Fix calculation of sizes and offset

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 <[email protected]>  Fri, 13 Oct 2023 11:26:23 +0100
 

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 {

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

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 {

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
 }

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,
 					},
 				},
 			},

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