[libvirt] [PATCH] qemu: Prevent disk corruption on domain shutdown

[Daniel P. Berrange]

On Tue, Sep 13, 2011 at 06:20:56PM +0200, Jiri Denemark wrote:
> Ever since we introduced fake reboot, we call qemuProcessKill as a
> reaction to SHUTDOWN event. Unfortunately, qemu doesn't guarantee it
> flushed all internal buffers before sending SHUTDOWN, in which case
> killing the process forcibly may result in (virtual) disk corruption.
> 
> By sending SIGQUIT instead of SIGTERM followed by SIGKILL we tell qemu
> to flush all buffers and exit. Once qemu exits, we will see an EOF on
> monitor connection and tear down the domain. In case qemu ignores
> SIGQUIT or just hangs there, the process stays running but that's not
> any different from a possible hang anytime during the shutdown process
> so I think it's just fine.

SIGQUIT is handled identically to SIGTERM, so I don't see a point
in adding a new method to send SIGQUIT. The important thing is just
that we don't send SIGKILL.

> ---
>  src/qemu/qemu_process.c |   21 +++++++++++++++++++--
>  src/qemu/qemu_process.h |    1 +
>  2 files changed, 20 insertions(+), 2 deletions(-)
> 
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index f8a8475..8a12e2a 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -445,12 +445,12 @@ qemuProcessHandleShutdown(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
>                              qemuProcessFakeReboot,
>                              vm) < 0) {
>              VIR_ERROR(_("Failed to create reboot thread, killing domain"));
> -            qemuProcessKill(vm);
> +            qemuProcessQuit(vm);
>              if (virDomainObjUnref(vm) == 0)
>                  vm = NULL;
>          }
>      } else {
> -        qemuProcessKill(vm);
> +        qemuProcessQuit(vm);
>      }
>      if (vm)
>          virDomainObjUnlock(vm);
> @@ -3182,6 +3182,23 @@ cleanup:
>  }
>  
>  
> +void qemuProcessQuit(virDomainObjPtr vm)
> +{
> +    VIR_DEBUG("vm=%s pid=%d", vm->def->name, vm->pid);
> +
> +    if (!virDomainObjIsActive(vm)) {
> +        VIR_DEBUG("VM '%s' not active", vm->def->name);
> +        return;
> +    }
> +
> +    if (virKillProcess(vm->pid, SIGQUIT) < 0 && errno != ESRCH) {
> +        char ebuf[1024];
> +        VIR_WARN("Failed to kill process %d: %s",
> +                 vm->pid, virStrerror(errno, ebuf, sizeof(ebuf)));
> +    }
> +}

So instead of this method, I'd just add a 'bool gracefully' to
the qemuProcessKill() method, and if that flag is true, then
skip the SIGKILL part. We can then also add a new flag:

   VIR_DOMAIN_DESTROY_GRACEFULLY

and if that flag is passed to virDomainDestroy, we will cause it
to skip the SIGKILL part too. This lets mgmt apps also have more
fine grained control over death

Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|