[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
signature.asc
Description: OpenPGP digital signature
- [Qemu-devel] [RFC V6 00/11] Quorum block filter., Benoît Canet, 2013/01/17
- [Qemu-devel] [RFC V6 03/11] quorum: Add quorum_open() and quorum_close()., Benoît Canet, 2013/01/17
- Re: [Qemu-devel] [RFC V6 03/11] quorum: Add quorum_open() and quorum_close().,
Eric Blake <=
- [Qemu-devel] [RFC V6 04/11] quorum: Add quorum_aio_writev and its dependencies., Benoît Canet, 2013/01/17
- [Qemu-devel] [RFC V6 06/11] quorum: Add quorum_aio_readv., Benoît Canet, 2013/01/17
- [Qemu-devel] [RFC V6 05/11] blkverify: Extract qemu_iovec_clone() and qemu_iovec_compare() from blkverify., Benoît Canet, 2013/01/17
- [Qemu-devel] [RFC V6 09/11] quorum: Add quorum_invalidate_cache()., Benoît Canet, 2013/01/17
- [Qemu-devel] [RFC V6 10/11] quorum: Add quorum_co_is_allocated., Benoît Canet, 2013/01/17
- [Qemu-devel] [RFC V6 08/11] quorum: Add quorum_getlength()., Benoît Canet, 2013/01/17
- [Qemu-devel] [RFC V6 07/11] quorum: Add quorum mechanism., Benoît Canet, 2013/01/17
- [Qemu-devel] [RFC V6 11/11] quorum: Add quorum_co_flush()., Benoît Canet, 2013/01/17