From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55492) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aXtMr-0002t1-Ba for qemu-devel@nongnu.org; Mon, 22 Feb 2016 11:29:50 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aXtMq-0003SQ-6W for qemu-devel@nongnu.org; Mon, 22 Feb 2016 11:29:49 -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> <87oab9i24b.fsf@blackfin.pond.sub.org> From: Max Reitz Message-ID: <56CB3773.1030107@redhat.com> Date: Mon, 22 Feb 2016 17:29:39 +0100 MIME-Version: 1.0 In-Reply-To: <87oab9i24b.fsf@blackfin.pond.sub.org> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="qvEnInl3tCONQnuXUvSsFrSfrVt5U6PIJ" 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: Markus Armbruster Cc: Kevin Wolf , qemu-block@nongnu.org, qemu-devel@nongnu.org, Stefan Hajnoczi This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --qvEnInl3tCONQnuXUvSsFrSfrVt5U6PIJ Content-Type: text/plain; charset=iso-8859-15 Content-Transfer-Encoding: quoted-printable On 22.02.2016 09:24, Markus Armbruster wrote: > Max Reitz writes: >=20 >> 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 shoul= d have >>>>> >>>>> s/does hold/holds/? >>>> >>>> It was intentional, so I'd keep it unless you drop the question mark= =2E >>> >>> For me it seems to imply something like "contrary to your expectation= s", >>> but maybe that's just my non-native English needing a fix. >>> >>> I don't find it surprising anyway that the monitor holds BB reference= s. >=20 > Me neither. >=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 ha= ve >> a list of the BBs it owns. >=20 > Oh yes, it has. It's just outsources their actual storage to > block-backend.c, but that's detail. In my opinion it's not a reference made by the monitor, then, especially because it's done through magic. With this interpretation, block-backend.c considers every BB to be monitor-owned (until blk_hide...() is called). Also, if blk_backends is supposed to be the list of monitor-owned BBs, then it's a really bad name and I put all the blame on that. >> 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 > I disagree :) Then I'd say "It's contrary to my interpretation of what the current code wants to do." Now it's my personal opinion! ;-) I wouldn't mind removing the "does" from the commit message (obviously), but that is not the only thing there which conflicts with your opinion. It clearly states that blk_backends is not the list of monitor-owned BlockBackends, whereas you are saying that it very much is. So...? Rephrase it entirely? State that blk_backends is a bad name and this commit is basically duplicating blk_backends as monitor_block_backends, which is the correct name, and that a follow-up commit will make blk_backends contain what it name implies it does? Or just call the commit "Remove magic", because it adds explicit functions for saying that a BB is monitor-owned or that it is not? Max >> 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 --qvEnInl3tCONQnuXUvSsFrSfrVt5U6PIJ 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 iQEcBAEBCAAGBQJWyzdzAAoJEDuxQgLoOKyt9QoH/0hLVZsvYij4jTAfJsPj3q1B gBv2EKmBDOctd7tTE8ahXf3pE4d13hSWq9I5/MdZmZh2ApeE6N9HcNUtSFaY68X6 +4T8tw74AjiPNiIJCGMQkOqqwHFFZM58ZDodB6DjEUZMWTmNguitxO4Sdet7nG7L ydApzPuUaBVOOoqaaVaDnbnRstl8mjrBjVs3Qy1aXAgvSbes8fCE9IHG9FSRuPj3 CWVjCDWe1t4dWbY2GOuVytUCUGbX4V0lCNgRufzG8zfPxQxL3+/X0DhbJ22zg1Je mpGXFMWlHBGNdx5RGtOQWg0DazhL7Z2Imq4rUAknCBusDjKWknH3ykhkS6qUxbU= =uPAQ -----END PGP SIGNATURE----- --qvEnInl3tCONQnuXUvSsFrSfrVt5U6PIJ--