From 56be87fe948f7534878880cb300f91550361c062 Mon Sep 17 00:00:00 2001
Message-Id: <56be87fe948f7534878880cb300f91550361c062@dist-git>
From: Peter Krempa <pkrempa@redhat.com>
Date: Mon, 4 Feb 2019 12:53:38 +0100
Subject: [PATCH] qemu: command: Don't skip 'readonly' and throttling info for
 empty drive
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

In commit f80eae8c2ae 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>
(cherry picked from commit 6db0d033839807aec885e10b5a45748da016e261)

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

 Conflicts:
        src/qemu/qemu_command.c
        tests/qemuxml2argvdata/disk-cdrom.args
            - context: bootindex not yet assumed, 'boot=on' code still
            present
        tests/qemuxml2argvdata/disk-cdrom.x86_64-2.12.0.args
        tests/qemuxml2argvdata/disk-cdrom.x86_64-latest.args
            - missing refactor to capability file based tests

Reviewed-by: Ján Tomko <jtomko@redhat.com>
---
 src/qemu/qemu_command.c                      | 18 +++++++++++++-----
 tests/qemuxml2argvdata/disk-cdrom-empty.args |  2 +-
 tests/qemuxml2argvdata/disk-cdrom.args       |  2 +-
 3 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index e153c011f6..66abd3fe86 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -1695,10 +1695,18 @@ qemuBuildDriveStr(virDomainDiskDefPtr disk,
         disk->bus != VIR_DOMAIN_DISK_BUS_IDE)
         virBufferAddLit(&opt, ",boot=on");
 
-    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));
@@ -1723,10 +1731,10 @@ qemuBuildDriveStr(virDomainDiskDefPtr disk,
             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-empty.args b/tests/qemuxml2argvdata/disk-cdrom-empty.args
index b84bdda143..1cbc76ab86 100644
--- a/tests/qemuxml2argvdata/disk-cdrom-empty.args
+++ b/tests/qemuxml2argvdata/disk-cdrom-empty.args
@@ -24,5 +24,5 @@ server,nowait \
 -usb \
 -drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \
 -device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \
--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
diff --git a/tests/qemuxml2argvdata/disk-cdrom.args b/tests/qemuxml2argvdata/disk-cdrom.args
index f519ea5236..973d8e9af1 100644
--- a/tests/qemuxml2argvdata/disk-cdrom.args
+++ b/tests/qemuxml2argvdata/disk-cdrom.args
@@ -27,5 +27,5 @@ server,nowait \
 -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
-- 
2.20.1