Merge pull request kubevirt#6754 from jean-edouard/padisk

Only create 1MiB-aligned disk images

pkg/emptydisk/BUILD.bazel

@@ -9,6 +9,7 @@ go_library(
         "//pkg/ephemeral-disk-utils:go_default_library",
         "//pkg/util:go_default_library",
         "//staging/src/kubevirt.io/api/core/v1:go_default_library",
+        "//staging/src/kubevirt.io/client-go/log:go_default_library",
     ],
 )
 

pkg/emptydisk/emptydisk.go

@@ -1,12 +1,14 @@
 package emptydisk
 
 import (
+	"fmt"
 	"os"
 	"os/exec"
 	"path"
 	"strconv"
 
 	v1 "kubevirt.io/api/core/v1"
+	"kubevirt.io/client-go/log"
 	ephemeraldiskutils "kubevirt.io/kubevirt/pkg/ephemeral-disk-utils"
 	"kubevirt.io/kubevirt/pkg/util"
 )
@@ -19,11 +21,19 @@ type emptyDiskCreator struct {
 }
 
 func (c *emptyDiskCreator) CreateTemporaryDisks(vmi *v1.VirtualMachineInstance) error {
-	for _, volume := range vmi.Spec.Volumes {
+	logger := log.Log.Object(vmi)
 
+	for _, volume := range vmi.Spec.Volumes {
 		if volume.EmptyDisk != nil {
 			// qemu-img takes the size in bytes or in Kibibytes/Mebibytes/...; lets take bytes
-			size := strconv.FormatInt(volume.EmptyDisk.Capacity.ToDec().ScaledValue(0), 10)
+			intSize := volume.EmptyDisk.Capacity.ToDec().ScaledValue(0)
+			// round down the size to the nearest 1MiB multiple
+			intSize = util.AlignImageSizeTo1MiB(intSize, logger.With("volume", volume.Name))
+			if intSize == 0 {
+				return fmt.Errorf("the size for volume %s is too low", volume.Name)
+			}
+			// convert the size to string for qemu-img
+			size := strconv.FormatInt(intSize, 10)
 			file := filePathForVolumeName(c.emptyDiskBaseDir, volume.Name)
 			if err := util.MkdirAllWithNosec(c.emptyDiskBaseDir); err != nil {
 				return err

pkg/host-disk/host-disk.go

@@ -78,7 +78,10 @@ func ReplacePVCByHostDisk(vmi *v1.VirtualMachineInstance) error {
 				continue
 			}
 
-			replaceForHostDisk(volumeSource, volume.Name, pvcVolume)
+			err := replaceForHostDisk(volumeSource, volume.Name, pvcVolume)
+			if err != nil {
+				return err
+			}
 			// PersistenVolumeClaim is replaced by HostDisk
 			volumeSource.PersistentVolumeClaim = nil
 		}
@@ -87,24 +90,36 @@ func ReplacePVCByHostDisk(vmi *v1.VirtualMachineInstance) error {
 				continue
 			}
 
-			replaceForHostDisk(volumeSource, volume.Name, pvcVolume)
+			err := replaceForHostDisk(volumeSource, volume.Name, pvcVolume)
+			if err != nil {
+				return err
+			}
 			// PersistenVolumeClaim is replaced by HostDisk
 			volumeSource.DataVolume = nil
 		}
 	}
 	return nil
 }
 
-func replaceForHostDisk(volumeSource *v1.VolumeSource, volumeName string, pvcVolume map[string]v1.VolumeStatus) {
+func replaceForHostDisk(volumeSource *v1.VolumeSource, volumeName string, pvcVolume map[string]v1.VolumeStatus) error {
 	volumeStatus := pvcVolume[volumeName]
 	isShared := types.HasSharedAccessMode(volumeStatus.PersistentVolumeClaimInfo.AccessModes)
 	file := getPVCDiskImgPath(volumeName, "disk.img")
+	capacity := volumeStatus.PersistentVolumeClaimInfo.Capacity[k8sv1.ResourceStorage]
+	// The host-disk must be 1MiB-aligned. If the volume specifies a misaligned size, shrink it down to the nearest multiple of 1MiB
+	size := util.AlignImageSizeTo1MiB(capacity.Value(), log.Log.V(2))
+	if size == 0 {
+		return fmt.Errorf("the size for volume %s is too low, must be at least 1MiB", volumeName)
+	}
+	capacity.Set(size)
 	volumeSource.HostDisk = &v1.HostDisk{
 		Path:     file,
 		Type:     v1.HostDiskExistsOrCreate,
-		Capacity: volumeStatus.PersistentVolumeClaimInfo.Capacity[k8sv1.ResourceStorage],
+		Capacity: capacity,
 		Shared:   &isShared,
 	}
+
+	return nil
 }
 
 func shouldSkipVolumeSource(passthoughFSVolumes map[string]struct{}, hotplugVolumes map[string]bool, pvcVolume map[string]v1.VolumeStatus, volumeName string) bool {

pkg/host-disk/host-disk_test.go

@@ -399,6 +399,9 @@ var _ = Describe("HostDisk", func() {
 					Name: volumeName,
 					PersistentVolumeClaimInfo: &v1.PersistentVolumeClaimInfo{
 						VolumeMode: &mode,
+						Capacity: map[k8sv1.ResourceName]resource.Quantity{
+							k8sv1.ResourceStorage: *resource.NewQuantity(1024*1024, resource.BinarySI),
+						},
 					},
 				},
 			}

pkg/util/util.go

@@ -6,6 +6,7 @@ import (
 	"strings"
 
 	v1 "kubevirt.io/api/core/v1"
+	"kubevirt.io/client-go/log"
 )
 
 const (
@@ -150,3 +151,23 @@ func IsReadOnlyDisk(disk *v1.Disk) bool {
 
 	return isReadOnlyCDRom
 }
+
+// AlignImageSizeTo1MiB rounds down the size to the nearest multiple of 1MiB
+// A warning or an error may get logged
+// The caller is responsible for ensuring the rounded-down size is not 0
+func AlignImageSizeTo1MiB(size int64, logger *log.FilteredLogger) int64 {
+	remainder := size % (1024 * 1024)
+	if remainder == 0 {
+		return size
+	} else {
+		newSize := size - remainder
+		if logger != nil {
+			if newSize == 0 {
+				logger.Errorf("disks must be at least 1MiB, %d bytes is too small", size)
+			} else {
+				logger.Warningf("disk size is not 1MiB-aligned. Adjusting from %d down to %d.", size, newSize)
+			}
+		}
+		return newSize
+	}
+}

pkg/virt-launcher/virtwrap/manager.go

@@ -38,6 +38,8 @@ import (
 	"sync"
 	"time"
 
+	util2 "kubevirt.io/kubevirt/pkg/util"
+
 	"kubevirt.io/kubevirt/pkg/virt-launcher/virtwrap/device/hostdevice/generic"
 	"kubevirt.io/kubevirt/pkg/virt-launcher/virtwrap/device/hostdevice/gpu"
 
@@ -649,10 +651,14 @@ func expandDiskImageOffline(imagePath string, size int64) error {
 	} else {
 		preallocateFlag = "--preallocation=off"
 	}
+	size = util2.AlignImageSizeTo1MiB(size, log.Log.With("image", imagePath))
+	if size == 0 {
+		return fmt.Errorf("%s must be at least 1MiB", imagePath)
+	}
 	cmd := exec.Command("/usr/bin/qemu-img", "resize", preallocateFlag, imagePath, strconv.FormatInt(size, 10))
 	out, err := cmd.CombinedOutput()
 	if err != nil {
-		return fmt.Errorf("Expanding image failed with error: %v, output: %s", err, out)
+		return fmt.Errorf("expanding image failed with error: %v, output: %s", err, out)
 	}
 	return nil
 }
@@ -672,7 +678,12 @@ func possibleGuestSize(disk api.Disk) (int64, bool) {
 		log.DefaultLogger().Reason(err).Error("Failed to parse filesystem overhead as float")
 		return 0, false
 	}
-	return int64((1 - filesystemOverhead) * float64(preferredSize)), true
+	size := int64((1 - filesystemOverhead) * float64(preferredSize))
+	size = util2.AlignImageSizeTo1MiB(size, log.DefaultLogger())
+	if size == 0 {
+		return 0, false
+	}
+	return size, true
 }
 
 func shouldExpandOffline(disk api.Disk) bool {

pkg/virt-launcher/virtwrap/manager_test.go

@@ -2384,7 +2384,7 @@ var _ = Describe("Manager helper functions", func() {
 		BeforeEach(func() {
 			fakePercentFloat = 0.7648
 			fakePercent := cdiv1beta1.Percent(fmt.Sprint(fakePercentFloat))
-			fakeCapacity := int64(123000)
+			fakeCapacity := int64(2345 * 3456) // We need (1-0.7648)*fakeCapacity to be > 1MiB and misaligned
 
 			properDisk = api.Disk{
 				FilesystemOverhead: &fakePercent,
@@ -2399,6 +2399,8 @@ var _ = Describe("Manager helper functions", func() {
 			Expect(capacity).ToNot(Equal(nil))
 
 			expectedSize := int64((1 - fakePercentFloat) * float64(*capacity))
+			// The size is expected to be 1MiB-aligned
+			expectedSize = expectedSize - expectedSize%(1024*1024)
 
 			Expect(size).To(Equal(expectedSize))
 		})

tests/storage/storage.go

@@ -329,18 +329,18 @@ var _ = SIGDescribe("Storage", func() {
 					Name: "emptydisk1",
 					VolumeSource: virtv1.VolumeSource{
 						EmptyDisk: &virtv1.EmptyDiskSource{
-							Capacity: resource.MustParse("2Gi"),
+							Capacity: resource.MustParse("1G"),
 						},
 					},
 				})
 				vmi = tests.RunVMIAndExpectLaunch(vmi, 90)
 
 				Expect(libnet.WithIPv6(console.LoginToCirros)(vmi)).To(Succeed())
 
-				By("Checking that /dev/vdc has a capacity of 2Gi")
+				By("Checking that /dev/vdc has a capacity of 1G, aligned to 4k")
 				Expect(console.SafeExpectBatch(vmi, []expect.Batcher{
 					&expect.BSnd{S: "sudo blockdev --getsize64 /dev/vdc\n"},
-					&expect.BExp{R: "2147483648"}, // 2Gi in bytes
+					&expect.BExp{R: "999292928"}, // 1G in bytes rounded down to nearest 1MiB boundary
 				}, 10)).To(Succeed())
 
 				By("Checking if we can write to /dev/vdc")
@@ -654,7 +654,7 @@ var _ = SIGDescribe("Storage", func() {
 
 						Expect(console.SafeExpectBatch(vmi, []expect.Batcher{
 							&expect.BSnd{S: "blockdev --getsize64 /dev/vdb\n"},
-							&expect.BExp{R: "1014686208"},
+							&expect.BExp{R: "1013972992"},
 						}, 200)).To(Succeed())
 					}
 

tests/vmi_configuration_test.go

@@ -2031,10 +2031,10 @@ var _ = Describe("[sig-compute]Configurations", func() {
 			// PVC is mounted as tmpfs on kind, which does not support direct I/O.
 			// As such, it behaves as plugging in a hostDisk - check disks[6].
 			if tests.IsRunningOnKindInfra() {
-				// The chache mode is set to cacheWritethrough
+				// The cache mode is set to cacheWritethrough
 				Expect(string(disks[2].Driver.IO)).To(Equal(ioNone))
 			} else {
-				// The chache mode is set to cacheNone
+				// The cache mode is set to cacheNone
 				Expect(disks[2].Driver.IO).To(Equal(ioNative))
 			}