At 04/16/2011 11:28 PM, Eric Blake Write:
> There's still work to add persistent callback functions, and to
> make sure this all works, but this demonstrates how having a
> single function makes it easy to support flags for all three
> types of device modifications.
>
> * src/qemu/qemu_driver.c (qemuDomainModifyDeviceFlags): Add
> parameter, and support VIR_DOMAIN_DEVICE_MODIFY_CURRENT.
> (qemuDomainAttachDeviceFlags, qemuDomainUpdateDeviceFlags)
> (qemuDomainDetachDeviceFlags): Update callers.
> ---
>
> After this point, we can use Kame's patch 1/4 to add in the
> persistent function callbacks (with slight tweaks to their
> signatures), and 2/4 to make it easier to to temporary
> modifications to a config:
>
> if (flags & CONFIG) {
> create temporary def
> call persistent callback
> }
> if (flags & LIVE) {
> call live callback
> }
> if (no errors)
> commit temporary def and live state changes, as needed
>
> But with the benefit that CURRENT support is now provided.
>
> src/qemu/qemu_driver.c | 47 +++++++++++++++++++++++++++++++++++------------
> 1 files changed, 35 insertions(+), 12 deletions(-)
>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 4f0a057..8c978be 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -4031,7 +4031,8 @@ cleanup:
> static int
> qemuDomainModifyDeviceFlags(virDomainPtr dom, const char *xml,
> unsigned int flags,
> - qemuDomainModifyDeviceCallback cb)
> + qemuDomainModifyDeviceCallback live,
> + qemuDomainModifyDeviceCallback persistent)
> {
> struct qemud_driver *driver = dom->conn->privateData;
> virDomainObjPtr vm;
> @@ -4039,12 +4040,7 @@ qemuDomainModifyDeviceFlags(virDomainPtr dom, const char *xml,
> virBitmapPtr qemuCaps = NULL;
> int ret = -1;
> bool force = (flags & VIR_DOMAIN_DEVICE_MODIFY_FORCE) != 0;
> -
> - if (flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG) {
> - qemuReportError(VIR_ERR_OPERATION_INVALID, "%s",
> - _("cannot modify domain persistent configuration"));
> - return -1;
> - }
> + bool active;
>
> qemuDriverLock(driver);
> vm = virDomainFindByUUID(&driver->domains, dom->uuid);
> @@ -4059,12 +4055,32 @@ qemuDomainModifyDeviceFlags(virDomainPtr dom, const char *xml,
> if (qemuDomainObjBeginJobWithDriver(driver, vm) < 0)
> goto cleanup;
>
> - if (!virDomainObjIsActive(vm)) {
> + active = virDomainObjIsActive(vm);
> +
> + if ((flags & (VIR_DOMAIN_DEVICE_MODIFY_LIVE
> + | VIR_DOMAIN_DEVICE_MODIFY_CONFIG)) == 0)
> + flags |= (active ? VIR_DOMAIN_DEVICE_MODIFY_LIVE
> + : VIR_DOMAIN_DEVICE_MODIFY_CONFIG);
> +
> + if ((flags & VIR_DOMAIN_DEVICE_MODIFY_LIVE) && !active) {
> qemuReportError(VIR_ERR_OPERATION_INVALID,
> "%s", _("cannot modify device on inactive domain"));
> goto endjob;
> }
>
> + if ((flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG) && !vm->persistent) {
> + qemuReportError(VIR_ERR_OPERATION_INVALID,
> + "%s", _("cannot modify device on transient domain"));
> + goto endjob;
> + }
> +
> + /* XXX add persistent support */
> + if (flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG) {
kamezawa-san is writing patch to support persistent device modification(attach/detach)
and Hu Tao is writing patch to support persistent device modification(update).
We can detect whether persistent device modification is supported like this:
if ((flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG) && !persistent) {
So the caller can pass NULL when persistent device modification is not supported safely.
> + qemuReportError(VIR_ERR_OPERATION_INVALID, "%s",
> + _("cannot modify domain persistent configuration"));
> + return -1;
We should goto endjob to do some cleanup.
> + }
> +
> dev = virDomainDeviceDefParse(driver->caps, vm->def, xml,
> VIR_DOMAIN_XML_INACTIVE);
> if (dev == NULL)
> @@ -4075,7 +4091,11 @@ qemuDomainModifyDeviceFlags(virDomainPtr dom, const char *xml,
> &qemuCaps) < 0)
> goto endjob;
>
> - ret = (cb)(dom, driver, vm, dev, qemuCaps, force);
> + ret = 0;
> + if (flags & VIR_DOMAIN_DEVICE_MODIFY_LIVE)
> + ret = (live)(dom, driver, vm, dev, qemuCaps, force);
> + if (ret == 0 && (flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG))
> + ret = (persistent)(dom, driver, vm, dev, qemuCaps, force);
>
> /* update domain status forcibly because the domain status may be changed
> * even if we attach the device failed. For example, a new controller may
> @@ -4105,7 +4125,8 @@ qemuDomainAttachDeviceFlags(virDomainPtr dom, const char *xml,
> virCheckFlags(VIR_DOMAIN_DEVICE_MODIFY_LIVE |
> VIR_DOMAIN_DEVICE_MODIFY_CONFIG, -1);
> return qemuDomainModifyDeviceFlags(dom, xml, flags,
> - qemuDomainAttachDeviceLive);
> + qemuDomainAttachDeviceLive,
> + NULL);
> }
>
> static int
> @@ -4124,7 +4145,8 @@ qemuDomainUpdateDeviceFlags(virDomainPtr dom, const char *xml,
> VIR_DOMAIN_DEVICE_MODIFY_CONFIG |
> VIR_DOMAIN_DEVICE_MODIFY_FORCE, -1);
> return qemuDomainModifyDeviceFlags(dom, xml, flags,
> - qemuDomainUpdateDeviceLive);
> + qemuDomainUpdateDeviceLive,
> + NULL);
> }
>
>
> @@ -4135,7 +4157,8 @@ qemuDomainDetachDeviceFlags(virDomainPtr dom, const char *xml,
> virCheckFlags(VIR_DOMAIN_DEVICE_MODIFY_LIVE |
> VIR_DOMAIN_DEVICE_MODIFY_CONFIG, -1);
> return qemuDomainModifyDeviceFlags(dom, xml, flags,
> - qemuDomainDetachDeviceLive);
> + qemuDomainDetachDeviceLive,
> + NULL);
> }
>
> static int
The other modification and the other 3 patches look good to me.
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list