Re: block stream and bitmaps

[Kevin Wolf]

Am 24.03.2020 um 20:19 hat John Snow geschrieben:
> 
> 
> 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!

Yes, if we hadn't talked about it, I probably wouldn't have had this
suggestion for you.

Anyway, someone still needs to write that patch. Should I try to find
time for it or are you (or someone else working on bitmaps) going to do
that?

> >> 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.

And then the result for that request would be that it doesn't dirty any
bitmaps at A, but it does dirty the bitmaps at B?

I mean, many things can be developed, but being sure that they actually
implement the right thing is harder... So as long as we have the
workaround you mentioned above, maybe let's just ignore this for now.

> >> 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...?

Ah. I just took it as a given that you need or want BlockBackend-based
bitmaps because that's how you started the email. Maybe I should look at
the bigger picture before suggesting things.

So what was the real motivation behind it?

> 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.

Which can change, though (taking a snapshot or through block jobs). Do
we want to copy the bitmap around then?

> 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'm not sure if I should even mention this, but I guess you could have a
metadata-only root qcow2 file that will always stay there, i.e. you only
ever do snapshots/block jobs below it, so the node gets only accessed by
the guest device:

    virtio-blk
        |
        | BlockBackend
        |
        v       file
      qcow2 <--------- file-posix (drive-bitmaps.qcow2)
        |
        | data_file
        |
        v
      qcow2  <--------- file-posix (top.qcow2)
        |
        | backing
        v
       ...

I wouldn't promise that managing this doesn't become a nightmare,
though.

> 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.

Yes, in-memory bitmaps for BlockBackends shouldn't be a big problem.

With merge-to/merge-from you mean that the user could manually save or
load the bitmap to/from an image file?

> > 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.)

But as soon as the management tool is actually node-name based (which I
think libvirt is now), shouldn't these problems disappear? This sounds
like a typical problem of exposing per-node data as per-drive, not like
an inherent problem of managing things at the node level.

Kevin