[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v9 4/4] block: Support GlusterFS as a QEMU block
From: |
Paolo Bonzini |
Subject: |
Re: [Qemu-devel] [PATCH v9 4/4] block: Support GlusterFS as a QEMU block backend. |
Date: |
Thu, 27 Sep 2012 09:19:34 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:15.0) Gecko/20120911 Thunderbird/15.0.1 |
Il 27/09/2012 08:41, Bharata B Rao ha scritto:
>>> As you note, I don't need 2nd strtok strictly since the rest of the string
>>> is available in saveptr. But I thought using saveptr is not ideal or
>>> preferred.
>>> I wanted to use the most appropriate/safe delimiter to extract the image
>>> string
>>> in the 2nd strtok and decided to use '?'.
>>
>> I don't think it is defined what saveptr points to.
>> http://publib.boulder.ibm.com/infocenter/pseries/v5r3/index.jsp?topic=/com.ibm.aix.basetechref/doc/basetrf2/strtok_r.htm
>> says "the strtok_r subroutine also updates the Pointer parameter with
>> the starting address of the token following the first occurrence of the
>> Separators parameter". I read this as:
>>
>> *saveptr = token + strlen(token) + 1;
>>
>> which is consistent with this strtok example from the C standard:
>>
>> #include <string.h>
>> static char str[] = "?a???b,";
>> char *t;
>>
>> t = strtok(str, "?"); // t points to the token "a"
>> t = strtok(str, ","); // t points to the token "??b"
>>
>> Have you tested this code with multiple consecutive slashes?
>
> Yes, both 2-strtok and strtok+saveptr approach work correctly (= as per
> my expectation!) with multiple consecutive slashes. I do understand that
> 2-strtok approach will not work when we have %3F in the path component of
> the URI.
>
> For URIs like gluster://server/volname/path/to/image, both the approaches
> extract image as "path/to/image".
>
> For URIs like gluster://server/volname//path/to/image, both the approaches
> extract image as "/path/to/image".
>
> For gluster://server/volname////path/to/image, the image is extracted as
> "//path/to/image".
Should there be three /'s here? I assume it's just a typo.
I'm concerned that there is no documentation of what saveptr actually
points to. In many cases the strtok specification doesn't leave much
free room, but in the case you're testing here:
>>> /* image */
>>> if (!*saveptr) {
>>> return -EINVAL;
>>> }
strtok_r may just as well leave saveptr = NULL for example.
>>> gconf->image = g_strdup(saveptr);
>>>
>>
>> I would avoid strtok_r completely:
>>
>> char *p = path + strcpsn(path, "/");
>> if (*p == '\0') {
>> return -EINVAL;
>> }
>> gconf->volname = g_strndup(path, p - path);
>>
>> p += strspn(p, "/");
>> if (*p == '\0') {
>> return -EINVAL;
>> }
>> gconf->image = g_strdup(p);
>
> This isn't working because for a URI like
> gluster://server/volname/path/to/image, uri_parse() will give
> "/volname/path/to/image" in uri->path. I would have expected to see
> uri->path as "volname/path/to/image" (without 1st slash).
Ok, that's easy enough to fix with an extra strspn,
char *p = path + strpsn(path, "/");
p += strcspn(p, "/");
> Note that gluster is currently able to resolve image paths that don't
> have a preceding slash (like dir/a.img). But I guess we should support
> specifying complete image paths too (like /dir/a.img)
How would the URIs look like?
Paolo
- Re: [Qemu-devel] [PATCH v9 2/4] qemu: URI parsing library, (continued)
- [Qemu-devel] [PATCH v9 3/4] configure: Add a config option for GlusterFS as block backend, Bharata B Rao, 2012/09/24
- [Qemu-devel] [PATCH v9 4/4] block: Support GlusterFS as a QEMU block backend., Bharata B Rao, 2012/09/24
- Re: [Qemu-devel] [PATCH v9 4/4] block: Support GlusterFS as a QEMU block backend., Paolo Bonzini, 2012/09/24
- Re: [Qemu-devel] [PATCH v9 4/4] block: Support GlusterFS as a QEMU block backend., Kevin Wolf, 2012/09/26
- Re: [Qemu-devel] [PATCH v9 4/4] block: Support GlusterFS as a QEMU block backend., Paolo Bonzini, 2012/09/26
- Re: [Qemu-devel] [PATCH v9 4/4] block: Support GlusterFS as a QEMU block backend., Bharata B Rao, 2012/09/26
- Re: [Qemu-devel] [PATCH v9 4/4] block: Support GlusterFS as a QEMU block backend., Paolo Bonzini, 2012/09/26
- Re: [Qemu-devel] [PATCH v9 4/4] block: Support GlusterFS as a QEMU block backend., Bharata B Rao, 2012/09/27
- Re: [Qemu-devel] [PATCH v9 4/4] block: Support GlusterFS as a QEMU block backend.,
Paolo Bonzini <=
- Re: [Qemu-devel] [PATCH v9 4/4] block: Support GlusterFS as a QEMU block backend., Bharata B Rao, 2012/09/27
- Re: [Qemu-devel] [PATCH v9 4/4] block: Support GlusterFS as a QEMU block backend., Paolo Bonzini, 2012/09/27