[libvirt] [PATCH 5/3] qemu: don't leave shutdown inhibited on attach failure

[Michal Privoznik]

On 29.08.2013 00:38, Eric Blake wrote:
> While debugging a failure of 'virsh qemu-attach', I noticed that
> we were leaking the count of active domains on failure.  This
> means that a libvirtd session that is supposed to quit after
> active domains disappear will hang around forever.
> 
> * src/qemu/qemu_process.c (qemuProcessAttach): Undo count of
> active domains on failure.
> 
> Signed-off-by: Eric Blake <eblake at redhat.com>
> ---
> 
> Quite a few latent bugs being uncovered while still drilling
> down to the root cause of attaching not working.
> 
>  src/qemu/qemu_process.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index 128618b..6fb4a4f 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -4353,6 +4353,7 @@ int qemuProcessAttach(virConnectPtr conn ATTRIBUTE_UNUSED,
>      const char *model;
>      virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
>      virCapsPtr caps = NULL;
> +    bool active = false;
> 
>      VIR_DEBUG("Beginning VM attach process");
> 
> @@ -4378,6 +4379,7 @@ int qemuProcessAttach(virConnectPtr conn ATTRIBUTE_UNUSED,
> 
>      if (virAtomicIntInc(&driver->nactive) == 1 && driver->inhibitCallback)
>          driver->inhibitCallback(true, driver->inhibitOpaque);
> +    active = true;
> 
>      if (virFileMakePath(cfg->logDir) < 0) {
>          virReportSystemError(errno,
> @@ -4549,9 +4551,12 @@ int qemuProcessAttach(virConnectPtr conn ATTRIBUTE_UNUSED,
>      return 0;
> 
>  cleanup:

I don't like this label being 'cleanup' when in fact it should be
'error'. But that's pre-existing.

> -    /* We jump here if we failed to start the VM for any reason, or
> -     * if we failed to initialize the now running VM. kill it off and
> -     * pretend we never started it */
> +    /* We jump here if we failed to attach to the VM for any reason.
> +     * Leave the domain running, but pretend we never attempted to
> +     * attach to it.  */
> +    if (active && virAtomicIntDecAndTest(&driver->nactive) &&
> +        driver->inhibitCallback)
> +        driver->inhibitCallback(false, driver->inhibitOpaque);
>      VIR_FORCE_CLOSE(logfile);
>      VIR_FREE(seclabel);
>      VIR_FREE(sec_managers);
> 

ACK

Michal