Re: [Xen-devel] [PATCH v3 6/9] libxl: add Qdisk backend launch helper

[Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>]

Roger Pau Monne writes ("[PATCH v3 6/9] libxl: add Qdisk backend launch 
helper"):
> Current Qemu launch functions in libxl require the usage of data
> structures only avaialbe on domain creation. All this information is
> not need in order to launch a Qemu instance to serve Qdisk backends,
> so introduce a new simplified helper that can be used to launch
> Qemu/Qdisk, that will be used to launch Qdisk in driver domains.
...
> +static int libxl__create_qemu_logfile(libxl__gc *gc, char *name)
> +{
...
> +    logfile_w = open(logfile, O_WRONLY|O_CREAT|O_APPEND, 0644);
> +    free(logfile);
> +
> +    if (logfile_w < 0)
> +        LOGE(ERROR, "unable to open Qemu logfile");
> +
> +    return logfile_w;

This function ought to have a comment stating its return type.  AFAICT
the return type is "the same as syscalls", but we don't do that in
libxl.  libxl functions ought to return libxl error codes on error.

> @@ -1204,11 +1219,12 @@ void libxl__spawn_local_dm(libxl__egc *egc, 
> libxl__dm_spawn_state *dmss)
...
> -    libxl_create_logfile(ctx,
> -                         libxl__sprintf(gc, "qemu-dm-%s", c_info->name),
> -                         &logfile);
> -    logfile_w = open(logfile, O_WRONLY|O_CREAT|O_APPEND, 0644);
> -    free(logfile);
> +    logfile_w = libxl__create_qemu_logfile(gc, GCSPRINTF("qemu-dm-%s",
> +                                                         c_info->name));
> +    if (logfile_w < 0) {
> +        rc = logfile_w;
> +        goto out;
> +    }

And here we assume that it does indeed return a libxl error code.

The other changes you've made here look good.

Thanks,
Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel