Re: [Xen-devel] [RFC Patch v4 8/9] store correct format into tapdisk-params/params

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Mon, 2014-09-22 at 13:59 +0800, Wen Congyang wrote:

Please prefix the subject "tools: libxl: "

> If the format is raw, we store aio into tapdisk-params/params.

I got confused and thought "tapdisl-params/params" was a path. Could you
spell it out the first time as "into either the tapdisk-params or params
xenstore node" please.

> And we cannot use the API libxl_disk_format_from_string()
> to get the format from the string.

... can you spell out why/where this is important please.

> The API libxl__device_disk_string_of_format() is just
> for blktap2, which needs to pass aio instead of raw to
> tapdisk2.

Should we move it to libxl_blktap2.c then?

>  So use libxl_disk_format_to_string() to
> instead of libxl__device_disk_string_of_format().
> 
> Also update libxl__device_destroy_tapdisk() due to
> tapdisk-params changed.

s/changed/change/.

> Note: the content of tapdisk-params/params has been changed...

AFAICT the actual change is that "aio:/path/to/a/thing" might become
either "raw:/path/to/a/thing" or "empty:/path/to/a/thing", or perhaps
even "unknown:/path/to/a/thing" (or more likely "aio:"->"empty:").

Did you test blktap2 and qemu to be sure that they handle "raw" in
particular correctly?

> @@ -67,6 +68,15 @@ int libxl__device_destroy_tapdisk(libxl__gc *gc, const 
> char *params)
>  
>      *disk++ = '\0';
>  
> +    /* type may be raw */
> +    rc = libxl_disk_format_from_string(type, &format);
> +    if (rc < 0) {
> +        LOG(ERROR, "invalid disk type %s", type);
> +        return ERROR_FAIL;

Please propagate rc.

> +    }
> +
> +    type = libxl__device_disk_string_of_format(format);
> +
>      err = tap_ctl_find(type, disk, &tap);
>      if (err < 0) {
>          /* returns -errno */



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