Patchwork [RFC,V5,03/11] quorum: Add quorum_open() and quorum_close().

login
register
mail settings
Submitter Benoit Canet
Date Aug. 27, 2012, 7:30 a.m.
Message ID <1346052629-15686-4-git-send-email-benoit@irqsave.net>
Download mbox | patch
Permalink /patch/180153/
State New
Headers show

Comments

Benoit Canet - Aug. 27, 2012, 7:30 a.m.
Valid quorum resources look like
quorum:threshold/total:path/to/image_1, ... ,path/to/image_total

',' is used as a separator to allow to use networked path
'\' is the escaping character for filename containing ','
'\' escape itself

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 \\)

Signed-off-by: Benoit Canet <benoit@irqsave.net>
---
 block/quorum.c |  123 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 123 insertions(+)
Eric Blake - Aug. 27, 2012, 5:59 p.m.
On 08/27/2012 01:30 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 to allow to use networked path

Isn't this a step backwards?  After all, on the command line, we would
have something like:

-drive file=quorum:...,readonly=on

but if the 'quorum:...' portion contains commas, then the qemu option
parsing requires you to write those commas as doubled.  Which means to
escape those commas for _both_ quorum: processing _and_ qemu command
line parsing, I'd have to write:

-drive "file=quorum:2/2:path/to/image\\,,1,,path/to/image\\\\2,readonly=on"

to use the two files:
path/to/image,1
path/to/image\2

with appropriate shell, qemu, and quorum escaping.  Using : rather than
, as the separator between quorum: elements was a bit simpler, since I
don't have to type double commas for every single comma to be taken by
the quorum parsing, escaped or not.

> '\' is the escaping character for filename containing ','
> '\' escape itself

s/escape/escapes/

> 
> 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 \\)

Yes, that's what I'm worried about.
Benoît Canet - Aug. 27, 2012, 7:20 p.m.
Le Monday 27 Aug 2012 à 11:59:34 (-0600), Eric Blake a écrit :
> On 08/27/2012 01:30 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 to allow to use networked path
> 
> Isn't this a step backwards?  After all, on the command line, we would
> have something like:
> 
> -drive file=quorum:...,readonly=on
> 
> but if the 'quorum:...' portion contains commas, then the qemu option
> parsing requires you to write those commas as doubled.  Which means to
> escape those commas for _both_ quorum: processing _and_ qemu command
> line parsing, I'd have to write:
> 
> -drive "file=quorum:2/2:path/to/image\\,,1,,path/to/image\\\\2,readonly=on"
> 
> to use the two files:
> path/to/image,1
> path/to/image\2
> 
> with appropriate shell, qemu, and quorum escaping.  Using : rather than
> , as the separator between quorum: elements was a bit simpler, since I
> don't have to type double commas for every single comma to be taken by
> the quorum parsing, escaped or not.
> 
> > '\' is the escaping character for filename containing ','
> > '\' escape itself
> 
> s/escape/escapes/
> 
> > 
> > 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 \\)
> 
> Yes, that's what I'm worried about.

I was following an idea of Blue Swirl.

You will have to use \\ on the command line.
':' can be used instead of ',' but it won't work
with networked urls.

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

Patch

diff --git a/block/quorum.c b/block/quorum.c
index 19a9a44..b9fb2b9 100644
--- a/block/quorum.c
+++ b/block/quorum.c
@@ -52,11 +52,134 @@  struct QuorumAIOCB {
     int vote_ret;
 };
 
+/* Valid quorum resources look like
+ * quorum:threshold/total:path/to/image_1, ... ,path/to/image_total
+ *
+ * ',' is used as a separator to allow to use network path
+ * '\' is the escaping character for filename containing ','
+ */
+static int quorum_open(BlockDriverState *bs, const char *filename, int flags)
+{
+    BDRVQuorumState *s = bs->opaque;
+    int i, j, k, len, ret = 0;
+    char *a, *b, *names;
+    bool escape;
+
+    /* Parse the quorum: prefix */
+    if (strncmp(filename, "quorum:", strlen("quorum:"))) {
+        return -EINVAL;
+    }
+
+    filename += strlen("quorum:");
+
+    /* Get threshold */
+    errno = 0;
+    s->threshold = strtoul(filename, &a, 10);
+    if (*a != '/' || errno) {
+        return -EINVAL;
+    }
+    a++;
+
+    /* Get total */
+    errno = 0;
+    s->total = strtoul(a, &b, 10);
+    if (*b != ':' || errno) {
+        return -EINVAL;
+    }
+    b++;
+
+    if (s->threshold < 1 || s->total < 2) {
+        return -EINVAL;
+    }
+
+    if (s->threshold > s->total) {
+        return -EINVAL;
+    }
+
+    s->bs = g_malloc0(sizeof(BlockDriverState *) * s->total);
+    /* Two allocations for all filenames: simpler to free */
+    s->filenames = g_malloc0(sizeof(char *) * s->total);
+    names = g_strdup(b);
+
+    /* Get the filenames pointers */
+    escape = false;
+    s->filenames[0] = names;
+    len = strlen(names);
+    for (i = j = k = 0; i < len && j < s->total; i++) {
+        /* separation between two files */
+        if (!escape && names[i] == ',') {
+            char *prev = s->filenames[j];
+            prev[k] = '\0';
+            s->filenames[++j] = prev + k + 1;
+            k = 0;
+            continue;
+        }
+
+        escape = !escape && names[i] == '\\';
+
+        /* if we are not escaping copy */
+        if (!escape) {
+            s->filenames[j][k++] = names[i];
+        }
+    }
+    /* terminate last string */
+    s->filenames[j][k] = '\0';
+
+    if ((j + 1) != s->total) {
+        ret = -EINVAL;
+        goto free_exit;
+    }
+
+    /* Open files */
+    for (i = 0; i < s->total; i++) {
+        s->bs[i] = bdrv_new("");
+        ret = bdrv_open(s->bs[i], s->filenames[i], flags, NULL);
+        if (ret < 0) {
+            goto error_exit;
+        }
+    }
+
+    goto exit;
+
+error_exit:
+    for (; i >= 0; i--) {
+        bdrv_delete(s->bs[i]);
+        s->bs[i] = NULL;
+    }
+free_exit:
+    g_free(s->filenames[0]);
+    g_free(s->filenames);
+    s->filenames = NULL;
+    g_free(s->bs);
+exit:
+    return ret;
+}
+
+static void quorum_close(BlockDriverState *bs)
+{
+    BDRVQuorumState *s = bs->opaque;
+    int i;
+
+    for (i = 0; i < s->total; i++) {
+        /* Ensure writes reach stable storage */
+        bdrv_flush(s->bs[i]);
+        bdrv_delete(s->bs[i]);
+    }
+
+    g_free(s->filenames[0]);
+    g_free(s->filenames);
+    s->filenames = NULL;
+    g_free(s->bs);
+}
+
 static BlockDriver bdrv_quorum = {
     .format_name        = "quorum",
     .protocol_name      = "quorum",
 
     .instance_size      = sizeof(BDRVQuorumState),
+
+    .bdrv_file_open     = quorum_open,
+    .bdrv_close         = quorum_close,
 };
 
 static void bdrv_quorum_init(void)