[libvirt] [PATCH v3 12/13] Pass virDomainDeviceDefListPtr to hotplug functions

This actually does all the things as though there were more than one device in list. Signed-off-by: Shivaprasad G Bhat <sbhat(a)linux.vnet.ibm.com&gt; --- src/qemu/qemu_domain.c | 50 +++++++++++++++------- src/qemu/qemu_domain.h | 6 +-- src/qemu/qemu_driver.c | 82 +++++++++++++++++++++++------------- src/qemu/qemu_hotplug.c | 108 ++++++++++++++++++++++++++++++++++++----------- src/qemu/qemu_hotplug.h | 6 +-- 5 files changed, 176 insertions(+), 76 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index da5f97d..9f9ad3a 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -5528,14 +5528,21 @@ qemuDomainAttachDeviceConfigInternal(virQEMUCapsPtr qemuCaps, int qemuDomainAttachDeviceConfig(virQEMUCapsPtr qemuCaps, virDomainDefPtr vmdef, - virDomainDeviceDefPtr dev, + virDomainDeviceDefListPtr devlist, virConnectPtr conn) { - if (virDomainDefCompatibleDevice(vmdef, dev, - VIR_DOMAIN_DEVICE_ACTION_ATTACH) < 0) - return -1; + size_t i; + + for (i = 0; i < devlist->count; i++) + if (virDomainDefCompatibleDevice(vmdef, devlist->devs[i], + VIR_DOMAIN_DEVICE_ACTION_ATTACH) < 0) + return -1; - return qemuDomainAttachDeviceConfigInternal(qemuCaps, vmdef, dev, conn); + for (i = 0; i < devlist->count; i++) + if (qemuDomainAttachDeviceConfigInternal(qemuCaps, vmdef, devlist->devs[i], conn) < 0) + return -1; + + return 0; } @@ -5676,13 +5683,20 @@ qemuDomainDetachDeviceConfigInternal(virDomainDefPtr vmdef, int qemuDomainDetachDeviceConfig(virDomainDefPtr vmdef, - virDomainDeviceDefPtr dev) + virDomainDeviceDefListPtr devlist) { - if (virDomainDefCompatibleDevice(vmdef, dev, - VIR_DOMAIN_DEVICE_ACTION_DETACH) < 0) - return -1; + size_t i; + + for (i = 0; i < devlist->count; i++) + if (virDomainDefCompatibleDevice(vmdef, devlist->devs[i], + VIR_DOMAIN_DEVICE_ACTION_DETACH) < 0) + return -1; - return qemuDomainDetachDeviceConfigInternal(vmdef, dev); + for (i = 0; i < devlist->count; i++) + if (qemuDomainDetachDeviceConfigInternal(vmdef, devlist->devs[i]) < 0) + return -1; + + return 0; } @@ -5784,11 +5798,17 @@ qemuDomainUpdateDeviceConfigInternal(virQEMUCapsPtr qemuCaps, int qemuDomainUpdateDeviceConfig(virQEMUCapsPtr qemuCaps, virDomainDefPtr vmdef, - virDomainDeviceDefPtr dev) + virDomainDeviceDefListPtr devlist) { - if (virDomainDefCompatibleDevice(vmdef, dev, - VIR_DOMAIN_DEVICE_ACTION_UPDATE) < 0) - return -1; + size_t i; + for (i = 0; i < devlist->count; i++) + if (virDomainDefCompatibleDevice(vmdef, devlist->devs[i], + VIR_DOMAIN_DEVICE_ACTION_UPDATE) < 0) + return -1; + + for (i = 0; i < devlist->count; i++) + if (qemuDomainUpdateDeviceConfigInternal(qemuCaps, vmdef, devlist->devs[i]) < 0) + return -1; - return qemuDomainUpdateDeviceConfigInternal(qemuCaps, vmdef, dev); + return 0; } diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 82e3308..acb3c4c 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -672,18 +672,18 @@ int qemuDomainDefValidateDiskLunSource(const virStorageSource *src) int qemuDomainAttachDeviceConfig(virQEMUCapsPtr qemuCaps, virDomainDefPtr vmdef, - virDomainDeviceDefPtr dev, + virDomainDeviceDefListPtr dev, virConnectPtr conn); int qemuDomainDetachDeviceConfig(virDomainDefPtr vmdef, - virDomainDeviceDefPtr dev); + virDomainDeviceDefListPtr dev); int qemuDomainUpdateDeviceConfig(virQEMUCapsPtr qemuCaps, virDomainDefPtr vmdef, - virDomainDeviceDefPtr dev); + virDomainDeviceDefListPtr dev); #endif /* __QEMU_DOMAIN_H__ */ diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 9484576..e970ad6 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7460,7 +7460,8 @@ static int qemuDomainAttachDeviceFlags(virDomainPtr dom, const char *xml, virQEMUDriverPtr driver = dom->conn->privateData; virDomainObjPtr vm = NULL; virDomainDefPtr vmdef = NULL; - virDomainDeviceDefPtr dev = NULL, dev_copy = NULL; + virDomainDeviceDefPtr dev = NULL; + virDomainDeviceDefListPtr devlist = NULL, devcopylist = NULL; int ret = -1; unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE | VIR_DOMAIN_DEF_PARSE_ABI_UPDATE; @@ -7493,20 +7494,27 @@ static int qemuDomainAttachDeviceFlags(virDomainPtr dom, const char *xml, if (virDomainObjUpdateModificationImpact(vm, &flags) < 0) goto endjob; - dev = dev_copy = virDomainDeviceDefParse(xml, vm->def, + dev = virDomainDeviceDefParse(xml, vm->def, caps, driver->xmlopt, parse_flags); - if (dev == NULL) + if (!dev || VIR_ALLOC(devlist) < 0) goto endjob; + if (VIR_APPEND_ELEMENT(devlist->devs, devlist->count, dev) < 0) + goto endjob; + + devcopylist = devlist; + if (flags & VIR_DOMAIN_AFFECT_CONFIG && flags & VIR_DOMAIN_AFFECT_LIVE) { /* If we are affecting both CONFIG and LIVE * create a deep copy of device as adding * to CONFIG takes one instance. */ - dev_copy = virDomainDeviceDefCopy(dev, vm->def, caps, driver->xmlopt); - if (!dev_copy) + if (VIR_ALLOC(devcopylist) < 0) + goto endjob; + if (virDomainDeviceDefListAddCopy(devcopylist, devlist->devs[0], + vm->def, caps, driver->xmlopt) < 0) goto endjob; } @@ -7521,13 +7529,13 @@ static int qemuDomainAttachDeviceFlags(virDomainPtr dom, const char *xml, if (!vmdef) goto endjob; - if ((ret = qemuDomainAttachDeviceConfig(qemuCaps, vmdef, dev, + if ((ret = qemuDomainAttachDeviceConfig(qemuCaps, vmdef, devlist, dom->conn)) < 0) goto endjob; } if (flags & VIR_DOMAIN_AFFECT_LIVE) { - if ((ret = qemuDomainAttachDeviceLive(vm, dev_copy, dom)) < 0) + if ((ret = qemuDomainAttachDeviceLive(vm, devcopylist, dom)) < 0) goto endjob; /* * update domain status forcibly because the domain status may be @@ -7555,9 +7563,9 @@ static int qemuDomainAttachDeviceFlags(virDomainPtr dom, const char *xml, cleanup: virObjectUnref(qemuCaps); virDomainDefFree(vmdef); - if (dev != dev_copy) - virDomainDeviceDefFree(dev_copy); - virDomainDeviceDefFree(dev); + if (devlist != devcopylist) + virDomainDeviceDefListFree(devcopylist); + virDomainDeviceDefListFree(devlist); virDomainObjEndAPI(&vm); virObjectUnref(caps); virObjectUnref(cfg); @@ -7579,7 +7587,8 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom, virQEMUDriverPtr driver = dom->conn->privateData; virDomainObjPtr vm = NULL; virDomainDefPtr vmdef = NULL; - virDomainDeviceDefPtr dev = NULL, dev_copy = NULL; + virDomainDeviceDefPtr dev = NULL; + virDomainDeviceDefListPtr devlist = NULL, devcopylist = NULL; bool force = (flags & VIR_DOMAIN_DEVICE_MODIFY_FORCE) != 0; int ret = -1; virQEMUCapsPtr qemuCaps = NULL; @@ -7609,12 +7618,17 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom, if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) goto cleanup; - dev = dev_copy = virDomainDeviceDefParse(xml, vm->def, + dev = virDomainDeviceDefParse(xml, vm->def, caps, driver->xmlopt, VIR_DOMAIN_DEF_PARSE_INACTIVE); - if (dev == NULL) + if (!dev || VIR_ALLOC(devlist) < 0) + goto endjob; + + if (VIR_APPEND_ELEMENT(devlist->devs, devlist->count, dev) < 0) goto endjob; + devcopylist = devlist; + if (virDomainObjUpdateModificationImpact(vm, &flags) < 0) goto endjob; @@ -7624,8 +7638,10 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom, * create a deep copy of device as adding * to CONFIG takes one instance. */ - dev_copy = virDomainDeviceDefCopy(dev, vm->def, caps, driver->xmlopt); - if (!dev_copy) + if (VIR_ALLOC(devcopylist) < 0) + goto endjob; + if (virDomainDeviceDefListAddCopy(devcopylist, devlist->devs[0], + vm->def, caps, driver->xmlopt) < 0) goto endjob; } @@ -7640,12 +7656,12 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom, if (!vmdef) goto endjob; - if ((ret = qemuDomainUpdateDeviceConfig(qemuCaps, vmdef, dev)) < 0) + if ((ret = qemuDomainUpdateDeviceConfig(qemuCaps, vmdef, devlist)) < 0) goto endjob; } if (flags & VIR_DOMAIN_AFFECT_LIVE) { - if ((ret = qemuDomainUpdateDeviceLive(dom->conn, vm, dev_copy, dom, force)) < 0) + if ((ret = qemuDomainUpdateDeviceLive(dom->conn, vm, devcopylist, dom, force)) < 0) goto endjob; /* * update domain status forcibly because the domain status may be @@ -7673,9 +7689,9 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom, cleanup: virObjectUnref(qemuCaps); virDomainDefFree(vmdef); - if (dev != dev_copy) - virDomainDeviceDefFree(dev_copy); - virDomainDeviceDefFree(dev); + if (devlist != devcopylist) + virDomainDeviceDefListFree(devcopylist); + virDomainDeviceDefListFree(devlist); virDomainObjEndAPI(&vm); virObjectUnref(caps); virObjectUnref(cfg); @@ -7690,7 +7706,8 @@ static int qemuDomainDetachDeviceFlags(virDomainPtr dom, const char *xml, virQEMUDriverPtr driver = dom->conn->privateData; virDomainObjPtr vm = NULL; virDomainDefPtr vmdef = NULL; - virDomainDeviceDefPtr dev = NULL, dev_copy = NULL; + virDomainDeviceDefPtr dev = NULL; + virDomainDeviceDefListPtr devlist = NULL, devcopylist = NULL; int ret = -1; unsigned int parse_flags = 0; virQEMUCapsPtr qemuCaps = NULL; @@ -7724,20 +7741,27 @@ static int qemuDomainDetachDeviceFlags(virDomainPtr dom, const char *xml, !(flags & VIR_DOMAIN_AFFECT_LIVE)) parse_flags |= VIR_DOMAIN_DEF_PARSE_INACTIVE; - dev = dev_copy = virDomainDeviceDefParse(xml, vm->def, + dev = virDomainDeviceDefParse(xml, vm->def, caps, driver->xmlopt, parse_flags); - if (dev == NULL) + if (!dev || VIR_ALLOC(devlist) < 0) goto endjob; + if (VIR_APPEND_ELEMENT(devlist->devs, devlist->count, dev) < 0) + goto endjob; + + devcopylist = devlist; + if (flags & VIR_DOMAIN_AFFECT_CONFIG && flags & VIR_DOMAIN_AFFECT_LIVE) { /* If we are affecting both CONFIG and LIVE * create a deep copy of device as adding * to CONFIG takes one instance. */ - dev_copy = virDomainDeviceDefCopy(dev, vm->def, caps, driver->xmlopt); - if (!dev_copy) + if (VIR_ALLOC(devcopylist) < 0) + goto endjob; + if (virDomainDeviceDefListAddCopy(devcopylist, devlist->devs[0], + vm->def, caps, driver->xmlopt) < 0) goto endjob; } @@ -7752,12 +7776,12 @@ static int qemuDomainDetachDeviceFlags(virDomainPtr dom, const char *xml, if (!vmdef) goto endjob; - if ((ret = qemuDomainDetachDeviceConfig(vmdef, dev)) < 0) + if ((ret = qemuDomainDetachDeviceConfig(vmdef, devlist)) < 0) goto endjob; } if (flags & VIR_DOMAIN_AFFECT_LIVE) { - if ((ret = qemuDomainDetachDeviceLive(vm, dev_copy, dom)) < 0) + if ((ret = qemuDomainDetachDeviceLive(vm, devcopylist, dom)) < 0) goto endjob; /* * update domain status forcibly because the domain status may be @@ -7785,8 +7809,8 @@ static int qemuDomainDetachDeviceFlags(virDomainPtr dom, const char *xml, cleanup: virObjectUnref(qemuCaps); virDomainDefFree(vmdef); - if (dev != dev_copy) - virDomainDeviceDefFree(dev_copy); + if (devlist != devcopylist) + virDomainDeviceDefListFree(devcopylist); virDomainDeviceDefFree(dev); virDomainObjEndAPI(&vm); virObjectUnref(caps); diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 6821ed5..adee545 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -4384,38 +4384,81 @@ qemuDomainAttachDeviceLiveInternal(virDomainObjPtr vm, int qemuDomainAttachDeviceLive(virDomainObjPtr vm, - virDomainDeviceDefPtr dev, + virDomainDeviceDefListPtr devlist, virDomainPtr dom) { + size_t i, j, d; + virDomainDeviceDefListPtr rollbacklist = NULL; virQEMUDriverPtr driver = dom->conn->privateData; + bool pciHostdevs = false; virQEMUCapsPtr qemuCaps = NULL; qemuDomainObjPrivatePtr priv = vm->privateData; + virCapsPtr caps = NULL; + int ret = -1; + + if (!(caps = virQEMUDriverGetCapabilities(driver, false))) + return -1; if (priv->qemuCaps) qemuCaps = virObjectRef(priv->qemuCaps); else if (!(qemuCaps = virQEMUCapsCacheLookup(driver->qemuCapsCache, vm->def->emulator))) return -1; - if (virDomainDefCompatibleDevice(vm->def, dev, - VIR_DOMAIN_DEVICE_ACTION_ATTACH) < 0) - return -1; + if (devlist->devs[0]->type == VIR_DOMAIN_DEVICE_HOSTDEV && + devlist->devs[0]->data.hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) + pciHostdevs = true; - if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV && - dev->data.hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI && - qemuDomainAttachPCIHostDevicePrepare(driver, vm->def, dev->data.hostdev, qemuCaps) < 0) - return -1; + for (i = 0; i < devlist->count; i++) + if (virDomainDefCompatibleDevice(vm->def, devlist->devs[i], + VIR_DOMAIN_DEVICE_ACTION_ATTACH) < 0) + return -1; + + for (d = 0; d < devlist->count; d++) + if (pciHostdevs && + qemuDomainAttachPCIHostDevicePrepare(driver, vm->def, + devlist->devs[d]->data.hostdev, qemuCaps) < 0) + goto undoprepare; - if (qemuDomainAttachDeviceLiveInternal(vm, dev, dom) < 0) + if (VIR_ALLOC(rollbacklist) < 0) goto undoprepare; - return 0; + for (i = 0; i < devlist->count; i++) { + if (virDomainDeviceDefListAddCopy(rollbacklist, devlist->devs[i], + vm->def, caps, driver->xmlopt) < 0) + goto rollback_livehotplug; + if ((ret = qemuDomainAttachDeviceLiveInternal(vm, devlist->devs[i], dom)) < 0) + goto rollback_livehotplug; + } - undoprepare: - if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV && - dev->data.hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) - qemuHostdevReAttachPCIDevices(driver, vm->def->name, &dev->data.hostdev, 1); + ret = 0; + exit: + virDomainDeviceDefListFree(rollbacklist); + return ret; + rollback_livehotplug: + /* The last hotplug on list failed. So "count-1". + * Rollback takes care of cleaning up of any preparations done + * by the respective devices. So, not necessary to do + * any cleanup explicitly here. + */ + VIR_DELETE_ELEMENT(rollbacklist->devs, rollbacklist->count, rollbacklist->count); - return -1; + if ((qemuDomainDetachDeviceLive(vm, rollbacklist, dom)) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Couldnt revert hotplug")); + ret = -1; + goto exit; + } + + undoprepare: + if (pciHostdevs) { + /* Devices from o to rollbacklist->count are reattached as part of + * device_deleted event rest of the devices should be bound back to host + * here. 'd' is the last device we detached */ + for (j = rollbacklist ? rollbacklist->count : 0; j < d; j++) + qemuHostdevReAttachPCIDevices(driver, vm->def->name, + &devlist->devs[j]->data.hostdev, 1); + } + ret = -1; + goto exit; } static int @@ -4503,14 +4546,21 @@ qemuDomainDetachDeviceLiveInternal(virDomainObjPtr vm, int qemuDomainDetachDeviceLive(virDomainObjPtr vm, - virDomainDeviceDefPtr dev, + virDomainDeviceDefListPtr devlist, virDomainPtr dom) { - if (virDomainDefCompatibleDevice(vm->def, dev, - VIR_DOMAIN_DEVICE_ACTION_DETACH) < 0) - return -1; + size_t i; + + for (i = 0; i < devlist->count; i++) + if (virDomainDefCompatibleDevice(vm->def, devlist->devs[i], + VIR_DOMAIN_DEVICE_ACTION_DETACH) < 0) + return -1; + + for (i = 0; i < devlist->count; i++) + if (qemuDomainDetachDeviceLiveInternal(vm, devlist->devs[i], dom) < 0) + return -1; - return qemuDomainDetachDeviceLiveInternal(vm, dev, dom); + return 0; } @@ -4649,14 +4699,20 @@ qemuDomainUpdateDeviceLiveInternal(virConnectPtr conn, int qemuDomainUpdateDeviceLive(virConnectPtr conn, virDomainObjPtr vm, - virDomainDeviceDefPtr dev, + virDomainDeviceDefListPtr devlist, virDomainPtr dom, bool force) { - if (virDomainDefCompatibleDevice(vm->def, dev, - VIR_DOMAIN_DEVICE_ACTION_UPDATE) < 0) - return -1; + size_t i; + + for (i = 0; i < devlist->count; i++) + if (virDomainDefCompatibleDevice(vm->def, devlist->devs[i], + VIR_DOMAIN_DEVICE_ACTION_UPDATE) < 0) + return -1; - return qemuDomainUpdateDeviceLiveInternal(conn, vm, dev, - dom, force); + for (i = 0; i < devlist->count; i++) + if (qemuDomainUpdateDeviceLiveInternal(conn, vm, devlist->devs[i], dom, force) < 0) + return -1; + + return 0; } diff --git a/src/qemu/qemu_hotplug.h b/src/qemu/qemu_hotplug.h index 3fedd25..47a5bb4 100644 --- a/src/qemu/qemu_hotplug.h +++ b/src/qemu/qemu_hotplug.h @@ -130,19 +130,19 @@ bool qemuDomainSignalDeviceRemoval(virDomainObjPtr vm, int qemuDomainAttachDeviceLive(virDomainObjPtr vm, - virDomainDeviceDefPtr dev, + virDomainDeviceDefListPtr devlist, virDomainPtr dom); int qemuDomainUpdateDeviceLive(virConnectPtr conn, virDomainObjPtr vm, - virDomainDeviceDefPtr dev, + virDomainDeviceDefListPtr dev, virDomainPtr dom, bool force); int qemuDomainDetachDeviceLive(virDomainObjPtr vm, - virDomainDeviceDefPtr dev, + virDomainDeviceDefListPtr dev, virDomainPtr dom);