diff --git a/debian/changelog b/debian/changelog index 3013cd71..07305b3f 100644 --- a/debian/changelog +++ b/debian/changelog @@ -33,6 +33,7 @@ ubuntu-image (3.0+23.04ubuntu1) UNRELEASED; urgency=medium [ Alfonso Sanchez-Beato ] * Update to latest snapd, adapting to changes in layouts code. + * Fix calculation of volume sizes after latest snapd changes -- Alfonso Sanchez-Beato Fri, 13 Oct 2023 11:26:23 +0100 diff --git a/internal/statemachine/common_states.go b/internal/statemachine/common_states.go index 35870b23..f930eedc 100644 --- a/internal/statemachine/common_states.go +++ b/internal/statemachine/common_states.go @@ -295,8 +295,6 @@ func (stateMachine *StateMachine) populatePreparePartitions() error { if err := stateMachine.handleLkBootloader(volume); err != nil { return err } - var farthestOffset quantity.Offset = 0 - //for structureNumber, structure := range volume.Structure { for structureNumber, structure := range volume.Structure { var contentRoot string if structure.Role == gadget.SystemData || structure.Role == gadget.SystemSeed { @@ -305,8 +303,6 @@ func (stateMachine *StateMachine) populatePreparePartitions() error { contentRoot = filepath.Join(stateMachine.tempDirs.volumes, volumeName, "part"+strconv.Itoa(structureNumber)) } - farthestOffset = maxOffset(farthestOffset, - quantity.Offset(structure.Size)+getStructureOffset(structure)) if shouldSkipStructure(structure, stateMachine.IsSeeded) { continue } @@ -319,8 +315,9 @@ func (stateMachine *StateMachine) populatePreparePartitions() error { return err } } - // set the image size values to be used by make_disk - stateMachine.handleContentSizes(farthestOffset, volumeName) + // Set the image size values to be used by make_disk, by using + // the minimum size that would be valid according to gadget.yaml. + stateMachine.handleContentSizes(quantity.Offset(volume.MinSize()), volumeName) } return nil } @@ -336,7 +333,9 @@ func (stateMachine *StateMachine) makeDisk() error { // Create the disk image imgSize, found := stateMachine.ImageSizes[volumeName] if !found { - imgSize, _ = stateMachine.calculateImageSize() + // Calculate the minimum size that would be + // valid according to gadget.yaml. + imgSize = volume.MinSize() } if err := osRemoveAll(imgName); err != nil { diff --git a/internal/statemachine/helper.go b/internal/statemachine/helper.go index 51ef8107..fa979e9f 100644 --- a/internal/statemachine/helper.go +++ b/internal/statemachine/helper.go @@ -439,22 +439,10 @@ func createPartitionTable(volumeName string, volume *gadget.Volume, sectorSize u return &partitionTable, rootfsPartitionNumber } -// calculateImageSize calculates the total sum of all partition sizes in an image -func (stateMachine *StateMachine) calculateImageSize() (quantity.Size, error) { - if stateMachine.GadgetInfo == nil { - return 0, fmt.Errorf("Cannot calculate image size before initializing GadgetInfo") - } - var imgSize quantity.Size = 0 - for _, volume := range stateMachine.GadgetInfo.Volumes { - for _, structure := range volume.Structure { - imgSize += structure.Size - } - } - return imgSize, nil -} - // copyDataToImage runs dd commands to copy the raw data to the final image with appropriate offsets func (stateMachine *StateMachine) copyDataToImage(volumeName string, volume *gadget.Volume, diskImg *disk.Disk) error { + // Resolve gadget information to on disk volume + onDisk := gadget.OnDiskStructsFromGadget(volume) for structureNumber, structure := range volume.Structure { if shouldSkipStructure(structure, stateMachine.IsSeeded) { continue @@ -463,8 +451,9 @@ func (stateMachine *StateMachine) copyDataToImage(volumeName string, volume *gad // set up the arguments to dd the structures into an image partImg := filepath.Join(stateMachine.tempDirs.volumes, volumeName, "part"+strconv.Itoa(structureNumber)+".img") - seek := strconv.FormatInt(int64(getStructureOffset(structure))/sectorSize, 10) - count := strconv.FormatFloat(math.Ceil(float64(structure.Size)/float64(sectorSize)), 'f', 0, 64) + onDiskStruct := onDisk[structure.YamlIndex] + seek := strconv.FormatInt(int64(onDiskStruct.StartOffset)/sectorSize, 10) + count := strconv.FormatFloat(math.Ceil(float64(onDiskStruct.Size)/float64(sectorSize)), 'f', 0, 64) ddArgs := []string{ "if=" + partImg, "of=" + diskImg.File.Name(), @@ -507,14 +496,6 @@ func writeOffsetValues(volume *gadget.Volume, imgName string, sectorSize, imgSiz return nil } -// getStructureOffset returns 0 if structure.Offset is nil, otherwise the value stored there -func getStructureOffset(structure gadget.VolumeStructure) quantity.Offset { - if structure.Offset == nil { - return 0 - } - return *structure.Offset -} - // generateUniqueDiskID returns a random 4-byte long disk ID, unique per the list of existing IDs func generateUniqueDiskID(existing *[][]byte) ([]byte, error) { var retry bool diff --git a/internal/statemachine/helper_test.go b/internal/statemachine/helper_test.go index c24d526b..e2d1e438 100644 --- a/internal/statemachine/helper_test.go +++ b/internal/statemachine/helper_test.go @@ -368,18 +368,6 @@ func TestFailedCleanup(t *testing.T) { }) } -// TestFailedCalculateImageSize tests a scenario when calculateImageSize() is called -// with a nil pointer to stateMachine.GadgetInfo -func TestFailedCalculateImageSize(t *testing.T) { - t.Run("test_failed_calculate_image_size", func(t *testing.T) { - asserter := helper.Asserter{T: t} - var stateMachine StateMachine - stateMachine.commonFlags, stateMachine.stateMachineFlags = helper.InitCommonOpts() - _, err := stateMachine.calculateImageSize() - asserter.AssertErrContains(err, "Cannot calculate image size before initializing GadgetInfo") - }) -} - // TestFailedWriteOffsetValues tests various error scenarios for writeOffsetValues func TestFailedWriteOffsetValues(t *testing.T) { t.Run("test_failed_write_offset_values", func(t *testing.T) { @@ -499,27 +487,6 @@ func TestWarningRootfsSizeTooSmall(t *testing.T) { }) } -// TestGetStructureOffset ensures structure offset safely dereferences structure.Offset -func TestGetStructureOffset(t *testing.T) { - var testOffset quantity.Offset = 1 - testCases := []struct { - name string - structure gadget.VolumeStructure - expected quantity.Offset - }{ - {"nil", gadget.VolumeStructure{Offset: nil}, 0}, - {"with_value", gadget.VolumeStructure{Offset: &testOffset}, 1}, - } - for _, tc := range testCases { - t.Run("test_get_structure_offset_"+tc.name, func(t *testing.T) { - offset := getStructureOffset(tc.structure) - if offset != tc.expected { - t.Errorf("Error, expected offset %d but got %d", tc.expected, offset) - } - }) - } -} - // TestGenerateUniqueDiskID ensures that we generate unique disk IDs func TestGenerateUniqueDiskID(t *testing.T) { testCases := []struct { diff --git a/internal/statemachine/state_machine.go b/internal/statemachine/state_machine.go index cd75097f..dcf91c82 100644 --- a/internal/statemachine/state_machine.go +++ b/internal/statemachine/state_machine.go @@ -229,10 +229,9 @@ func (stateMachine *StateMachine) postProcessGadgetYaml() error { var farthestOffset quantity.Offset var lastOffset quantity.Offset farthestOffsetUnknown := false - var lastVolumeName string + var volume *gadget.Volume for _, volumeName := range stateMachine.VolumeOrder { - volume := stateMachine.GadgetInfo.Volumes[volumeName] - lastVolumeName = volumeName + volume = stateMachine.GadgetInfo.Volumes[volumeName] volumeBaseDir := filepath.Join(stateMachine.tempDirs.volumes, volumeName) if err := osMkdirAll(volumeBaseDir, 0755); err != nil { return fmt.Errorf("Error creating volume dir: %s", err.Error()) @@ -314,6 +313,8 @@ func (stateMachine *StateMachine) postProcessGadgetYaml() error { // // Since so far we have no knowledge of the rootfs contents, the // size is set to 0, and will be calculated later + + // Note that there is only one volume, so "volume" points to it rootfsStructure := gadget.VolumeStructure{ Name: "", Label: "writable", @@ -325,12 +326,13 @@ func (stateMachine *StateMachine) postProcessGadgetYaml() error { Filesystem: "ext4", Content: []gadget.VolumeContent{}, Update: gadget.VolumeUpdate{}, + // "virtual" yaml index for the new structure (it would + // be the last one in gadget.yaml) + YamlIndex: len(volume.Structure), } - // There is only one volume, so lastVolumeName is its name // we now add the rootfs structure to the volume - stateMachine.GadgetInfo.Volumes[lastVolumeName].Structure = - append(stateMachine.GadgetInfo.Volumes[lastVolumeName].Structure, rootfsStructure) + volume.Structure = append(volume.Structure, rootfsStructure) } return nil } diff --git a/internal/statemachine/state_machine_test.go b/internal/statemachine/state_machine_test.go index 175138b0..7ff3aeb2 100644 --- a/internal/statemachine/state_machine_test.go +++ b/internal/statemachine/state_machine_test.go @@ -622,6 +622,7 @@ func TestPostProcessGadgetYaml(t *testing.T) { Label: "writable", Offset: createOffsetPointer(537919488), Content: []gadget.VolumeContent{}, + YamlIndex: 1, }, }, }, @@ -699,6 +700,7 @@ func TestPostProcessGadgetYaml(t *testing.T) { Label: "writable", Offset: createOffsetPointer(54525952), Content: []gadget.VolumeContent{}, + YamlIndex: 3, }, }, }, diff --git a/snapcraft.yaml b/snapcraft.yaml index c1c34e20..f4175384 100644 --- a/snapcraft.yaml +++ b/snapcraft.yaml @@ -3,7 +3,7 @@ summary: Create Ubuntu images description: | Official tool for building Ubuntu images, currently supporing Ubuntu Core snap-based images and preinstalled Ubuntu classic images. -version: 3.0+snap13 +version: 3.0+snap14 grade: stable confinement: classic base: core22