[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