On 3/24/20 6:18 AM, Kevin Wolf wrote:
Am 23.03.2020 um 19:06 hat John Snow geschrieben:
Hi Kevin,
I'm hoping to get some preliminary ideas from you (capped at five
minutes' effort?) on this subject. My ideas here are a bit shaky, I only
have really rough notions here.
We want to use bitmaps with 'drive' semantics; i.e. tracking changes to
the visible guest data. What we have are bitmaps with node semantics,
tracking changes to the data at a particular level in the graph.
For commit, this isn't a big deal: we can disable bitmaps in the backing
file while we commit and then re-enable it on completion. We usually
have a separate bitmap enabled on the root node that is recording writes
during this time, and can be moved later.
For streaming, this is more challenging: new writes will dirty the
bitmap, but so will writes from the stream job itself.
Semantically, we want to ignore writes from the stream while recording
them from the guest -- differentiating based on source.
No, based on source is actually not what you want. What you really want
is that BDRV_REQ_WRITE_UNCHANGED doesn't mark any blocks dirty.
This is why I sent the mail, I figured you'd know the better incision
point, and I was right!
We discussed this specific case of streaming at FOSDEM (with Paolo and
probably Nir). Paolo was even convinced that unchanged writes already
behave like this, but we agreed that dirtying blocks for them would be a
bug. After checking that the code is indeed buggy, I was planning to
send a patch, but never got around to actually do that. Sorry about
that.
Glad to hear it has been given consideration, though!
Bitmaps aren't really geared to do that right now. With the changes to
Bdrv Roles that Max was engineering, do you think it's possible to add
some kind of write source discrimination to bitmaps, or is that too messy?
I don't think it would work because copy-on-read requests come from the
same parent node as writes (no matter whether the legacy code in
block/io.c or a copy-on-read filter node is used).
Oh, understood. Rule that approach out, then.
For both commit and stream, it might be nice to say: "This bitmap is
enabled, but ignores writes from [all? specific types? specific
instances?] jobs.
Commit is a bit trickier, because it's not WRITE_UNCHANGED. The result
is only unchanged for the top layer, but not for the backing file you're
committing to. Not sure if we can represent this condition somehow.
Nothing comes to mind apart from a semantic that applies to a graph
subsection instead of an individual node.
i.e. UNCHANGED as applied to [A --> B].
Not saying that's reasonable to develop... or necessarily even possible
to enforce, just nothing else comes to mind.
Or, I wonder if what we truly want is some kind of bitmap "forwarder"
object on block-backend objects that represent the semantic drive view,
and only writes through that *backend* get forwarded to the bitmaps
attached to whatever node the bitmap is actually associated with.
(That might wind up causing weird problems too, though... since those
objects are no longer intended to be user-addressable, managing that
configuration might get intensely strange.)
Hm... Drive-based does suggest that it's managed at the BlockBackend
level. So having a bitmap that isn't added as a dirty bitmap to the BDS,
but only to the BB does make sense to me. The BB would be addressed
with the qdev ID of the device, as usual (which underlines that it's
really per device).
That's the rough idea, though if it's needed or not is unclear. We might
be able to get by with node semantics if we jazz them up enough...?
Working around all the edge bases of a drive-semantic bitmap seem
difficult to reason about.
In general, it should likely be made persistent against the root-most
node to which writes are routed to a protocol node.
In the common case, that means the top-most qcow2 format node of a chain.
But, many of our job filters also route writes to format nodes too, so
which one is the "canonical" store? Is it even possible to define?
Hm.
I suppose we can always allow bitmaps being attached to the BB and
*disallow them* from being made persistent; but simply exist as an
in-memory tool to help ease the pain of managing data consistency during
critical sections.
We can offer merge-to, merge-from semantics for these in-memory-only
bitmaps. Maybe.
Still mulling it over.
The part that's unclear to me is how to make such bitmaps persistent.
You can change the root node of a BB and even remove the root node
completely (for removable devices; but even changing is technically
remove followed by insert), so you may need to move the bitmap around
between image files and at least for some time you might not have any
place to store the bitmap.
Yeah, we've had discussions about this in the past. When you were
removing/reworking bdrv_swap there were discussions about the right way
to migrate or not-migrate bitmaps. We went with a node-centric design to
stay consistent with existing code. It's mostly been the correct
decision. Mostly.
(It gets weird when you attach a bitmap to the root node associated with
a device and then use filter-based drive-backup and the root node
changes, making it appear as if your bitmaps vanished. Endless BZs due
to this phenomenon. drive-semantic bitmaps would alleviate this
particular pain point.)
Or you say that you store it in one specific node, be it the root node
of the BB or not, and it will always stay there no matter how you change
the graph and whether the BB and that node are even in the same subtree.
That node would just get an additonal refcount, so you can't remove it
until the BB goes away.
That was the 5 second thought. It's possible to make graphs that make no
sense with this feature, but I guess we can always say "Well, don't do
that, then."
It has been difficult or impossible to identify what counts as the
'"real" root node' in a graph. We want to exclude filters and jobs, but
allow temporary things on occasion.
Unless you already have a better plan (I hope you do, I didn't think
about it for more than a few minutes), maybe the latter would actually
be the most reasonable solution.
I regret to inform you that I have no preferred plan right now. I will
try to prototype a few things soon and see if I come up with anything
not disgusting.