Sign Up
Log In
Log In
or
Sign Up
Places
All Projects
Status Monitor
Collapse sidebar
SUSE:SLE-15-SP4:Update
kubevirt.26647
0004-only-create-1MiB-aligned-disk-images.patch
Overview
Repositories
Revisions
Requests
Users
Attributes
Meta
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
Locations
Projects
Search
Status Monitor
Help
OpenBuildService.org
Documentation
API Documentation
Code of Conduct
Contact
Support
@OBShq
Terms
openSUSE Build Service is sponsored by
The Open Build Service is an
openSUSE project
.
Sign Up
Log In
Places
Places
All Projects
Status Monitor
