qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Eric Blake
Subject: Re: [Qemu-devel] [RFC V6 03/11] quorum: Add quorum_open() and quorum_close().
Date: Thu, 17 Jan 2013 15:13:04 -0700
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130110 Thunderbird/17.0.2

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 <address@hidden>

> +    /* 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: signature.asc
Description: OpenPGP digital signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]