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

[libvirt] [PATCH 2/4] qemu: refactor device modification functions



The device modify functions (Attach, Update, Detach) are rather
long, with boilerplate to obtain locks surrounding a case statement
for acting on particular device types.  In order for a future patch
to share the boilerplate and allow persistent modification, factor
out the device actions.

* src/qemu/qemu_driver.c (qemuDomainAttachDeviceLive)
(qemuDomainUpdateDeviceLive, qemuDomainDetachDeviceLive): New
helper methods.
(qemuDomainAttachDeviceFlags, qemuDomainUpdateDeviceFlags)
(qemuDomainDetachDeviceFlags): Use it to separate case statements
from locking setup.
---
 src/qemu/qemu_driver.c |  385 ++++++++++++++++++++++++++++--------------------
 1 files changed, 225 insertions(+), 160 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index ca06dd6..c1a5ebb 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -3776,72 +3776,36 @@ cleanup:
 }


+/* Helper called on active vm while job condition is held */
 static int
-qemuDomainAttachDeviceFlags(virDomainPtr dom, const char *xml,
-                            unsigned int flags)
+qemuDomainAttachDeviceLive(virDomainPtr dom, struct qemud_driver *driver,
+                           virDomainObjPtr vm, virDomainDeviceDefPtr dev,
+                           virBitmapPtr qemuCaps,
+                           bool force ATTRIBUTE_UNUSED)
 {
-    struct qemud_driver *driver = dom->conn->privateData;
-    virDomainObjPtr vm;
-    virDomainDeviceDefPtr dev = NULL;
-    virBitmapPtr qemuCaps = NULL;
     virCgroupPtr cgroup = NULL;
     int ret = -1;

-    virCheckFlags(VIR_DOMAIN_DEVICE_MODIFY_LIVE |
-                  VIR_DOMAIN_DEVICE_MODIFY_CONFIG, -1);
-    if (flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG) {
-        qemuReportError(VIR_ERR_OPERATION_INVALID,
-                        "%s", _("cannot modify the persistent configuration of a domain"));
-        return -1;
-    }
-
-    qemuDriverLock(driver);
-    vm = virDomainFindByUUID(&driver->domains, dom->uuid);
-    if (!vm) {
-        char uuidstr[VIR_UUID_STRING_BUFLEN];
-        virUUIDFormat(dom->uuid, uuidstr);
-        qemuReportError(VIR_ERR_NO_DOMAIN,
-                        _("no domain with matching uuid '%s'"), uuidstr);
-        goto cleanup;
-    }
-
-    if (qemuDomainObjBeginJobWithDriver(driver, vm) < 0)
-        goto cleanup;
-
-    if (!virDomainObjIsActive(vm)) {
-        qemuReportError(VIR_ERR_OPERATION_INVALID,
-                        "%s", _("cannot attach device on inactive domain"));
-        goto endjob;
-    }
-
-    dev = virDomainDeviceDefParse(driver->caps, vm->def, xml,
-                                  VIR_DOMAIN_XML_INACTIVE);
-    if (dev == NULL)
-        goto endjob;
-
-    if (qemuCapsExtractVersionInfo(vm->def->emulator, vm->def->os.arch,
-                                   NULL,
-                                   &qemuCaps) < 0)
-        goto endjob;
-
-    if (dev->type == VIR_DOMAIN_DEVICE_DISK) {
+    switch (dev->type) {
+    case VIR_DOMAIN_DEVICE_DISK:
         if (dev->data.disk->driverName != NULL &&
             !STREQ(dev->data.disk->driverName, "qemu")) {
             qemuReportError(VIR_ERR_INTERNAL_ERROR,
                             _("unsupported driver name '%s' for disk '%s'"),
                             dev->data.disk->driverName, dev->data.disk->src);
-            goto endjob;
+            goto cleanup;
         }

         if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_DEVICES)) {
-            if (virCgroupForDomain(driver->cgroup, vm->def->name, &cgroup, 0) !=0 ) {
+            if (virCgroupForDomain(driver->cgroup, vm->def->name,
+                                   &cgroup, 0) !=0 ) {
                 qemuReportError(VIR_ERR_INTERNAL_ERROR,
                                 _("Unable to find cgroup for %s"),
                                 vm->def->name);
-                goto endjob;
+                goto cleanup;
             }
             if (qemuSetupDiskCgroup(driver, vm, cgroup, dev->data.disk) < 0)
-                goto endjob;
+                goto cleanup;
         }

         switch (dev->data.disk->device) {
@@ -3856,69 +3820,256 @@ qemuDomainAttachDeviceFlags(virDomainPtr dom, const char *xml,
             break;

         case VIR_DOMAIN_DISK_DEVICE_DISK:
-            if (dev->data.disk->bus == VIR_DOMAIN_DISK_BUS_USB) {
+            switch (dev->data.disk->bus) {
+            case VIR_DOMAIN_DISK_BUS_USB:
                 ret = qemuDomainAttachUsbMassstorageDevice(driver, vm,
-                                                           dev->data.disk, qemuCaps);
-                if (ret == 0)
-                    dev->data.disk = NULL;
-            } else if (dev->data.disk->bus == VIR_DOMAIN_DISK_BUS_VIRTIO) {
+                                                           dev->data.disk,
+                                                           qemuCaps);
+                break;
+            case VIR_DOMAIN_DISK_BUS_VIRTIO:
                 ret = qemuDomainAttachPciDiskDevice(driver, vm,
                                                     dev->data.disk, qemuCaps);
-                if (ret == 0)
-                    dev->data.disk = NULL;
-            } else if (dev->data.disk->bus == VIR_DOMAIN_DISK_BUS_SCSI) {
+                break;
+            case VIR_DOMAIN_DISK_BUS_SCSI:
                 ret = qemuDomainAttachSCSIDisk(driver, vm,
                                                dev->data.disk, qemuCaps);
-                if (ret == 0)
-                    dev->data.disk = NULL;
-            } else {
+                break;
+            default:
                 qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
                                 _("disk bus '%s' cannot be hotplugged."),
                                 virDomainDiskBusTypeToString(dev->data.disk->bus));
-                /* fallthrough */
+                goto cleanup;
             }
+            if (ret == 0)
+                dev->data.disk = NULL;
             break;

         default:
             qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
                             _("disk device type '%s' cannot be hotplugged"),
                             virDomainDiskDeviceTypeToString(dev->data.disk->device));
-            /* Fallthrough */
-        }
-        if (ret != 0 && cgroup) {
-            if (qemuTeardownDiskCgroup(driver, vm, cgroup, dev->data.disk) < 0)
-                VIR_WARN("Failed to teardown cgroup for disk path %s",
-                         NULLSTR(dev->data.disk->src));
+            goto cleanup;
         }
-    } else if (dev->type == VIR_DOMAIN_DEVICE_CONTROLLER) {
+        break;
+    case VIR_DOMAIN_DEVICE_CONTROLLER:
         if (dev->data.controller->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI) {
             ret = qemuDomainAttachPciControllerDevice(driver, vm,
-                                                      dev->data.controller, qemuCaps);
+                                                      dev->data.controller,
+                                                      qemuCaps);
             if (ret == 0)
                 dev->data.controller = NULL;
         } else {
             qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
                             _("disk controller bus '%s' cannot be hotplugged."),
                             virDomainControllerTypeToString(dev->data.controller->type));
