From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52362) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WGwL3-0004iO-Sm for qemu-devel@nongnu.org; Fri, 21 Feb 2014 15:04:54 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WGwKy-0001VA-Nc for qemu-devel@nongnu.org; Fri, 21 Feb 2014 15:04:49 -0500 Received: from lnantes-156-75-100-125.w80-12.abo.wanadoo.fr ([80.12.84.125]:58630 helo=paradis.irqsave.net) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WGwKy-0001V4-Dq for qemu-devel@nongnu.org; Fri, 21 Feb 2014 15:04:44 -0500 Date: Fri, 21 Feb 2014 21:04:43 +0100 From: =?iso-8859-1?Q?Beno=EEt?= Canet Message-ID: <20140221200443.GA3165@irqsave.net> References: <1393011848-18742-1-git-send-email-mreitz@redhat.com> <1393011848-18742-2-git-send-email-mreitz@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: <1393011848-18742-2-git-send-email-mreitz@redhat.com> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 1/2] quorum: Simplify quorum_open() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Max Reitz Cc: Kevin Wolf , qemu-devel@nongnu.org, Stefan Hajnoczi The Friday 21 Feb 2014 =E0 20:44:07 (+0100), Max Reitz wrote : > Although it may not look like it, this patch simplifies quorum_open(). > qdict_array_split() is now able to return QLists with different objects > than only QDicts, therefore it will now do all the work and > quorum_open() does not have to handle reference strings by itself. >=20 > This allows mixing full option dicts and reference strings for > specifying the child block devices of quorum; furthermore, it improves > handling of malformed specifications. >=20 > Signed-off-by: Max Reitz > --- > block/quorum.c | 62 +++++++++++++++++++++++++++++++++++---------------= -------- > 1 file changed, 38 insertions(+), 24 deletions(-) >=20 > diff --git a/block/quorum.c b/block/quorum.c > index 18721ba..8f5833d 100644 > --- a/block/quorum.c > +++ b/block/quorum.c > @@ -720,7 +720,6 @@ static int quorum_open(BlockDriverState *bs, QDict = *options, int flags, > QDict *sub =3D NULL; > QList *list =3D NULL; > const QListEntry *lentry; > - const QDictEntry *dentry; > int i; > int ret =3D 0; > =20 > @@ -728,10 +727,17 @@ static int quorum_open(BlockDriverState *bs, QDic= t *options, int flags, > qdict_extract_subqdict(options, &sub, "children."); > qdict_array_split(sub, &list); > =20 > + if (qdict_size(sub)) { > + error_setg(&local_err, "Invalid option children.%s", > + qdict_first(sub)->key); > + ret =3D -EINVAL; > + goto exit; > + } > + > /* count how many different children are present and validate > * qdict_size(sub) address the open by reference case > */ I think the second part of the comment about qlist_size(sub) is an extra = now. Aside from that: Reviewed-by: Benoit Canet > - s->num_children =3D !qlist_size(list) ? qdict_size(sub) : qlist_si= ze(list); > + s->num_children =3D qlist_size(list); > if (s->num_children < 2) { > error_setg(&local_err, > "Number of provided children must be greater than 1= "); > @@ -767,30 +773,38 @@ static int quorum_open(BlockDriverState *bs, QDic= t *options, int flags, > s->bs =3D g_new0(BlockDriverState *, s->num_children); > opened =3D g_new0(bool, s->num_children); > =20 > - /* 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); > - if (ret < 0) { > - goto close_exit; > - } > - opened[i] =3D true; > + for (i =3D 0, lentry =3D qlist_first(list); lentry; > + lentry =3D qlist_next(lentry), i++) { > + QDict *d; > + QString *string; > + > + switch (qobject_type(lentry->value)) > + { > + /* List of options */ > + case QTYPE_QDICT: > + d =3D qobject_to_qdict(lentry->value); > + QINCREF(d); > + ret =3D bdrv_open(&s->bs[i], NULL, NULL, d, flags, NUL= L, > + &local_err); > + break; > + > + /* QMP reference */ > + case QTYPE_QSTRING: > + string =3D qobject_to_qstring(lentry->value); > + ret =3D bdrv_open(&s->bs[i], NULL, qstring_get_str(str= ing), NULL, > + flags, NULL, &local_err); > + break; > + > + default: > + error_setg(&local_err, "Specification of child block d= evice %i " > + "is invalid", i); > + ret =3D -EINVAL; > } > - /* 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); > - if (ret < 0) { > - goto close_exit; > - } > - opened[i] =3D true; > + > + if (ret < 0) { > + goto close_exit; > } > + opened[i] =3D true; > } > =20 > g_free(opened); > --=20 > 1.9.0 >=20