Re: [libvirt] [PATCH] lxc: fix show the wrong xml when guest start failed

On 01/06/2015 09:31 AM, Luyao Huang wrote: > On 01/06/2015 10:11 PM, Michal Privoznik wrote: >> On 22.12.2014 08:21, Luyao Huang wrote: >>> https://bugzilla.redhat.com/show_bug.cgi?id=1176503 >>> >>> When guest start failed, libvirt will keep the current vm->def, >>> this will make a issue that we cannot get a right xml after guest >>> start failed. Pass the newDef to def will make it work well. >>> >>> Signed-off-by: Luyao Huang <lhuang(a)redhat.com&gt; >>> --- >>> src/lxc/lxc_process.c | 10 ++++++---- >>> 1 file changed, 6 insertions(+), 4 deletions(-) >>> >>> diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c >>> index 1c0d4e5..b7171ac 100644 >>> --- a/src/lxc/lxc_process.c >>> +++ b/src/lxc/lxc_process.c >>> @@ -1353,10 +1353,6 @@ int virLXCProcessStart(virConnectPtr conn, >>> VIR_FREE(veths[i]); >>> } >>> if (rc != 0) { >>> - if (vm->newDef) { >>> - virDomainDefFree(vm->newDef); >>> - vm->newDef = NULL; >>> - } >>> if (priv->monitor) { >>> virObjectUnref(priv->monitor); >>> priv->monitor = NULL; >>> @@ -1373,6 +1369,12 @@ int virLXCProcessStart(virConnectPtr conn, >>> VIR_FREE(vm->def->seclabels[0]->label); >>> VIR_FREE(vm->def->seclabels[0]->imagelabel); >>> } >>> + if (vm->newDef) { >>> + virDomainDefFree(vm->def); >>> + vm->def = vm->newDef; >>> + vm->def->id = -1; >>> + vm->newDef = NULL; >>> + } >>> } >>> for (i = 0; i < nttyFDs; i++) >>> VIR_FORCE_CLOSE(ttyFDs[i]); >>> >> Shouldn't this be in virLXCProcessStop() like it is in other drivers, >> e.g. qemu? >> > These code is already in virLXCProcessStop(), but if we meet some > issue and do 'goto cleanup' in > virLXCProcessStart, we won't call virLXCProcessStop. > > Maybe we can merge what we do in cleanup to virLXCProcessStop() ? I haven't looked in detail, but I'm guessing there are likely other things done in virLXCProcessCleanup() that should be done in certain cases of a failed virLXCProcessStart(), but aren't. One example is that the lxc hook is called three times during virLXCProcessStart() (prepare, start, and started), and really should be called again with stopped and/or release if the start fails (since the start hooks may be allocating resources).