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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxx>
---
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?

Martin

    if (disk->transient) {
        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
                       _("transient disks not supported yet"));
--
1.9.3

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

Attachment: signature.asc
Description: Digital signature

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

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]