Re: [Qemu-devel] [RFC V6 03/11] quorum: Add quorum_open() and quorum_close().

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1898 bytes --]

On 01/17/2013 08:51 AM, Benoît Canet wrote:
> Valid quorum resources look like
> quorum:threshold/total:path/to/image_1: ... :path/to/image_total
> 
> ':' is used as a separator
> '\' is the escaping character for filename containing ':'
> '\' escape itself
> ',' must be escaped with ','
> 
> On the command line for quorum files "img:test.raw", "img2,raw"
> and "img3.raw" invocation look like:
> 
> -drive file=quorum:2/3:img\\:test.raw:img2,,raw:img3.raw
> (note the double \\ and the double ,,)
> 
> Signed-off-by: Benoit Canet <benoit@irqsave.net>

> +    /* Get threshold */
> +    errno = 0;
> +    s->threshold = strtoul(start, &a, 10);
> +    if (*a != '/' || errno) {
> +        return -EINVAL;
> +    }
> +    a++;

Hmm - you can fail to reject file=quorum:/3:... (strtoul happily parses
a to 0 in that case, and is not required to set errno).  But see below...

> +
> +    /* Get total */
> +    errno = 0;
> +    s->total = strtoul(a, &b, 10);
> +    if (*b != ':' || errno) {
> +        return -EINVAL;
> +    }
> +    b++;

Again, you fail to reject file=qourum:1/:... (strtoul happily parses b
to 0 in that case, and is not required to set errno)...

> +
> +    if (s->threshold < 1 || s->total < 2) {
> +        return -EINVAL;
> +    }

...but you got lucky: this check rejects either a or b being set to 0.
Still, you may want to refactor this patch on top of
https://lists.gnu.org/archive/html/qemu-devel/2013-01/msg03238.html.


> +    if ((j + 1) != s->total) {
> +        ret = -EINVAL;
> +        goto free_exit;
> +    }

You have a lot of reasons why this function can fail with -EINVAL; it
would be nicer if you actually set an error object describing each
failure, instead of making the user guess.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]