From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45983) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Yj9QT-0006Bd-62 for qemu-devel@nongnu.org; Fri, 17 Apr 2015 12:47:37 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Yj9QP-0007fi-RW for qemu-devel@nongnu.org; Fri, 17 Apr 2015 12:47:33 -0400 Message-ID: <5531384F.3090100@redhat.com> Date: Fri, 17 Apr 2015 10:43:59 -0600 From: Eric Blake MIME-Version: 1.0 References: <1427484005-31120-1-git-send-email-jsnow@redhat.com> <1427484005-31120-9-git-send-email-jsnow@redhat.com> <55312E76.8040808@redhat.com> In-Reply-To: <55312E76.8040808@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="hnb3nNT4hGb1R2QSliTJ9qeqCsXnWIjVG" Subject: Re: [Qemu-devel] [PATCH v2 08/11] block: move transactions beneath qmp interfaces List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Max Reitz , John Snow , qemu-block@nongnu.org Cc: kwolf@redhat.com, vsementsov@parallels.com, famz@redhat.com, qemu-devel@nongnu.org, stefanha@redhat.com This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --hnb3nNT4hGb1R2QSliTJ9qeqCsXnWIjVG Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 04/17/2015 10:01 AM, Max Reitz wrote: > On 27.03.2015 20:20, John Snow wrote: >> In general, since transactions may reference QMP function helpers, >> it would be nice for them to sit beneath them. >> >> This will avoid the need for forward declaring any QMP interfaces, >> which would be aggravating to update in so many places. >> >> Signed-off-by: John Snow >> --- >> blockdev.c | 2581 >> ++++++++++++++++++++++++++++++------------------------------ >> 1 file changed, 1292 insertions(+), 1289 deletions(-) >=20 > I'm not so sure about this patch. I mean... 2581 changes? :-/ >=20 > If you (or someone else) can convince me that forward declarations are > really worse than this (is it more aggravating than having a patch with= > this diffcount?), well... I like avoiding forward declarations of static functions, where it makes sense, but I'm not going to insist on reordering code if it makes things ugly. >=20 > But even then, I'd strongly vote for removing dropping all functional > changes beside the code movement from this patch. Because I'm seeing th= is: >=20 > 2096c2096,2097 > < error_set(errp, QERR_INVALID_PARAMETER_VALUE, "granularity", > "power of 2"); > --- >> error_set(errp, QERR_INVALID_PARAMETER_VALUE, >> "granularity", "power of 2"); Indeed - you WANT code motion patches to be as easy as possible to review. From http://wiki.qemu.org/Contribute/SubmitAPatch "Ideally, a code motion patch can be reviewed by doing git format-patch --stdout -1 > patch; diff -u <(sed -n 's/^-//p' patch) <(sed -n 's/^\+//p' patch), to focus on the few changes that weren't wholesale code motion." > Probably sensible changes, but if you really, really, really want to go= > for this code movement, please apply them in an own patch before this > one so that this one really is nothing but code movement. >=20 > Max >=20 >=20 --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --hnb3nNT4hGb1R2QSliTJ9qeqCsXnWIjVG Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJVMThPAAoJEKeha0olJ0NqE8UH/1k5ntBqrdMUlSoSZOvJnbD6 KBlgmba888KRjibgS2Cd8L+QcnhA9nTfLbU9L84zSTCmOGVWDDGIGuRQ8PibRkSq AT+UbkzVyMPDdpjFgwjvYw3iR3yM6havqnJHTWhmRk45EWL/tuc66PTiel6xzrrn /IsgCDIdkkYSv1+aAnmV941zoF6mKObSVm6WGahkj4kcszGGgKM+68UGTRnDZM2p 2Ezk2hkHOlqUPu6a5nSzZcVisYM7c5A6STh6LEaqZsDft8KF9jnNGLenL6yYKdj+ MFxOej/u3iopUaDjX/Rv92FBci7OULWL1cM+5KVy5SmkOB9J2wVW5jDdPAOiYdU= =oAAF -----END PGP SIGNATURE----- --hnb3nNT4hGb1R2QSliTJ9qeqCsXnWIjVG--