Re: [Qemu-block] block: Dirty bitmaps and COR in bdrv_move_feature_fields()

[Max Reitz]

First of all: I heard someone (not that someone, the other someone)
asking whether I had any opinion on the matter, and my response was the
following:

I don't like "bool float", because of two reasons:

First, "floating" implies that thinks can be forced somewhere else
temporarily before floating back, whereas I think any bitmap that has
been attached to a BB should always stick to the BB, unless forcefully
removed (and then it should no longer float).

Second, "bool float" will give you a syntax error.

So "bool sticky" or "bool glued" would be better.

Now that was pretty unexciting (which is why I hadn't written an email
about it so far), so let's come to my second point:

I think it makes sense to attach bitmaps to BBs directly (as well as to
BDSs). The use case of having bitmaps attached to BBs is common enough
not to force people to move BDS-attached bitmaps manually to the root
when the graph is modified. However, I believe that BDS-attached bitmaps
make sense, too, for one because it's basically what we already have and
because I think that we may need them in the future for node-level
operations (maybe quorum can make some crazy things with them).

On 04.03.2016 21:02, John Snow wrote:
> On 03/04/2016 04:30 AM, Kevin Wolf wrote:
>> Am 03.03.2016 um 21:12 hat John Snow geschrieben:
>>> On 03/03/2016 02:08 PM, Markus Armbruster wrote:
>>>> John Snow <address@hidden> writes:
>>>>
>>>>> On 03/02/2016 04:31 AM, Kevin Wolf wrote:
>>>>>> Am 01.03.2016 um 17:51 hat John Snow geschrieben:
>>>>>>>
>>>>>>>
>>>>>>> On 03/01/2016 04:15 AM, Kevin Wolf wrote:
>>>>>>>> Am 01.03.2016 um 00:23 hat John Snow geschrieben:
>>>>>>>>> Rule of thumb: follow the node such that the (node, name) pair given 
>>>>>>>>> at
>>>>>>>>> creation time continues to work to identify the bitmap.
>>>>
>>>> Yet another ignorant / forgetful question: what is a (node, name) pair?
>>>> Since it's "given at creation time", I guess it must be something in
>>>> QMP.
>>>
>>> The node=[node-name|device] and name=[bitmap-name] parameters given at
>>> QMP creation. You need both parameters to address the bitmap in the
>>> future, so I consider them an address tuple of sorts.
>>
>> (node-name, bitmap-name) is the canonical address. device-name is just
>> an alias for node-name and it can change over time.
>>
> 
> Disagree: whatever the use provided is the canonical address. There may
> be no node-name at all. It cannot be the canonical address, currently.

I thought that every BDS had a node name now?

Anyway, I do agree that the "device name is an alias for the root node"
is too common to make a distinction for this very command, however nice
it may be.

>> This is the semantics of node parameters in all other QMP commands on
>> the BDS level that take both node-names and device-names, so if we're
>> talking about another BDS level command, it should do the same.
>>
> 
> Agree here.

[...]

>> What we need to do is to fix the command so that it's clearly one or the
>> other. Currently, it's _almost_ on the BDS level, so I'd be inclined to
>> make it fully BDS level rather than making it fully BB (and I freely
>> admit that a reason for this is that the series I'm working on is long
>> enough without completely reworking bitmaps to make it fully BB, and
>> moving it fully to the BDS level is a one-liner).
>>
> 
> Problems:
> 
> - Bitmap-add works at the BDS level.
> - drive-backup works at the device level.
> 
> Solutions:
> - Add a new BB-bitmap management command to complement drive-backup,
> realizing that block-dirty-bitmap-add is inadequate for such.

I'm in favor of this. Either add blockdev-dirty-bitmap-add or add the
@attach-to option.

> - Implement incrementals for blockdev-backup to consume bitmaps created
> by block-dirty-bitmap-add, realizing that drive-backup is inadequate for
> such.

That won't help you if you change a device's BDS tree. You'll probably
want the bitmap to always stay on the drive.

(You could just move the bitmap to the new root manually, but I find
that not so nice.)

OTOH, I do think that sometimes we want to exchange a BB's bitmap if the
BDS tree changed (e.g. because you're using a diskette and want the
bitmap to stay with the medium :-)). This can be achieved by manually
moving the bitmap from the root BDS to the BB (and back during
ejection), though.

> Either way we skew the problem for now, there's a significant chunk that
> needs to be done somewhere.
> 
>> After that, we can discuss extending the interface to introduce BB level
>> bitmaps. They should be either a new command, or we introduce a new QMP
>> option like 'attach-to': 'node'/'device'. I would definitely not
>> overload only based on the name given; device names as an alias for the
>> root node are too common to make this not confusing.
>>
>>> All of the existing documentation uses drive names to reference the
>>> creation and management of bitmaps, and the only supported interface for
>>> consuming the bitmaps is /drive-backup/, which again only takes
>>> backends. blockdev-backup does not currently support naming a bitmap or
>>> the incremental backup mode.
>>>
>>> So it would be odd to tilt the only add mechanism in favor of the node,
>>> when the only usage mechanism uses devices exclusively.
>>>
>>> This is why I wonder if we can't attach bitmaps to both BlockBackends
>>> *and* BDS nodes,
>>
>> We can. Except that someone (TM) needs to implement it.
>>
> 
> Hello, is it me you're looking for?

You were the first to propose it on-list. I was clever enough to do it
in a private conversation.

Max

>>> based on what the user provides in the block-dirty-bitmap-add command.
>>
>> But preferably not by inferring from the given node or device name.
>> I think that would be a bad idea.
>>
> 
> Will concede this idea.
> 
>> Kevin
>>
> 
> --js
>

signature.asc
Description: OpenPGP digital signature