[libvirt] [PATCH v2 0/4] qemu: Fix handling of 'dir' volumes with disk type='volume'
Peter Krempa (4): util: storage: Fix virStorageSourceGetActualType if volume was not translated qemu: command: Use 'actualType' when deciding whether to use disk format 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_command.c | 2 +- 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 ++++++++++++++ 6 files changed, 59 insertions(+), 6 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
On Tue, Jun 25, 2019 at 04:30:30PM +0200, Peter Krempa wrote:
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(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
qemuBuildDriveSourceStr omits the disk format string when we are emulating a 'fat' filesystem froma directory. The logic should decide based on the 'actualType' as a disk type=pool may be converted to a directory. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_command.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index b767a1e15f..c04bba4f02 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1730,7 +1730,7 @@ qemuBuildDriveSourceStr(virDomainDiskDefPtr disk, } if (disk->src->format > 0 && - disk->src->type != VIR_STORAGE_TYPE_DIR) { + actualType != VIR_STORAGE_TYPE_DIR) { const char *qemuformat = virStorageFileFormatTypeToString(disk->src->format); if (rawluks) qemuformat = "luks"; -- 2.21.0
On Tue, Jun 25, 2019 at 04:30:31PM +0200, Peter Krempa wrote:
qemuBuildDriveSourceStr omits the disk format string when we are emulating a 'fat' filesystem froma directory. The logic should decide
from a
based on the 'actualType' as a disk type=pool may be converted to a directory.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_command.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
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..65565fe49e 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,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
On Tue, Jun 25, 2019 at 04:30:32PM +0200, Peter Krempa wrote:
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(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
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 65565fe49e..7c05599822 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,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 04:30:33PM +0200, Peter Krempa wrote:
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(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
participants (2)
-
Ján Tomko -
Peter Krempa