Re: [PATCH v3 1/6] block: add bitmap-populate job

[Eric Blake]

[reviving an old thread]

On 6/22/20 4:44 PM, Eric Blake wrote:
> On 6/19/20 11:16 PM, Vladimir Sementsov-Ogievskiy wrote:
> > 19.06.2020 22:56, Eric Blake wrote:
> > > From: John Snow <jsnow@redhat.com>
> > >
> > > This job copies the allocation map into a bitmap. It's a job because
> > > there's no guarantee that allocation interrogation will be quick (or
> > > won't hang), so it cannot be retrofitted into block-dirty-bitmap-merge.
> > >
> > > It was designed with different possible population patterns in mind,
> > > but only top layer allocation was implemented for now.
> > >
> > > Signed-off-by: John Snow <jsnow@redhat.com>
> > > Signed-off-by: Eric Blake <eblake@redhat.com>
> > > ---
>
> > > +{ 'struct': 'BlockDirtyBitmapPopulate',
> > > +  'base': 'BlockDirtyBitmap',
> > > +  'data': { 'job-id': 'str',
> > > +            'pattern': 'BitmapPattern',

As written, "pattern":"allocate-top" is rather limited - it can only 
grab allocation from the top node.  Being able to grab the allocation 
from a specific node may indeed be more useful.  Another bitmap patterns 
that might be useful would be an all-one pattern (create a bitmap that 
treats the entire disk as dirty).  I also remember brainstorming with 
John the question of whether we want bitmap-populate to have different 
mask modes: does the population perform an overwrite (the bitmap now 
matches the source pattern exactly, even if some bits were set and 
others cleared), a union merge (any bits already set in the bitmap 
remain set, additional bits are set according to pattern), or even a 
difference (any bits already cleared in the bitmap remain clear, while 
bits in the pattern can also clear additional bits in the bitmap).

If I understand Peter's goals, the initial libvirt use is a union mode 
(keep bits in the bitmap that are already set, but set additional bits 
according to the population pattern).

> > > +            '*on-error': 'BlockdevOnError',
> > > +            '*auto-finalize': 'bool',
> > > +            '*auto-dismiss': 'bool' } }
> > > +
> >
> > Peter said about a possibility of populating several target bitmaps 
> > simultaneously.
> >
> > What about such a generalized semantics:
> >
> > Merge all sources to each target
> >
> > @targets: list of bitmaps to be populated by the job
> > { 'struct': 'BlockDirtyBitmapPopulate',
> >    'data': { <common job fields>,
> >              'targets': ['BlockDirtyBitmap'],
> >              'sources': ['BitmapPopulateSource'] } }
>
> We still need the 'pattern' argument (the idea being that if we have: 
> Base <- Active, we want to be able to merge in the allocation map of 
> Active into bitmaps stored in Base as part of a commit operation, 
> whether that is active commit of a live guest or offline commit while 
> the guest is offline).  Having an array for 'targets' to merge into is 
> fine, but for 'sources', it's less a concern about selecting from 
> multiple sources, and more a concern about selecting the allocation 
> pattern to be merged in (libvirt wants to merge the same allocation 
> pattern into each bitmap in Base).  Generalizing things to allow the 
> merge of more than one source at once might not hurt, but I'm not sure 
> we need it yet.

But when it comes to multiple destinations or multiple sources, while it 
seems like it might be a convenience factor, I also worry that it is 
over-engineering.  See more below...

