[libvirt] [PATCH] qemu: raise an error when trying to use readonly ide disks

[Giuseppe Scrivano]

Martin Kletzander <mkletzan at redhat.com> writes:

Q> On Mon, Jun 30, 2014 at 12:05:06PM +0200, Giuseppe Scrivano wrote:
>>The IDE bus doesn't support readonly disks, so inform the user with an
>>error message instead of let qemu fail with a more obscure "Device
>>'ide-hd' could not be initialized" error message.
>>
>>Closes: https://bugzilla.redhat.com/show_bug.cgi?id=1112939
>>
>>Signed-off-by: Giuseppe Scrivano <gscrivan at redhat.com>
>>---
>> src/qemu/qemu_command.c | 9 ++++++++-
>> 1 file changed, 8 insertions(+), 1 deletion(-)
>>
>>diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
>>index 63f322a..4829176 100644
>>--- a/src/qemu/qemu_command.c
>>+++ b/src/qemu/qemu_command.c
>>@@ -3385,8 +3385,15 @@ qemuBuildDriveStr(virConnectPtr conn,
>>         disk->bus != VIR_DOMAIN_DISK_BUS_IDE)
>>         virBufferAddLit(&opt, ",boot=on");
>>     if (disk->readonly &&
>>-        virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE_READONLY))
>>+        virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE_READONLY)) {
>>+        if (disk->bus == VIR_DOMAIN_DISK_BUS_IDE &&
>>+            disk->device == VIR_DOMAIN_DISK_DEVICE_DISK) {
>>+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>>+                           _("readonly ide disks are not supported"));
>>+            goto error;
>>+        }
>>         virBufferAddLit(&opt, ",readonly=on");
>>+    }
>
> I'm not very sure we should do that.  Second opinion would be great.
> My point is that if qemu does not support the readonly flag, we just
> skip setting it, but if it supports it, we will error out?  OTOH
> skipping the readonly=on when user requests it is, ehm, not nice
> either.  I think Eric was saying that *some* readonly flags do not
> make much of a sense, but that was probably WRT backing chains or
> something like that.  Eric?

I don't think that this error is related to the presence of the
DRIVE_READONLY Qemu caps.  My understanding of this comment on the Qemu
bug conterpart (https://bugzilla.redhat.com/show_bug.cgi?id=915162#c1) is
that this is not going to change as readonly disks are not supported by
the IDE bus, and this can be considered just an early detection of this
situation.

Regards,
Giuseppe