From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44931) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VqPxy-00039i-3z for qemu-devel@nongnu.org; Tue, 10 Dec 2013 11:15:26 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VqPxt-0004K9-12 for qemu-devel@nongnu.org; Tue, 10 Dec 2013 11:15:22 -0500 Received: from mx1.redhat.com ([209.132.183.28]:41247) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VqPxs-0004Jw-PZ for qemu-devel@nongnu.org; Tue, 10 Dec 2013 11:15:16 -0500 Message-ID: <52A73E0F.9010208@redhat.com> Date: Tue, 10 Dec 2013 09:15:11 -0700 From: Eric Blake MIME-Version: 1.0 References: <1386263703-19292-1-git-send-email-benoit@irqsave.net> <1386263703-19292-5-git-send-email-benoit@irqsave.net> <20131206092703.5d60345a@redhat.com> <52A1EC31.7000709@redhat.com> <20131206115215.0427a956@redhat.com> <20131209162309.GJ3549@dhcp-200-207.str.redhat.com> <20131209114109.1c4a8d5f@redhat.com> <20131210095750.GC3656@dhcp-200-207.str.redhat.com> <20131210090620.4bc73895@redhat.com> <20131210142507.GE3656@dhcp-200-207.str.redhat.com> <20131210101613.7a38dadc@redhat.com> In-Reply-To: <20131210101613.7a38dadc@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="PnoP5cNA3POEwMbRHdgAIpoooXol804UQ" Subject: Re: [Qemu-devel] [PATCH V4 4/7] qmp: Allow to change password on names block driver states. List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Luiz Capitulino , Kevin Wolf Cc: famz@redhat.com, =?UTF-8?B?QmVub8OudCBDYW5ldA==?= , jcody@redhat.com, armbru@redhat.com, qemu-devel@nongnu.org, stefanha@redhat.com This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --PnoP5cNA3POEwMbRHdgAIpoooXol804UQ Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 12/10/2013 08:16 AM, Luiz Capitulino wrote: > On Tue, 10 Dec 2013 15:25:07 +0100 > Kevin Wolf wrote: >=20 >> My objection to your approach is strong because Beno=C3=AEt already se= nt an >> alternative which I believe is less worse because with it, arguments >> actually mean what their names tell instead of having additional bools= >> for "oh, and I said A, but I didn't mean it, I really want B". >=20 > Current proposal: >=20 > { 'command': 'block_passwd', 'data': {'*device': 'str', > '*node-name': 'str', 'password': = 'str'} } >=20 > When I look at it, I ask myself: >=20 > - What happens when device=3DNULL? Then, per docs, node-name must be non-NULL. >=20 > - What happens when node-name=3DNULL? Then, per docs, device must be non-NULL. >=20 > - What happens when device=3DNULL and node-name=3DNULL? Error; violates docs. >=20 > - What happens when device !=3D NULL and node-node !=3D NULL? Error; violates docs. >=20 > - What happens when device !=3D NULL but node-node=3DNULL? Operate on the device. >=20 > - What happens when device=3DNULL but node-node !=3D NULL? Operate on the node-name. >=20 > My proposal: >=20 > { 'command': 'block_passwd', 'data': {'device': 'str', > '*device-is-node': 'bool', 'passw= ord': 'str'} } >=20 > - What happens when device-is-node=3DNULL? Operate on the device (same as if device-is-node were false) >=20 > - What happens when device-is-node !=3D NULL? The state of the bool determines whether we are operating on device or on node. >=20 > PS: I'm not NACKing anything. My review to this series started with a > "what about" question. I did voice my objections against this > proposal, but didn't NACK it. Besides you're a maintainer as much > as I am, so I could NACK this as much as you could push this > through your tree ignoring review. And I appreciate the critical review - if we capture design decisions like this in the commit message, then a year from now when someone asks "why didn't you do it this alternative way" we can say "look at the commit message which explores the tradeoffs, and why we settled for what we did" rather than saying "oops, we painted ourselves into a corner because we didn't think about that". The more this goes on, the more I like the mutually-exclusive device[str]/node-name[str] arguments, and the less I like the device[str]/device-is-node[bool] solution, but I can still live with either approach from libvirt's point of view. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --PnoP5cNA3POEwMbRHdgAIpoooXol804UQ 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.4.15 (GNU/Linux) Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJSpz4PAAoJEKeha0olJ0Nq63gH/0RjUztvZAzB87vpB3ZPW+6y 72VEuAwm4A/dyuRoCkL0yYbyCI6I0z3cDRAPeuO7D20sZ21n/rqeO0T7eZ2NOGeU 1UHnhFEdoYK0GEqjgEcedqECQCr5SYJD/sLOGatczlNhlWcR6cQH+y4YOYJNn7fx p8AbbmGKzckSPXOkPCW0rFiL84FBetpcqSFoMTklVRwhZlS+n+rC6FyOZ8cyljYX CM/zDsYFTePRuKAiDKTTJBgRdIMtwRtTDGNhnriD3FmUtZomqoOGMsr3BcgElx/M gRGvxLV26l7hAy9ovreyTxMD0q9EI9NgbT/u3ZzZpj6knYIqdinhEr9WrNxxHug= =sDU8 -----END PGP SIGNATURE----- --PnoP5cNA3POEwMbRHdgAIpoooXol804UQ--