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

[Kevin Wolf]

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?

If it relies on new_bitmap, I think this deserves a comment here.

In the documentation of BitmapPattern and specifically @allocation-top,
we should probably also be more explicit what this means for requests
that the guest makes while the job is running.

> Do we need to worry about the converse case where the job started with
> something allocated but runs in parallel with the guest trimming, such that
> our bitmap marks something as set even though at the conclusion of our job
> it is no longer allocated?

Discard shouldn't make the backing file visible (didn't we fix this for
qcow2 v2 only recently?), so in the sense of bdrv_is_allocated(), they
will still be allocated, even though they are probably zero clusters
now.

Kevin