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

[Kevin Wolf]

Am 04.03.2016 um 20:53 hat John Snow geschrieben:
> >>>> I think if we want to say something like:
> >>>>
> >>>> - If the user provides a BB name, the bitmap attaches to the *BB*,
> >>>> - If the user provides a node name, the bitmap attaches to the node (and
> >>>> will always stay on THAT node. QMP commands to move the bitmap in this
> >>>> scenario are appropriate. As long as the node we attach to always keeps
> >>>> the name it had when we attached to it, I'm happy.
> >>>
> >>> Now you confused me.  Example: user attaches bitmap to the BDS / node
> >>> with node name "Alice".  There's a QMP command to move the bitmap to
> >>> another node.  What does it move?  The bitmap?  The bitmap and the node
> >>> name?
> >>>
> >>
> >> User creates bitmap "foobar" on BDS "Alice". The user-visible address
> >> for accessing this bitmap is now and forever (Alice, foobar).
> > 
> > What if the BDS doesn't have a node-name?
> > 
> 
> Well, you can't attach the bitmap to a node you can't identify. Sort of;
> If that BDS does not have a name and exists as the root node under a BB,
> you can use the device/BB name to attach to that node.

Every node has a name these days.

> An unintended consequence is that if the root node also has its own name
> and you happen to know both the BB and the BDS name, you can use either
> interchangeably, but that wasn't the intent. The intent was that you
> always use the same pair consistently, a fixed address.

That's essentially the alias behaviour that is consistent with every
other command.

> In replying to you this time, I found out that I'm OK with scrapping
> node support until we have our story straight. It's not useful for
> anything yet, I doubt anyone is using it, and it is just confusing
> everything right now.
> 
> So let me adjust my story:
> 
> I'm fine with bitmap manipulation commands that work only for "devices"
> (with implementation on the BB) with the design goal of leaving an
> obvious extension for adding support for nodes once we have a solid game
> plan for them.
> 
> I am in favor of making the choice to choose a BDS or a BB more explicit
> in the interface -- the traditional way is the
> device-or-node-name-but-not-both approach, which is not well liked but
> it is at least explicitly clear in this case.
> 
> We could also do something like...
> 
> bitmap-add-to-node   node-name, name, ...
> bitmap-add-to-drive  device, name, ...
> 
> which I feel is a little verbose but is explicit, introspectable, etc.
> Maybe it's the only right thing.

I actually agree with most of what you're saying here, and I still like
the idea of the 'to-attach' idea to distinguish between attaching to a
BB and attaching to a BDS.

The problem is the order and timeline for implementing things. The code
for attaching to a BDS is almost there [1], whereas we don't have any
dirty tracking on the BB level yet. Essentially it turns out that the
user bitmap patches implemented the wrong thing.

By the way, another thing that we haven't discussed yet is the
difference between attaching to the BB and attaching to the root node.
It doesn't make a difference now, but as soon as we allow multiple BBs
per BDS, attaching to the BB would only get you the writes from one
source (e.g. the guest device) and ignore other sources (e.g. NBD server
or block jobs).

I assume that this is actually something that you want for your primary
use case of dirty bitmaps, but it's definitely something to be aware of.

Kevin

[1] This is the patch: http://repo.or.cz/qemu/kevin.git/commitdiff/aac1a0e