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@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
