Re: [libvirt] [PATCH 5/7] Add pidfile argument to __virExec

["Daniel P. Berrange" <berrange redhat com>]

On Tue, Apr 28, 2009 at 11:31:55AM -0400, Cole Robinson wrote:
> virExec will write out the pid of the daemonized process only. Use this
> in the QEMU driver, rather than QEMU's pidfile, so we can errors we might
> miss if the emulator bails early.

> diff --git a/src/qemu_driver.c b/src/qemu_driver.c
> index 5200532..2f17f6c 100644
> --- a/src/qemu_driver.c
> +++ b/src/qemu_driver.c
> @@ -1396,6 +1397,11 @@ static int qemudStartVMDaemon(virConnectPtr conn,
>          goto cleanup;
>      }
>  
> +    if (!(pidfile = virFilePid(driver->stateDir, vm->def->name))) {
> +        virReportSystemError(conn, errno,
> +                             "%s", _("Failed to build pidfile path."));
> +        goto cleanup;
> +    }
>  
>      vm->def->id = driver->nextvmid++;
>      if (qemudBuildCommandLine(conn, driver, vm->def,
> @@ -1437,13 +1443,15 @@ static int qemudStartVMDaemon(virConnectPtr conn,
>      ret = virExecDaemonize(conn, argv, progenv, &keepfd, &child,
>                             stdin_fd, &vm->logfile, &vm->logfile,
>                             VIR_EXEC_NONBLOCK | VIR_EXEC_DAEMON,
> -                           qemudSecurityHook, &hookData);
> +                           qemudSecurityHook, &hookData,
> +                           pidfile);
> +    VIR_FREE(pidfile);
>  
>      /* wait for qemu process to to show up */
>      if (ret == 0) {
> -        int retries = 100;
> +        int retries = 10;
>  
> -        while (retries) {
> +        while (retries*10) {
>              if ((ret = virFileReadPid(driver->stateDir,
>                                        vm->def->name, &vm->pid)) == 0)
>                  break;

I'm thinking that we don't need to loop here waiting for the 
pidfile to be created.

Since virExeDaemonize is calling waitpid() on the intermediate
program, it will block until the intermediate program has 
exited. Since the intermediate program is now writing the pidfile
instead of QEMU,  when  virExecDaemonize returns, we *must* have
written the pidfile (or had a earlier failure). So I believe
we just need to check if it exists once, with no need to wait in
a loop.


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 :|