[libvirt] [PATCH] lxc: fix show the wrong xml when guest start failed
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@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]); -- 1.8.3.1
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@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? Michal
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@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 think use this way to fix this issue maybe toodangerous for me (maybe will cause some other issue when rework the cleanup and virLXCProcessStop), because i am not familiar with LXC. So i use this easy way to fix this issue. Thanks for your review :)
Michal Luyao
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@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).
On 01/06/2015 11:14 PM, Laine Stump 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@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
On 01/06/2015 09:31 AM, Luyao Huang wrote: 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).
Thanks for your review and i noticed this function before, but i forgot the stop/release hook should be called in this place. So I think maybe call virLXCProcessCleanup() in this place is better than free the source one by one, or just call virLXCProcessStop() in the cleanup and merge 'goto error' to 'goto cleanup'?(just like what we do in qemu drivers) I found if we use virLXCProcessCleanup() in this place, the left things need free is security labels (which have been done in virLXCProcessStop()). And another way is call virLXCProcessStop() (maybe need change some thing in virLXCProcessStop to suit this ). Luyao
participants (4)
-
Laine Stump -
lhuang -
Luyao Huang
-
Michal Privoznik