On 22.02.2016 09:24, Markus Armbruster wrote: > Max Reitz <mre...@redhat.com> writes: > >> 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. > > Me neither. > >> 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 have >> a list of the BBs it owns. > > 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). > > 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. > [...] >
signature.asc
Description: OpenPGP digital signature
