Skip to content

Commit

Permalink
fix: align partition to 1M boundary by default
Browse files Browse the repository at this point in the history
See e.g. https://www.thomas-krenn.com/en/wiki/Partition_Alignment_detailed_explanation

This is the default used by modern disk partition utilities.

Align partition end as well to the same boundary when the partition is
grown to the maximum size (see
siderolabs/talos#4985).

Fix the problem with `WithSingleResult` which never worked properly (?).

It looks like `probe.All` the way it is designed will never work
properly, as the partition name check is done for the blockdevice as a
whole, while the iteration goes over all partition. This goes unnoticed
because of the way this function is actually used.

Signed-off-by: Andrey Smirnov <[email protected]>
  • Loading branch information
smira committed Mar 29, 2022
1 parent ec428fe commit b374eb4
Show file tree
Hide file tree
Showing 6 changed files with 65 additions and 29 deletions.
17 changes: 15 additions & 2 deletions blockdevice/lba/lba.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ import (
"os"
)

// RecommendedAlignment is recommended alignment for LBA.
const RecommendedAlignment = 1048576

// LBA represents logical block addressing.
//
//nolint:govet
Expand All @@ -23,18 +26,28 @@ type LBA struct {
}

// AlignToPhysicalBlockSize aligns LBA value in LogicalBlockSize multiples to be aligned to PhysicalBlockSize.
func (l *LBA) AlignToPhysicalBlockSize(lba uint64) uint64 {
func (l *LBA) AlignToPhysicalBlockSize(lba uint64, roundUp bool) uint64 {
physToLogical := uint64(l.PhysicalBlockSize / l.LogicalBlockSize)
minIOToLogical := uint64(l.MinimalIOSize / l.LogicalBlockSize)
recommended := uint64(RecommendedAlignment / l.LogicalBlockSize)

// find max ratio
ratio := physToLogical
if minIOToLogical > ratio {
ratio = minIOToLogical
}

if recommended > ratio {
ratio = recommended
}

if ratio <= 1 {
return lba
}

return (lba + ratio - 1) / ratio * ratio
if roundUp {
return (lba + ratio - 1) / ratio * ratio
}

return lba / ratio * ratio
}
52 changes: 35 additions & 17 deletions blockdevice/lba/lba_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,34 +12,52 @@ import (
"github.com/talos-systems/go-blockdevice/blockdevice/lba"
)

func TestAlignToRecommended(t *testing.T) {
l := lba.LBA{ //nolint: exhaustivestruct
PhysicalBlockSize: 512,
LogicalBlockSize: 512,
}

assert.EqualValues(t, 0, l.AlignToPhysicalBlockSize(0, true))
assert.EqualValues(t, 2048, l.AlignToPhysicalBlockSize(1, true))
assert.EqualValues(t, 2048, l.AlignToPhysicalBlockSize(2, true))
assert.EqualValues(t, 2048, l.AlignToPhysicalBlockSize(3, true))
assert.EqualValues(t, 2048, l.AlignToPhysicalBlockSize(4, true))
assert.EqualValues(t, 2048, l.AlignToPhysicalBlockSize(2048, true))
assert.EqualValues(t, 4096, l.AlignToPhysicalBlockSize(2049, true))
assert.EqualValues(t, 2048, l.AlignToPhysicalBlockSize(2049, false))
}

func TestAlignToPhysicalBlockSize(t *testing.T) {
l := lba.LBA{ //nolint: exhaustivestruct
PhysicalBlockSize: 4096,
PhysicalBlockSize: 2 * 1048576,
LogicalBlockSize: 512,
}

assert.EqualValues(t, 0, l.AlignToPhysicalBlockSize(0))
assert.EqualValues(t, 8, l.AlignToPhysicalBlockSize(1))
assert.EqualValues(t, 8, l.AlignToPhysicalBlockSize(2))
assert.EqualValues(t, 8, l.AlignToPhysicalBlockSize(3))
assert.EqualValues(t, 8, l.AlignToPhysicalBlockSize(4))
assert.EqualValues(t, 8, l.AlignToPhysicalBlockSize(8))
assert.EqualValues(t, 16, l.AlignToPhysicalBlockSize(9))
assert.EqualValues(t, 0, l.AlignToPhysicalBlockSize(0, true))
assert.EqualValues(t, 4096, l.AlignToPhysicalBlockSize(1, true))
assert.EqualValues(t, 4096, l.AlignToPhysicalBlockSize(2, true))
assert.EqualValues(t, 4096, l.AlignToPhysicalBlockSize(3, true))
assert.EqualValues(t, 4096, l.AlignToPhysicalBlockSize(4, true))
assert.EqualValues(t, 4096, l.AlignToPhysicalBlockSize(4096, true))
assert.EqualValues(t, 8192, l.AlignToPhysicalBlockSize(4097, true))
assert.EqualValues(t, 4096, l.AlignToPhysicalBlockSize(4097, false))
}

func TestAlignToMinIOkSize(t *testing.T) {
l := lba.LBA{ //nolint: exhaustivestruct
MinimalIOSize: 262144,
MinimalIOSize: 4 * 1048576,
PhysicalBlockSize: 512,
LogicalBlockSize: 512,
}

assert.EqualValues(t, 0, l.AlignToPhysicalBlockSize(0))
assert.EqualValues(t, 512, l.AlignToPhysicalBlockSize(1))
assert.EqualValues(t, 512, l.AlignToPhysicalBlockSize(2))
assert.EqualValues(t, 512, l.AlignToPhysicalBlockSize(3))
assert.EqualValues(t, 512, l.AlignToPhysicalBlockSize(4))
assert.EqualValues(t, 512, l.AlignToPhysicalBlockSize(8))
assert.EqualValues(t, 512, l.AlignToPhysicalBlockSize(512))
assert.EqualValues(t, 1024, l.AlignToPhysicalBlockSize(513))
assert.EqualValues(t, 0, l.AlignToPhysicalBlockSize(0, true))
assert.EqualValues(t, 8192, l.AlignToPhysicalBlockSize(1, true))
assert.EqualValues(t, 8192, l.AlignToPhysicalBlockSize(2, true))
assert.EqualValues(t, 8192, l.AlignToPhysicalBlockSize(3, true))
assert.EqualValues(t, 8192, l.AlignToPhysicalBlockSize(4, true))
assert.EqualValues(t, 8192, l.AlignToPhysicalBlockSize(8, true))
assert.EqualValues(t, 8192, l.AlignToPhysicalBlockSize(8192, true))
assert.EqualValues(t, 16384, l.AlignToPhysicalBlockSize(8193, true))
assert.EqualValues(t, 8192, l.AlignToPhysicalBlockSize(8193, false))
}
5 changes: 3 additions & 2 deletions blockdevice/partition/gpt/gpt.go
Original file line number Diff line number Diff line change
Expand Up @@ -263,10 +263,10 @@ func (g *GPT) InsertAt(idx int, size uint64, setters ...PartitionOption) (*Parti
// Find partition boundaries.
var start, end uint64

start = g.l.AlignToPhysicalBlockSize(minLBA)
start = g.l.AlignToPhysicalBlockSize(minLBA, true)

if opts.MaximumSize {
end = maxLBA
end = g.l.AlignToPhysicalBlockSize(maxLBA+1, false) - 1

if end < start {
return nil, outOfSpaceError{fmt.Errorf("requested partition with maximum size, but no space available")}
Expand Down Expand Up @@ -338,6 +338,7 @@ func (g *GPT) Resize(part *Partition) (bool, error) {
}

maxLBA := g.h.LastUsableLBA
maxLBA = g.l.AlignToPhysicalBlockSize(maxLBA+1, false) - 1

for i := idx + 1; i < len(g.e.p); i++ {
if g.e.p[i] != nil {
Expand Down
16 changes: 9 additions & 7 deletions blockdevice/partition/gpt/gpt_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"github.com/stretchr/testify/suite"

"github.com/talos-systems/go-blockdevice/blockdevice"
"github.com/talos-systems/go-blockdevice/blockdevice/lba"
"github.com/talos-systems/go-blockdevice/blockdevice/loopback"
"github.com/talos-systems/go-blockdevice/blockdevice/partition/gpt"
"github.com/talos-systems/go-blockdevice/blockdevice/test"
Expand All @@ -21,9 +22,10 @@ import (
const (
size = 1024 * 1024 * 1024 * 1024
blockSize = 512
alignment = lba.RecommendedAlignment / blockSize

headReserved = 34
tailReserved = 33
headReserved = (34 + alignment - 1) / alignment * alignment
tailReserved = (33 + alignment - 1) / alignment * alignment
)

type GPTSuite struct {
Expand Down Expand Up @@ -238,15 +240,15 @@ func (suite *GPTSuite) TestPartitionAddOutOfSpace() {

_, err = g.Add(size, gpt.WithPartitionName("boot"))
suite.Require().Error(err)
suite.Assert().EqualError(err, `requested partition size 1099511627776, available is 1099511592960 (34816 too many bytes)`)
suite.Assert().EqualError(err, `requested partition size 1099511627776, available is 1099510561792 (1065984 too many bytes)`)
suite.Assert().True(blockdevice.IsOutOfSpaceError(err))

_, err = g.Add(size/2, gpt.WithPartitionName("boot"))
suite.Require().NoError(err)

_, err = g.Add(size/2, gpt.WithPartitionName("boot2"))
suite.Require().Error(err)
suite.Assert().EqualError(err, `requested partition size 549755813888, available is 549755779072 (34816 too many bytes)`)
suite.Assert().EqualError(err, `requested partition size 549755813888, available is 549754747904 (1065984 too many bytes)`)
suite.Assert().True(blockdevice.IsOutOfSpaceError(err))

_, err = g.Add(size/2-(headReserved+tailReserved)*blockSize, gpt.WithPartitionName("boot2"))
Expand All @@ -266,7 +268,7 @@ func (suite *GPTSuite) TestPartitionDelete() {
bootSize = 1048576
grubSize = 2 * bootSize
efiSize = 512 * 1048576
configSize = blockSize
configSize = 1048576
)

_, err = g.Add(bootSize, gpt.WithPartitionName("boot"))
Expand Down Expand Up @@ -335,10 +337,10 @@ func (suite *GPTSuite) TestPartitionInsertAt() {
suite.Require().NoError(err)

const (
oldBootSize = 1048576
oldBootSize = 4 * 1048576
newBootSize = oldBootSize / 2
grubSize = newBootSize / 2
configSize = blockSize
configSize = 1048576
efiSize = 512 * 1048576
)

Expand Down
2 changes: 2 additions & 0 deletions blockdevice/probe/probe.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,8 @@ func WithSingleResult() SelectOption {
return false, fmt.Errorf("got more than one blockdevice with provided criteria")
}

count++

return true, nil
}
}
Expand Down
2 changes: 1 addition & 1 deletion blockdevice/probe/probe_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ func (suite *ProbeSuite) TestProbeByPartitionLabel() {
suite.addPartition("test", size)
suite.addPartition("test2", size)

probed, err := probe.All(probe.WithPartitionLabel("test"))
probed, err := probe.All(probe.WithPartitionLabel("test"), probe.WithSingleResult())
suite.Require().NoError(err)
suite.Require().Equal(1, len(probed))

Expand Down

0 comments on commit b374eb4

Please sign in to comment.