qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Eric Blake <eblake@redhat.com>
To: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>,
	qemu-devel@nongnu.org
Cc: kwolf@redhat.com, pkrempa@redhat.com, qemu-block@nongnu.org,
	armbru@redhat.com, mreitz@redhat.com, jsnow@redhat.com
Subject: Re: [PATCH v3 1/6] block: add bitmap-populate job
Date: Mon, 22 Jun 2020 16:44:30 -0500	[thread overview]
Message-ID: <74dc0ce7-2c0e-c987-cbc8-398d2c23f21a@redhat.com> (raw)
In-Reply-To: <074b3859-a6e1-1388-2142-5a7af8ee3fdb@virtuozzo.com>

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',
>> +            '*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 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, 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.

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


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



  reply	other threads:[~2020-06-22 21:46 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-19 19:56 [PATCH v3 0/6] block: add block-dirty-bitmap-populate job Eric Blake
2020-06-19 19:56 ` [PATCH v3 1/6] block: add bitmap-populate job Eric Blake
2020-06-20  4:16   ` Vladimir Sementsov-Ogievskiy
2020-06-22 21:44     ` Eric Blake [this message]
2020-06-23  7:01       ` Vladimir Sementsov-Ogievskiy
2020-09-02 15:58       ` Eric Blake
2020-06-23 11:50   ` Kevin Wolf
2020-06-19 19:56 ` [PATCH v3 2/6] blockdev: combine DriveBackupState and BlockdevBackupState Eric Blake
2020-06-22  5:12   ` Vladimir Sementsov-Ogievskiy
2020-06-22 21:45     ` Eric Blake
2020-06-19 19:56 ` [PATCH v3 3/6] qmp: expose block-dirty-bitmap-populate Eric Blake
2020-06-19 19:56 ` [PATCH v3 4/6] iotests: move bitmap helpers into their own file Eric Blake
2020-06-22  5:26   ` Vladimir Sementsov-Ogievskiy
2020-06-19 19:56 ` [PATCH v3 5/6] iotests: add 298 for block-dirty-bitmap-populate Eric Blake
2020-06-19 19:56 ` [PATCH v3 6/6] bitmaps: Use x- prefix for block-dirty-bitmap-popluate Eric Blake
2020-06-23 11:45   ` Kevin Wolf
2020-06-23 11:47 ` [PATCH v3 0/6] block: add block-dirty-bitmap-populate job Kevin Wolf

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=74dc0ce7-2c0e-c987-cbc8-398d2c23f21a@redhat.com \
    --to=eblake@redhat.com \
    --cc=armbru@redhat.com \
    --cc=jsnow@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=mreitz@redhat.com \
    --cc=pkrempa@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=vsementsov@virtuozzo.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).