> But there are other patterns that we may want to support: an all-ones 
> pattern, or maybe a pattern that tracks known-zeros instead of allocation.
>
> > @bitmap: specify dirty bitmap to be merged to target bitamp(s)
> > @node: specify a node name, which allocation-map is to be merged to 
> > target bitmap(s)
> > { 'alternate': 'BitmapPopulateSource',
> >    'data': { 'bitmap': 'BlockDirtyBitmap',
> >              'node': 'str' } }
>
> This design is clever in that it lets us merge in both existing bitmaps 
> and using a node-name for merging in an allocation map instead of a 
> bitmap; but it limits us to only one pattern.  Better might be something 
> where we supply a union (hmm, we've had proposals in the past for a 
> default value to the discriminator to allow it to be optional, so I'll 
> proceed as if we will finally implement that):
>
> { 'enum': 'BitmapPattern', 'data': [ 'bitmap', 'allocation-top' ] }
> { 'union': 'BitmapPopulateSource',
>    'base': { '*pattern': 'BitmapPattern' },
>    'discriminator': { 'name': 'pattern', 'default': 'bitmap' },
>    'data': { 'bitmap': 'BitmapPopulateSource',
>              'allocation-top': { 'node': 'str' } } }
>
> so that you can then do:
>
> { "execute": "block-dirty-bitmap-populate",
>    "arguments": { "targets": [ { "node": "base", "name": "b1" },
>                                { "node": "base", "name": "b2" } ],
>          "sources": [ { "pattern": "allocation-top", "node": "top" } ]
>    } }
>
> to merge in the allocation information of top into multiple bitmaps of 
> base at once,

Hmm, I left out the mandatory "job-id" parameter here; one of the key 
points of the new command is that some patterns (like allocation) may 
involve potentially lengthy I/O, so we need a job-id (the existing 
block-dirty-bitmap-merge does not).  But since the existing 
block-dirty-bitmap-merge supports multiple sources to one destination, 
supporting multiple patterns to one destination tracked by a single job 
id does have some appeal.

> or conversely, do:
>
> { "execute": "block-dirty-bitmap-populate",
>    "arguments": { "targets": [ { "node": "base", "name": "b1" } ],
>          "sources": [ { "pattern": "bitmap",
>                         "node": "top", "name": "b1" } ]
>    } }
> { "execute": "block-dirty-bitmap-populate",
>    "arguments": { "targets": [ { "node": "base", "name": "b2" } ],
>          "sources": [ { "node": "top", "name": "b2" } ]
>    } }
>
> and of course, wrap this in a "transaction" to ensure that it all 
> succeeds or fails as a unit, rather than messing up one bitmap if 
> another fails, while also allowing future extension for additional 
> patterns.

We already have transactions that let us perform multiple destinations 
as a group.  So what is the difference in the end results between 
merging one source into two separate destinations in one command spelled 
this way:

# proposal with many:many bitmap populate
{ "execute": "block-dirty-bitmap-populate",
  "arguments": { "job-id": "job0",
                 "targets": [ { "node": "base", "name": "b1" },
                              { "node": "base", "name": "b2" } ],
                 "source": { "pattern": "allocation", "node": "top" } } }
wait for job to complete

vs. spelled this way:

# patch as written with 1:1 bitmap-populate, but tweak to source
{ "execute": "block-dirty-bitmap-add",
  "arguments": { "node": "top", "name": "tmp" } }
{ "execute": "block-dirty-bitmap-populate",
  "arguments": { "job-id": "job0",
                 "node": "top", "name": "tmp",
                 "source": { "pattern": "allocation", "node": "top" } } }
wait for job to complete
{ "execute": "transaction",
  "arguments": { "actions": [
    { "type": "block-dirty-bitmap-merge", "node": "base", "name": "b1",
        "bitmaps": [ { "node": "top", "name": "tmp" } ] },
    { "type": "block-dirty-bitmap-merge", "node": "base", "name": "b2",
        "bitmaps": [ { "node": "top", "name": "tmp" } ] }
] } }



> > - so, we can merge several bitmaps together with several allocation 
> > maps into several target bitmaps.
> > (I remember, we also said about a possibility of starting several 
> > populating jobs, populating into
> >   same bitmap, I think it may be substituted by one job with several 
> > sources. Still, it's not hard to
> >   allow to use target bitmaps in a several jobs simultaneously and 
> > this is not about the QAPI interface)
> >
> > Will this simplify things in libvirt?
>
> Peter, in your preliminary experiments with block-dirty-bitmap-populate, 
> did you ever need to start more than one job to a single bitmap 
> destination, or was it merely starting multiple jobs because you had 
> multiple destinations but always just a single source?

I guess I'm struggling in posting a v4 in part because I don't have a 
good answer to what is easiest for Peter to use.

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org