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

[Laine Stump]

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 at redhat.com>
>>> ---
>>>   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).