Re: [libvirt] PATCH: 28/28: Remove unused param in QEMU monitor APIs

Sunday, 30 November 2008

The QEMU driver internal API for interacting with the monitor console takes
a 'struct qemud_driver *' parameter. This is a problem because any access
to this struct must be protected by a lock, but we do not want to hold the
global driver lock for all monitor operations since it would destroy any
hope of concurrency. Fortunately tracing down many levels of call reveals
this parameter is totally unused, so this patch removes it from a tonne of
internal APIs

 qemu_driver.c |  109 ++++++++++++++++++++++++++--------------------------------
 1 file changed, 49 insertions(+), 60 deletions(-)

Daniel

diff --git a/src/qemu_driver.c b/src/qemu_driver.c
--- a/src/qemu_driver.c
+++ b/src/qemu_driver.c
@@ -136,8 +136,7 @@ static void qemudShutdownVMDaemon(virCon
 
 static int qemudDomainGetMaxVcpus(virDomainPtr dom);
 
-static int qemudMonitorCommand (const struct qemud_driver *driver,
-                                const virDomainObjPtr vm,
+static int qemudMonitorCommand (const virDomainObjPtr vm,
                                 const char *cmd,
                                 char **reply);
 
@@ -386,14 +385,12 @@ qemudShutdown(void) {
 
 /* Return -1 for error, 1 to continue reading and 0 for success */
 typedef int qemudHandlerMonitorOutput(virConnectPtr conn,
-                                      struct qemud_driver *driver,
                                       virDomainObjPtr vm,
                                       const char *output,
                                       int fd);
 
 static int
 qemudReadMonitorOutput(virConnectPtr conn,
-                       struct qemud_driver *driver,
                        virDomainObjPtr vm,
                        int fd,
                        char *buf,
@@ -452,7 +449,7 @@ qemudReadMonitorOutput(virConnectPtr con
         } else {
             got += ret;
             buf[got] = '\0';
-            if ((ret = func(conn, driver, vm, buf, fd)) != 1)
+            if ((ret = func(conn, vm, buf, fd)) != 1)
                 return ret;
         }
     }
@@ -466,7 +463,6 @@ qemudReadMonitorOutput(virConnectPtr con
 
 static int
 qemudCheckMonitorPrompt(virConnectPtr conn ATTRIBUTE_UNUSED,
-                        struct qemud_driver *driver ATTRIBUTE_UNUSED,
                         virDomainObjPtr vm,
                         const char *output,
                         int fd)
@@ -480,7 +476,6 @@ qemudCheckMonitorPrompt(virConnectPtr co
 }
 
 static int qemudOpenMonitor(virConnectPtr conn,
-                            struct qemud_driver *driver,
                             virDomainObjPtr vm,
                             const char *monitor) {
     int monfd;
@@ -504,7 +499,7 @@ static int qemudOpenMonitor(virConnectPt
     }
 
     ret = qemudReadMonitorOutput(conn,
-                                 driver, vm, monfd,
+                                 vm, monfd,
                                  buf, sizeof(buf),
                                  qemudCheckMonitorPrompt,
                                  "monitor");
@@ -564,7 +559,6 @@ static int qemudExtractMonitorPath(virCo
 
 static int
 qemudFindCharDevicePTYs(virConnectPtr conn,
-                        struct qemud_driver *driver,
                         virDomainObjPtr vm,
                         const char *output,
                         int fd ATTRIBUTE_UNUSED)
@@ -603,7 +597,7 @@ qemudFindCharDevicePTYs(virConnectPtr co
     }
 
     /* Got them all, so now open the monitor console */
-    ret = qemudOpenMonitor(conn, driver, vm, monitor);
+    ret = qemudOpenMonitor(conn, vm, monitor);
 
 cleanup:
     VIR_FREE(monitor);
@@ -611,11 +605,10 @@ cleanup:
 }
 
 static int qemudWaitForMonitor(virConnectPtr conn,
-                               struct qemud_driver *driver,
                                virDomainObjPtr vm) {
     char buf[1024]; /* Plenty of space to get startup greeting */
     int ret = qemudReadMonitorOutput(conn,
-                                     driver, vm, vm->stderr_fd,
+                                     vm, vm->stderr_fd,
                                      buf, sizeof(buf),
                                      qemudFindCharDevicePTYs,
                                      "console");
@@ -632,7 +625,6 @@ static int qemudWaitForMonitor(virConnec
 
 static int
 qemudDetectVcpuPIDs(virConnectPtr conn,
-                    struct qemud_driver *driver,
                     virDomainObjPtr vm) {
     char *qemucpus = NULL;
     char *line;
@@ -656,7 +648,7 @@ qemudDetectVcpuPIDs(virConnectPtr conn,
         return 0;
     }
 
-    if (qemudMonitorCommand(driver, vm, "info cpus", &qemucpus) < 0) {
+    if (qemudMonitorCommand(vm, "info cpus", &qemucpus) < 0) {
         qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
                          "%s", _("cannot run monitor command to fetch CPU
thread info"));
         VIR_FREE(vm->vcpupids);
@@ -732,7 +724,6 @@ error:
 
 static int
 qemudInitCpus(virConnectPtr conn,
-              struct qemud_driver *driver,
               virDomainObjPtr vm,
               const char *migrateFrom) {
     char *info = NULL;
@@ -772,7 +763,7 @@ qemudInitCpus(virConnectPtr conn,
 
     if (migrateFrom == NULL) {
         /* Allow the CPUS to start executing */
-        if (qemudMonitorCommand(driver, vm, "cont", &info) < 0) {
+        if (qemudMonitorCommand(vm, "cont", &info) < 0) {
             qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
                              "%s", _("resume operation failed"));
             return -1;
@@ -999,9 +990,9 @@ static int qemudStartVMDaemon(virConnect
                                                    VIR_EVENT_HANDLE_HANGUP,
                                                    qemudDispatchVMEvent,
                                                    driver, NULL)) < 0) ||
-            (qemudWaitForMonitor(conn, driver, vm) < 0) ||
-            (qemudDetectVcpuPIDs(conn, driver, vm) < 0) ||
-            (qemudInitCpus(conn, driver, vm, migrateFrom) < 0)) {
+            (qemudWaitForMonitor(conn, vm) < 0) ||
+            (qemudDetectVcpuPIDs(conn, vm) < 0) ||
+            (qemudInitCpus(conn, vm, migrateFrom) < 0)) {
             qemudShutdownVMDaemon(conn, driver, vm);
             return -1;
         }
@@ -1143,8 +1134,7 @@ cleanup:
 }
 
 static int
-qemudMonitorCommand (const struct qemud_driver *driver ATTRIBUTE_UNUSED,
-                     const virDomainObjPtr vm,
+qemudMonitorCommand (const virDomainObjPtr vm,
                      const char *cmd,
                      char **reply) {
     int size = 0;
@@ -1683,7 +1673,7 @@ static int qemudDomainSuspend(virDomainP
         goto cleanup;
     }
     if (vm->state != VIR_DOMAIN_PAUSED) {
-        if (qemudMonitorCommand(driver, vm, "stop", &info) < 0) {
+        if (qemudMonitorCommand(vm, "stop", &info) < 0) {
             qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
                              "%s", _("suspend operation failed"));
             goto cleanup;
@@ -1732,7 +1722,7 @@ static int qemudDomainResume(virDomainPt
         goto cleanup;
     }
     if (vm->state == VIR_DOMAIN_PAUSED) {
-        if (qemudMonitorCommand(driver, vm, "cont", &info) < 0) {
+        if (qemudMonitorCommand(vm, "cont", &info) < 0) {
             qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
                              "%s", _("resume operation failed"));
             goto cleanup;
@@ -1774,7 +1764,7 @@ static int qemudDomainShutdown(virDomain
         goto cleanup;
     }
 
-    if (qemudMonitorCommand(driver, vm, "system_powerdown", &info) < 0)
{
+    if (qemudMonitorCommand(vm, "system_powerdown", &info) < 0) {
         qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
                          "%s", _("shutdown operation failed"));
         goto cleanup;
@@ -2171,7 +2161,7 @@ static int qemudDomainSave(virDomainPtr 
         goto cleanup;
     }
 
-    if (qemudMonitorCommand(driver, vm, command, &info) < 0) {
+    if (qemudMonitorCommand(vm, command, &info) < 0) {
         qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
                          "%s", _("migrate operation failed"));
         goto cleanup;
@@ -2541,7 +2531,7 @@ static int qemudDomainRestore(virConnect
     /* If it was running before, resume it now. */
     if (header.was_running) {
         char *info;
-        if (qemudMonitorCommand(driver, vm, "cont", &info) < 0) {
+        if (qemudMonitorCommand(vm, "cont", &info) < 0) {
             qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED,
                              "%s", _("failed to resume domain"));
             goto cleanup;
@@ -2821,7 +2811,6 @@ static char *qemudDiskDeviceName(const v
 }
 
 static int qemudDomainChangeEjectableMedia(virConnectPtr conn,
-                                           struct qemud_driver *driver,
                                            virDomainObjPtr vm,
                                            virDomainDeviceDefPtr dev)
 {
@@ -2905,7 +2894,7 @@ static int qemudDomainChangeEjectableMed
     }
     VIR_FREE(devname);
 
-    if (qemudMonitorCommand(driver, vm, cmd, &reply) < 0) {
+    if (qemudMonitorCommand(vm, cmd, &reply) < 0) {
         qemudReportError(conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
                          "%s", _("cannot change cdrom media"));
         VIR_FREE(cmd);
@@ -2934,7 +2923,6 @@ static int qemudDomainChangeEjectableMed
 }
 
 static int qemudDomainAttachPciDiskDevice(virConnectPtr conn,
-                                          struct qemud_driver *driver,
                                           virDomainObjPtr vm,
                                           virDomainDeviceDefPtr dev)
 {
@@ -2971,7 +2959,7 @@ static int qemudDomainAttachPciDiskDevic
         return ret;
     }
 
-    if (qemudMonitorCommand(driver, vm, cmd, &reply) < 0) {
+    if (qemudMonitorCommand(vm, cmd, &reply) < 0) {
         qemudReportError(conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
                          _("cannot attach %s disk"), type);
         VIR_FREE(cmd);
@@ -3006,7 +2994,6 @@ static int qemudDomainAttachPciDiskDevic
 }
 
 static int qemudDomainAttachUsbMassstorageDevice(virConnectPtr conn,
-                                                 struct qemud_driver *driver,
                                                  virDomainObjPtr vm,
                                                  virDomainDeviceDefPtr dev)
 {
@@ -3041,7 +3028,7 @@ static int qemudDomainAttachUsbMassstora
         return ret;
     }
 
-    if (qemudMonitorCommand(driver, vm, cmd, &reply) < 0) {
+    if (qemudMonitorCommand(vm, cmd, &reply) < 0) {
         qemudReportError(conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
                          "%s", _("cannot attach usb disk"));
         VIR_FREE(cmd);
@@ -3070,7 +3057,6 @@ static int qemudDomainAttachUsbMassstora
 }
 
 static int qemudDomainAttachHostDevice(virConnectPtr conn,
-                                       struct qemud_driver *driver,
                                        virDomainObjPtr vm,
                                        virDomainDeviceDefPtr dev)
 {
@@ -3096,7 +3082,7 @@ static int qemudDomainAttachHostDevice(v
         return -1;
     }
 
-    if (qemudMonitorCommand(driver, vm, cmd, &reply) < 0) {
+    if (qemudMonitorCommand(vm, cmd, &reply) < 0) {
         qemudReportError(conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
                          "%s", _("cannot attach usb device"));
         VIR_FREE(cmd);
@@ -3132,12 +3118,14 @@ static int qemudDomainAttachDevice(virDo
     qemuDriverLock(driver);
     vm = virDomainFindByUUID(&driver->domains, dom->uuid);
     if (!vm) {
-        qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN,
-                         "%s", _("no domain with matching uuid"));
-        goto cleanup;
-    }
-
-    if (!virDomainIsActive(vm)) {
+        qemuDriverUnlock(driver);
+        qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN,
+                         "%s", _("no domain with matching uuid"));
+        goto cleanup;
+    }
+
+    if (!virDomainIsActive(vm)) {
+        qemuDriverUnlock(driver);
         qemudReportError(dom->conn, dom, NULL, VIR_ERR_INTERNAL_ERROR,
                          "%s", _("cannot attach device on inactive
domain"));
         goto cleanup;
@@ -3146,6 +3134,7 @@ static int qemudDomainAttachDevice(virDo
     dev = virDomainDeviceDefParse(dom->conn,
                                   driver->caps,
                                   vm->def, xml);
+    qemuDriverUnlock(driver);
     if (dev == NULL)
         goto cleanup;
 
@@ -3154,14 +3143,14 @@ static int qemudDomainAttachDevice(virDo
         switch (dev->data.disk->device) {
         case VIR_DOMAIN_DISK_DEVICE_CDROM:
         case VIR_DOMAIN_DISK_DEVICE_FLOPPY:
-            ret = qemudDomainChangeEjectableMedia(dom->conn, driver, vm, dev);
+            ret = qemudDomainChangeEjectableMedia(dom->conn, vm, dev);
             break;
         case VIR_DOMAIN_DISK_DEVICE_DISK:
             if (dev->data.disk->bus == VIR_DOMAIN_DISK_BUS_USB) {
-                ret = qemudDomainAttachUsbMassstorageDevice(dom->conn, driver, vm,
dev);
+                ret = qemudDomainAttachUsbMassstorageDevice(dom->conn, vm, dev);
             } else if (dev->data.disk->bus == VIR_DOMAIN_DISK_BUS_SCSI ||
                        dev->data.disk->bus == VIR_DOMAIN_DISK_BUS_VIRTIO) {
-                ret = qemudDomainAttachPciDiskDevice(dom->conn, driver, vm, dev);
+                ret = qemudDomainAttachPciDiskDevice(dom->conn, vm, dev);
             }
             break;
         default:
@@ -3172,7 +3161,7 @@ static int qemudDomainAttachDevice(virDo
     } else if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV &&
                dev->data.hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS
&&
                dev->data.hostdev->source.subsys.type ==
VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB) {
-        ret = qemudDomainAttachHostDevice(dom->conn, driver, vm, dev);
+        ret = qemudDomainAttachHostDevice(dom->conn, vm, dev);
     } else {
         qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_SUPPORT,
                          "%s", _("this device type cannot be
attached"));
@@ -3183,12 +3172,10 @@ cleanup:
     virDomainDeviceDefFree(dev);
     if (vm)
         virDomainObjUnlock(vm);
-    qemuDriverUnlock(driver);
     return ret;
 }
 
 static int qemudDomainDetachPciDiskDevice(virConnectPtr conn,
-                                          struct qemud_driver *driver,
                                           virDomainObjPtr vm, virDomainDeviceDefPtr dev)
 {
     int i, ret = -1;
@@ -3222,7 +3209,7 @@ static int qemudDomainDetachPciDiskDevic
         goto cleanup;
     }
 
-    if (qemudMonitorCommand(driver, vm, cmd, &reply) < 0) {
+    if (qemudMonitorCommand(vm, cmd, &reply) < 0) {
         qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED,
                           _("failed to execute detach disk %s command"),
detach->dst);
         goto cleanup;
@@ -3268,18 +3255,21 @@ static int qemudDomainDetachDevice(virDo
     qemuDriverLock(driver);
     vm = virDomainFindByUUID(&driver->domains, dom->uuid);
     if (!vm) {
-        qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN,
-                         "%s", _("no domain with matching uuid"));
-        goto cleanup;
-    }
-
-    if (!virDomainIsActive(vm)) {
+        qemuDriverUnlock(driver);
+        qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN,
+                         "%s", _("no domain with matching uuid"));
+        goto cleanup;
+    }
+
+    if (!virDomainIsActive(vm)) {
+        qemuDriverUnlock(driver);
         qemudReportError(dom->conn, dom, NULL, VIR_ERR_INTERNAL_ERROR,
                          "%s", _("cannot attach device on inactive
domain"));
         goto cleanup;
     }
 
     dev = virDomainDeviceDefParse(dom->conn, driver->caps, vm->def, xml);
+    qemuDriverUnlock(driver);
     if (dev == NULL)
         goto cleanup;
 
@@ -3288,7 +3278,7 @@ static int qemudDomainDetachDevice(virDo
         dev->data.disk->device == VIR_DOMAIN_DISK_DEVICE_DISK &&
         (dev->data.disk->bus == VIR_DOMAIN_DISK_BUS_SCSI ||
          dev->data.disk->bus == VIR_DOMAIN_DISK_BUS_VIRTIO))
-        ret = qemudDomainDetachPciDiskDevice(dom->conn, driver, vm, dev);
+        ret = qemudDomainDetachPciDiskDevice(dom->conn, vm, dev);
     else
         qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_SUPPORT,
                          "%s", _("only SCSI or virtio disk device can be
detached dynamically"));
@@ -3297,7 +3287,6 @@ cleanup:
     virDomainDeviceDefFree(dev);
     if (vm)
         virDomainObjUnlock(vm);
-    qemuDriverUnlock(driver);
     return ret;
 }
 
@@ -3444,7 +3433,7 @@ qemudDomainBlockStats (virDomainPtr dom,
         goto cleanup;
     len = strlen (qemu_dev_name);
 
-    if (qemudMonitorCommand (driver, vm, "info blockstats", &info) < 0)
{
+    if (qemudMonitorCommand (vm, "info blockstats", &info) < 0) {
         qemudReportError (dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
                           "%s", _("'info blockstats' command
failed"));
         goto cleanup;
@@ -3710,7 +3699,7 @@ qemudDomainMemoryPeek (virDomainPtr dom,
 
     /* Issue the memsave command. */
     snprintf (cmd, sizeof cmd, "memsave %llu %zi \"%s\"", offset,
size, tmp);
-    if (qemudMonitorCommand (driver, vm, cmd, &info) < 0) {
+    if (qemudMonitorCommand (vm, cmd, &info) < 0) {
         qemudReportError (dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
                           "%s", _("'memsave' command
failed"));
         goto cleanup;
@@ -4028,7 +4017,7 @@ qemudDomainMigratePerform (virDomainPtr 
     if (!(flags & VIR_MIGRATE_LIVE)) {
         /* Pause domain for non-live migration */
         snprintf(cmd, sizeof cmd, "%s", "stop");
-        qemudMonitorCommand (driver, vm, cmd, &info);
+        qemudMonitorCommand (vm, cmd, &info);
         DEBUG ("stop reply: %s", info);
         VIR_FREE(info);
 
@@ -4043,7 +4032,7 @@ qemudDomainMigratePerform (virDomainPtr 
     if (resource > 0) {
         /* Issue migrate_set_speed command.  Don't worry if it fails. */
         snprintf (cmd, sizeof cmd, "migrate_set_speed %lum", resource);
-        qemudMonitorCommand (driver, vm, cmd, &info);
+        qemudMonitorCommand (vm, cmd, &info);
 
         DEBUG ("migrate_set_speed reply: %s", info);
         VIR_FREE (info);
@@ -4059,7 +4048,7 @@ qemudDomainMigratePerform (virDomainPtr 
     snprintf (cmd, sizeof cmd, "migrate \"%s\"", safe_uri);
     VIR_FREE (safe_uri);
 
-    if (qemudMonitorCommand (driver, vm, cmd, &info) < 0) {
+    if (qemudMonitorCommand (vm, cmd, &info) < 0) {
         qemudReportError (dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
                           "%s", _("migrate operation failed"));
         goto cleanup;


-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|


    

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

2008

2007

2006

2005

Re: [libvirt] PATCH: 28/28: Remove unused param in QEMU monitor APIs