[libvirt] [v2] 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 --- src/qemu/qemu_driver.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 47 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 19ce9a6..a3d87eb 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4429,11 +4429,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) { @@ -6473,7 +6480,17 @@ qemudDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, break; case VIR_DOMAIN_VCPU_LIVE: + if (virDomainObjSetDefTransient(driver->caps, vm) < 0) { + VIR_ERROR("Unable to set domain %s's running config as transient", + vm->def->name); + + goto endjob; + } + ret = qemudDomainHotplugVcpus(vm, nvcpus); + + if (ret == 0) + vm->updated = 1; break; case VIR_DOMAIN_VCPU_LIVE | VIR_DOMAIN_VCPU_CONFIG: @@ -8819,6 +8836,13 @@ static int qemudDomainAttachDevice(virDomainPtr dom, goto endjob; } + if (virDomainObjSetDefTransient(driver->caps, vm) < 0) { + VIR_ERROR("Unable to set domain %s's running config as transient", + vm->def->name); + + goto endjob; + } + dev = virDomainDeviceDefParse(driver->caps, vm->def, xml, VIR_DOMAIN_XML_INACTIVE); if (dev == NULL) @@ -8916,6 +8940,9 @@ static int qemudDomainAttachDevice(virDomainPtr dom, goto endjob; } + if (ret == 0) + vm->updated = 1; + if (!ret && virDomainSaveStatus(driver->caps, driver->stateDir, vm) < 0) ret = -1; @@ -9066,6 +9093,13 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom, goto endjob; } + if (virDomainObjSetDefTransient(driver->caps, vm) < 0) { + VIR_ERROR("Unable to set domain %s's running config as transient", + vm->def->name); + + goto endjob; + } + dev = virDomainDeviceDefParse(driver->caps, vm->def, xml, VIR_DOMAIN_XML_INACTIVE); if (dev == NULL) @@ -9126,6 +9160,9 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom, break; } + if (ret == 0) + vm->updated = 1; + if (!ret && virDomainSaveStatus(driver->caps, driver->stateDir, vm) < 0) ret = -1; @@ -9786,6 +9823,13 @@ static int qemudDomainDetachDevice(virDomainPtr dom, goto endjob; } + if (virDomainObjSetDefTransient(driver->caps, vm) < 0) { + VIR_ERROR("Unable to set domain %s's running config as transient", + vm->def->name); + + goto endjob; + } + dev = virDomainDeviceDefParse(driver->caps, vm->def, xml, VIR_DOMAIN_XML_INACTIVE); if (dev == NULL) @@ -9828,6 +9872,9 @@ static int qemudDomainDetachDevice(virDomainPtr dom, "%s", _("This type of device cannot be hot unplugged")); } + if (ret == 0) + vm->updated = 1; + if (!ret && virDomainSaveStatus(driver->caps, driver->stateDir, vm) < 0) ret = -1; -- 1.7.3.2
Based on "virDomainIsUpdated" API, to tell if the running domain configuration is updated by "attach/detach/update device" and "setvcpus" via "dominfo". * tools/virsh.c --- tools/virsh.c | 10 ++++++++++ 1 files changed, 10 insertions(+), 0 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 4e37f2d..2960b79 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -2038,6 +2038,7 @@ cmdDominfo(vshControl *ctl, const vshCmd *cmd) virSecurityModel secmodel; virSecurityLabel seclabel; int persistent = 0; + int updated = -1; int ret = TRUE, autostart; unsigned int id; char *str, uuid[VIR_UUID_STRING_BUFLEN]; @@ -2099,6 +2100,15 @@ cmdDominfo(vshControl *ctl, const vshCmd *cmd) else vshPrint(ctl, "%-15s %s\n", _("Persistent:"), persistent ? _("yes") : _("no")); + /* Check and display whether the domain running configuration is + * updated or not. + */ + updated = virDomainIsUpdated(dom); + if (updated < 0) + vshPrint(ctl, "%-15s %s\n", _("Updated:"), _("unknown")); + else + vshPrint(ctl, "%-15s %s\n", _("Updated:"), updated ? _("yes") : _("no")); + /* Check and display whether the domain autostarts or not */ if (!virDomainGetAutostart(dom, &autostart)) { vshPrint(ctl, "%-15s %s\n", _("Autostart:"), -- 1.7.3.2
On Fri, Dec 10, 2010 at 01:06:55PM +0800, Osier Yang wrote:
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 --- src/qemu/qemu_driver.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 47 insertions(+), 0 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 19ce9a6..a3d87eb 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4429,11 +4429,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) { @@ -6473,7 +6480,17 @@ qemudDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, break;
case VIR_DOMAIN_VCPU_LIVE: + if (virDomainObjSetDefTransient(driver->caps, vm) < 0) { + VIR_ERROR("Unable to set domain %s's running config as transient", + vm->def->name); + + goto endjob; + }
Why do we need this, or any other of the calls to SetDefTransient ? Since the VM is already running, we know this has already been called in qemudStartVMDaemon(). Regards, Daniel
于 2010年12月10日 18:58, Daniel P. Berrange 写道:
On Fri, Dec 10, 2010 at 01:06:55PM +0800, Osier Yang wrote:
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 --- src/qemu/qemu_driver.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 47 insertions(+), 0 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 19ce9a6..a3d87eb 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4429,11 +4429,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) { @@ -6473,7 +6480,17 @@ qemudDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, break;
case VIR_DOMAIN_VCPU_LIVE: + if (virDomainObjSetDefTransient(driver->caps, vm)< 0) { + VIR_ERROR("Unable to set domain %s's running config as transient", + vm->def->name); + + goto endjob; + }
Why do we need this, or any other of the calls to SetDefTransient ? Since the VM is already running, we know this has already been called in qemudStartVMDaemon().
Actually IMHO "SetDefTransient" in qemudStartVMDemon should be removed, as Eric said in the comment of v1. [quote] 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 [/quote] It's not reasonable to set the def transient regardless of if we want to do persistent changes or not, even if we won't support the persistent changes, we just need to set the def as transient only in the "update/attach/detach/", and "setvcpus" functions, but not thoroughly set the def as transient when domain start. If this reason is fine, I'd like make another patch to remove the "SetDefTransient" in qemudStartVMDaemon. Thanks for the review. - Osier
On 12/10/2010 12:06 AM, Osier Yang wrote:
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 --- src/qemu/qemu_driver.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 47 insertions(+), 0 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 19ce9a6..a3d87eb 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4429,11 +4429,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; }
I think it would be a good idea to break out this whole if() block into a helper function, since similar changes are needed in LXC, UML, and test drivers. - Cole
于 2010年12月10日 22:42, Cole Robinson 写道:
On 12/10/2010 12:06 AM, Osier Yang wrote:
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 --- src/qemu/qemu_driver.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 47 insertions(+), 0 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 19ce9a6..a3d87eb 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4429,11 +4429,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; }
I think it would be a good idea to break out this whole if() block into a helper function, since similar changes are needed in LXC, UML, and test drivers.
Yep, but I want it to be another patch, :-) Thanks - Osier
participants (3)
-
Cole Robinson -
Daniel P. Berrange -
Osier Yang