Re: [libvirt] [PATCH v2] qemu: Allow to attach/detach controller device persistently

* src/conf/domain_conf.c: - Add virDomainControllerFind to find controller device by type and index. - Add virDomainControllerRemove to remove the controller device from maintained controler list. * src/conf/domain_conf.h: - Declare the two new helpers. * src/libvirt_private.syms: - Expose private symbols for the two new helpers. * src/qemu/qemu_driver.c: - Support attach/detach controller device persistently * src/qemu/qemu_hotplug.c: - Use the two helpers to simplify the codes. v1 - v2: - Allow to detach the controller too. --- src/conf/domain_conf.c | 37 ++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 3 +- src/libvirt_private.syms | 2 + src/qemu/qemu_driver.c | 35 ++++++++++++++++++++++++++++++++- src/qemu/qemu_hotplug.c | 47 ++++++++++++--------------------------------- 5 files changed, 87 insertions(+), 37 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 41726ff..a7ae604 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -7554,6 +7554,43 @@ void virDomainControllerInsertPreAlloced(virDomainDefPtr def, def->ncontrollers++; } +int +virDomainControllerFind(virDomainDefPtr def, + int type, int idx) +{ + int i; + + for (i = 0 ; i< def->ncontrollers ; i++) { + if ((def->controllers[i]->type == type)&& + (def->controllers[i]->idx == idx)) { + return i; + } + } + + return -1; +} + +virDomainControllerDefPtr +virDomainControllerRemove(virDomainDefPtr def, size_t i) +{ + virDomainControllerDefPtr controller = def->controllers[i]; + + if (def->ncontrollers> 1) { + memmove(def->controllers + i, + def->controllers + i + 1, + sizeof(*def->controllers) * + (def->ncontrollers - (i + 1))); + def->ncontrollers--; + if (VIR_REALLOC_N(def->controllers, def->ncontrollers)< 0) { + /* ignore, harmless */ + } + } else { + VIR_FREE(def->controllers); + def->ncontrollers = 0; + } + + return controller; +} int virDomainLeaseIndex(virDomainDefPtr def, virDomainLeaseDefPtr lease) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 469d3b6..b102a43 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2043,7 +2043,8 @@ int virDomainControllerInsert(virDomainDefPtr def, virDomainControllerDefPtr controller); void virDomainControllerInsertPreAlloced(virDomainDefPtr def, virDomainControllerDefPtr controller); - +int virDomainControllerFind(virDomainDefPtr def, int type, int idx); +virDomainControllerDefPtr virDomainControllerRemove(virDomainDefPtr def, size_t i); int virDomainLeaseIndex(virDomainDefPtr def, virDomainLeaseDefPtr lease); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 734c881..9f14077 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -266,12 +266,14 @@ virDomainClockOffsetTypeFromString; virDomainClockOffsetTypeToString; virDomainConfigFile; virDomainControllerDefFree; +virDomainControllerFind; virDomainControllerInsert; virDomainControllerInsertPreAlloced; virDomainControllerModelSCSITypeFromString; virDomainControllerModelSCSITypeToString; virDomainControllerModelUSBTypeFromString; virDomainControllerModelUSBTypeToString; +virDomainControllerRemove; virDomainControllerTypeToString; virDomainCpuPlacementModeTypeFromString; virDomainCpuPlacementModeTypeToString; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 6cf3882..2d9c3a9 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5535,6 +5535,7 @@ qemuDomainAttachDeviceConfig(virDomainDefPtr vmdef, virDomainNetDefPtr net; virDomainHostdevDefPtr hostdev; virDomainLeaseDefPtr lease; + virDomainControllerDefPtr controller; switch (dev->type) { case VIR_DOMAIN_DEVICE_DISK: @@ -5607,6 +5608,23 @@ qemuDomainAttachDeviceConfig(virDomainDefPtr vmdef, dev->data.lease = NULL; break; + case VIR_DOMAIN_DEVICE_CONTROLLER: + controller = dev->data.controller; + if (virDomainControllerFind(vmdef, controller->type, + controller->idx)> 0) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("Target already exists")); + return -1; + } + + if (virDomainControllerInsert(vmdef, controller)< 0) + return -1; + dev->data.controller = NULL; + + if (qemuDomainAssignAddresses(vmdef, NULL, NULL)< 0) + return -1; + break; + default: virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("persistent attach of device is not supported")); @@ -5624,6 +5642,8 @@ qemuDomainDetachDeviceConfig(virDomainDefPtr vmdef, virDomainNetDefPtr net, det_net; virDomainHostdevDefPtr hostdev, det_hostdev; virDomainLeaseDefPtr lease, det_lease; + virDomainControllerDefPtr cont, det_cont; + int idx; switch (dev->type) { case VIR_DOMAIN_DEVICE_DISK: @@ -5650,8 +5670,6 @@ qemuDomainDetachDeviceConfig(virDomainDefPtr vmdef, break; case VIR_DOMAIN_DEVICE_HOSTDEV: { - int idx; - hostdev = dev->data.hostdev; if ((idx = virDomainHostdevFind(vmdef, hostdev,&det_hostdev))< 0) { virReportError(VIR_ERR_INVALID_ARG, "%s", @@ -5674,6 +5692,19 @@ qemuDomainDetachDeviceConfig(virDomainDefPtr vmdef, virDomainLeaseDefFree(det_lease); break; + case VIR_DOMAIN_DEVICE_CONTROLLER: + cont = dev->data.controller; + if ((idx = virDomainControllerFind(vmdef, cont->type, + cont->idx))< 0) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("device not present in domain configuration")); + return -1; + } + det_cont = virDomainControllerRemove(vmdef, idx); + virDomainControllerDefFree(det_cont); + + break; + default: virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("persistent detach of device is not supported")); diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 7880606..c3ac938 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -308,21 +308,17 @@ int qemuDomainAttachPciControllerDevice(struct qemud_driver *driver, virDomainObjPtr vm, virDomainControllerDefPtr controller) { - int i; int ret = -1; const char* type = virDomainControllerTypeToString(controller->type); char *devstr = NULL; qemuDomainObjPrivatePtr priv = vm->privateData; bool releaseaddr = false; - for (i = 0 ; i< vm->def->ncontrollers ; i++) { - if ((vm->def->controllers[i]->type == controller->type)&& - (vm->def->controllers[i]->idx == controller->idx)) { - virReportError(VIR_ERR_OPERATION_FAILED, - _("target %s:%d already exists"), - type, controller->idx); - return -1; - } + if (virDomainControllerFind(vm->def, controller->type, controller->idx)> 0) { + virReportError(VIR_ERR_OPERATION_FAILED, + _("target %s:%d already exists"), + type, controller->idx); + return -1; } if (qemuCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) { @@ -1874,19 +1870,13 @@ int qemuDomainDetachPciControllerDevice(struct qemud_driver *driver, virDomainObjPtr vm, virDomainDeviceDefPtr dev) { - int i, ret = -1; + int idx, ret = -1; virDomainControllerDefPtr detach = NULL; qemuDomainObjPrivatePtr priv = vm->privateData; - for (i = 0 ; i< vm->def->ncontrollers ; i++) { - if ((vm->def->controllers[i]->type == dev->data.controller->type)&& - (vm->def->controllers[i]->idx == dev->data.controller->idx)) { - detach = vm->def->controllers[i]; - break; - } - } - - if (!detach) { + if ((idx = virDomainControllerFind(vm->def, + dev->data.controller->type, + dev->data.controller->idx))< 0) { virReportError(VIR_ERR_OPERATION_FAILED, _("disk controller %s:%d not found"), virDomainControllerTypeToString(dev->data.controller->type), @@ -1894,6 +1884,8 @@ int qemuDomainDetachPciControllerDevice(struct qemud_driver *driver, goto cleanup; } + detach = vm->def->controllers[idx]; + if (!virDomainDeviceAddressIsValid(&detach->info, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI)) { virReportError(VIR_ERR_OPERATION_FAILED, "%s", @@ -1934,27 +1926,14 @@ int qemuDomainDetachPciControllerDevice(struct qemud_driver *driver, } qemuDomainObjExitMonitorWithDriver(driver, vm); - if (vm->def->ncontrollers> 1) { - memmove(vm->def->controllers + i, - vm->def->controllers + i + 1, - sizeof(*vm->def->controllers) * - (vm->def->ncontrollers - (i + 1))); - vm->def->ncontrollers--; - if (VIR_REALLOC_N(vm->def->controllers, vm->def->ncontrollers)< 0) { - /* ignore, harmless */ - } - } else { - VIR_FREE(vm->def->controllers); - vm->def->ncontrollers = 0; - } + virDomainControllerRemove(vm->def, idx); + virDomainControllerDefFree(detach); if (qemuCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)&& qemuDomainPCIAddressReleaseSlot(priv->pciaddrs, detach->info.addr.pci.slot)< 0) VIR_WARN("Unable to release PCI address on controller"); - virDomainControllerDefFree(detach); - ret = 0; cleanup: