From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:51255) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aX7hj-0003M5-Ht for qemu-devel@nongnu.org; Sat, 20 Feb 2016 08:36:12 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aX7hi-0004ZW-J3 for qemu-devel@nongnu.org; Sat, 20 Feb 2016 08:36:11 -0500 References: <1455646106-2047-1-git-send-email-mreitz@redhat.com> <1455646106-2047-8-git-send-email-mreitz@redhat.com> <20160217105345.GE29494@noname.str.redhat.com> <56C494B4.8020707@redhat.com> <20160217162053.GK29494@noname.str.redhat.com> <56C86B70.8090102@redhat.com> From: Max Reitz Message-ID: <56C86BC2.1070406@redhat.com> Date: Sat, 20 Feb 2016 14:36:02 +0100 MIME-Version: 1.0 In-Reply-To: <56C86B70.8090102@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="OtJHk9bIlOCPjqR1ASC7WhLft2wGUWUn9" Subject: Re: [Qemu-devel] [PATCH v3 07/14] blockdev: Add list of monitor-owned BlockBackends List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: Stefan Hajnoczi , qemu-devel@nongnu.org, qemu-block@nongnu.org, Markus Armbruster This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --OtJHk9bIlOCPjqR1ASC7WhLft2wGUWUn9 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: quoted-printable On 20.02.2016 14:34, Max Reitz wrote: > On 17.02.2016 17:20, Kevin Wolf wrote: >> Am 17.02.2016 um 16:41 hat Max Reitz geschrieben: >>> On 17.02.2016 11:53, Kevin Wolf wrote: >>>> Am 16.02.2016 um 19:08 hat Max Reitz geschrieben: >>>>> The monitor does hold references to some BlockBackends so it should= have >>>> >>>> s/does hold/holds/? >>> >>> It was intentional, so I'd keep it unless you drop the question mark.= >> >> For me it seems to imply something like "contrary to your expectations= ", >> but maybe that's just my non-native English needing a fix. >> >> I don't find it surprising anyway that the monitor holds BB references= =2E >=20 > The contrast I tried to point out is that while we do have these > references in theory, and they are reflected by a refcount, too, we do > not actually have these references because the monitor does not yet hav= e > a list of the BBs it owns. >=20 > So it's not an "emphasize the verb because it may be contrary to your > expectations", but an "emphasize it because it is contrary to what the > current code does" (which is not actually referencing these BBs). >=20 > Like: It is supposed to have references. It says it does. But it > actually doesn't. It does "hold" them, however, because they are > accounted for in the BBs' refcounts. >=20 >>>>> a list of those BBs; blk_backends is a different list, as it contai= ns >>>>> references to all BBs (after a follow-up patch, that is), and that >>>>> should not be changed because we do need such a list. >>>>> >>>>> monitor_remove_blk() is idempotent so that we can call it in >>>>> blockdev_auto_del() without having to care whether it had been call= ed in >>>>> do_drive_del() before. monitor_add_blk() is idempotent for symmetry= >>>>> reasons (monitor_remove_blk() is, so it would be strange for >>>>> monitor_add_blk() not to be). >>>>> >>>>> Signed-off-by: Max Reitz >>>> >>>> I think hmp_drive_add() needs a monitor_remove_blk() in its error pa= th. >>> >>> You're right, thanks. >>> >>> In addition, if we really do say that a BB having a name equals being= >>> referenced by the monitor, then maybe we don't need explicit calls to= >>> monitor_add_blk() because any BB that is created with a non-NULL name= >>> should be automatically added to the list of monitor BBs. >> >> While probably workable, I'd rather avoid this kind of magic where the= >> presence of a name parameter decides whether a reference is taken or >> not. It makes the interface of the function a lot less obvious. >=20 > Still you want the name to be the monitor's reference to the BB. Thus, > if monitor_add_blk() should not be called implicitly by blk_new(), then= > I'd instead move the @name parameter from blk_new() to monitor_add_blk(= ). =2E..aaand I noticed that this is what you said for patch 8 anyway. Good.= Max --OtJHk9bIlOCPjqR1ASC7WhLft2wGUWUn9 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 iQEcBAEBCAAGBQJWyGvCAAoJEDuxQgLoOKytPTEIAK32dYpZbs7szstMpeH02AuJ te8hjYg7xeQyXv7OiVo8vcL5NiOKq+IoLUTBUteuy5//fOTWTJQbGXwDwDgfJD3E HorG9IVR+HBSlUPpQaqX45rJgZTreb2gvz2BqsmGwdJSVAAC5etyr3VdGslEXY0f fNlLWrMqEQ3a34TOHsLraSQNpf0cH26AMaPhPxL67V3PQhqYov482fGiolzgqXXx HtliZlKYDdR5bMGLjiA6r/MGG6jWnYN72DBorIvWafbsruFUb3Hlx0JpqUY9LgSG smGnoqzIC6+7I/YVIjDO6dKvx9qVAFZELVMCmRAYIq3KBzeqAcclhnGeLcrgEDw= =XESS -----END PGP SIGNATURE----- --OtJHk9bIlOCPjqR1ASC7WhLft2wGUWUn9--