[Libguestfs] [PATCH 3/3] Add APIs for resolving the appliance kernel/initrd

[Richard W.M. Jones]

On Mon, Jul 05, 2010 at 12:26:01PM -0400, Daniel P. Berrange wrote:
> +  ("find_appliance", (RErr, []), -1, [FishAlias "find_appliance"],
> +   [],
> +   "find the appliance kernel/initrd",
> +   "\
> +Find the guest daemon appliance kernel and initrd images.
> +
> +This prefers to build a super-min appliance, but falls
> +back to a traditional appliance if the former was not found.");

I think you should add to this documentation that most users will not
need to call this.  Same for get_kernel and get_initrd.

Also on the documentation front we need a section added to
src/guestfs.pod to explain how to use all of this.

> +  ("get_kernel", (RConstString "kernel", []), -1, [],

RConstString is the wrong type to use here, I think.

If users should call find_appliance before this, then it should be an
error not to have called it, so RString would be the correct return
type.

See also:

http://git.annexia.org/?p=libguestfs.git;a=blob;f=src/generator.ml;h=d640343e7a407543401dc30c58451f7c6e7a74ae;hb=HEAD#l75

(Note that use of RConstString elsewhere in your code was, I think,
correct).

> +  ("get_initrd", (RConstString "initrd", []), -1, [],

RString also.

> +const char *
> +guestfs__get_kernel (guestfs_h *g)
> +{
> +  return g->kernel;
> +}
> +
> +const char *
> +guestfs__get_initrd (guestfs_h *g)
> +{
> +  return g->initrd;
> +}

If the above becomes RString, then you need to check for error here
(and call error / return NULL if so), and also strdup the non-NULL
returned value.

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