From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59492) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WGymo-0004oU-MH for qemu-devel@nongnu.org; Fri, 21 Feb 2014 17:41:43 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WGymj-0001vz-Pi for qemu-devel@nongnu.org; Fri, 21 Feb 2014 17:41:38 -0500 Received: from mx1.redhat.com ([209.132.183.28]:17763) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WGymj-0001vu-HX for qemu-devel@nongnu.org; Fri, 21 Feb 2014 17:41:33 -0500 Message-ID: <5307D618.4090408@redhat.com> Date: Fri, 21 Feb 2014 15:41:28 -0700 From: Eric Blake MIME-Version: 1.0 References: <1393017681-12794-1-git-send-email-benoit.canet@irqsave.net> <1393017681-12794-7-git-send-email-benoit.canet@irqsave.net> <5307CEA6.5050003@redhat.com> <20140221223045.GB13076@irqsave.net> In-Reply-To: <20140221223045.GB13076@irqsave.net> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="eq6CHdLATk8nqH8xbcN03OwsdLQHbXiqc" Subject: Re: [Qemu-devel] [PATCH V19 06/12] quorum: Add quorum mechanism. List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?UTF-8?B?QmVub8OudCBDYW5ldA==?= Cc: kwolf@redhat.com, qemu-devel@nongnu.org, stefanha@redhat.com, mreitz@redhat.com This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --eq6CHdLATk8nqH8xbcN03OwsdLQHbXiqc Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 02/21/2014 03:30 PM, Beno=C3=AEt Canet wrote: >>> + >>> +- "ret": The IO return code. >> >> What values is this likely to contain? Is it a finite set, in which >> case it would be nice to have a QAPI enum that describes the set of >> return codes, rather than a raw number? >=20 > It's anything that the block stack could return as an error. Yes, but returning a raw integer is not friendly. What do those integers mean? In this patch, I found only two callers that set this parameter: quorum_report_bad_versions: + quorum_report_bad(acb, s->bs[item->index]->node_name, 0); So the code means nothing there, because you always pass 0. quroum_aio_cb: if (ret =3D=3D 0) { acb->success_count++; + } else { + quorum_report_bad(acb, sacb->aiocb->bs->node_name, ret); Here, ret is non-zero - but will it ever be anything other than -1? Seriously, I think you should change this to NOT be an integer, but instead be an enum value where the enum is tracked in qapi-schema.json. Change the signature of quorum_report_bad to take the enum value, rather than a raw int. And over the wire, the QMP event will appear with a string encoding of the enum name, where we can add named errors to the enum type as we come up with what different error codes we want to distinguish. I'm opposed to the current nebulous "it's whatever the block stack wanted to report" without knowing what it means. If you can't switch to an enum, it may be better to just delete the field entirely, if you can't justify what it is doing. QMP-wise, we can always add more information later, but once the release happens, we can't take away information, even if later refactoring makes it harder to support. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --eq6CHdLATk8nqH8xbcN03OwsdLQHbXiqc Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJTB9YZAAoJEKeha0olJ0NqH7YH/iXZMV6z9Nih3ORSRshuEg7v P2cplErQ9Aw3LUM7b4TpZgL0U3sHfDiBkIdNvKf0E701RAJxAFiAyAocaIxFYrmU D7N5lJDHfcWTjzZpTKfglGGoWsU2K0DYgPHi3gK35tSscHevTVQrGWls6/dUoDN+ CO/s5btqGMB/eiTYSFsbq7y63VshhDUkOf2xs/uKWVzHdAf+HYnYBpIFSB8rtvAP /MK4j9BTykQlsmjs7neju7VeSi5xioJcjTQ7HKo8AXLJDKrtUMP87fIanSQUEmOF oFVi5UVZlfmmFuPaNqHks1RPjxBWF94voKJQSf6Obv+SX7UTnZzVD/S9/T/hfVQ= =/mFp -----END PGP SIGNATURE----- --eq6CHdLATk8nqH8xbcN03OwsdLQHbXiqc--