From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50365) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Wl14i-0007NN-Ut for qemu-devel@nongnu.org; Thu, 15 May 2014 15:12:22 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Wl14d-0007ut-Vz for qemu-devel@nongnu.org; Thu, 15 May 2014 15:12:16 -0400 Received: from mx1.redhat.com ([209.132.183.28]:62570) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Wl14d-0007uh-NU for qemu-devel@nongnu.org; Thu, 15 May 2014 15:12:11 -0400 Message-ID: <53751187.5050608@redhat.com> Date: Thu, 15 May 2014 13:12:07 -0600 From: Eric Blake MIME-Version: 1.0 References: <5374E445.9070609@redhat.com> <20140515184103.GM8452@localhost.localdomain> In-Reply-To: <20140515184103.GM8452@localhost.localdomain> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="HXHh1lrMwsbt10lv3TT6dk8PkNSBpdoKI" Subject: Re: [Qemu-devel] [PATCH 1/5] block: Auto-generate node_names for each BDS entry List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jeff Cody Cc: kwolf@redhat.com, benoit.canet@irqsave.net, pkrempa@redhat.com, famz@redhat.com, qemu-devel@nongnu.org, stefanha@redhat.com This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --HXHh1lrMwsbt10lv3TT6dk8PkNSBpdoKI Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 05/15/2014 12:41 PM, Jeff Cody wrote: >>> The prefix is to aid in identifying it as a qemu-generated name, the >>> numeric portion is to guarantee uniqueness in a given qemu session, a= nd >>> the random characters are to further avoid any accidental collisions >>> with user-specified node-names. >>> >>> Signed-off-by: Jeff Cody >>> --- >>> block.c | 16 +++++++++++++++- >>> 1 file changed, 15 insertions(+), 1 deletion(-) >> >> This patch feels incomplete. We probably also ought to modify >> qapi-schema.json to make BlockDeviceInfo's output of 'node-name' by >> unconditional (as an output-only struct, changing from optional to >> mandatory is a safe change for back-compat API considerations); which >> implies further modifying code to get rid of has_node_name arguments i= n >> all places that generate BlockDeviceInfo details. >=20 > I think it should be a separate patch (but still in this series). > Strictly speaking, this patch doesn't force the argument to be > mandatory, the value just happens to always be populated now. True, keeping it as two patches is cleaner. >=20 >> See also my thoughts on patch 5/5 - can this patch be rebased to be LA= ST >> in the series, rather than first, so that it serves as a reliable >> witness that everything else related to block operations on node-names= >> is complete? >> > How about this becomes the penultimate patch, and the patch to make > BlockDeviceInfo's node-name field to be unconditional becomes the last > patch? Or, if we go the route of adding a new command for setting the backing name in isolation, then _that_ patch should be last, at which point leaving this patch early in the series no longer hurts, because it is no longer the witness that libvirt would be relying on. >=20 > The only thing I don't like about moving this further back in the > patch series is it makes the earlier patches untestable; I can't > easily test the usage of the node-names for various intermediate BDS > because they don't have node-names set. So that means I'll just need > to rebase the patches prior to sending. So let me revise my above > statement - how about this patch stays at the beginning, which makes > development / testing easier, with the patch that modifies > BlockDeviceInfo as the final (not including tests) patch? Yes, keeping this patch first sounds reasonable after all. The patch modifying BlockDeviceInfo isn't introspectible (there's nothing libvirt can probe that says whether the QMP code switched from optional to mandatory - all libvirt can probe is whether a name gets generated - and that is THIS patch, not the BlockDeviceInfo patch). But adding a new command is easier to use than probing whether a name was generated. >=20 >=20 --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --HXHh1lrMwsbt10lv3TT6dk8PkNSBpdoKI 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/ iQEcBAEBCAAGBQJTdRGHAAoJEKeha0olJ0NqkxMH/iesQgWUZPrZtq2r3PDgVdE7 HKFZAOhrg0gxj3sFqql8EQ2jc3gi+bs63654jqEyLPvDJq/TJdS1A3OsQrfBFFlt 86C6tmSr+rEDTVryUxWCqc9bYY6cxxSfU3W1JxK84N55Xa9B9iF/ftrnf3Zib7wI uc+UwmEXgUKMXmxoxLiVvEGcYJLwqjd8Yiop368qw+Tp4I2qW1wbPkC0v/FWdgNm VDihx7lmhNes9fpco3AHOX37hGbDIuzrJ5HOUrDGgPLZ0+027jMVAMWCAGPEloze 6b7umgpPqRD03IeQE2T/xU/bVUjJbHNkxj4LU3JK93zzcpwib5S1e+RooTlgDyY= =i07y -----END PGP SIGNATURE----- --HXHh1lrMwsbt10lv3TT6dk8PkNSBpdoKI--