From db0a588b9498fd337b424525df05010d9729be80 Mon Sep 17 00:00:00 2001 From: Tim Serong Date: Wed, 20 Mar 2024 18:12:47 +1100 Subject: [PATCH 1/4] Set bus and disk type correctly when creating VMs from ISO images The onImageChange() function in edit/kubevirt.io.virtualmachine/VirtualMachineVolume/type/vmImage.vue includes logic to set type="cd-rom" and bus="sata" when you select an ISO image as the source for a volume. This commit applies the same logic when initially creating a VM from a given image. Signed-off-by: Tim Serong --- pkg/harvester/mixins/harvester-vm/index.js | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/pkg/harvester/mixins/harvester-vm/index.js b/pkg/harvester/mixins/harvester-vm/index.js index ea31a5a3213..380ca00d767 100644 --- a/pkg/harvester/mixins/harvester-vm/index.js +++ b/pkg/harvester/mixins/harvester-vm/index.js @@ -412,15 +412,25 @@ export default { let out = []; if (_disks.length === 0) { + let bus = 'virtio'; + let type = HARD_DISK; + + const imageResource = this.images.find( I => this.imageId === I.id); + + if (/iso$/i.test(imageResource?.imageSuffix)) { + bus = 'sata'; + type = CD_ROM; + } + out.push({ id: randomStr(5), source: SOURCE_TYPE.IMAGE, name: 'disk-0', accessMode: 'ReadWriteMany', - bus: 'virtio', + bus, volumeName: '', size: '10Gi', - type: HARD_DISK, + type, storageClassName: '', image: this.imageId, volumeMode: 'Block', From 90e65358d6ecbf10b32cdf2f7e7c539a5c8924c3 Mon Sep 17 00:00:00 2001 From: Tim Serong Date: Wed, 20 Mar 2024 19:03:30 +1100 Subject: [PATCH 2/4] Always set VM volume bus and disk type when changing image type The onImageChange() function sets the VM volume type="cd-rom" and bus="sata" when an ISO image is selected, but it only works for the first volume. This commit enables the automatic type and bus selection for all volumes attached to a VM. Related issue: https://github.com/harvester/harvester/issues/5142 Signed-off-by: Tim Serong --- .../VirtualMachineVolume/type/vmImage.vue | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/pkg/harvester/edit/kubevirt.io.virtualmachine/VirtualMachineVolume/type/vmImage.vue b/pkg/harvester/edit/kubevirt.io.virtualmachine/VirtualMachineVolume/type/vmImage.vue index 3f5f9e094a3..be545945fcf 100644 --- a/pkg/harvester/edit/kubevirt.io.virtualmachine/VirtualMachineVolume/type/vmImage.vue +++ b/pkg/harvester/edit/kubevirt.io.virtualmachine/VirtualMachineVolume/type/vmImage.vue @@ -141,14 +141,12 @@ export default { onImageChange() { const imageResource = this.$store.getters['harvester/all'](HCI.IMAGE)?.find( I => this.value.image === I.id); - if (this.idx === 0) { - if (/iso$/i.test(imageResource?.imageSuffix)) { - this.$set(this.value, 'type', 'cd-rom'); - this.$set(this.value, 'bus', 'sata'); - } else { - this.$set(this.value, 'type', 'disk'); - this.$set(this.value, 'bus', 'virtio'); - } + if (/iso$/i.test(imageResource?.imageSuffix)) { + this.$set(this.value, 'type', 'cd-rom'); + this.$set(this.value, 'bus', 'sata'); + } else { + this.$set(this.value, 'type', 'disk'); + this.$set(this.value, 'bus', 'virtio'); } this.update(); From ec15f49975d452332e7d99ab6688de102c8805b2 Mon Sep 17 00:00:00 2001 From: Tim Serong Date: Wed, 20 Mar 2024 19:53:02 +1100 Subject: [PATCH 3/4] Take image virtual size into account when creating VMs Prior to this, the default size for volumes attached to VMs was 10GiB, regardless of the size of the underlying image. This is fine if the image is smaller, but if the image is either physically larger (as in the case of a >10GiB ISO), or has a larger virtual size (as can be true with qcow2 images), the result is a corrupt volume. This commit fixes the problem by checking both the physical size and virtual size of the image, and using whichever is greater as the default size of the volume. Virtual size should always be >= physical size, but it is still theoretically possible for either value to be zero in case of error, so taking the greater of the two is the safest approach. There are two exceptions to the above rule: 1) For non-ISO images, if the image is < 10GiB, we still default to a size of 10GiB for consistency with the behaviour of earlier Harvester releases. The user can always lower this if they want/need to. 2) ISO images smaller than 1GiB will be given a size of 1GiB, simply because the size control in the UI is set to work in GiB units. It is actually possible to get a value of 0.2 GiB in there for a 200MiB ISO, but doing that is pretty ugly IMO, and there's no real harm in having the volume that backs a CD-ROM image be a bit larger than the source in this case. Related issue: https://github.com/harvester/harvester/issues/4905 Related issue: https://github.com/harvester/harvester/issues/2189 Signed-off-by: Tim Serong --- .../VirtualMachineVolume/type/vmImage.vue | 13 ++++++++++++- pkg/harvester/mixins/harvester-vm/index.js | 16 ++++++++++++++-- 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/pkg/harvester/edit/kubevirt.io.virtualmachine/VirtualMachineVolume/type/vmImage.vue b/pkg/harvester/edit/kubevirt.io.virtualmachine/VirtualMachineVolume/type/vmImage.vue index be545945fcf..68b46bb2881 100644 --- a/pkg/harvester/edit/kubevirt.io.virtualmachine/VirtualMachineVolume/type/vmImage.vue +++ b/pkg/harvester/edit/kubevirt.io.virtualmachine/VirtualMachineVolume/type/vmImage.vue @@ -140,8 +140,10 @@ export default { onImageChange() { const imageResource = this.$store.getters['harvester/all'](HCI.IMAGE)?.find( I => this.value.image === I.id); + const isIsoImage = /iso$/i.test(imageResource?.imageSuffix); + const imageSize = Math.max(imageResource?.status?.size, imageResource?.status?.virtualSize); - if (/iso$/i.test(imageResource?.imageSuffix)) { + if (isIsoImage) { this.$set(this.value, 'type', 'cd-rom'); this.$set(this.value, 'bus', 'sata'); } else { @@ -149,6 +151,15 @@ export default { this.$set(this.value, 'bus', 'virtio'); } + if (imageSize) { + let imageSizeGiB = Math.ceil(imageSize / 1024 / 1024 / 1024); + + if (!isIsoImage) { + imageSizeGiB = Math.max(imageSizeGiB, 10); + } + this.$set(this.value, 'size', `${ imageSizeGiB }Gi`); + } + this.update(); }, diff --git a/pkg/harvester/mixins/harvester-vm/index.js b/pkg/harvester/mixins/harvester-vm/index.js index 380ca00d767..17f0b729c6c 100644 --- a/pkg/harvester/mixins/harvester-vm/index.js +++ b/pkg/harvester/mixins/harvester-vm/index.js @@ -414,14 +414,26 @@ export default { if (_disks.length === 0) { let bus = 'virtio'; let type = HARD_DISK; + let size = '10Gi'; const imageResource = this.images.find( I => this.imageId === I.id); + const isIsoImage = /iso$/i.test(imageResource?.imageSuffix); + const imageSize = Math.max(imageResource?.status?.size, imageResource?.status?.virtualSize); - if (/iso$/i.test(imageResource?.imageSuffix)) { + if (isIsoImage) { bus = 'sata'; type = CD_ROM; } + if (imageSize) { + let imageSizeGiB = Math.ceil(imageSize / 1024 / 1024 / 1024); + + if (!isIsoImage) { + imageSizeGiB = Math.max(imageSizeGiB, 10); + } + size = `${ imageSizeGiB }Gi`; + } + out.push({ id: randomStr(5), source: SOURCE_TYPE.IMAGE, @@ -429,7 +441,7 @@ export default { accessMode: 'ReadWriteMany', bus, volumeName: '', - size: '10Gi', + size, type, storageClassName: '', image: this.imageId, From a0cc74ce44d3aacf42bf85014c56a45645431be0 Mon Sep 17 00:00:00 2001 From: Tim Serong Date: Wed, 20 Mar 2024 22:00:55 +1100 Subject: [PATCH 4/4] Add Virtual Size to Image List and Image Detail page This commit adds Virtual Size as an additional column when listing images. I don't know whether or not this is the best way to present this information. Another alternative could be to make the existing Size column simply report the greater of Size and Virtual Size, on the assumption that what you really care about is the size that a volume needs to be when based on that image. I've also added Virtual Size to the Image Detail page. Related issue: https://github.com/harvester/harvester/issues/4905 Signed-off-by: Tim Serong --- pkg/harvester/config/harvester.js | 2 ++ pkg/harvester/config/table-headers.js | 8 ++++++++ .../harvesterhci.io.virtualmachineimage/index.vue | 10 ++++++++++ pkg/harvester/l10n/en-us.yaml | 2 ++ .../models/harvesterhci.io.virtualmachineimage.js | 15 +++++++++++++++ 5 files changed, 37 insertions(+) diff --git a/pkg/harvester/config/harvester.js b/pkg/harvester/config/harvester.js index 3d54eb0c1e8..4cb4ecee538 100644 --- a/pkg/harvester/config/harvester.js +++ b/pkg/harvester/config/harvester.js @@ -28,6 +28,7 @@ import { import { IMAGE_DOWNLOAD_SIZE, + IMAGE_VIRTUAL_SIZE, FINGERPRINT, IMAGE_PROGRESS, SNAPSHOT_TARGET_VOLUME, @@ -220,6 +221,7 @@ export function init($plugin, store) { NAMESPACE_COL, IMAGE_PROGRESS, IMAGE_DOWNLOAD_SIZE, + IMAGE_VIRTUAL_SIZE, AGE ]); virtualType({ diff --git a/pkg/harvester/config/table-headers.js b/pkg/harvester/config/table-headers.js index ecd7afa808c..fa0c42bfcb4 100644 --- a/pkg/harvester/config/table-headers.js +++ b/pkg/harvester/config/table-headers.js @@ -11,6 +11,14 @@ export const IMAGE_DOWNLOAD_SIZE = { width: 120 }; +export const IMAGE_VIRTUAL_SIZE = { + name: 'virtualSize', + labelKey: 'harvester.tableHeaders.virtualSize', + value: 'virtualSize', + sort: 'status.virtualSize', + width: 120 +}; + export const IMAGE_PROGRESS = { name: 'Uploaded', labelKey: 'tableHeaders.progress', diff --git a/pkg/harvester/detail/harvesterhci.io.virtualmachineimage/index.vue b/pkg/harvester/detail/harvesterhci.io.virtualmachineimage/index.vue index fcb523d3a15..98677d4e27c 100644 --- a/pkg/harvester/detail/harvesterhci.io.virtualmachineimage/index.vue +++ b/pkg/harvester/detail/harvesterhci.io.virtualmachineimage/index.vue @@ -35,6 +35,10 @@ export default { return this.value?.downSize; }, + virtualSize() { + return this.value?.virtualSize; + }, + url() { return this.value?.spec?.url || '-'; }, @@ -100,6 +104,12 @@ export default { +
+
+ +
+
+
diff --git a/pkg/harvester/l10n/en-us.yaml b/pkg/harvester/l10n/en-us.yaml index 1907d6e0ba9..d4e1523f07c 100644 --- a/pkg/harvester/l10n/en-us.yaml +++ b/pkg/harvester/l10n/en-us.yaml @@ -159,6 +159,7 @@ harvester: forceStop: Force Stop tableHeaders: size: Size + virtualSize: Virtual Size progress: Progress message: Message phase: Phase @@ -696,6 +697,7 @@ harvester: basics: Basics url: URL size: Size + virtualSize: Virtual Size urlTip: 'supports the raw and qcow2 image formats which are supported by qemu. Bootable ISO images can also be used and are treated like raw images.' fileName: File Name uploadFile: Upload File diff --git a/pkg/harvester/models/harvesterhci.io.virtualmachineimage.js b/pkg/harvester/models/harvesterhci.io.virtualmachineimage.js index 47d9cb2e6d5..5480a74c675 100644 --- a/pkg/harvester/models/harvesterhci.io.virtualmachineimage.js +++ b/pkg/harvester/models/harvesterhci.io.virtualmachineimage.js @@ -168,6 +168,21 @@ export default class HciVmImage extends HarvesterResource { }); } + get virtualSize() { + const virtualSize = this.status?.virtualSize; + + if (!virtualSize) { + return '-'; + } + + return formatSi(virtualSize, { + increment: 1024, + maxPrecision: 2, + suffix: 'B', + firstSuffix: 'B', + }); + } + getStatusConditionOfType(type, defaultValue = []) { const conditions = Array.isArray(get(this, 'status.conditions')) ? this.status.conditions : defaultValue;