[libvirt] [PATCH] Move most of qemuProcessKill into virProcessKillPainfully

[Eric Blake]

On 09/26/2012 08:44 AM, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange at redhat.com>
> 
> In the cgroups APIs we have a virCgroupKillPainfully function
> which does the loop sending SIGTERM, then SIGKILL and waiting
> for the process to exit. There is similar functionality for
> simple processes in qemuProcessKill, but it is tangled with
> the QEMU code. Untangle it to provide a virProcessKillPainfuly
> function

It is also similar to virProcessAbort, although the two differ on how
long to wait between SIGTERM and an eventual SIGKILL.  Maybe those
should be consolidated in a later patch?

> ---
>  src/libvirt_private.syms |  1 +
>  src/qemu/qemu_driver.c   |  8 ++---
>  src/qemu/qemu_process.c  | 79 ++++++++----------------------------------------
>  src/util/virprocess.c    | 57 ++++++++++++++++++++++++++++++++++
>  src/util/virprocess.h    |  2 ++
>  5 files changed, 76 insertions(+), 71 deletions(-)

> +++ b/src/qemu/qemu_driver.c
> @@ -2006,13 +2006,11 @@ qemuDomainDestroyFlags(virDomainPtr dom,
>       * it now means the job will be released
>       */
>      if (flags & VIR_DOMAIN_DESTROY_GRACEFUL) {
> -        if (qemuProcessKill(driver, vm, 0) < 0) {
> -            virReportError(VIR_ERR_OPERATION_FAILED, "%s",
> -                           _("failed to kill qemu process with SIGTERM"));
> +        if (qemuProcessKill(driver, vm, 0) < 0)
>              goto cleanup;
> -        }
>      } else {
> -        ignore_value(qemuProcessKill(driver, vm, VIR_QEMU_PROCESS_KILL_FORCE));
> +        if (qemuProcessKill(driver, vm, VIR_QEMU_PROCESS_KILL_FORCE) < 0)
> +            goto cleanup;

Why are you changing this from ignore_value() into a cleanup path on
failure?

> +++ b/src/util/virprocess.c
> @@ -235,3 +235,60 @@ int virProcessKill(pid_t pid, int sig)
>      return kill(pid, sig);
>  #endif
>  }
> +
> +
> +/*
> + * Try to kill the process and verify it has exited
> + *
> + * Returns 0 if it was killed gracefully, 1 if it
> + * was killed forcably, -1 if it is still alive,

s/forcably/forcibly/

> + * or another error occurred.
> + */
> +int
> +virProcessKillPainfully(pid_t pid, bool force)
> +{
> +    int i, ret = -1;
> +    const char *signame = "TERM";
> +
> +    VIR_DEBUG("vpid=%d force=%d", pid, force);
> +
> +    /* This loop sends SIGTERM, then waits a few iterations (10 seconds)
> +     * to see if it dies. If the process still hasn't exited, and
> +     * @force is requested, a SIGKILL will be sent, and this will
> +     * wait upto 5 seconds more for the process to exit before

s/upto/up to/

> +     * returning.
> +     *
> +     * Note that setting @force could result in dataloss for the process.

s/dataloss/data loss/

The move looks okay if you can answer my question about the
ignore_value() change.

-- 
Eric Blake   eblake at redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 617 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20120926/9c157e96/attachment-0001.sig>