[Libguestfs] [PATCH 2/3] Allow connecting to an externally spawned QEMU + appliance

[Richard W.M. Jones]

On Mon, Jul 05, 2010 at 12:26:00PM -0400, Daniel P. Berrange wrote:
> +   "\
> +Set the appliance launch method that we will use.
> +
> +Valid methods are
> +
> + * spawn:  spawn a QEMU process for the appliance (default)
> + * attach: attach to existing QEMU over a UNIX socket

This string is POD, so use:

=over 4

=item \"spawn\"

spawn a QEMU process for the appliance (default)

[etc]

=back

> +You can also override this by setting the C<LIBGUESTFS_METHOD>
> +environment variable.

This should be LIBGUESTFS_LAUNCH_METHOD for consistency.

> +  ("set_sockpath", (RErr, [OptString "sockpath"]), -1, [FishAlias "sockpath"],
> +   [],
> +   "set the UNIX domain socket path",
> +   "\
> +Set the UNIX domain socket path that we will use.
> +
> +The default is a file in a randomly named temporary directory.

Is it?  For null vmchannel there's no Unix domain socket at all.  This
indicates that NULL is a distinct value for this variable.

> +
> +#define GUESTFS_LAUNCH_METHOD_SPAWN "spawn"
> +#define GUESTFS_LAUNCH_METHOD_ATTACH "attach"
> +

Aarrgh vertical whitespace.

Aaarrrgghhhh unnecessary macros!!

Just use "spawn" and "attach" in the code.  Or ...

> +  char *method;                 /* Appliance launch method */

... make this into an enum.  (Which means you don't need to strdup
the string too).

>  int
> -guestfs__launch (guestfs_h *g)
> +guestfs__launch_spawn (guestfs_h *g)

Static?

>    /* Start the clock ... */
>    gettimeofday (&g->launch_t, NULL);

This code needs to be in guestfs_launch.

> @@ -1905,6 +1996,11 @@ guestfs__kill_subprocess (guestfs_h *g)
>      return -1;
>    }
>  
> +  if (strcmp(g->method, GUESTFS_LAUNCH_METHOD_SPAWN) != 0) {
> +    error (g, _("cannot kill subprocess with launch method '%s'"), g->method);
> +    return -1;
> +  }
> +

What does guestfs_get_pid return?  Possibly it will return -1
(because g->pid == -1) but it won't set the error string.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://et.redhat.com/~rjones/virt-top