[libvirt] [PATCH 0/3] qemu: Fix handling of 'dir' volumes with disk type='volume'
Peter Krempa (3): util: storage: Fix virStorageSourceGetActualType if volume was not translated qemu: domain: Allow 'VIR_STORAGE_TYPE_VOLUME' disks with 'fat' format qemu: Supply correct default type for 'dir' based VIR_STORAGE_TYPE_VOLUME src/qemu/qemu_domain.c | 13 ++++++++++--- src/util/virstoragefile.c | 12 +++++++++++- tests/qemuxml2argvdata/disk-source-pool.args | 8 ++++++++ tests/qemuxml2argvdata/disk-source-pool.xml | 16 +++++++++++++++- tests/qemuxml2xmloutdata/disk-source-pool.xml | 14 ++++++++++++++ 5 files changed, 58 insertions(+), 5 deletions(-) -- 2.21.0
virStorageSourceGetActualType would return VIR_STORAGE_TYPE_NONE in case when a virStorageSource of (top level) type VIR_STORAGE_TYPE_VOLUME was not prepared to use by the vm by calling virDomainDiskTranslateSourcePool. Fix this issue by returning VIR_STORAGE_TYPE_VOLUME in case when the volume was not translated yet. Additionally also add documentation for the function describing the quirk. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virstoragefile.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index f7495ab6da..269d0050fd 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -2430,10 +2430,20 @@ virStorageSourcePoolDefFree(virStorageSourcePoolDefPtr def) } +/** + * virStorageSourceGetActualType: + * @def: storage source definition + * + * Returns type of @def. In case when the type is VIR_STORAGE_TYPE_VOLUME + * and virDomainDiskTranslateSourcePool was called on @def the actual type + * of the storage volume is returned rather than VIR_STORAGE_TYPE_VOLUME. + */ int virStorageSourceGetActualType(const virStorageSource *def) { - if (def->type == VIR_STORAGE_TYPE_VOLUME && def->srcpool) + if (def->type == VIR_STORAGE_TYPE_VOLUME && + def->srcpool && + def->srcpool->actualtype != VIR_STORAGE_TYPE_NONE) return def->srcpool->actualtype; return def->type; -- 2.21.0
The storage volume may in fact convert into a directory when starting the VM so that it may be actually possible to use it. This is a regression caused by c9b27af32d5 as moving the check to validation time without adjustment causes problems as the volumes are not translated yet. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.c | 1 + tests/qemuxml2argvdata/disk-source-pool.args | 4 ++++ tests/qemuxml2argvdata/disk-source-pool.xml | 10 +++++++++- tests/qemuxml2xmloutdata/disk-source-pool.xml | 7 +++++++ 4 files changed, 21 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 1326c3d6b1..5b72b7f7bf 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -5078,6 +5078,7 @@ qemuDomainValidateStorageSource(virStorageSourcePtr src, } if (src->format == VIR_STORAGE_FILE_FAT && + actualType != VIR_STORAGE_TYPE_VOLUME && actualType != VIR_STORAGE_TYPE_DIR) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("storage format 'fat' is supported only with 'dir' " diff --git a/tests/qemuxml2argvdata/disk-source-pool.args b/tests/qemuxml2argvdata/disk-source-pool.args index 676ffb5768..10e6fcf1f2 100644 --- a/tests/qemuxml2argvdata/disk-source-pool.args +++ b/tests/qemuxml2argvdata/disk-source-pool.args @@ -32,4 +32,8 @@ readonly=on \ -device ide-cd,bus=ide.1,unit=0,drive=drive-ide0-1-0,id=ide0-1-0 \ -drive file=/tmp/idedisk.img,format=raw,if=none,id=drive-ide0-0-2 \ -device ide-hd,bus=ide.0,unit=2,drive=drive-ide0-0-2,id=ide0-0-2,bootindex=1 \ +-drive file=fat:/some/dir/device/vol1,format=fat,if=none,id=drive-virtio-disk0,\ +readonly=on \ +-device virtio-blk-pci,bus=pci.0,addr=0x4,drive=drive-virtio-disk0,\ +id=virtio-disk0 \ -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvdata/disk-source-pool.xml b/tests/qemuxml2argvdata/disk-source-pool.xml index ed326d8d49..96bcae9a57 100644 --- a/tests/qemuxml2argvdata/disk-source-pool.xml +++ b/tests/qemuxml2argvdata/disk-source-pool.xml @@ -37,9 +37,17 @@ <target dev='hdd' bus='ide'/> <address type='drive' controller='0' bus='0' target='0' unit='2'/> </disk> + <disk type='volume' device='disk'> + <driver name='qemu' type='fat'/> + <source pool='pool-disk' volume='dir+vol1'/> + <target dev='vda' bus='virtio'/> + <readonly/> + </disk> <controller type='usb' index='0'/> <controller type='ide' index='0'/> <controller type='pci' index='0' model='pci-root'/> - <memballoon model='virtio'/> + <memballoon model='virtio'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </memballoon> </devices> </domain> diff --git a/tests/qemuxml2xmloutdata/disk-source-pool.xml b/tests/qemuxml2xmloutdata/disk-source-pool.xml index 567b22db84..7e3961381e 100644 --- a/tests/qemuxml2xmloutdata/disk-source-pool.xml +++ b/tests/qemuxml2xmloutdata/disk-source-pool.xml @@ -37,6 +37,13 @@ <target dev='hdd' bus='ide'/> <address type='drive' controller='0' bus='0' target='0' unit='2'/> </disk> + <disk type='volume' device='disk'> + <driver name='qemu' type='fat'/> + <source pool='pool-disk' volume='dir+vol1'/> + <target dev='vda' bus='virtio'/> + <readonly/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> + </disk> <controller type='usb' index='0'> <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> </controller> -- 2.21.0
Our code would skip adding the default type in this cases, but since we know that the only reasonable option here is 'fat' we can add it while starting the VM. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.c | 12 +++++++++--- tests/qemuxml2argvdata/disk-source-pool.args | 4 ++++ tests/qemuxml2argvdata/disk-source-pool.xml | 6 ++++++ tests/qemuxml2xmloutdata/disk-source-pool.xml | 7 +++++++ 4 files changed, 26 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 5b72b7f7bf..d71d9b3273 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -14217,10 +14217,16 @@ qemuDomainPrepareDiskSource(virDomainDiskDefPtr disk, { qemuDomainPrepareDiskCachemode(disk); - /* add raw file format if the storage pool did not fill it in */ + /* set default format for storage pool based disks */ if (disk->src->type == VIR_STORAGE_TYPE_VOLUME && - disk->src->format <= VIR_STORAGE_FILE_NONE) - disk->src->format = VIR_STORAGE_FILE_RAW; + disk->src->format <= VIR_STORAGE_FILE_NONE) { + int actualType = virStorageSourceGetActualType(disk->src); + + if (actualType == VIR_STORAGE_TYPE_DIR) + disk->src->format = VIR_STORAGE_FILE_FAT; + else + disk->src->format = VIR_STORAGE_FILE_RAW; + } if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV)) { if (qemuDomainPrepareDiskSourceBlockdev(disk, priv, cfg) < 0) diff --git a/tests/qemuxml2argvdata/disk-source-pool.args b/tests/qemuxml2argvdata/disk-source-pool.args index 10e6fcf1f2..3a3ef44506 100644 --- a/tests/qemuxml2argvdata/disk-source-pool.args +++ b/tests/qemuxml2argvdata/disk-source-pool.args @@ -36,4 +36,8 @@ readonly=on \ readonly=on \ -device virtio-blk-pci,bus=pci.0,addr=0x4,drive=drive-virtio-disk0,\ id=virtio-disk0 \ +-drive file=fat:/some/dir/device/vol2,format=fat,if=none,id=drive-virtio-disk1,\ +readonly=on \ +-device virtio-blk-pci,bus=pci.0,addr=0x5,drive=drive-virtio-disk1,\ +id=virtio-disk1 \ -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvdata/disk-source-pool.xml b/tests/qemuxml2argvdata/disk-source-pool.xml index 96bcae9a57..31b148b4bb 100644 --- a/tests/qemuxml2argvdata/disk-source-pool.xml +++ b/tests/qemuxml2argvdata/disk-source-pool.xml @@ -43,6 +43,12 @@ <target dev='vda' bus='virtio'/> <readonly/> </disk> + <disk type='volume' device='disk'> + <driver name='qemu'/> + <source pool='pool-disk' volume='dir+vol2'/> + <target dev='vdb' bus='virtio'/> + <readonly/> + </disk> <controller type='usb' index='0'/> <controller type='ide' index='0'/> <controller type='pci' index='0' model='pci-root'/> diff --git a/tests/qemuxml2xmloutdata/disk-source-pool.xml b/tests/qemuxml2xmloutdata/disk-source-pool.xml index 7e3961381e..78e0449dfd 100644 --- a/tests/qemuxml2xmloutdata/disk-source-pool.xml +++ b/tests/qemuxml2xmloutdata/disk-source-pool.xml @@ -44,6 +44,13 @@ <readonly/> <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> </disk> + <disk type='volume' device='disk'> + <driver name='qemu'/> + <source pool='pool-disk' volume='dir+vol2'/> + <target dev='vdb' bus='virtio'/> + <readonly/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x0'/> + </disk> <controller type='usb' index='0'> <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> </controller> -- 2.21.0
On Tue, Jun 25, 2019 at 15:31:40 +0200, Peter Krempa wrote:
Peter Krempa (3): util: storage: Fix virStorageSourceGetActualType if volume was not translated qemu: domain: Allow 'VIR_STORAGE_TYPE_VOLUME' disks with 'fat' format qemu: Supply correct default type for 'dir' based VIR_STORAGE_TYPE_VOLUME
src/qemu/qemu_domain.c | 13 ++++++++++--- src/util/virstoragefile.c | 12 +++++++++++- tests/qemuxml2argvdata/disk-source-pool.args | 8 ++++++++ tests/qemuxml2argvdata/disk-source-pool.xml | 16 +++++++++++++++- tests/qemuxml2xmloutdata/disk-source-pool.xml | 14 ++++++++++++++ 5 files changed, 58 insertions(+), 5 deletions(-)
SNACK, one more fix is necessary
participants (1)
-
Peter Krempa