Re: [PATCH RFC v2 1/5] block: add bitmap-populate job

[Peter Krempa <pkrempa@redhat.com>]

On Thu, Jun 04, 2020 at 13:31:45 +0200, Kevin Wolf wrote:
> Am 04.06.2020 um 11:16 hat Peter Krempa geschrieben:
> > On Thu, Jun 04, 2020 at 11:12:31 +0200, Kevin Wolf wrote:
> > > Am 18.05.2020 um 22:49 hat Eric Blake geschrieben:
> > > > > +
> > > > > +    /* NB: new bitmap is anonymous and enabled */
> > > > > +    cluster_size = bdrv_dirty_bitmap_granularity(target_bitmap);
> > > > > +    new_bitmap = bdrv_create_dirty_bitmap(bs, cluster_size, NULL, errp);
> > > > > +    if (!new_bitmap) {
> > > > > +        return NULL;
> > > > > +    }
> > > > 
> > > > This means if the guest writes to the disk while the job is ongoing, the
> > > > bitmap will be updated to mark that portion of the bitmap as set, even if it
> > > > was not allocated at the time the job started.  But then again, the guest
> > > > writes are causing allocation, so this seems like the right thing to do.
> > > 
> > > Is the target bitmap active at the same time, i.e. will it get the
> > > correct information only from new_bitmap or are the bits already set in
> > > it anyway?
> > 
> > Yes, libvirt plans to use it with an active non-persistent bitmap which
> > will in subsequent steps be merged into others. The bitmap is added in
> > the same transaction. The bitmap must be active, because we need to wait
> > for the block jobs to finish before it becomes usable and thus can't
> > sequence in other operations until later.
> 
> A lot of bitmap merging then, because the block job in this series
> already creates a temporary internal bitmap that is merged into the
> target bitmap on completion. But if the target bitmap is only libvirt's
> temporary bitmap to be merged to yet another bitmap, I wonder if this
> process shouldn't be simplified.

Possibly yes, but I'll leave that for later. All of this is done when
executin very expensive operations anyways so for our first
implementation it IMO won't matter that much.