-            /* fallthrough */
+            goto cleanup;
         }
-    } else if (dev->type == VIR_DOMAIN_DEVICE_NET) {
+        break;
+    case VIR_DOMAIN_DEVICE_NET:
         ret = qemuDomainAttachNetDevice(dom->conn, driver, vm,
                                         dev->data.net, qemuCaps);
         if (ret == 0)
             dev->data.net = NULL;
-    } else if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV) {
+        break;
+    case VIR_DOMAIN_DEVICE_HOSTDEV:
         ret = qemuDomainAttachHostDevice(driver, vm,
                                          dev->data.hostdev, qemuCaps);
         if (ret == 0)
             dev->data.hostdev = NULL;
-    } else {
+        break;
+    default:
         qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
                         _("device type '%s' cannot be attached"),
                         virDomainDeviceTypeToString(dev->type));
+        goto cleanup;
+    }
+
+cleanup:
+    if (ret != 0 && cgroup) {
+        if (qemuTeardownDiskCgroup(driver, vm, cgroup, dev->data.disk) < 0)
+            VIR_WARN("Failed to teardown cgroup for disk path %s",
+                     NULLSTR(dev->data.disk->src));
+    }
+    if (cgroup)
+        virCgroupFree(&cgroup);
+    return ret;
+}
+
+/* Helper called on active vm while job condition is held */
+static int
+qemuDomainUpdateDeviceLive(virDomainPtr dom ATTRIBUTE_UNUSED,
+                           struct qemud_driver *driver,
+                           virDomainObjPtr vm, virDomainDeviceDefPtr dev,
+                           virBitmapPtr qemuCaps, bool force)
+{
+    virCgroupPtr cgroup = NULL;
+    int ret = -1;
+
+    switch (dev->type) {
+    case VIR_DOMAIN_DEVICE_DISK:
+        if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_DEVICES)) {
+            if (virCgroupForDomain(driver->cgroup, vm->def->name,
+                                   &cgroup, 0) !=0 ) {
+                qemuReportError(VIR_ERR_INTERNAL_ERROR,
+                                _("Unable to find cgroup for %s"),
+                                vm->def->name);
+                goto cleanup;
+            }
+            if (qemuSetupDiskCgroup(driver, vm, cgroup, dev->data.disk) < 0)
+                goto cleanup;
+        }
+
+        switch (dev->data.disk->device) {
+        case VIR_DOMAIN_DISK_DEVICE_CDROM:
+        case VIR_DOMAIN_DISK_DEVICE_FLOPPY:
+            ret = qemuDomainChangeEjectableMedia(driver, vm,
+                                                 dev->data.disk,
+                                                 qemuCaps,
+                                                 force);
+            if (ret == 0)
+                dev->data.disk = NULL;
+            break;
+
+        default:
+            qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                            _("disk bus '%s' cannot be updated."),
+                            virDomainDiskBusTypeToString(dev->data.disk->bus));
+            goto cleanup;
+        }
+        break;
+
+    case VIR_DOMAIN_DEVICE_GRAPHICS:
+        ret = qemuDomainChangeGraphics(driver, vm, dev->data.graphics);
+        break;
+
+    default:
+        qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                        _("device type '%s' cannot be updated"),
+                        virDomainDeviceTypeToString(dev->type));
+        goto cleanup;
+    }
+
+cleanup:
+    if (ret != 0 && cgroup) {
+        if (qemuTeardownDiskCgroup(driver, vm, cgroup, dev->data.disk) < 0)
+            VIR_WARN("Failed to teardown cgroup for disk path %s",
+                     NULLSTR(dev->data.disk->src));
+    }
+    if (cgroup)
+        virCgroupFree(&cgroup);
+    return ret;
+}
+
+/* Helper called on active vm while job condition is held */
+static int
+qemuDomainDetachDeviceLive(virDomainPtr dom ATTRIBUTE_UNUSED,
+                           struct qemud_driver *driver,
+                           virDomainObjPtr vm, virDomainDeviceDefPtr dev,
+                           virBitmapPtr qemuCaps,
+                           bool force ATTRIBUTE_UNUSED)
+{
+    int ret = -1;
+
+    switch (dev->type) {
+    case VIR_DOMAIN_DEVICE_DISK:
+        if (dev->data.disk->device == VIR_DOMAIN_DISK_DEVICE_DISK) {
+            switch (dev->data.disk->bus) {
+            case VIR_DOMAIN_DISK_BUS_VIRTIO:
+                ret = qemuDomainDetachPciDiskDevice(driver, vm, dev, qemuCaps);
+                break;
+            case VIR_DOMAIN_DISK_BUS_SCSI:
+                ret = qemuDomainDetachDiskDevice(driver, vm, dev, qemuCaps);
+                break;
+            case VIR_DOMAIN_DISK_BUS_USB:
+                ret = qemuDomainDetachDiskDevice(driver, vm, dev, qemuCaps);
+                break;
+            default:
+                qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                                _("This type of disk cannot be hot unplugged"));
+                goto cleanup;
+            }
+        } else {
+            qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                            _("This type of disk cannot be hot unplugged"));
+            goto cleanup;
+        }
+        break;
+    case VIR_DOMAIN_DEVICE_NET:
+        ret = qemuDomainDetachNetDevice(driver, vm, dev, qemuCaps);
+        break;
+    case VIR_DOMAIN_DEVICE_CONTROLLER:
+        if (dev->data.controller->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI) {
+            ret = qemuDomainDetachPciControllerDevice(driver, vm, dev,
+                                                      qemuCaps);
+        } else {
+            qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                            _("disk controller bus '%s' cannot be hotunplugged."),
+                            virDomainControllerTypeToString(dev->data.controller->type));
+            goto cleanup;
+        }
+        break;
+    case VIR_DOMAIN_DEVICE_HOSTDEV:
+        ret = qemuDomainDetachHostDevice(driver, vm, dev, qemuCaps);
+        break;
+    default:
+        qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                        "%s", _("This type of device cannot be hot unplugged"));
+        goto cleanup;
+    }
+
+cleanup:
+    return ret;
+}
+
+static int
+qemuDomainAttachDeviceFlags(virDomainPtr dom, const char *xml,
+                            unsigned int flags)
+{
+    struct qemud_driver *driver = dom->conn->privateData;
+    virDomainObjPtr vm;
+    virDomainDeviceDefPtr dev = NULL;
+    virBitmapPtr qemuCaps = NULL;
+    int ret = -1;
+
+    virCheckFlags(VIR_DOMAIN_DEVICE_MODIFY_LIVE |
+                  VIR_DOMAIN_DEVICE_MODIFY_CONFIG, -1);
+    if (flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG) {
+        qemuReportError(VIR_ERR_OPERATION_INVALID,
+                        "%s", _("cannot modify the persistent configuration of a domain"));
+        return -1;
+    }
+
+    qemuDriverLock(driver);
+    vm = virDomainFindByUUID(&driver->domains, dom->uuid);
+    if (!vm) {
+        char uuidstr[VIR_UUID_STRING_BUFLEN];
+        virUUIDFormat(dom->uuid, uuidstr);
+        qemuReportError(VIR_ERR_NO_DOMAIN,
+                        _("no domain with matching uuid '%s'"), uuidstr);
+        goto cleanup;
+    }
+
+    if (qemuDomainObjBeginJobWithDriver(driver, vm) < 0)
+        goto cleanup;
+
+    if (!virDomainObjIsActive(vm)) {
+        qemuReportError(VIR_ERR_OPERATION_INVALID,
+                        "%s", _("cannot attach device on inactive domain"));
         goto endjob;
     }

+    dev = virDomainDeviceDefParse(driver->caps, vm->def, xml,
+                                  VIR_DOMAIN_XML_INACTIVE);
+    if (dev == NULL)
+        goto endjob;
+
+    if (qemuCapsExtractVersionInfo(vm->def->emulator, vm->def->os.arch,
+                                   NULL,
+                                   &qemuCaps) < 0)
+        goto endjob;
+
+    ret = qemuDomainAttachDeviceLive(dom, driver, vm, dev, qemuCaps, false);
+
     /* update domain status forcibly because the domain status may be changed
      * even if we attach the device failed. For example, a new controller may
      * be created.
@@ -3931,9 +4082,6 @@ endjob:
         vm = NULL;

 cleanup:
-    if (cgroup)
-        virCgroupFree(&cgroup);
-
     qemuCapsFree(qemuCaps);
     virDomainDeviceDefFree(dev);
     if (vm)
@@ -3958,7 +4106,6 @@ qemuDomainUpdateDeviceFlags(virDomainPtr dom, const char *xml,
     virDomainObjPtr vm;
     virDomainDeviceDefPtr dev = NULL;
     virBitmapPtr qemuCaps = NULL;
-    virCgroupPtr cgroup = NULL;
     int ret = -1;
     bool force = (flags & VIR_DOMAIN_DEVICE_MODIFY_FORCE) != 0;

@@ -4002,55 +4149,7 @@ qemuDomainUpdateDeviceFlags(virDomainPtr dom, const char *xml,
                                    &qemuCaps) < 0)
         goto endjob;

-    switch (dev->type) {
-    case VIR_DOMAIN_DEVICE_DISK:
-        if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_DEVICES)) {
-            if (virCgroupForDomain(driver->cgroup, vm->def->name, &cgroup, 0) !=0 ) {
-                qemuReportError(VIR_ERR_INTERNAL_ERROR,
-                                _("Unable to find cgroup for %s"),
-                                vm->def->name);
-                goto endjob;
-            }
-            if (qemuSetupDiskCgroup(driver, vm, cgroup, dev->data.disk) < 0)
-                goto endjob;
-        }
-
-        switch (dev->data.disk->device) {
-        case VIR_DOMAIN_DISK_DEVICE_CDROM:
-        case VIR_DOMAIN_DISK_DEVICE_FLOPPY:
-            ret = qemuDomainChangeEjectableMedia(driver, vm,
-                                                 dev->data.disk,
-                                                 qemuCaps,
-                                                 force);
-            if (ret == 0)
-                dev->data.disk = NULL;
-            break;
-
-
-        default:
-            qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-                            _("disk bus '%s' cannot be updated."),
-                            virDomainDiskBusTypeToString(dev->data.disk->bus));
-            break;
-        }
-
-        if (ret != 0 && cgroup) {
-            if (qemuTeardownDiskCgroup(driver, vm, cgroup, dev->data.disk) < 0)
-                VIR_WARN("Failed to teardown cgroup for disk path %s",
-                         NULLSTR(dev->data.disk->src));
-        }
-        break;
-
-    case VIR_DOMAIN_DEVICE_GRAPHICS:
-        ret = qemuDomainChangeGraphics(driver, vm, dev->data.graphics);
-        break;
-
-    default:
-        qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-                        _("device type '%s' cannot be updated"),
-                        virDomainDeviceTypeToString(dev->type));
-        break;
-    }
+    ret = qemuDomainUpdateDeviceLive(dom, driver, vm, dev, qemuCaps, force);

     if (!ret && virDomainSaveStatus(driver->caps, driver->stateDir, vm) < 0)
         ret = -1;
@@ -4060,9 +4159,6 @@ endjob:
         vm = NULL;

 cleanup:
-    if (cgroup)
-        virCgroupFree(&cgroup);
-
     qemuCapsFree(qemuCaps);
     virDomainDeviceDefFree(dev);
     if (vm)
@@ -4120,38 +4216,7 @@ qemuDomainDetachDeviceFlags(virDomainPtr dom, const char *xml,
                                    &qemuCaps) < 0)
         goto endjob;

-    if (dev->type == VIR_DOMAIN_DEVICE_DISK &&
-        dev->data.disk->device == VIR_DOMAIN_DISK_DEVICE_DISK) {
-        if (dev->data.disk->bus == VIR_DOMAIN_DISK_BUS_VIRTIO) {
-            ret = qemuDomainDetachPciDiskDevice(driver, vm, dev, qemuCaps);
-        }
-        else if (dev->data.disk->bus == VIR_DOMAIN_DISK_BUS_SCSI) {
-            ret = qemuDomainDetachDiskDevice(driver, vm, dev, qemuCaps);
-        } else if (dev->data.disk->bus == VIR_DOMAIN_DISK_BUS_USB) {
-            ret = qemuDomainDetachDiskDevice(driver, vm, dev, qemuCaps);
-        }
-        else {
-            qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-                            _("This type of disk cannot be hot unplugged"));
-        }
-    } else if (dev->type == VIR_DOMAIN_DEVICE_NET) {
-        ret = qemuDomainDetachNetDevice(driver, vm, dev, qemuCaps);
-    } else if (dev->type == VIR_DOMAIN_DEVICE_CONTROLLER) {
-        if (dev->data.controller->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI) {
-            ret = qemuDomainDetachPciControllerDevice(driver, vm, dev,
-                                                      qemuCaps);
-        } else {
-            qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-                            _("disk controller bus '%s' cannot be hotunplugged."),
-                            virDomainControllerTypeToString(dev->data.controller->type));
-            /* fallthrough */
-        }
-    } else if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV) {
-        ret = qemuDomainDetachHostDevice(driver, vm, dev, qemuCaps);
-    } else {
-        qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-                        "%s", _("This type of device cannot be hot unplugged"));
-    }
+    ret = qemuDomainDetachDeviceLive(dom, driver, vm, dev, qemuCaps, false);

     if (!ret && virDomainSaveStatus(driver->caps, driver->stateDir, vm) < 0)
         ret = -1;
-- 
1.7.4.2


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