[libvirt] [PATCH] qemu: Set domain def as updated and transient if changes

As qemu driver doesn't allow to make changes on persistent domain configuration via "attach/detach/update device", and all the changes made on the running domain configuration should not be persistent across next boot (without the need of restarting libvirtd), so: 1) Set the running domain def as transient, and restore the domain configuration to original configuration when shutdown. 2) Set the running domain def as updated, and reset it as not updated when shutdown. Also for "live VCPU set", it doesn't change the persistent domain configuration, so, we also set the running domain def as updated and transient, and restore the original def when shutdown. * src/qemu/qemu_driver.c (qemudDomainSetVcpusFlags, qemudDomainAttachDevice, qemuDomainUpdateDeviceFlags, qemudDomainDetachDevice) --- src/qemu/qemu_driver.c | 45 +++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 45 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 1b86b5e..7825e2b 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4369,11 +4369,18 @@ retry: VIR_FREE(priv->vcpupids); priv->nvcpupids = 0; + /* Restore original domain def, so that changes on running domain def + * will not be persistent across next boot + */ if (vm->newDef) { virDomainDefFree(vm->def); vm->def = vm->newDef; vm->def->id = -1; vm->newDef = NULL; + + /* Now set domain def as not updated */ + if (vm->updated) + vm->updated = 0; } if (orig_err) { @@ -6353,6 +6360,17 @@ qemudDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, if (flags & VIR_DOMAIN_VCPU_CONFIG) ret = virDomainSaveConfig(driver->configDir, persistentDef); + if (flags & VIR_DOMAIN_VCPU_LIVE) { + /* Set running domain def as updated */ + if (ret == 0) { + vm->updated = 1; + + if (virDomainObjSetDefTransient(driver->caps, vm) < 0) + VIR_ERROR("Unable to set domain %s's running config as transient", + vm->def->name); + } + } + endjob: if (qemuDomainObjEndJob(vm) == 0) vm = NULL; @@ -8781,6 +8799,15 @@ static int qemudDomainAttachDevice(virDomainPtr dom, goto endjob; } + /* Set running domain def as updated */ + if (ret == 0) { + vm->updated = 1; + + if (virDomainObjSetDefTransient(driver->caps, vm) < 0) + VIR_ERROR("Unable to set domain %s's running config as transient", + vm->def->name); + } + if (!ret && virDomainSaveStatus(driver->caps, driver->stateDir, vm) < 0) ret = -1; @@ -8991,6 +9018,15 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom, break; } + /* Set running domain def as updated */ + if (ret == 0) { + vm->updated = 1; + + if (virDomainObjSetDefTransient(driver->caps, vm) < 0) + VIR_ERROR("Unable to set domain %s's running config as transient", + vm->def->name); + } + if (!ret && virDomainSaveStatus(driver->caps, driver->stateDir, vm) < 0) ret = -1; @@ -9665,6 +9701,15 @@ static int qemudDomainDetachDevice(virDomainPtr dom, "%s", _("This type of device cannot be hot unplugged")); } + /* Set running domain def as updated */ + if (ret == 0) { + vm->updated = 1; + + if (virDomainObjSetDefTransient(driver->caps, vm) < 0) + VIR_ERROR("Unable to set domain %s's running config as transient", + vm->def->name); + } + if (!ret && virDomainSaveStatus(driver->caps, driver->stateDir, vm) < 0) ret = -1; -- 1.7.3.2

On 12/06/2010 05:35 AM, Osier Yang wrote:
As qemu driver doesn't allow to make changes on persistent domain configuration via "attach/detach/update device",
However, this feature has been requested [1] - how easy will it be to add that support after this patch is applied? [1] https://bugzilla.redhat.com/show_bug.cgi?id=658713
and all the changes made on the running domain configuration should not be persistent across next boot (without the need of restarting libvirtd), so: 1) Set the running domain def as transient, and restore the domain configuration to original configuration when shutdown. 2) Set the running domain def as updated, and reset it as not updated when shutdown.
Also for "live VCPU set", it doesn't change the persistent domain configuration, so, we also set the running domain def as updated and transient, and restore the original def when shutdown.
@@ -6353,6 +6360,17 @@ qemudDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, if (flags & VIR_DOMAIN_VCPU_CONFIG) ret = virDomainSaveConfig(driver->configDir, persistentDef);
+ if (flags & VIR_DOMAIN_VCPU_LIVE) { + /* Set running domain def as updated */ + if (ret == 0) { + vm->updated = 1; + + if (virDomainObjSetDefTransient(driver->caps, vm) < 0) + VIR_ERROR("Unable to set domain %s's running config as transient", + vm->def->name); + } + }
In all of these cases, you issue VIR_ERROR but don't change the return status. Is that intentional, or should we make the overall API fail if we can't create the transient counterpart? And depending on the answer to that question, should we be attempting the virDomainObjSetDefTransient sooner, so as to guarantee that we have the transient counterpart prior to making any attempt to change the running domain, so that we don't have to worry about rolling back changes as part of returning failure? -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

于 2010年12月08日 23:42, Eric Blake 写道:
On 12/06/2010 05:35 AM, Osier Yang wrote:
As qemu driver doesn't allow to make changes on persistent domain configuration via "attach/detach/update device",
However, this feature has been requested [1] - how easy will it be to add that support after this patch is applied?
yeah, it will affects patch for that feature, but not much I think, just need to skip the block like: if (!persistent) { if (!vm->updated) vm->updated = 1; ...... }
and all the changes made on the running domain configuration should not be persistent across next boot (without the need of restarting libvirtd), so: 1) Set the running domain def as transient, and restore the domain configuration to original configuration when shutdown. 2) Set the running domain def as updated, and reset it as not updated when shutdown.
Also for "live VCPU set", it doesn't change the persistent domain configuration, so, we also set the running domain def as updated and transient, and restore the original def when shutdown.
@@ -6353,6 +6360,17 @@ qemudDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, if (flags& VIR_DOMAIN_VCPU_CONFIG) ret = virDomainSaveConfig(driver->configDir, persistentDef);
+ if (flags& VIR_DOMAIN_VCPU_LIVE) { + /* Set running domain def as updated */ + if (ret == 0) { + vm->updated = 1; + + if (virDomainObjSetDefTransient(driver->caps, vm)< 0) + VIR_ERROR("Unable to set domain %s's running config as transient", + vm->def->name); + } + }
In all of these cases, you issue VIR_ERROR but don't change the return status. Is that intentional, or should we make the overall API fail if we can't create the transient counterpart?
It was intentional, as probly the domain configuration was updated successfully by previous codes, but yeah, it's not much reasonable here.
And depending on the answer to that question, should we be attempting the virDomainObjSetDefTransient sooner, so as to guarantee that we have the transient counterpart prior to making any attempt to change the running domain, so that we don't have to worry about rolling back changes as part of returning failure?
Ah, yeah, good idea, using virDomainObjSetDefTransient sooner will be avoid considering the upper question, as the running domain configuration should be transient properly, we even don't need to worry about whether the coming changes on the config be success or not. Will update, thanks, Eric - Osier
participants (2)
-
Eric Blake
-
Osier Yang