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

[John Ferlan]

On 01/12/2015 09:33 AM, 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. And don't call the stop/release hook to do some
> other clean work.
> 
> Call virLXCProcessCleanup to help us clean the source and call
> the hooks if start a vm failed.
> 
> Signed-off-by: Luyao Huang <lhuang at redhat.com>
> ---
> v2: use virLXCProcessCleanup to free the source and call the hook.
> 
>  src/lxc/lxc_process.c | 69 ++++++++++++++++++++++-----------------------------
>  1 file changed, 29 insertions(+), 40 deletions(-)
> 

There's been some changes to virLXCProcessStart since this was written -
a revisit/rework is in order.

John
> diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c
> index d7eb8bc..7ba0da8 100644
> --- a/src/lxc/lxc_process.c
> +++ b/src/lxc/lxc_process.c
> @@ -1006,6 +1006,7 @@ int virLXCProcessStart(virConnectPtr conn,
>      virLXCDriverConfigPtr cfg = virLXCDriverGetConfig(driver);
>      virCgroupPtr selfcgroup;
>      int status;
> +    bool need_stop = false;
>  
>      if (virCgroupNewSelf(&selfcgroup) < 0)
>          return -1;
> @@ -1265,18 +1266,20 @@ int virLXCProcessStart(virConnectPtr conn,
>          goto cleanup;
>      }
>  
> +    need_stop = true;
> +
>      if (virCgroupNewDetectMachine(vm->def->name, "lxc", vm->pid,
>                                    vm->def->resource ?
>                                    vm->def->resource->partition :
>                                    NULL,
>                                    -1, &priv->cgroup) < 0)
> -        goto error;
> +        goto cleanup;
>  
>      if (!priv->cgroup) {
>          virReportError(VIR_ERR_INTERNAL_ERROR,
>                         _("No valid cgroup for machine %s"),
>                         vm->def->name);
> -        goto error;
> +        goto cleanup;
>      }
>  
>      priv->stopReason = VIR_DOMAIN_EVENT_STOPPED_FAILED;
> @@ -1296,17 +1299,17 @@ int virLXCProcessStart(virConnectPtr conn,
>                             _("guest failed to start: %s"), out);
>          }
>  
> -        goto error;
> +        goto cleanup;
>      }
>  
>      if (autoDestroy &&
>          virCloseCallbacksSet(driver->closeCallbacks, vm,
>                               conn, lxcProcessAutoDestroy) < 0)
> -        goto error;
> +        goto cleanup;
>  
>      if (virDomainObjSetDefTransient(caps, driver->xmlopt,
>                                      vm, false) < 0)
> -        goto error;
> +        goto cleanup;
>  
>      /* We don't need the temporary NIC names anymore, clear them */
>      virLXCProcessCleanInterfaces(vm->def);
> @@ -1318,7 +1321,7 @@ int virLXCProcessStart(virConnectPtr conn,
>       * it with the live status XML instead. This is a (currently
>       * harmless) inconsistency we should fix one day */
>      if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0)
> -        goto error;
> +        goto cleanup;
>  
>      /* finally we can call the 'started' hook script if any */
>      if (virHookPresent(VIR_HOOK_DRIVER_LXC)) {
> @@ -1334,47 +1337,38 @@ int virLXCProcessStart(virConnectPtr conn,
>           * If the script raised an error abort the launch
>           */
>          if (hookret < 0)
> -            goto error;
> +            goto cleanup;
>      }
>  
>      rc = 0;
>  
>   cleanup:
> -    if (rc != 0 && !err)
> -        err = virSaveLastError();
> -    virCommandFree(cmd);
>      if (VIR_CLOSE(logfd) < 0) {
>          virReportSystemError(errno, "%s", _("could not close logfile"));
>          rc = -1;
>      }
> -    for (i = 0; i < nveths; i++) {
> -        if (rc != 0 && veths[i])
> -            ignore_value(virNetDevVethDelete(veths[i]));
> -        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;
> -        }
> -        virDomainConfVMNWFilterTeardown(vm);
> -
> -        virSecurityManagerRestoreAllLabel(driver->securityManager,
> -                                          vm->def, false);
> -        virSecurityManagerReleaseLabel(driver->securityManager, vm->def);
> -        /* Clear out dynamically assigned labels */
> -        if (vm->def->nseclabels &&
> -            vm->def->seclabels[0]->type == VIR_DOMAIN_SECLABEL_DYNAMIC) {
> -            VIR_FREE(vm->def->seclabels[0]->model);
> -            VIR_FREE(vm->def->seclabels[0]->label);
> -            VIR_FREE(vm->def->seclabels[0]->imagelabel);
> -            VIR_DELETE_ELEMENT(vm->def->seclabels, 0, vm->def->nseclabels);
> +        err = virSaveLastError();
> +        if (need_stop) {
> +            virLXCProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_FAILED);
> +        } else {
> +            virSecurityManagerRestoreAllLabel(driver->securityManager,
> +                                              vm->def, false);
> +            virSecurityManagerReleaseLabel(driver->securityManager, vm->def);
> +            /* Clear out dynamically assigned labels */
> +            if (vm->def->nseclabels &&
> +                vm->def->seclabels[0]->type == VIR_DOMAIN_SECLABEL_DYNAMIC) {
> +                VIR_FREE(vm->def->seclabels[0]->model);
> +                VIR_FREE(vm->def->seclabels[0]->label);
> +                VIR_FREE(vm->def->seclabels[0]->imagelabel);
> +                VIR_DELETE_ELEMENT(vm->def->seclabels, 0, vm->def->nseclabels);
> +            }
> +            virLXCProcessCleanup(driver, vm, VIR_DOMAIN_SHUTOFF_FAILED);
>          }
>      }
> +    virCommandFree(cmd);
> +    for (i = 0; i < nveths; i++)
> +        VIR_FREE(veths[i]);
>      for (i = 0; i < nttyFDs; i++)
>          VIR_FORCE_CLOSE(ttyFDs[i]);
>      VIR_FREE(ttyFDs);
> @@ -1390,11 +1384,6 @@ int virLXCProcessStart(virConnectPtr conn,
>      }
>  
>      return rc;
> -
> - error:
> -    err = virSaveLastError();
> -    virLXCProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_FAILED);
> -    goto cleanup;
>  }
>  
>  struct virLXCProcessAutostartData {
>