[libvirt] [PATCH 2/4] qemu: refactor device modification functions
Eric Blake
eblake at redhat.com
Sat Apr 16 15:28:20 UTC 2011
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
More information about the libvir-list
mailing list