From b374eb48148dc92a82d8bf9540432bb8531f73f3 Mon Sep 17 00:00:00 2001 From: Andrey Smirnov Date: Tue, 29 Mar 2022 23:17:18 +0300 Subject: [PATCH] fix: align partition to 1M boundary by default 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 https://github.com/siderolabs/talos/issues/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 --- blockdevice/lba/lba.go | 17 +++++++-- blockdevice/lba/lba_test.go | 52 ++++++++++++++++++--------- blockdevice/partition/gpt/gpt.go | 5 +-- blockdevice/partition/gpt/gpt_test.go | 16 +++++---- blockdevice/probe/probe.go | 2 ++ blockdevice/probe/probe_test.go | 2 +- 6 files changed, 65 insertions(+), 29 deletions(-) diff --git a/blockdevice/lba/lba.go b/blockdevice/lba/lba.go index 002c62f..8b9afc6 100644 --- a/blockdevice/lba/lba.go +++ b/blockdevice/lba/lba.go @@ -8,6 +8,9 @@ import ( "os" ) +// RecommendedAlignment is recommended alignment for LBA. +const RecommendedAlignment = 1048576 + // LBA represents logical block addressing. // //nolint:govet @@ -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 } diff --git a/blockdevice/lba/lba_test.go b/blockdevice/lba/lba_test.go index 4d6739f..90aa212 100644 --- a/blockdevice/lba/lba_test.go +++ b/blockdevice/lba/lba_test.go @@ -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)) } diff --git a/blockdevice/partition/gpt/gpt.go b/blockdevice/partition/gpt/gpt.go index 59653bd..dd95b07 100644 --- a/blockdevice/partition/gpt/gpt.go +++ b/blockdevice/partition/gpt/gpt.go @@ -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")} @@ -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 { diff --git a/blockdevice/partition/gpt/gpt_test.go b/blockdevice/partition/gpt/gpt_test.go index ccb2eab..081414e 100644 --- a/blockdevice/partition/gpt/gpt_test.go +++ b/blockdevice/partition/gpt/gpt_test.go @@ -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" @@ -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 { @@ -238,7 +240,7 @@ 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")) @@ -246,7 +248,7 @@ func (suite *GPTSuite) TestPartitionAddOutOfSpace() { _, 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")) @@ -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")) @@ -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 ) diff --git a/blockdevice/probe/probe.go b/blockdevice/probe/probe.go index 3208a8e..cfeddd8 100644 --- a/blockdevice/probe/probe.go +++ b/blockdevice/probe/probe.go @@ -84,6 +84,8 @@ func WithSingleResult() SelectOption { return false, fmt.Errorf("got more than one blockdevice with provided criteria") } + count++ + return true, nil } } diff --git a/blockdevice/probe/probe_test.go b/blockdevice/probe/probe_test.go index ad6c700..8efa850 100644 --- a/blockdevice/probe/probe_test.go +++ b/blockdevice/probe/probe_test.go @@ -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))