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

[libvirt] [PATCH] Fix ejecting cdroms with latest qemu syntax



Originally, ejecting a cdrom from a qemu guest entailed
passing 'eject cdrom' to the monitor. But since qemu
added the -drive option, more than one cdrom can be
specified, so just using 'cdrom' isn't explicit enough.

The attached patch updates media change/eject to use
the current qemu syntax. The new generated commands
look something like "eject ide0-cd1", with the name
derived from device target and bus type.

While I was in there I added support for inserting/
ejecting media from scsi cdroms and floppy devices.

This is built around my previous two patches:
- Fix cd eject segfault
- Attempt to detect cdrom change failures

Thanks,
Cole

diff --git a/src/qemu_driver.c b/src/qemu_driver.c
index 05e7402..a6fd53b 100644
--- a/src/qemu_driver.c
+++ b/src/qemu_driver.c
@@ -2948,48 +2948,107 @@ static int qemudDomainUndefine(virDomainPtr dom) {
     return 0;
 }
 
-static int qemudDomainChangeCDROM(virDomainPtr dom,
-                                  virDomainObjPtr vm,
-                                  virDomainDiskDefPtr olddisk,
-                                  virDomainDiskDefPtr newdisk) {
+static int qemudDomainChangeEjectableMedia(virDomainPtr dom,
+                                           virDomainDeviceDefPtr dev)
+{
     struct qemud_driver *driver = (struct qemud_driver *)dom->conn->privateData;
+    virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid);
+    virDomainDiskDefPtr origdisk, newdisk;
     char *cmd, *reply, *safe_path;
+    char *devname = NULL;
+    int ret;
+
+    if (!vm) {
+        qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN,
+                         "%s", _("no domain with matching uuid"));
+        return -1;
+    }
+
+    newdisk = dev->data.disk;
+    origdisk = vm->def->disks;
+    while (origdisk) {
+        if (origdisk->bus == newdisk->bus &&
+            STREQ(origdisk->dst, newdisk->dst))
+            break;
+        origdisk = origdisk->next;
+    }
+
+    if (!origdisk) {
+        qemudReportError(dom->conn, dom, NULL, VIR_ERR_INTERNAL_ERROR,
+                         _("No device with bus '%s' and target '%s'"),
+                         virDomainDiskBusTypeToString(newdisk->bus),
+                         newdisk->dst);
+        return -1;
+    }
+
+    int idx = virDiskNameToIndex(newdisk->dst);
+    switch (newdisk->bus) {
+        /* Assume that if we are here, device targets don't exceed hypervisor
+         * limits, and we are already an appropriate device type */
+        case VIR_DOMAIN_DISK_BUS_IDE:
+            /* Device name of the form 'ide{0-1}-cd{0-1}' */
+            ret = asprintf(&devname, "ide%d-cd%d", ((idx - (idx % 2)) / 2),
+                                                   (idx % 2));
+            break;
+        case VIR_DOMAIN_DISK_BUS_SCSI:
+            /* Device name of the form 'scsi{bus#}-cd{0-6}
+             * Each bus holds seven devs */
+            ret = asprintf(&devname, "scsi%d-cd%d", ((idx - (idx % 7)) / 7),
+                                                    (idx % 7));
+            break;
+        case VIR_DOMAIN_DISK_BUS_FDC:
+            /* Device name is 'floppy{0-1}' */
+            ret = asprintf(&devname, "floppy%d", idx);
+            break;
+
+        default:
+            qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_SUPPORT,
+                             _("Cannot hotplug device with bus '%s'"),
+                             virDomainDiskBusTypeToString(newdisk->bus));
+            return -1;
+    }
+
+    if (!devname || ret == -1) {
+        qemudReportError(dom->conn, NULL, NULL, VIR_ERR_NO_MEMORY, NULL);
+        return -1;
+    }
 
     if (newdisk->src) {
         safe_path = qemudEscapeMonitorArg(newdisk->src);
         if (!safe_path) {
             qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
                              "%s", _("out of memory"));
+            VIR_FREE(devname);
             return -1;
         }
