[libvirt] [PATCH v3 12/13] Pass virDomainDeviceDefListPtr to hotplug functions
Nikunj A Dadhania
nikunj at linux.vnet.ibm.com
Tue May 24 14:15:40 UTC 2016
Shivaprasad G Bhat <shivaprasadbhat at gmail.com> writes:
> This actually does all the things as though there were more than one
> device in list.
>
> Signed-off-by: Shivaprasad G Bhat <sbhat at linux.vnet.ibm.com>
> ---
> 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)
devcopylist = NULL ?
> + 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)
set to null
> + 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);
free devlist ??
> 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;
Can go in the above patch where prepare is introduced.
> - 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);
unref of caps?
> + 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++)
********* -1
> + qemuHostdevReAttachPCIDevices(driver, vm->def->name,
> + &devlist->devs[j]->data.hostdev, 1);
> + }
> + ret = -1;
> + goto exit;
unref of caps?
> }
>
> 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);
>
>
>
> --
> libvir-list mailing list
> libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
More information about the libvir-list
mailing list