[libvirt] [PATCH 2/2] qemu: command: Don't format image properties for empty -drive

[Peter Krempa]

If a -drive has no image, using image properties makes qemu whine that
they should not be used.

This patch stops formating cache/readonly/... for empty drives
for the pre-blockdev syntax. Unfortunately those parameters can't be
added later when inserting media, but on the other hand qemu will start
with an empty drive.

Since we already were able to start a VM with such config previously due
to qemu ignoring them I've opted just to skip formatting them.
Additionally with -blockdev support it will work as expected as the
image properties will be formatted when adding the image itself which is
not possible without it.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1651457

Signed-off-by: Peter Krempa <pkrempa at redhat.com>
---
 src/qemu/qemu_command.c                       | 47 ++++++++++---------
 tests/qemuxml2argvdata/disk-cdrom.args        |  4 +-
 .../disk-cdrom.x86_64-2.12.0.args             |  4 +-
 .../disk-cdrom.x86_64-latest.args             |  4 +-
 4 files changed, 30 insertions(+), 29 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 822d5f8669..3913ac4c15 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -1745,37 +1745,38 @@ qemuBuildDriveStr(virDomainDiskDefPtr disk,
         }
     }

-    if (disk->src->readonly)
-        virBufferAddLit(&opt, ",readonly=on");
+    if (!virStorageSourceIsEmpty(disk->src)) {
+        if (disk->src->readonly)
+            virBufferAddLit(&opt, ",readonly=on");

+        if (disk->cachemode) {
+            virBufferAsprintf(&opt, ",cache=%s",
+                              qemuDiskCacheV2TypeToString(disk->cachemode));
+        }

-    if (disk->cachemode) {
-        virBufferAsprintf(&opt, ",cache=%s",
-                          qemuDiskCacheV2TypeToString(disk->cachemode));
-    }
+        if (disk->copy_on_read) {
+            virBufferAsprintf(&opt, ",copy-on-read=%s",
+                              virTristateSwitchTypeToString(disk->copy_on_read));
+        }

-    if (disk->copy_on_read) {
-        virBufferAsprintf(&opt, ",copy-on-read=%s",
-                          virTristateSwitchTypeToString(disk->copy_on_read));
-    }
+        if (disk->discard) {
+            virBufferAsprintf(&opt, ",discard=%s",
+                              virDomainDiskDiscardTypeToString(disk->discard));
+        }

-    if (disk->discard) {
-        virBufferAsprintf(&opt, ",discard=%s",
-                          virDomainDiskDiscardTypeToString(disk->discard));
-    }
+        if (detect_zeroes) {
+            virBufferAsprintf(&opt, ",detect-zeroes=%s",
+                              virDomainDiskDetectZeroesTypeToString(detect_zeroes));
+        }

-    if (detect_zeroes) {
-        virBufferAsprintf(&opt, ",detect-zeroes=%s",
-                          virDomainDiskDetectZeroesTypeToString(detect_zeroes));
-    }
+        if (disk->iomode) {
+            virBufferAsprintf(&opt, ",aio=%s",
+                              virDomainDiskIoTypeToString(disk->iomode));
+        }

-    if (disk->iomode) {
-        virBufferAsprintf(&opt, ",aio=%s",
-                          virDomainDiskIoTypeToString(disk->iomode));
+        qemuBuildDiskThrottling(disk, &opt);
     }

-    qemuBuildDiskThrottling(disk, &opt);
-
     if (virBufferCheckError(&opt) < 0)
         goto error;

diff --git a/tests/qemuxml2argvdata/disk-cdrom.args b/tests/qemuxml2argvdata/disk-cdrom.args
index 0b62e6ee9f..a9f60aa477 100644
--- a/tests/qemuxml2argvdata/disk-cdrom.args
+++ b/tests/qemuxml2argvdata/disk-cdrom.args
@@ -27,7 +27,7 @@ bootindex=1 \
 -drive file=/root/boot.iso,format=raw,if=none,id=drive-ide0-0-1,media=cdrom,\
 readonly=on \
 -device ide-drive,bus=ide.0,unit=1,drive=drive-ide0-0-1,id=ide0-0-1 \
--drive if=none,id=drive-ide0-1-0,media=cdrom,readonly=on,cache=none \
+-drive if=none,id=drive-ide0-1-0,media=cdrom \
 -device ide-drive,bus=ide.1,unit=0,drive=drive-ide0-1-0,id=ide0-1-0 \
--drive if=none,id=drive-ide0-1-1,media=cdrom,readonly=on \
+-drive if=none,id=drive-ide0-1-1,media=cdrom \
 -device ide-drive,bus=ide.1,unit=1,drive=drive-ide0-1-1,id=ide0-1-1
diff --git a/tests/qemuxml2argvdata/disk-cdrom.x86_64-2.12.0.args b/tests/qemuxml2argvdata/disk-cdrom.x86_64-2.12.0.args
index b51c0919cc..a39d920f67 100644
--- a/tests/qemuxml2argvdata/disk-cdrom.x86_64-2.12.0.args
+++ b/tests/qemuxml2argvdata/disk-cdrom.x86_64-2.12.0.args
@@ -28,10 +28,10 @@ file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \
 -device ide-hd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=1 \
 -drive file=/root/boot.iso,format=raw,if=none,id=drive-ide0-0-1,readonly=on \
 -device ide-cd,bus=ide.0,unit=1,drive=drive-ide0-0-1,id=ide0-0-1 \
--drive if=none,id=drive-ide0-1-0,readonly=on,cache=none \
+-drive if=none,id=drive-ide0-1-0 \
 -device ide-cd,bus=ide.1,unit=0,drive=drive-ide0-1-0,id=ide0-1-0,\
 write-cache=on \
--drive if=none,id=drive-ide0-1-1,readonly=on \
+-drive if=none,id=drive-ide0-1-1 \
 -device ide-cd,bus=ide.1,unit=1,drive=drive-ide0-1-1,id=ide0-1-1 \
 -sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,\
 resourcecontrol=deny \
diff --git a/tests/qemuxml2argvdata/disk-cdrom.x86_64-latest.args b/tests/qemuxml2argvdata/disk-cdrom.x86_64-latest.args
index 8bdcffada3..029ae23dfa 100644
--- a/tests/qemuxml2argvdata/disk-cdrom.x86_64-latest.args
+++ b/tests/qemuxml2argvdata/disk-cdrom.x86_64-latest.args
@@ -28,10 +28,10 @@ file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \
 -device ide-hd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=1 \
 -drive file=/root/boot.iso,format=raw,if=none,id=drive-ide0-0-1,readonly=on \
 -device ide-cd,bus=ide.0,unit=1,drive=drive-ide0-0-1,id=ide0-0-1 \
--drive if=none,id=drive-ide0-1-0,readonly=on,cache=none \
+-drive if=none,id=drive-ide0-1-0 \
 -device ide-cd,bus=ide.1,unit=0,drive=drive-ide0-1-0,id=ide0-1-0,\
 write-cache=on \
--drive if=none,id=drive-ide0-1-1,readonly=on \
+-drive if=none,id=drive-ide0-1-1 \
 -device ide-cd,bus=ide.1,unit=1,drive=drive-ide0-1-1,id=ide0-1-1 \
 -sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,\
 resourcecontrol=deny \
-- 
2.20.1