-        if (asprintf (&cmd, "change %s \"%s\"",
-                      /* XXX qemu may support multiple CDROM in future */
-                      /* olddisk->dst */ "cdrom",
-                      safe_path) == -1) {
+        if (asprintf (&cmd, "change %s \"%s\"", devname, safe_path) == -1) {
             qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
                              "%s", _("out of memory"));
             VIR_FREE(safe_path);
+            VIR_FREE(devname);
             return -1;
         }
         VIR_FREE(safe_path);
 
-    } else if (asprintf(&cmd, "eject cdrom") == -1) {
+    } else if (asprintf(&cmd, "eject %s", devname) == -1) {
         qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
                          "%s", _("out of memory"));
+        VIR_FREE(devname);
         return -1;
     }
+    VIR_FREE(devname);
 
     if (qemudMonitorCommand(driver, vm, cmd, &reply) < 0) {
         qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
                          "%s", _("cannot change cdrom media"));
         VIR_FREE(cmd);
         return -1;
     }
 
     /* If the command failed qemu prints:
      * device not found, device is locked ...
      * No message is printed on success it seems */
-    DEBUG ("cdrom change reply: %s", reply);
+    DEBUG ("ejectable media change reply: %s", reply);
     if (strstr(reply, "\ndevice ")) {
         qemudReportError (dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
                           _("changing cdrom media failed: %s"), reply);
@@ -3006,49 +3065,16 @@ static int qemudDomainChangeCDROM(virDomainPtr dom,
         VIR_FREE(cmd);
         return -1;
     }
-
     VIR_FREE(reply);
     VIR_FREE(cmd);
 
-    VIR_FREE(olddisk->src);
-    olddisk->src = newdisk->src;
-    newdisk->src = NULL;
-    olddisk->type = newdisk->type;
+    VIR_FREE(origdisk->src);
+    origdisk->src = newdisk->src;
+    newdisk->src = NULL;
+    origdisk->type = newdisk->type;
     return 0;
 }
 
-static int qemudDomainAttachCdromDevice(virDomainPtr dom,
-                                        virDomainDeviceDefPtr dev)
-{
-    struct qemud_driver *driver = (struct qemud_driver *)dom->conn->privateData;
-    virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid);
-    virDomainDiskDefPtr disk;
-
-    if (!vm) {
-        qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN,
-                         "%s", _("no domain with matching uuid"));
-        return -1;
-    }
-
-    disk = vm->def->disks;
-    while (disk) {
-        if (disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM &&
-            STREQ(disk->dst, dev->data.disk->dst))
-            break;
-        disk = disk->next;
-    }
-
-    if (!disk) {
-        qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_SUPPORT,
-                         "%s", _("CDROM not attached, cannot change media"));
-        return -1;
-    }
-
-    if (qemudDomainChangeCDROM(dom, vm, disk, dev->data.disk) < 0) {
-        return -1;
-    }
-    return 0;
-}
 
 static int qemudDomainAttachUsbMassstorageDevice(virDomainPtr dom, virDomainDeviceDefPtr dev)
 {
@@ -3202,10 +3228,11 @@ static int qemudDomainAttachDevice(virDomainPtr dom,
     }
 
     if (dev->type == VIR_DOMAIN_DEVICE_DISK &&
-        dev->data.disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM) {
-                ret = qemudDomainAttachCdromDevice(dom, dev);
+        (dev->data.disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM ||
+         dev->data.disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY)) {
+                ret = qemudDomainChangeEjectableMedia(dom, dev);
     } else if (dev->type == VIR_DOMAIN_DEVICE_DISK &&
-        dev->data.disk->device == VIR_DOMAIN_DEVICE_DISK &&
+        dev->data.disk->device == VIR_DOMAIN_DISK_DEVICE_DISK &&
         dev->data.disk->bus == VIR_DOMAIN_DISK_BUS_USB) {
                 ret = qemudDomainAttachUsbMassstorageDevice(dom, dev);
     } else if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV &&

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