Skip to content

Commit

Permalink
Take image virtual size into account when creating VMs
Browse files Browse the repository at this point in the history
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: harvester/harvester#4905
Related issue: harvester/harvester#2189

Signed-off-by: Tim Serong <tserong@suse.com>
(cherry picked from commit ec15f49)
  • Loading branch information
tserong authored and a110605 committed Aug 16, 2024
1 parent d3bbeb1 commit 0760a51
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -140,15 +140,26 @@ 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 {
this.$set(this.value, 'type', 'disk');
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();
},
Expand Down
16 changes: 14 additions & 2 deletions pkg/harvester/mixins/harvester-vm/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -414,22 +414,34 @@ 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,
name: 'disk-0',
accessMode: 'ReadWriteMany',
bus,
volumeName: '',
size: '10Gi',
size,
type,
storageClassName: '',
image: this.imageId,
Expand Down

0 comments on commit 0760a51

Please sign in to comment.