[libvirt] [PATCH 1/2] qemu: Don't mangle the storage format for type='dir'

Martin Kletzander posted 2 patches 8 years, 3 months ago
There is a newer version of this series
[libvirt] [PATCH 1/2] qemu: Don't mangle the storage format for type='dir'
Posted by Martin Kletzander 8 years, 3 months ago
Partially-resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1443434

Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
---
 src/storage/storage_source.c                       |  5 ++++
 .../qemuxml2argv-floppy-drive-noformat.args        | 24 +++++++++++++++++
 .../qemuxml2argv-floppy-drive-noformat.xml         | 31 ++++++++++++++++++++++
 tests/qemuxml2argvtest.c                           |  2 ++
 4 files changed, 62 insertions(+)
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-floppy-drive-noformat.args
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-floppy-drive-noformat.xml

diff --git a/src/storage/storage_source.c b/src/storage/storage_source.c
index b620153f1e5a..bbc5cc77be1a 100644
--- a/src/storage/storage_source.c
+++ b/src/storage/storage_source.c
@@ -527,11 +527,16 @@ virStorageFileGetMetadata(virStorageSourcePtr src,
               allow_probe, report_broken);
 
     virHashTablePtr cycle = NULL;
+    virStorageType actualType = virStorageSourceGetActualType(src);
     int ret = -1;
 
     if (!(cycle = virHashCreate(5, NULL)))
         return -1;
 
+    /* No backing chains for type='dir' */
+    if (actualType == VIR_STORAGE_TYPE_DIR)
+        return 0;
+
     if (src->format <= VIR_STORAGE_FILE_NONE)
         src->format = allow_probe ?
             VIR_STORAGE_FILE_AUTO : VIR_STORAGE_FILE_RAW;
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-floppy-drive-noformat.args b/tests/qemuxml2argvdata/qemuxml2argv-floppy-drive-noformat.args
new file mode 100644
index 000000000000..214067b50345
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-floppy-drive-noformat.args
@@ -0,0 +1,24 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/home/test \
+USER=test \
+LOGNAME=test \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu-system-i686 \
+-name QEMUGuest1 \
+-S \
+-M pc \
+-m 214 \
+-smp 1,sockets=1,cores=1,threads=1 \
+-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
+-nographic \
+-nodefaults \
+-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\
+server,nowait \
+-mon chardev=charmonitor,id=monitor,mode=readline \
+-no-acpi \
+-boot a \
+-usb \
+-drive file=fat:floppy:/var/somefiles,if=none,id=drive-fdc0-0-0,readonly=on \
+-global isa-fdc.driveA=drive-fdc0-0-0 \
+-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-floppy-drive-noformat.xml b/tests/qemuxml2argvdata/qemuxml2argv-floppy-drive-noformat.xml
new file mode 100644
index 000000000000..9d9dcfe5e603
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-floppy-drive-noformat.xml
@@ -0,0 +1,31 @@
+<domain type='qemu'>
+  <name>QEMUGuest1</name>
+  <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+  <memory unit='KiB'>219136</memory>
+  <currentMemory unit='KiB'>219136</currentMemory>
+  <vcpu placement='static'>1</vcpu>
+  <os>
+    <type arch='i686' machine='pc'>hvm</type>
+    <boot dev='fd'/>
+  </os>
+  <clock offset='utc'/>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>destroy</on_crash>
+  <devices>
+    <emulator>/usr/bin/qemu-system-i686</emulator>
+    <disk type='dir' device='floppy'>
+      <source dir='/var/somefiles'/>
+      <target dev='fda' bus='fdc'/>
+      <readonly/>
+      <address type='drive' controller='0' bus='0' target='0' unit='0'/>
+    </disk>
+    <controller type='usb' index='0'/>
+    <controller type='ide' index='0'/>
+    <controller type='fdc' index='0'/>
+    <controller type='pci' index='0' model='pci-root'/>
+    <input type='mouse' bus='ps2'/>
+    <input type='keyboard' bus='ps2'/>
+    <memballoon model='virtio'/>
+  </devices>
+</domain>
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 77cfe8d28fd1..b7cf2a22b196 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -887,6 +887,8 @@ mymain(void)
             QEMU_CAPS_DRIVE_BOOT);
     DO_TEST("floppy-drive-fat",
             QEMU_CAPS_DRIVE_BOOT);
+    DO_TEST("floppy-drive-noformat",
+            QEMU_CAPS_DRIVE_BOOT);
     DO_TEST("disk-drive-readonly-disk",
             QEMU_CAPS_NODEFCONFIG);
     DO_TEST("disk-drive-readonly-no-device",
-- 
2.14.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] qemu: Don't mangle the storage format for type='dir'
Posted by John Ferlan 8 years, 3 months ago

On 08/18/2017 11:36 AM, Martin Kletzander wrote:
> Partially-resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1443434
> 

How so?  Seeing as backing chains weren't mentioned in the bz. If I
remove the changes from this patch, the new XML tests still pass (at
least for me), so I'm curious at the relationship.

Regardless, a bit more beef to the commit message would be nice.

> Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
> ---
>  src/storage/storage_source.c                       |  5 ++++
>  .../qemuxml2argv-floppy-drive-noformat.args        | 24 +++++++++++++++++
>  .../qemuxml2argv-floppy-drive-noformat.xml         | 31 ++++++++++++++++++++++
>  tests/qemuxml2argvtest.c                           |  2 ++
>  4 files changed, 62 insertions(+)
>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-floppy-drive-noformat.args
>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-floppy-drive-noformat.xml
> 
> diff --git a/src/storage/storage_source.c b/src/storage/storage_source.c
> index b620153f1e5a..bbc5cc77be1a 100644
> --- a/src/storage/storage_source.c
> +++ b/src/storage/storage_source.c
> @@ -527,11 +527,16 @@ virStorageFileGetMetadata(virStorageSourcePtr src,
>                allow_probe, report_broken);
>  
>      virHashTablePtr cycle = NULL;
> +    virStorageType actualType = virStorageSourceGetActualType(src);
>      int ret = -1;
>  
>      if (!(cycle = virHashCreate(5, NULL)))
>          return -1;
>  
> +    /* No backing chains for type='dir' */
> +    if (actualType == VIR_STORAGE_TYPE_DIR)
> +        return 0;
> +

This would leak cycle

Reviewed-by: John Ferlan <jferlan@redhat.com>

With the obvious adjustment and commit message adjustment...

John

[...]

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list