From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34349) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WEUXy-0006VP-VQ for qemu-devel@nongnu.org; Fri, 14 Feb 2014 21:00:08 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WEUXt-0006gJ-Rv for qemu-devel@nongnu.org; Fri, 14 Feb 2014 21:00:02 -0500 Received: from mx1.redhat.com ([209.132.183.28]:43820) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WEUXt-0006gD-Jv for qemu-devel@nongnu.org; Fri, 14 Feb 2014 20:59:57 -0500 Message-ID: <52FECAA8.7060401@redhat.com> Date: Sat, 15 Feb 2014 03:02:16 +0100 From: Max Reitz MIME-Version: 1.0 References: <1392242799-16364-1-git-send-email-benoit.canet@irqsave.net> <1392242799-16364-12-git-send-email-benoit.canet@irqsave.net> In-Reply-To: <1392242799-16364-12-git-send-email-benoit.canet@irqsave.net> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH V17 11/12] quorum: Add quorum_open() and quorum_close(). List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?UTF-8?B?QmVub8OudCBDYW5ldA==?= , qemu-devel@nongnu.org Cc: kwolf@redhat.com, famz@redhat.com, =?UTF-8?B?QmVub8OudCBDYW5ldA==?= , stefanha@redhat.com On 12.02.2014 23:06, Beno=C3=AEt Canet wrote: > From: Beno=C3=AEt Canet > > Example of command line: > -drive if=3Dvirtio,file.driver=3Dquorum,\ > file.children.0.file.filename=3D1.raw,\ > file.children.0.node-name=3D1.raw,\ > file.children.0.driver=3Draw,\ > file.children.1.file.filename=3D2.raw,\ > file.children.1.node-name=3D2.raw,\ > file.children.1.driver=3Draw,\ > file.children.2.file.filename=3D3.raw,\ > file.children.2.node-name=3D3.raw,\ > file.children.2.driver=3Draw,\ > file.vote-threshold=3D2 > > file.blkverify=3Don with file.vote-threshold=3D2 and two files can be p= assed to > emulated blkverify. > > Signed-off-by: Benoit Canet > --- > block/quorum.c | 165 ++++++++++++++++++++++++++++++++++++++++++++++= +++++++++ > monitor.c | 2 + > qapi-schema.json | 21 ++++++- > 3 files changed, 187 insertions(+), 1 deletion(-) > > diff --git a/block/quorum.c b/block/quorum.c > index 8e033ef..d3a90fb 100644 > --- a/block/quorum.c > +++ b/block/quorum.c > @@ -20,6 +20,9 @@ > =20 > #define HASH_LENGTH 32 > =20 > +#define QUORUM_OPT_VOTE_THRESHOLD "vote-threshold" > +#define QUORUM_OPT_BLKVERIFY "blkverify" > + > /* This union holds a vote hash value */ > typedef union QuorumVoteValue { > char h[HASH_LENGTH]; /* SHA-256 hash */ > @@ -677,12 +680,174 @@ static bool quorum_recurse_is_first_non_filter(B= lockDriverState *bs, > return false; > } > =20 > +static int quorum_valid_threshold(int threshold, > + int num_children, > + Error **errp) This fits on a single line and I'm all for maximizing usage of the 80=20 character limit. > +{ > + > + if (threshold < 1) { > + error_set(errp, QERR_INVALID_PARAMETER_VALUE, > + "vote-threshold", "value >=3D 1"); > + return -ERANGE; > + } > + > + if (threshold > num_children) { > + error_setg(errp, "threshold may not exceed children count"); > + return -ERANGE; > + } > + > + return 0; > +} > + > +static QemuOptsList quorum_runtime_opts =3D { > + .name =3D "quorum", > + .head =3D QTAILQ_HEAD_INITIALIZER(quorum_runtime_opts.head), > + .desc =3D { > + { > + .name =3D QUORUM_OPT_VOTE_THRESHOLD, > + .type =3D QEMU_OPT_NUMBER, > + .help =3D "The number of vote needed for reaching quorum", > + }, > + { > + .name =3D QUORUM_OPT_BLKVERIFY, > + .type =3D QEMU_OPT_BOOL, > + .help =3D "Trigger block verify mode if set", > + }, > + { /* end of list */ } > + }, > +}; > + > +static int quorum_open(BlockDriverState *bs, > + QDict *options, > + int flags, > + Error **errp) This takes two lines (although since you have to split at all, I can=20 cope with this being split once per parameter). > +{ > + BDRVQuorumState *s =3D bs->opaque; > + Error *local_err =3D NULL; > + QemuOpts *opts; > + bool *opened; > + QDict *sub =3D NULL; > + QList *list =3D NULL; > + const QListEntry *lentry; > + const QDictEntry *dentry; > + int i; > + int ret =3D 0; > + > + qdict_flatten(options); > + qdict_extract_subqdict(options, &sub, "children."); > + qdict_array_split(sub, &list); > + > + /* count how many different children are present and validate > + * qdict_size(sub) address the open by reference case > + */ > + s->num_children =3D !qlist_size(list) ? qdict_size(sub) : qlist_si= ze(list); Okay, so eventually we probably want to add these values in order to=20 allow "mixed" parameters (some references, some "full" specifications).=20 But for that to work without much hassle, qdict_array_split() still=20 needs some extension (or be fixed?) to split not just QDicts but other=20 objects as well. I think we (probably I) should amend=20 qdict_array_split() first (perhaps add some boolean parameter to specify=20 what it should do) and then fix this (that is, make mixed parameters=20 work and allow detection of malformed parameters). For this series, this is fine. > + if (s->num_children < 2) { > + error_setg(&local_err, > + "Number of provided children must be greater than 1= "); > + ret =3D -EINVAL; > + goto exit; > + } > + > + opts =3D qemu_opts_create(&quorum_runtime_opts, NULL, 0, &error_ab= ort); > + qemu_opts_absorb_qdict(opts, options, &local_err); > + if (error_is_set(&local_err)) { > + ret =3D -EINVAL; > + goto exit; > + } > + > + s->threshold =3D qemu_opt_get_number(opts, QUORUM_OPT_VOTE_THRESHO= LD, 0); > + > + /* and validate it against s->num_children */ > + ret =3D quorum_valid_threshold(s->threshold, s->num_children, &loc= al_err); > + if (ret < 0) { > + goto exit; > + } > + > + /* is the driver in blkverify mode */ > + if (qemu_opt_get_bool(opts, QUORUM_OPT_BLKVERIFY, false) && > + s->num_children =3D=3D 2 && s->threshold =3D=3D 2) { If qemu_opt_get_bool() returns true but the other conditions aren't met,=20 I'd prefer, again, a warning. > + s->is_blkverify =3D true; > + } > + > + /* allocate the children BlockDriverState array */ > + s->bs =3D g_new0(BlockDriverState *, s->num_children); > + opened =3D g_new0(bool, s->num_children); > + > + /* Open by file name or options dict (command line or QMP) */ > + if (s->num_children =3D=3D qlist_size(list)) { > + for (i =3D 0, lentry =3D qlist_first(list); lentry; > + lentry =3D qlist_next(lentry), i++) { > + QDict *d =3D qobject_to_qdict(lentry->value); > + QINCREF(d); > + ret =3D bdrv_open(&s->bs[i], NULL, NULL, > + d, > + flags, NULL, &local_err); This actually fits on a single line (sorry for my obsession with this). > + if (ret < 0) { > + goto close_exit; > + } > + opened[i] =3D true; > + } > + /* Open by QMP references */ > + } else { > + for (i =3D 0, dentry =3D qdict_first(sub); dentry; > + dentry =3D qdict_next(sub, dentry), i++) { > + QString *string =3D qobject_to_qstring(dentry->value); > + ret =3D bdrv_open(&s->bs[i], NULL, > + qstring_get_str(string), > + NULL, flags, NULL, &local_err); This takes two. > + if (ret < 0) { > + goto close_exit; > + } > + opened[i] =3D true; > + } > + } > + > + g_free(opened); > + goto exit; > + > +close_exit: > + /* cleanup on error */ > + for (i =3D 0; i < s->num_children; i++) { > + if (!opened[i]) { > + continue; > + } > + bdrv_unref(s->bs[i]); > + } > + g_free(s->bs); > + g_free(opened); > +exit: > + /* propagate error */ > + if (error_is_set(&local_err)) { > + error_propagate(errp, local_err); > + } > + QDECREF(list); > + QDECREF(sub); > + return ret; > +} > + > +static void quorum_close(BlockDriverState *bs) > +{ > + BDRVQuorumState *s =3D bs->opaque; > + int i; > + > + for (i =3D 0; i < s->num_children; i++) { > + bdrv_unref(s->bs[i]); > + } > + > + g_free(s->bs); > +} > + > static BlockDriver bdrv_quorum =3D { > .format_name =3D "quorum", > .protocol_name =3D "quorum", > =20 > .instance_size =3D sizeof(BDRVQuorumState), > =20 > + .bdrv_file_open =3D quorum_open, > + .bdrv_close =3D quorum_close, > + > + .authorizations =3D { true, true }, > + > .bdrv_co_flush_to_disk =3D quorum_co_flush, > =20 > .bdrv_getlength =3D quorum_getlength, > diff --git a/monitor.c b/monitor.c > index 81ffa0f..8b175fb 100644 > --- a/monitor.c > +++ b/monitor.c > @@ -639,6 +639,8 @@ static void monitor_protocol_event_init(void) > monitor_protocol_event_throttle(QEVENT_RTC_CHANGE, 1000); > monitor_protocol_event_throttle(QEVENT_BALLOON_CHANGE, 1000); > monitor_protocol_event_throttle(QEVENT_WATCHDOG, 1000); > + monitor_protocol_event_throttle(QEVENT_QUORUM_REPORT_BAD, 1000); > + monitor_protocol_event_throttle(QEVENT_QUORUM_FAILURE, 1000); Hm, what is this for? Max > } > =20 > /** > diff --git a/qapi-schema.json b/qapi-schema.json > index 7cfb5e5..990d0c5 100644 > --- a/qapi-schema.json > +++ b/qapi-schema.json > @@ -4352,6 +4352,24 @@ > 'raw': 'BlockdevRef' } } > =20 > ## > +# @BlockdevOptionsQuorum > +# > +# Driver specific block device options for Quorum > +# > +# @blkverify: #optional true if the driver must print content mis= match > +# > +# @children: the children block device to use > +# > +# @vote_threshold: the vote limit under which a read will fail > +# > +# Since: 2.0 > +## > +{ 'type': 'BlockdevOptionsQuorum', > + 'data': { '*blkverify': 'bool', > + 'children': [ 'BlockdevRef' ], > + 'vote-threshold': 'int' } } > + > +## > # @BlockdevOptions > # > # Options for creating a block device. > @@ -4390,7 +4408,8 @@ > 'vdi': 'BlockdevOptionsGenericFormat', > 'vhdx': 'BlockdevOptionsGenericFormat', > 'vmdk': 'BlockdevOptionsGenericCOWFormat', > - 'vpc': 'BlockdevOptionsGenericFormat' > + 'vpc': 'BlockdevOptionsGenericFormat', > + 'quorum': 'BlockdevOptionsQuorum' > } } > =20 > ##