qemu: command: Don't skip 'readonly' and throttling info for empty drive

In commit f80eae8c2a I was too agresive in removing properties of
-drive for empty drives. It turns out that qemu actually persists the
state of 'readonly' and the throttling information even for the empty
drive.

Removing 'readonly' thus made qemu open any subsequent images added via
the 'change' command as RW which was forbidden by selinux thanks to the
restrictive sVirt label for readonly media.

Fix this by formating the property again and bump the tests and leave a
note detailing why the rest of the properties needs to be skipped.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

src/qemu/qemu_command.c

@@ -1773,10 +1773,18 @@ qemuBuildDriveStr(virDomainDiskDefPtr disk,
         }
     }
 
-    if (!virStorageSourceIsEmpty(disk->src)) {
-        if (disk->src->readonly)
-            virBufferAddLit(&opt, ",readonly=on");
+    if (disk->src->readonly)
+        virBufferAddLit(&opt, ",readonly=on");
 
+    /* qemu rejects some parameters for an empty -drive, so we need to skip
+     * them in that case:
+     * cache: modifies properties of the format driver which is not present
+     * copy_on_read: really only works for floppies
+     * discard: modifies properties of format driver
+     * detect_zeroes: works but really depends on discard so it's useless
+     * iomode: setting it to 'native' requires a specific cache mode
+     */
+    if (!virStorageSourceIsEmpty(disk->src)) {
         if (disk->cachemode) {
             virBufferAsprintf(&opt, ",cache=%s",
                               qemuDiskCacheV2TypeToString(disk->cachemode));
@@ -1801,10 +1809,10 @@ qemuBuildDriveStr(virDomainDiskDefPtr disk,
             virBufferAsprintf(&opt, ",aio=%s",
                               virDomainDiskIoTypeToString(disk->iomode));
         }
-
-        qemuBuildDiskThrottling(disk, &opt);
     }
 
+    qemuBuildDiskThrottling(disk, &opt);
+
     if (virBufferCheckError(&opt) < 0)
         goto error;
 

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 \
+-drive if=none,id=drive-ide0-1-0,media=cdrom,readonly=on \
 -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 \
+-drive if=none,id=drive-ide0-1-1,media=cdrom,readonly=on \
 -device ide-drive,bus=ide.1,unit=1,drive=drive-ide0-1-1,id=ide0-1-1

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 \
+-drive if=none,id=drive-ide0-1-0,readonly=on \
 -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 \
+-drive if=none,id=drive-ide0-1-1,readonly=on \
 -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 \

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 \
+-drive if=none,id=drive-ide0-1-0,readonly=on \
 -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 \
+-drive if=none,id=drive-ide0-1-1,readonly=on \
 -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 \