Re: [libvirt] [PATCH 2/4] daemonize qemu processes

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sun, Jan 18, 2009 at 08:28:13PM +0100, Guido G?nther wrote:
> so they doen't get killed when the libvirtd quits unexpectedly
> 
> changes:
>   * longer timeouts when waiting for procs to show up 
>   * check if process exists before killing it
>   * use virKill
> 
> this one basically got acked before but makes little sense without the
> follow up patches.
> Cheers,
>  -- Guido
> 
> ---
>  src/qemu_driver.c |   39 ++++++++++++++++++++++++++-------------
>  1 files changed, 26 insertions(+), 13 deletions(-)
> 
> diff --git a/src/qemu_driver.c b/src/qemu_driver.c
> index 1619154..06f444b 100644
> --- a/src/qemu_driver.c
> +++ b/src/qemu_driver.c
> @@ -941,6 +941,7 @@ static int qemudStartVMDaemon(virConnectPtr conn,
>      unsigned int qemuCmdFlags;
>      fd_set keepfd;
>      const char *emulator;
> +    pid_t child;
>  
>      FD_ZERO(&keepfd);
>  
> @@ -1039,12 +1040,26 @@ static int qemudStartVMDaemon(virConnectPtr conn,
>      for (i = 0 ; i < ntapfds ; i++)
>          FD_SET(tapfds[i], &keepfd);
>  
> -    ret = virExec(conn, argv, progenv, &keepfd, &vm->pid,
> +    ret = virExec(conn, argv, progenv, &keepfd, &child,
>                    vm->stdin_fd, &vm->stdout_fd, &vm->stderr_fd,
> -                  VIR_EXEC_NONBLOCK);
> -    if (ret == 0)
> +                  VIR_EXEC_NONBLOCK | VIR_EXEC_DAEMON);
> +
> +    /* wait for qemu process to to show up */
> +    if (ret == 0) {
> +        int retries = 100;
> +        while (retries) {
> +            if ((ret = virFileReadPid(driver->stateDir, vm->def->name, &vm->pid)) == 0)
> +                break;
> +            usleep(100*1000);
> +            retries--;
> +        }
> +        if (ret)
> +            qemudLog(QEMUD_WARN, _("Domain %s didn't show up\n"), vm->def->name);
> +    }
> +
> +    if (ret == 0) {
>          vm->state = migrateFrom ? VIR_DOMAIN_PAUSED : VIR_DOMAIN_RUNNING;
> -    else
> +    } else
>          vm->def->id = -1;
>  
>      for (i = 0 ; argv[i] ; i++)
> @@ -1121,7 +1136,10 @@ static void qemudShutdownVMDaemon(virConnectPtr conn ATTRIBUTE_UNUSED,
>  
>      qemudLog(QEMUD_INFO, _("Shutting down VM '%s'\n"), vm->def->name);
>  
> -    kill(vm->pid, SIGTERM);
> +    if (virKillProcess(vm->pid, 0) == 0 &&
> +        virKillProcess(vm->pid, SIGTERM) < 0)
> +        qemudLog(QEMUD_ERROR, _("Failed to send SIGTERM to %s (%d): %s\n"),
> +                 vm->def->name, vm->pid, strerror(errno));
>  
>      qemudVMData(driver, vm, vm->stdout_fd);
>      qemudVMData(driver, vm, vm->stderr_fd);
> @@ -1141,15 +1159,10 @@ static void qemudShutdownVMDaemon(virConnectPtr conn ATTRIBUTE_UNUSED,
>      vm->stderr_fd = -1;
>      vm->monitor = -1;
>  
> -    if (waitpid(vm->pid, NULL, WNOHANG) != vm->pid) {
> -        kill(vm->pid, SIGKILL);
> -        if (waitpid(vm->pid, NULL, 0) != vm->pid) {
> -            qemudLog(QEMUD_WARN,
> -                     "%s", _("Got unexpected pid, damn\n"));
> -        }
> -    }
> -    qemudRemoveDomainStatus(conn, driver, vm);
> +    /* shut it off for sure */
> +    virKillProcess(vm->pid, SIGKILL);
>  
> +    qemudRemoveDomainStatus(conn, driver, vm);
>      vm->pid = -1;
>      vm->def->id = -1;
>      vm->state = VIR_DOMAIN_SHUTOFF;

ACK

The retrying loop looking for the PID upon exec  isn't great long term, 
but good enough for now. It is more important to get this working and
optimize later.


Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]