Skip to content
This repository has been archived by the owner on May 30, 2023. It is now read-only.

Relocate GPT headers for expanded disks #158

Merged
merged 2 commits into from
Mar 25, 2022
+33 −0
Merged

Relocate GPT headers for expanded disks #158

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
36f042c
Relocate GPT headers for expanded disks
davidcassany Mar 24, 2022
c0c3926
Move GPT headers fix logic to Disk only
davidcassany Mar 25, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions pkg/partitioner/disk.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,19 @@ func (dev *Disk) Reload() error {
return err
}

if pc.HasUnallocatedSpace(prnt) {
// Parted has not a proper way to doing it in non interactive mode,
// because of that we use sgdisk for that...
_, err = dev.runner.Run("sgdisk", "-e", dev.device)
if err != nil {
return err
}
prnt, err = pc.Print()
if err != nil {
return err
}
}

sectorS, err := pc.GetSectorSize(prnt)
if err != nil {
return err
Expand Down
11 changes: 11 additions & 0 deletions pkg/partitioner/parted.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,17 @@ func (pc PartedCall) Print() (string, error) {
return string(out), err
}

// HasUnallocatedSpace parses a parted print command output and checks for a
// warning within the output complaining about wrong GPT end header. If
// the warning is found it assumes the GPT end header is not at the end of
// the disk, hence it needs to be adapted to get valid sizes from parted
// print. Parted has no proper command to verify GPT headers neither a
// proper command to fix them whenever is possible in a non interactive mode.
func (pc PartedCall) HasUnallocatedSpace(printOut string) bool {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

small nit here, better keep this private. It is parsing a string for a known pattern so also the regexp could be compiled instead of being a func wrapper

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

besides, it is safe to assume that we want to always call sgdisk here in any case this pattern matches?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

besides, it is safe to assume that we want to always call sgdisk here in any case this pattern matches?

I think so and it is aligned with the --fix flag that parted should include within the next release (I am not sure which is the criteria to cut releases, since it is more than a year already since last one). So if we'd use parted from master upstream we would not need this check at all and we would simply call parted print command with --fix flag, which essentially fixes gpt headers if there is some inconsistency. So the Disk.Reload method would essentially run parted -s --fix -m <device> unit s print and it would load the disk partition table at the time it fixes GTP header inconsistencies, so the values of the disk size print outputs matches the actual disk geometry.

In parted there is no such command to actually fix headers, it can only do it on the fly in interactive mode or fully automated with the --fix flag in non interactive mode (up coming releases). sgdisk has verify and expand commands, so you have the tools to add logic to handle this situation in different scenarios if you want.

sgdisk is a well designed tool to script on top of it, parted is not that good in that regard, scripted commands are convoluted (probably to match GPT and MSDOS support in a single UI) and not as powerfull as in interactive mode. The only reason to use parted is that is supports MSDOS partition tables and it allows us to support GPT and MSDOS with a single logic or wrapper.

unallocated, _ := regexp.MatchString("Not all of the space available", printOut)
return unallocated
}

// Parses the output of a PartedCall.Print call
func (pc PartedCall) parseHeaderFields(printOut string, field int) (string, error) {
re := regexp.MustCompile(`^(.*):(\d+)s:(.*):(\d+):(\d+):(.*):(.*):(.*);$`)
Expand Down
9 changes: 9 additions & 0 deletions pkg/partitioner/partitioner_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,15 @@ var _ = Describe("Partitioner", Label("disk", "partition", "partitioner"), func(
dev.Reload()
Expect(dev.GetLabel()).To(Equal("msdos"))
})
It("It fixes GPT headers if the disk was expanded", func() {
runner.ReturnValue = []byte("Warning: Not all of the space available to /dev/loop0...\n" + printOutput)
Expect(dev.Reload()).To(BeNil())
Expect(runner.MatchMilestones([][]string{
{"parted", "--script", "--machine", "--", "/some/device", "unit", "s", "print"},
{"sgdisk", "-e", "/some/device"},
{"parted", "--script", "--machine", "--", "/some/device", "unit", "s", "print"},
})).To(BeNil())
})
})
Describe("Modify disk", func() {
It("Format an already existing partition", func() {
Expand Down