File 0004-only-create-1MiB-aligned-disk-images.patch of Package kubevirt.26647
From 7fbe61db7b02e2e30e2c5d39b708fc61a96115e1 Mon Sep 17 00:00:00 2001
From: Maya Rashish <mrashish@redhat.com>
Date: Sun, 9 Jan 2022 16:55:20 +0200
Subject: [PATCH 1/3] Limit ourselves to expanding to
min(spec.request,status.capacity)
With storage classes like NFS, HPP and local storage, the listed
status.capacity is the full underlying file system.
Using all of it for a single PVC is unexpected.
Don't expand beyond what we requested for the PVC in this scenario.
Signed-off-by: Maya Rashish <mrashish@redhat.com>
---
api/openapi-spec/swagger.json | 9 ++++-
pkg/virt-controller/watch/vmi.go | 1 +
pkg/virt-launcher/virtwrap/api/BUILD.bazel | 1 -
.../virtwrap/api/deepcopy_generated.go | 4 +-
pkg/virt-launcher/virtwrap/api/schema.go | 37 +++++++++---------
.../virtwrap/converter/converter.go | 38 +++++++++++++++++--
.../virtwrap/converter/converter_test.go | 30 +++++++++++++++
pkg/virt-launcher/virtwrap/manager.go | 14 ++-----
pkg/virt-launcher/virtwrap/manager_test.go | 18 +++------
.../components/validations_generated.go | 12 +++++-
.../api/core/v1/deepcopy_generated.go | 7 ++++
staging/src/kubevirt.io/api/core/v1/types.go | 6 ++-
.../api/core/v1/types_swagger_generated.go | 3 +-
.../client-go/api/openapi_generated.go | 16 +++++++-
14 files changed, 143 insertions(+), 53 deletions(-)
diff --git a/api/openapi-spec/swagger.json b/api/openapi-spec/swagger.json
index 059056118..cd9070cda 100644
--- a/api/openapi-spec/swagger.json
+++ b/api/openapi-spec/swagger.json
@@ -14448,7 +14448,7 @@
"x-kubernetes-list-type": "atomic"
},
"capacity": {
- "description": "Capacity represents the capacity set on the corresponding PVC spec",
+ "description": "Capacity represents the capacity set on the corresponding PVC status",
"type": "object",
"additionalProperties": {
"$ref": "#/definitions/k8s.io.apimachinery.pkg.api.resource.Quantity"
@@ -14462,6 +14462,13 @@
"description": "Preallocated indicates if the PVC's storage is preallocated or not",
"type": "boolean"
},
+ "requests": {
+ "description": "Requests represents the resources requested by the corresponding PVC spec",
+ "type": "object",
+ "additionalProperties": {
+ "$ref": "#/definitions/k8s.io.apimachinery.pkg.api.resource.Quantity"
+ }
+ },
"volumeMode": {
"description": "VolumeMode defines what type of volume is required by the claim. Value of Filesystem is implied when not included in claim spec.",
"type": "string"
diff --git a/pkg/virt-controller/watch/vmi.go b/pkg/virt-controller/watch/vmi.go
index ba7b72a0c..a3fca9ab5 100644
--- a/pkg/virt-controller/watch/vmi.go
+++ b/pkg/virt-controller/watch/vmi.go
@@ -2056,6 +2056,7 @@ func (c *VMIController) updateVolumeStatus(vmi *virtv1.VirtualMachineInstance, v
AccessModes: pvc.Spec.AccessModes,
VolumeMode: pvc.Spec.VolumeMode,
Capacity: pvc.Status.Capacity,
+ Requests: pvc.Spec.Resources.Requests,
Preallocated: kubevirttypes.IsPreallocated(pvc.ObjectMeta.Annotations),
}
filesystemOverhead, err := c.getFilesystemOverhead(pvc)
diff --git a/pkg/virt-launcher/virtwrap/api/BUILD.bazel b/pkg/virt-launcher/virtwrap/api/BUILD.bazel
index 97d593a38..f95c4dbe9 100644
--- a/pkg/virt-launcher/virtwrap/api/BUILD.bazel
+++ b/pkg/virt-launcher/virtwrap/api/BUILD.bazel
@@ -15,7 +15,6 @@ go_library(
"//staging/src/kubevirt.io/client-go/precond:go_default_library",
"//vendor/k8s.io/api/core/v1:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/api/meta:go_default_library",
- "//vendor/k8s.io/apimachinery/pkg/api/resource:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/runtime/schema:go_default_library",
diff --git a/pkg/virt-launcher/virtwrap/api/deepcopy_generated.go b/pkg/virt-launcher/virtwrap/api/deepcopy_generated.go
index 877cb9c14..f4f5a0092 100644
--- a/pkg/virt-launcher/virtwrap/api/deepcopy_generated.go
+++ b/pkg/virt-launcher/virtwrap/api/deepcopy_generated.go
@@ -803,8 +803,8 @@ func (in *Disk) DeepCopyInto(out *Disk) {
}
if in.Capacity != nil {
in, out := &in.Capacity, &out.Capacity
- x := (*in).DeepCopy()
- *out = &x
+ *out = new(int64)
+ **out = **in
}
return
}
diff --git a/pkg/virt-launcher/virtwrap/api/schema.go b/pkg/virt-launcher/virtwrap/api/schema.go
index 34e29a954..539e78c5c 100644
--- a/pkg/virt-launcher/virtwrap/api/schema.go
+++ b/pkg/virt-launcher/virtwrap/api/schema.go
@@ -27,7 +27,6 @@ import (
kubev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/meta"
- "k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/types"
@@ -568,24 +567,24 @@ type ControllerDriver struct {
// BEGIN Disk -----------------------------
type Disk struct {
- Device string `xml:"device,attr"`
- Snapshot string `xml:"snapshot,attr,omitempty"`
- Type string `xml:"type,attr"`
- Source DiskSource `xml:"source"`
- Target DiskTarget `xml:"target"`
- Serial string `xml:"serial,omitempty"`
- Driver *DiskDriver `xml:"driver,omitempty"`
- ReadOnly *ReadOnly `xml:"readonly,omitempty"`
- Auth *DiskAuth `xml:"auth,omitempty"`
- Alias *Alias `xml:"alias,omitempty"`
- BackingStore *BackingStore `xml:"backingStore,omitempty"`
- BootOrder *BootOrder `xml:"boot,omitempty"`
- Address *Address `xml:"address,omitempty"`
- Model string `xml:"model,attr,omitempty"`
- BlockIO *BlockIO `xml:"blockio,omitempty"`
- FilesystemOverhead *cdiv1.Percent `xml:"filesystemOverhead,omitempty"`
- Capacity *resource.Quantity `xml:"capacity,omitempty"`
- ExpandDisksEnabled bool `xml:"expandDisksEnabled,omitempty"`
+ Device string `xml:"device,attr"`
+ Snapshot string `xml:"snapshot,attr,omitempty"`
+ Type string `xml:"type,attr"`
+ Source DiskSource `xml:"source"`
+ Target DiskTarget `xml:"target"`
+ Serial string `xml:"serial,omitempty"`
+ Driver *DiskDriver `xml:"driver,omitempty"`
+ ReadOnly *ReadOnly `xml:"readonly,omitempty"`
+ Auth *DiskAuth `xml:"auth,omitempty"`
+ Alias *Alias `xml:"alias,omitempty"`
+ BackingStore *BackingStore `xml:"backingStore,omitempty"`
+ BootOrder *BootOrder `xml:"boot,omitempty"`
+ Address *Address `xml:"address,omitempty"`
+ Model string `xml:"model,attr,omitempty"`
+ BlockIO *BlockIO `xml:"blockio,omitempty"`
+ FilesystemOverhead *cdiv1.Percent `xml:"filesystemOverhead,omitempty"`
+ Capacity *int64 `xml:"capacity,omitempty"`
+ ExpandDisksEnabled bool `xml:"expandDisksEnabled,omitempty"`
}
type DiskAuth struct {
diff --git a/pkg/virt-launcher/virtwrap/converter/converter.go b/pkg/virt-launcher/virtwrap/converter/converter.go
index 9330364f4..2e6ea10d4 100644
--- a/pkg/virt-launcher/virtwrap/converter/converter.go
+++ b/pkg/virt-launcher/virtwrap/converter/converter.go
@@ -224,10 +224,7 @@ func Convert_v1_Disk_To_api_Disk(c *ConverterContext, diskDevice *v1.Disk, disk
volumeStatus, ok := volumeStatusMap[diskDevice.Name]
if ok && volumeStatus.PersistentVolumeClaimInfo != nil {
disk.FilesystemOverhead = volumeStatus.PersistentVolumeClaimInfo.FilesystemOverhead
- capacity, ok := volumeStatus.PersistentVolumeClaimInfo.Capacity[k8sv1.ResourceStorage]
- if ok {
- disk.Capacity = &capacity
- }
+ disk.Capacity = getDiskCapacity(volumeStatus.PersistentVolumeClaimInfo)
}
disk.ExpandDisksEnabled = c.ExpandDisksEnabled
}
@@ -245,6 +242,39 @@ func Convert_v1_Disk_To_api_Disk(c *ConverterContext, diskDevice *v1.Disk, disk
return nil
}
+// Get expected disk capacity - a minimum between the request and the PVC capacity.
+// Returns nil when we have insufficient data to calculate this minimum.
+func getDiskCapacity(pvcInfo *v1.PersistentVolumeClaimInfo) *int64 {
+ logger := log.DefaultLogger()
+ storageCapacityResource, ok := pvcInfo.Capacity[k8sv1.ResourceStorage]
+ if !ok {
+ return nil
+ }
+ storageCapacity, ok := storageCapacityResource.AsInt64()
+ if !ok {
+ logger.Infof("Failed to convert storage capacity %+v to int64", storageCapacityResource)
+ return nil
+ }
+ storageRequestResource, ok := pvcInfo.Requests[k8sv1.ResourceStorage]
+ if !ok {
+ return nil
+ }
+ storageRequest, ok := storageRequestResource.AsInt64()
+ if !ok {
+ logger.Infof("Failed to convert storage request %+v to int64", storageRequestResource)
+ return nil
+ }
+ preferredSize := min(storageRequest, storageCapacity)
+ return &preferredSize
+}
+
+func min(one, two int64) int64 {
+ if one < two {
+ return one
+ }
+ return two
+}
+
type DirectIOChecker interface {
CheckBlockDevice(path string) (bool, error)
CheckFile(path string) (bool, error)
diff --git a/pkg/virt-launcher/virtwrap/converter/converter_test.go b/pkg/virt-launcher/virtwrap/converter/converter_test.go
index f1c9e52e0..181cfc789 100644
--- a/pkg/virt-launcher/virtwrap/converter/converter_test.go
+++ b/pkg/virt-launcher/virtwrap/converter/converter_test.go
@@ -103,6 +103,36 @@ var _ = Describe("Converter", func() {
})
Context("with v1.Disk", func() {
+ table.DescribeTable("Should define disk capacity as the minimum of capacity and request", func(requests, capacity int64) {
+ context := &ConverterContext{}
+ v1Disk := v1.Disk{
+ Name: "myvolume",
+ DiskDevice: v1.DiskDevice{
+ Disk: &v1.DiskTarget{Bus: "virtio"},
+ },
+ }
+ apiDisk := api.Disk{}
+ devicePerBus := map[string]deviceNamer{}
+ numQueues := uint(2)
+ volumeStatusMap := make(map[string]v1.VolumeStatus)
+ volumeStatusMap["myvolume"] = v1.VolumeStatus{
+ PersistentVolumeClaimInfo: &v1.PersistentVolumeClaimInfo{
+ Capacity: k8sv1.ResourceList{
+ k8sv1.ResourceStorage: *resource.NewQuantity(capacity, resource.DecimalSI),
+ },
+ Requests: k8sv1.ResourceList{
+ k8sv1.ResourceStorage: *resource.NewQuantity(requests, resource.DecimalSI),
+ },
+ },
+ }
+ Convert_v1_Disk_To_api_Disk(context, &v1Disk, &apiDisk, devicePerBus, &numQueues, volumeStatusMap)
+ Expect(apiDisk.Capacity).ToNot(BeNil())
+ Expect(*apiDisk.Capacity).To(Equal(min(capacity, requests)))
+ },
+ table.Entry("Higher request than capacity", int64(9999), int64(1111)),
+ table.Entry("Lower request than capacity", int64(1111), int64(9999)),
+ )
+
It("Should add boot order when provided", func() {
order := uint(1)
kubevirtDisk := &v1.Disk{
diff --git a/pkg/virt-launcher/virtwrap/manager.go b/pkg/virt-launcher/virtwrap/manager.go
index 2dc8ab0cb..b81bc6c84 100644
--- a/pkg/virt-launcher/virtwrap/manager.go
+++ b/pkg/virt-launcher/virtwrap/manager.go
@@ -618,17 +618,11 @@ func expandDiskImageOffline(imagePath string, size int64) error {
}
func possibleGuestSize(disk api.Disk) (int64, bool) {
- var err error
- capacityResource := disk.Capacity
- if capacityResource == nil {
- log.DefaultLogger().Error("Failed to get storage capacity")
- return 0, false
- }
- capacity, ok := capacityResource.AsInt64()
- if !ok {
- log.DefaultLogger().Error("Failed to convert capacity to int64")
+ if disk.Capacity == nil {
+ log.DefaultLogger().Error("No disk capacity")
return 0, false
}
+ preferredSize := *disk.Capacity
if disk.FilesystemOverhead == nil {
log.DefaultLogger().Errorf("No filesystem overhead found for disk %v", disk)
return 0, false
@@ -638,7 +632,7 @@ 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(capacity)), true
+ return int64((1 - filesystemOverhead) * float64(preferredSize)), true
}
func shouldExpandOffline(disk api.Disk) bool {
diff --git a/pkg/virt-launcher/virtwrap/manager_test.go b/pkg/virt-launcher/virtwrap/manager_test.go
index 79cfcd09f..e8097b901 100644
--- a/pkg/virt-launcher/virtwrap/manager_test.go
+++ b/pkg/virt-launcher/virtwrap/manager_test.go
@@ -2250,20 +2250,21 @@ var _ = Describe("Manager helper functions", func() {
BeforeEach(func() {
fakePercentFloat = 0.7648
fakePercent := cdiv1beta1.Percent(fmt.Sprint(fakePercentFloat))
+ fakeCapacity := int64(123000)
properDisk = api.Disk{
FilesystemOverhead: &fakePercent,
- Capacity: resource.NewScaledQuantity(123, 4),
+ Capacity: &fakeCapacity,
}
})
It("should return correct value", func() {
size, ok := possibleGuestSize(properDisk)
Expect(ok).To(BeTrue())
- capacity, ok := properDisk.Capacity.AsInt64()
- Expect(ok).To(BeTrue())
+ capacity := properDisk.Capacity
+ Expect(capacity).ToNot(Equal(nil))
- expectedSize := int64((1 - fakePercentFloat) * float64(capacity))
+ expectedSize := int64((1 - fakePercentFloat) * float64(*capacity))
Expect(size).To(Equal(expectedSize))
})
@@ -2276,19 +2277,12 @@ var _ = Describe("Manager helper functions", func() {
disk.Capacity = nil
return disk
}),
- table.Entry("disk capacity non-int", func() api.Disk {
- disk := properDisk
- nonIntQuantity, ok := resource.ParseQuantity("0.456546456")
- Expect(ok).To(BeTrue())
- disk.Capacity = &nonIntQuantity
- return disk
- }),
table.Entry("filesystem overhead is nil", func() api.Disk {
disk := properDisk
disk.FilesystemOverhead = nil
return disk
}),
- table.Entry("filesystem is non-float", func() api.Disk {
+ table.Entry("filesystem overhead is non-float", func() api.Disk {
disk := properDisk
fakePercent := cdiv1beta1.Percent(fmt.Sprint("abcdefg"))
disk.FilesystemOverhead = &fakePercent
diff --git a/pkg/virt-operator/resource/generate/components/validations_generated.go b/pkg/virt-operator/resource/generate/components/validations_generated.go
index 0461a06e5..86c21f019 100644
--- a/pkg/virt-operator/resource/generate/components/validations_generated.go
+++ b/pkg/virt-operator/resource/generate/components/validations_generated.go
@@ -8842,7 +8842,7 @@ var CRDsValidation map[string]string = map[string]string{
pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$
x-kubernetes-int-or-string: true
description: Capacity represents the capacity set on the corresponding
- PVC spec
+ PVC status
type: object
filesystemOverhead:
description: Percentage of filesystem's size to be reserved when
@@ -8853,6 +8853,16 @@ var CRDsValidation map[string]string = map[string]string{
description: Preallocated indicates if the PVC's storage is preallocated
or not
type: boolean
+ requests:
+ additionalProperties:
+ anyOf:
+ - type: integer
+ - type: string
+ pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$
+ x-kubernetes-int-or-string: true
+ description: Requests represents the resources requested by the
+ corresponding PVC spec
+ type: object
volumeMode:
description: VolumeMode defines what type of volume is required
by the claim. Value of Filesystem is implied when not included
diff --git a/staging/src/kubevirt.io/api/core/v1/deepcopy_generated.go b/staging/src/kubevirt.io/api/core/v1/deepcopy_generated.go
index 7da3beaa2..62be5003e 100644
--- a/staging/src/kubevirt.io/api/core/v1/deepcopy_generated.go
+++ b/staging/src/kubevirt.io/api/core/v1/deepcopy_generated.go
@@ -2865,6 +2865,13 @@ func (in *PersistentVolumeClaimInfo) DeepCopyInto(out *PersistentVolumeClaimInfo
(*out)[key] = val.DeepCopy()
}
}
+ if in.Requests != nil {
+ in, out := &in.Requests, &out.Requests
+ *out = make(corev1.ResourceList, len(*in))
+ for key, val := range *in {
+ (*out)[key] = val.DeepCopy()
+ }
+ }
if in.FilesystemOverhead != nil {
in, out := &in.FilesystemOverhead, &out.FilesystemOverhead
*out = new(v1beta1.Percent)
diff --git a/staging/src/kubevirt.io/api/core/v1/types.go b/staging/src/kubevirt.io/api/core/v1/types.go
index 58383d064..8bb22a6b8 100644
--- a/staging/src/kubevirt.io/api/core/v1/types.go
+++ b/staging/src/kubevirt.io/api/core/v1/types.go
@@ -244,10 +244,14 @@ type PersistentVolumeClaimInfo struct {
// +optional
VolumeMode *k8sv1.PersistentVolumeMode `json:"volumeMode,omitempty"`
- // Capacity represents the capacity set on the corresponding PVC spec
+ // Capacity represents the capacity set on the corresponding PVC status
// +optional
Capacity k8sv1.ResourceList `json:"capacity,omitempty"`
+ // Requests represents the resources requested by the corresponding PVC spec
+ // +optional
+ Requests k8sv1.ResourceList `json:"requests,omitempty"`
+
// Preallocated indicates if the PVC's storage is preallocated or not
// +optional
Preallocated bool `json:"preallocated,omitempty"`
diff --git a/staging/src/kubevirt.io/api/core/v1/types_swagger_generated.go b/staging/src/kubevirt.io/api/core/v1/types_swagger_generated.go
index 0f9e81214..b83583a79 100644
--- a/staging/src/kubevirt.io/api/core/v1/types_swagger_generated.go
+++ b/staging/src/kubevirt.io/api/core/v1/types_swagger_generated.go
@@ -81,7 +81,8 @@ func (PersistentVolumeClaimInfo) SwaggerDoc() map[string]string {
"": "PersistentVolumeClaimInfo contains the relavant information virt-handler needs cached about a PVC",
"accessModes": "AccessModes contains the desired access modes the volume should have.\nMore info: https://kubernetes.io/docs/concepts/storage/persistent-volumes#access-modes-1\n+listType=atomic\n+optional",
"volumeMode": "VolumeMode defines what type of volume is required by the claim.\nValue of Filesystem is implied when not included in claim spec.\n+optional",
- "capacity": "Capacity represents the capacity set on the corresponding PVC spec\n+optional",
+ "capacity": "Capacity represents the capacity set on the corresponding PVC status\n+optional",
+ "requests": "Requests represents the resources requested by the corresponding PVC spec\n+optional",
"preallocated": "Preallocated indicates if the PVC's storage is preallocated or not\n+optional",
"filesystemOverhead": "Percentage of filesystem's size to be reserved when resizing the PVC\n+optional",
}
diff --git a/staging/src/kubevirt.io/client-go/api/openapi_generated.go b/staging/src/kubevirt.io/client-go/api/openapi_generated.go
index 8a941827f..4f86ca0dd 100644
--- a/staging/src/kubevirt.io/client-go/api/openapi_generated.go
+++ b/staging/src/kubevirt.io/client-go/api/openapi_generated.go
@@ -18298,7 +18298,21 @@ func schema_kubevirtio_api_core_v1_PersistentVolumeClaimInfo(ref common.Referenc
},
"capacity": {
SchemaProps: spec.SchemaProps{
- Description: "Capacity represents the capacity set on the corresponding PVC spec",
+ Description: "Capacity represents the capacity set on the corresponding PVC status",
+ Type: []string{"object"},
+ AdditionalProperties: &spec.SchemaOrBool{
+ Allows: true,
+ Schema: &spec.Schema{
+ SchemaProps: spec.SchemaProps{
+ Ref: ref("k8s.io/apimachinery/pkg/api/resource.Quantity"),
+ },
+ },
+ },
+ },
+ },
+ "requests": {
+ SchemaProps: spec.SchemaProps{
+ Description: "Requests represents the resources requested by the corresponding PVC spec",
Type: []string{"object"},
AdditionalProperties: &spec.SchemaOrBool{
Allows: true,
--
2.36.1
From e818c70fdd0d5757ec6b7d8a91e2fadfd8004ae3 Mon Sep 17 00:00:00 2001
From: Jed Lejosne <jed@redhat.com>
Date: Mon, 8 Nov 2021 09:04:57 -0500
Subject: [PATCH 2/3] Only create 4k-aligned disk images
Signed-off-by: Jed Lejosne <jed@redhat.com>
---
pkg/emptydisk/BUILD.bazel | 1 +
pkg/emptydisk/emptydisk.go | 10 ++++++++--
pkg/host-disk/host-disk.go | 5 ++++-
pkg/util/util.go | 14 ++++++++++++++
pkg/virt-launcher/virtwrap/manager.go | 9 +++++++--
pkg/virt-launcher/virtwrap/manager_test.go | 2 ++
tests/storage/storage.go | 8 ++++----
tests/vmi_configuration_test.go | 4 ++--
8 files changed, 42 insertions(+), 11 deletions(-)
diff --git a/pkg/emptydisk/BUILD.bazel b/pkg/emptydisk/BUILD.bazel
index 5dc6af303..c47a4e9f9 100644
--- a/pkg/emptydisk/BUILD.bazel
+++ b/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",
],
)
diff --git a/pkg/emptydisk/emptydisk.go b/pkg/emptydisk/emptydisk.go
index d8dfe47f5..7a0ba2f02 100644
--- a/pkg/emptydisk/emptydisk.go
+++ b/pkg/emptydisk/emptydisk.go
@@ -7,6 +7,7 @@ import (
"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 +20,16 @@ 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 4KiB multiple
+ intSize = util.AlignImageSizeTo4k(intSize, logger.With("volume", 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
diff --git a/pkg/host-disk/host-disk.go b/pkg/host-disk/host-disk.go
index 852e71567..2b5adb73e 100644
--- a/pkg/host-disk/host-disk.go
+++ b/pkg/host-disk/host-disk.go
@@ -99,10 +99,13 @@ func replaceForHostDisk(volumeSource *v1.VolumeSource, volumeName string, pvcVol
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 4k-aligned. If the volume specifies a misaligned size, shrink it down to the nearest multiple of 4096
+ capacity.Set(util.AlignImageSizeTo4k(capacity.Value(), log.Log.V(2)))
volumeSource.HostDisk = &v1.HostDisk{
Path: file,
Type: v1.HostDiskExistsOrCreate,
- Capacity: volumeStatus.PersistentVolumeClaimInfo.Capacity[k8sv1.ResourceStorage],
+ Capacity: capacity,
Shared: &isShared,
}
}
diff --git a/pkg/util/util.go b/pkg/util/util.go
index 60f6262d7..b2984a176 100644
--- a/pkg/util/util.go
+++ b/pkg/util/util.go
@@ -6,6 +6,7 @@ import (
"strings"
v1 "kubevirt.io/api/core/v1"
+ "kubevirt.io/client-go/log"
)
const (
@@ -144,3 +145,16 @@ func IsReadOnlyDisk(disk *v1.Disk) bool {
return isReadOnlyCDRom
}
+
+func AlignImageSizeTo4k(size int64, logger *log.FilteredLogger) int64 {
+ remainder := size % 4096
+ if remainder == 0 {
+ return size
+ } else {
+ newSize := size - remainder
+ if logger != nil {
+ logger.Warningf("new size is not 4k-aligned. Adjusting from %d down to %d.", size, newSize)
+ }
+ return newSize
+ }
+}
diff --git a/pkg/virt-launcher/virtwrap/manager.go b/pkg/virt-launcher/virtwrap/manager.go
index b81bc6c84..e8e7234a5 100644
--- a/pkg/virt-launcher/virtwrap/manager.go
+++ b/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"
@@ -609,10 +611,11 @@ func expandDiskImageOffline(imagePath string, size int64) error {
} else {
preallocateFlag = "--preallocation=off"
}
+ size = util2.AlignImageSizeTo4k(size, log.Log.With("image", 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
}
@@ -632,7 +635,9 @@ 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.AlignImageSizeTo4k(size, log.DefaultLogger())
+ return size, true
}
func shouldExpandOffline(disk api.Disk) bool {
diff --git a/pkg/virt-launcher/virtwrap/manager_test.go b/pkg/virt-launcher/virtwrap/manager_test.go
index e8097b901..75f74a772 100644
--- a/pkg/virt-launcher/virtwrap/manager_test.go
+++ b/pkg/virt-launcher/virtwrap/manager_test.go
@@ -2265,6 +2265,8 @@ var _ = Describe("Manager helper functions", func() {
Expect(capacity).ToNot(Equal(nil))
expectedSize := int64((1 - fakePercentFloat) * float64(*capacity))
+ // The size is expected to be 4k-aligned
+ expectedSize = expectedSize - expectedSize%4096
Expect(size).To(Equal(expectedSize))
})
diff --git a/tests/storage/storage.go b/tests/storage/storage.go
index a5ebb9b08..7ba436ba8 100644
--- a/tests/storage/storage.go
+++ b/tests/storage/storage.go
@@ -320,7 +320,7 @@ var _ = SIGDescribe("Storage", func() {
Name: "emptydisk1",
VolumeSource: virtv1.VolumeSource{
EmptyDisk: &virtv1.EmptyDiskSource{
- Capacity: resource.MustParse("2Gi"),
+ Capacity: resource.MustParse("1G"),
},
},
})
@@ -328,10 +328,10 @@ var _ = SIGDescribe("Storage", func() {
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: "999997440"}, // 1G in bytes rounded down to nearest 4k boundary
}, 10)).To(Succeed())
By("Checking if we can write to /dev/vdc")
@@ -645,7 +645,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: "1014685696"},
}, 200)).To(Succeed())
}
diff --git a/tests/vmi_configuration_test.go b/tests/vmi_configuration_test.go
index 64b62ec6d..05802f37f 100644
--- a/tests/vmi_configuration_test.go
+++ b/tests/vmi_configuration_test.go
@@ -2011,10 +2011,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))
}
--
2.36.1
From 0bc593aad3f1fe8ca9fd39f62e45e6511f023282 Mon Sep 17 00:00:00 2001
From: Jed Lejosne <jed@redhat.com>
Date: Tue, 15 Feb 2022 15:05:16 -0500
Subject: [PATCH 3/3] Align disks on 1MiB instead of 4KiB
Signed-off-by: Jed Lejosne <jed@redhat.com>
---
pkg/emptydisk/emptydisk.go | 8 ++++++--
pkg/host-disk/host-disk.go | 22 +++++++++++++++++-----
pkg/host-disk/host-disk_test.go | 3 +++
pkg/util/util.go | 13 ++++++++++---
pkg/virt-launcher/virtwrap/manager.go | 10 ++++++++--
pkg/virt-launcher/virtwrap/manager_test.go | 6 +++---
tests/storage/storage.go | 4 ++--
7 files changed, 49 insertions(+), 17 deletions(-)
diff --git a/pkg/emptydisk/emptydisk.go b/pkg/emptydisk/emptydisk.go
index 7a0ba2f02..ad72a7004 100644
--- a/pkg/emptydisk/emptydisk.go
+++ b/pkg/emptydisk/emptydisk.go
@@ -1,6 +1,7 @@
package emptydisk
import (
+ "fmt"
"os"
"os/exec"
"path"
@@ -26,8 +27,11 @@ func (c *emptyDiskCreator) CreateTemporaryDisks(vmi *v1.VirtualMachineInstance)
if volume.EmptyDisk != nil {
// qemu-img takes the size in bytes or in Kibibytes/Mebibytes/...; lets take bytes
intSize := volume.EmptyDisk.Capacity.ToDec().ScaledValue(0)
- // round down the size to the nearest 4KiB multiple
- intSize = util.AlignImageSizeTo4k(intSize, logger.With("volume", volume.Name))
+ // 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)
diff --git a/pkg/host-disk/host-disk.go b/pkg/host-disk/host-disk.go
index 2b5adb73e..a44231794 100644
--- a/pkg/host-disk/host-disk.go
+++ b/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,7 +90,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.DataVolume = nil
}
@@ -95,19 +101,25 @@ func ReplacePVCByHostDisk(vmi *v1.VirtualMachineInstance) error {
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 4k-aligned. If the volume specifies a misaligned size, shrink it down to the nearest multiple of 4096
- capacity.Set(util.AlignImageSizeTo4k(capacity.Value(), log.Log.V(2)))
+ // 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: capacity,
Shared: &isShared,
}
+
+ return nil
}
func shouldSkipVolumeSource(passthoughFSVolumes map[string]struct{}, hotplugVolumes map[string]bool, pvcVolume map[string]v1.VolumeStatus, volumeName string) bool {
diff --git a/pkg/host-disk/host-disk_test.go b/pkg/host-disk/host-disk_test.go
index 9fccd9be8..68f3fe8aa 100644
--- a/pkg/host-disk/host-disk_test.go
+++ b/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),
+ },
},
},
}
diff --git a/pkg/util/util.go b/pkg/util/util.go
index b2984a176..3ba7c8386 100644
--- a/pkg/util/util.go
+++ b/pkg/util/util.go
@@ -146,14 +146,21 @@ func IsReadOnlyDisk(disk *v1.Disk) bool {
return isReadOnlyCDRom
}
-func AlignImageSizeTo4k(size int64, logger *log.FilteredLogger) int64 {
- remainder := size % 4096
+// 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 {
- logger.Warningf("new size is not 4k-aligned. Adjusting from %d down to %d.", size, newSize)
+ 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
}
diff --git a/pkg/virt-launcher/virtwrap/manager.go b/pkg/virt-launcher/virtwrap/manager.go
index e8e7234a5..2276f1ea9 100644
--- a/pkg/virt-launcher/virtwrap/manager.go
+++ b/pkg/virt-launcher/virtwrap/manager.go
@@ -611,7 +611,10 @@ func expandDiskImageOffline(imagePath string, size int64) error {
} else {
preallocateFlag = "--preallocation=off"
}
- size = util2.AlignImageSizeTo4k(size, log.Log.With("image", imagePath))
+ 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 {
@@ -636,7 +639,10 @@ func possibleGuestSize(disk api.Disk) (int64, bool) {
return 0, false
}
size := int64((1 - filesystemOverhead) * float64(preferredSize))
- size = util2.AlignImageSizeTo4k(size, log.DefaultLogger())
+ size = util2.AlignImageSizeTo1MiB(size, log.DefaultLogger())
+ if size == 0 {
+ return 0, false
+ }
return size, true
}
diff --git a/pkg/virt-launcher/virtwrap/manager_test.go b/pkg/virt-launcher/virtwrap/manager_test.go
index 75f74a772..96956ba5d 100644
--- a/pkg/virt-launcher/virtwrap/manager_test.go
+++ b/pkg/virt-launcher/virtwrap/manager_test.go
@@ -2250,7 +2250,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,
@@ -2265,8 +2265,8 @@ var _ = Describe("Manager helper functions", func() {
Expect(capacity).ToNot(Equal(nil))
expectedSize := int64((1 - fakePercentFloat) * float64(*capacity))
- // The size is expected to be 4k-aligned
- expectedSize = expectedSize - expectedSize%4096
+ // The size is expected to be 1MiB-aligned
+ expectedSize = expectedSize - expectedSize%(1024*1024)
Expect(size).To(Equal(expectedSize))
})
diff --git a/tests/storage/storage.go b/tests/storage/storage.go
index 7ba436ba8..5dce49ee9 100644
--- a/tests/storage/storage.go
+++ b/tests/storage/storage.go
@@ -331,7 +331,7 @@ var _ = SIGDescribe("Storage", func() {
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: "999997440"}, // 1G in bytes rounded down to nearest 4k boundary
+ &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")
@@ -645,7 +645,7 @@ var _ = SIGDescribe("Storage", func() {
Expect(console.SafeExpectBatch(vmi, []expect.Batcher{
&expect.BSnd{S: "blockdev --getsize64 /dev/vdb\n"},
- &expect.BExp{R: "1014685696"},
+ &expect.BExp{R: "1013972992"},
}, 200)).To(Succeed())
}
--
2.36.1
