Re: [PATCH] qemu: save config after pivot only if domain is persistent

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

 



On Wed, 6 Jan 2016, Peter Krempa wrote:
On Tue, Jan 05, 2016 at 11:38:07 +1100, Michael Chapman wrote:
On Mon, 4 Jan 2016, Peter Krempa wrote:
On Thu, Dec 31, 2015 at 16:42:52 +1100, Michael Chapman wrote:

...

@@ -188,8 +182,8 @@ qemuBlockJobEventProcess(virQEMUDriverPtr driver,
         if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0)
             VIR_WARN("Unable to save status on vm %s after block job",
                      vm->def->name);
-        if (persistDisk && virDomainSaveConfig(cfg->configDir,
-                                               vm->newDef) < 0)
+        if (vm->persistent && persistDisk &&

I'm not quite sure how this would happen. If there isn't a persistent
configuration, how did we get to having the persistDisk object?

If this happened in some way, the problem then is that vm->newDef was
not freed or is present in any way so we should fix the origin and not
this symptom.

I spent a lot of time trying to work this out myself, and although I can
see what the code is doing I don't really understand the rationale or
history behind it all.

vm->newDef is supposed to be the "new domain definition to activate on
shutdown", according to domain_conf.h. What's confusing though is that it
is possible for this to exist even on transient domains.

That is not confusing but a bug. The newDef should not exist if the
domain is transient.

Well, it was certainly confusing to me, but that's probably because I was
assuming the code was correct and it was just my understanding of it that
was wrong. :-)

[...]

As a side note, all the naming of def, newDef and
virDomainObjSetDefTransient is very unfortunate. Having a persistentDef
and liveDef or similarly named pointers would be more clear, easier to
differentiate states and would additionally allow to get rid of
vm->persistent since it would be equal to vm->persistentDef being set or
not.

If you want to fix the undefine bug you are welcome, otherwise I'll do
it in a few days.

Peter

I would be more comfortable if you were to fix up the undefine bug. You've
clearly got a better understanding of how it's all supposed to work.

However, I will send through a patch fixing the possible NULL dereference
should virStorageSourceCopy fail.

- Michael

--
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]