| Message ID | 1344347073-7773-4-git-send-email-benoit@irqsave.net |
|---|---|
| State | New |
| Headers | show |
On Tue, Aug 7, 2012 at 1:44 PM, Benoît Canet <benoit.canet@gmail.com> wrote: > Signed-off-by: Benoit Canet <benoit@irqsave.net> > --- > block/quorum.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 62 insertions(+) > > diff --git a/block/quorum.c b/block/quorum.c > index e0405b6..de58ab8 100644 > --- a/block/quorum.c > +++ b/block/quorum.c > @@ -47,11 +47,73 @@ struct QuorumAIOCB { > int vote_ret; > }; > > +/* Valid quorum filenames look like > + * quorum:path/to/a_image:path/to/b_image:path/to/c_image This syntax would mean that stacking for example curl or other network paths would not be possible. How about comma as separator? > + */ > +static int quorum_open(BlockDriverState *bs, const char *filename, int flags) > +{ > + BDRVQuorumState *s = bs->opaque; > + int ret, i; > + char *a, *b, *c, *filenames[3]; > + > + /* Parse the quorum: prefix */ > + if (strncmp(filename, "quorum:", strlen("quorum:"))) { > + return -EINVAL; > + } > + a = g_strdup(filename + strlen("quorum:")); > + > + /* Find separators */ > + b = strchr(a, ':'); > + if (b == NULL) { > + return -EINVAL; > + } > + > + c = strrchr(a, ':'); > + if (c == NULL) { > + return -EINVAL; > + } > + > + /* Check that filename contains two separate ':' */ > + if (b == c) { > + return -EINVAL; > + } > + > + /* Split string */ > + *b = '\0'; > + *c = '\0'; > + > + filenames[0] = a; > + filenames[1] = b + 1; > + filenames[2] = c + 1; > + > + /* Open files */ > + for (i = 0; i <= 2; i++) { > + s->bs[i] = bdrv_new(""); > + ret = bdrv_open(s->bs[i], filenames[i], flags, NULL); > + if (ret < 0) { > + goto error_exit; Successfully opening two out of three should be enough, but maybe it does not make much sense. > + } > + } > + > + goto clean_exit; > + > +error_exit: > + for (; i >= 0; i--) { > + bdrv_delete(s->bs[i]); bdrv_close() instead? > + s->bs[i] = NULL; > + } > +clean_exit: > + g_free(a); > + return ret; > +} > + > static BlockDriver bdrv_quorum = { > .format_name = "quorum", > .protocol_name = "quorum", > > .instance_size = sizeof(BDRVQuorumState), > + > + .bdrv_file_open = quorum_open, > }; > > static void bdrv_quorum_init(void) > -- > 1.7.9.5 >
On 08/07/2012 02:30 PM, Blue Swirl wrote: > On Tue, Aug 7, 2012 at 1:44 PM, Benoît Canet <benoit.canet@gmail.com> wrote: >> Signed-off-by: Benoit Canet <benoit@irqsave.net> >> --- >> block/quorum.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 62 insertions(+) >> >> diff --git a/block/quorum.c b/block/quorum.c >> index e0405b6..de58ab8 100644 >> --- a/block/quorum.c >> +++ b/block/quorum.c >> @@ -47,11 +47,73 @@ struct QuorumAIOCB { >> int vote_ret; >> }; >> >> +/* Valid quorum filenames look like >> + * quorum:path/to/a_image:path/to/b_image:path/to/c_image > > This syntax would mean that stacking for example curl or other network > paths would not be possible. How about comma as separator? Also, what escaping mechanism is in place for allowing a file containing the same character as the separator (whether you end up with : or , as the separator)? If you support a larger quorum (whether always n/(2n-1) or whether fully configurable n/m), rather than hard-coded 2/3, then you also need a way to specify how many quorum members will follow.
On Tue, Aug 7, 2012 at 2:44 PM, Benoît Canet <benoit.canet@gmail.com> wrote: > Signed-off-by: Benoit Canet <benoit@irqsave.net> > --- > block/quorum.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 62 insertions(+) > > diff --git a/block/quorum.c b/block/quorum.c > index e0405b6..de58ab8 100644 > --- a/block/quorum.c > +++ b/block/quorum.c > @@ -47,11 +47,73 @@ struct QuorumAIOCB { > int vote_ret; > }; > > +/* Valid quorum filenames look like > + * quorum:path/to/a_image:path/to/b_image:path/to/c_image > + */ > +static int quorum_open(BlockDriverState *bs, const char *filename, int flags) > +{ > + BDRVQuorumState *s = bs->opaque; > + int ret, i; > + char *a, *b, *c, *filenames[3]; > + > + /* Parse the quorum: prefix */ > + if (strncmp(filename, "quorum:", strlen("quorum:"))) { > + return -EINVAL; > + } > + a = g_strdup(filename + strlen("quorum:")); > + > + /* Find separators */ > + b = strchr(a, ':'); > + if (b == NULL) { > + return -EINVAL; Leaks a. Same for returns below. > static BlockDriver bdrv_quorum = { > .format_name = "quorum", > .protocol_name = "quorum", > > .instance_size = sizeof(BDRVQuorumState), > + > + .bdrv_file_open = quorum_open, Please squash quorum_close() into this commit so the g_strdup() filesnames[] aren't leaked on close.
Le Tuesday 07 Aug 2012 à 20:30:09 (+0000), Blue Swirl a écrit : > On Tue, Aug 7, 2012 at 1:44 PM, Benoît Canet <benoit.canet@gmail.com> wrote: > > Signed-off-by: Benoit Canet <benoit@irqsave.net> > > --- > > block/quorum.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 62 insertions(+) > > > > diff --git a/block/quorum.c b/block/quorum.c > > index e0405b6..de58ab8 100644 > > --- a/block/quorum.c > > +++ b/block/quorum.c > > @@ -47,11 +47,73 @@ struct QuorumAIOCB { > > int vote_ret; > > }; > > > > +/* Valid quorum filenames look like > > + * quorum:path/to/a_image:path/to/b_image:path/to/c_image > > This syntax would mean that stacking for example curl or other network > paths would not be possible. How about comma as separator? I just tried comma but it fail because the qemu command line parsing catch it believing that the string next to the coma is another "file=" like options. Is there any other separator we can use ? Benoît
Am 10.08.2012 19:48, schrieb Benoît Canet: > Le Tuesday 07 Aug 2012 à 20:30:09 (+0000), Blue Swirl a écrit : >> On Tue, Aug 7, 2012 at 1:44 PM, Benoît Canet <benoit.canet@gmail.com> wrote: >>> Signed-off-by: Benoit Canet <benoit@irqsave.net> >>> --- >>> block/quorum.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ >>> 1 file changed, 62 insertions(+) >>> >>> diff --git a/block/quorum.c b/block/quorum.c >>> index e0405b6..de58ab8 100644 >>> --- a/block/quorum.c >>> +++ b/block/quorum.c >>> @@ -47,11 +47,73 @@ struct QuorumAIOCB { >>> int vote_ret; >>> }; >>> >>> +/* Valid quorum filenames look like >>> + * quorum:path/to/a_image:path/to/b_image:path/to/c_image >> >> This syntax would mean that stacking for example curl or other network >> paths would not be possible. How about comma as separator? > > I just tried comma but it fail because the qemu command line parsing > catch it believing that the string next to the coma is another "file=" > like options. > > Is there any other separator we can use ? I would ignore that for now, or you can introduce your own escaping of colons. The real solution is, once again, -blockdev. Kevin
Le Monday 13 Aug 2012 à 09:41:00 (+0200), Kevin Wolf a écrit : > Am 10.08.2012 19:48, schrieb Benoît Canet: > > Le Tuesday 07 Aug 2012 à 20:30:09 (+0000), Blue Swirl a écrit : > >> On Tue, Aug 7, 2012 at 1:44 PM, Benoît Canet <benoit.canet@gmail.com> wrote: > >>> Signed-off-by: Benoit Canet <benoit@irqsave.net> > >>> --- > >>> block/quorum.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > >>> 1 file changed, 62 insertions(+) > >>> > >>> diff --git a/block/quorum.c b/block/quorum.c > >>> index e0405b6..de58ab8 100644 > >>> --- a/block/quorum.c > >>> +++ b/block/quorum.c > >>> @@ -47,11 +47,73 @@ struct QuorumAIOCB { > >>> int vote_ret; > >>> }; > >>> > >>> +/* Valid quorum filenames look like > >>> + * quorum:path/to/a_image:path/to/b_image:path/to/c_image > >> > >> This syntax would mean that stacking for example curl or other network > >> paths would not be possible. How about comma as separator? > > > > I just tried comma but it fail because the qemu command line parsing > > catch it believing that the string next to the coma is another "file=" > > like options. > > > > Is there any other separator we can use ? > > I would ignore that for now, or you can introduce your own escaping of > colons. The real solution is, once again, -blockdev. Is -blockdev related to BlockBackend ? Benoît > > Kevin >
Am 14.08.2012 18:56, schrieb Benoît Canet: > Le Monday 13 Aug 2012 à 09:41:00 (+0200), Kevin Wolf a écrit : >> Am 10.08.2012 19:48, schrieb Benoît Canet: >>> Le Tuesday 07 Aug 2012 à 20:30:09 (+0000), Blue Swirl a écrit : >>>> On Tue, Aug 7, 2012 at 1:44 PM, Benoît Canet <benoit.canet@gmail.com> wrote: >>>>> Signed-off-by: Benoit Canet <benoit@irqsave.net> >>>>> --- >>>>> block/quorum.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ >>>>> 1 file changed, 62 insertions(+) >>>>> >>>>> diff --git a/block/quorum.c b/block/quorum.c >>>>> index e0405b6..de58ab8 100644 >>>>> --- a/block/quorum.c >>>>> +++ b/block/quorum.c >>>>> @@ -47,11 +47,73 @@ struct QuorumAIOCB { >>>>> int vote_ret; >>>>> }; >>>>> >>>>> +/* Valid quorum filenames look like >>>>> + * quorum:path/to/a_image:path/to/b_image:path/to/c_image >>>> >>>> This syntax would mean that stacking for example curl or other network >>>> paths would not be possible. How about comma as separator? >>> >>> I just tried comma but it fail because the qemu command line parsing >>> catch it believing that the string next to the coma is another "file=" >>> like options. >>> >>> Is there any other separator we can use ? >> >> I would ignore that for now, or you can introduce your own escaping of >> colons. The real solution is, once again, -blockdev. > > Is -blockdev related to BlockBackend ? BlockBackend is a prerequisite for it. One of the goals with -blockdev is to have not only a filename for bdrv_open from which you can parse options, but that you can get structured and driver-specific options. Kevin
diff --git a/block/quorum.c b/block/quorum.c index e0405b6..de58ab8 100644 --- a/block/quorum.c +++ b/block/quorum.c @@ -47,11 +47,73 @@ struct QuorumAIOCB { int vote_ret; }; +/* Valid quorum filenames look like + * quorum:path/to/a_image:path/to/b_image:path/to/c_image + */ +static int quorum_open(BlockDriverState *bs, const char *filename, int flags) +{ + BDRVQuorumState *s = bs->opaque; + int ret, i; + char *a, *b, *c, *filenames[3]; + + /* Parse the quorum: prefix */ + if (strncmp(filename, "quorum:", strlen("quorum:"))) { + return -EINVAL; + } + a = g_strdup(filename + strlen("quorum:")); + + /* Find separators */ + b = strchr(a, ':'); + if (b == NULL) { + return -EINVAL; + } + + c = strrchr(a, ':'); + if (c == NULL) { + return -EINVAL; + } + + /* Check that filename contains two separate ':' */ + if (b == c) { + return -EINVAL; + } + + /* Split string */ + *b = '\0'; + *c = '\0'; + + filenames[0] = a; + filenames[1] = b + 1; + filenames[2] = c + 1; + + /* Open files */ + for (i = 0; i <= 2; i++) { + s->bs[i] = bdrv_new(""); + ret = bdrv_open(s->bs[i], filenames[i], flags, NULL); + if (ret < 0) { + goto error_exit; + } + } + + goto clean_exit; + +error_exit: + for (; i >= 0; i--) { + bdrv_delete(s->bs[i]); + s->bs[i] = NULL; + } +clean_exit: + g_free(a); + return ret; +} + static BlockDriver bdrv_quorum = { .format_name = "quorum", .protocol_name = "quorum", .instance_size = sizeof(BDRVQuorumState), + + .bdrv_file_open = quorum_open, }; static void bdrv_quorum_init(void)
Signed-off-by: Benoit Canet <benoit@irqsave.net> --- block/quorum.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+)