Re: [v2] qemu: Set domain def as updated and transient if changes

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



ä 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

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]