On 04/08/2015 04:19 PM, John Snow wrote: > A bitmap successor is an anonymous BdrvDirtyBitmap that is intended to > be created just prior to a sensitive operation (e.g. Incremental Backup) > that can either succeed or fail, but during the course of which we still > want a bitmap tracking writes. > > On creating a successor, we "freeze" the parent bitmap which prevents > its deletion, enabling, anonymization, or creating a bitmap with the > same name. > > On success, the parent bitmap can "abdicate" responsibility to the > successor, which will inherit its name. The successor will have been > tracking writes during the course of the backup operation. The parent > will be safely deleted. > > On failure, we can "reclaim" the successor from the parent, unifying > them such that the resulting bitmap describes all writes occurring since > the last successful backup, for instance. Reclamation will thaw the > parent, but not explicitly re-enable it. > > BdrvDirtyBitmap operations that target a single bitmap are protected > by assertions that the bitmap is not frozen and/or disabled. > > BdrvDirtyBitmap operations that target a group of bitmaps, such as > bdrv_{set,reset}_dirty will ignore frozen/disabled drives with a > conditional instead. > > QMP transactions that enable/disable bitmaps have extra error checking > surrounding them that prevent modifying bitmaps that are frozen.
Is this last paragraph stale now? > > Signed-off-by: John Snow <js...@redhat.com> > Reviewed-by: Max Reitz <mre...@redhat.com> > Reviewed-by: Stefan Hajnoczi <stefa...@redhat.com> > --- > block.c | 104 > +++++++++++++++++++++++++++++++++++++++++++++++++- > blockdev.c | 7 ++++ > include/block/block.h | 10 +++++ > qapi/block-core.json | 1 + > 4 files changed, 121 insertions(+), 1 deletion(-) > > +/** > + * A BdrvDirtyBitmap can be in three possible states: > + * (1) successor is false and disabled is false: full r/w mode > + * (2) successor is false and disabled is true: read only mode ("disabled") > + * (3) successor is set: frozen mode. > + * A frozen bitmap cannot be renamed, deleted, anonymized, cleared, set, > + * or enabled. A frozen bitmap can only abdicate() or reclaim(). > + */ > struct BdrvDirtyBitmap { > HBitmap *bitmap; > + BdrvDirtyBitmap *successor; 'successor is false' reads awkwardly given that it is not a bool; maybe 'successor is NULL' is better? > > +bool bdrv_dirty_bitmap_frozen(BdrvDirtyBitmap *bitmap) > +{ > + return bitmap->successor; Good thing C99 requires conversion of pointer to bool to work :) > + > +/** > + * For a bitmap with a successor, yield our name to the successor, > + * Delete the old bitmap, and return a handle to the new bitmap. Sentences with capitals after comma, Even with a line break, Look weird. (s/Delete/delete/) > + > +/** > + * In cases of failure where we can no longer safely delete the parent, > + * We may wish to re-join the parent and child/successor. and again (s/We/we/) Grammar fixes are minor, so: Reviewed-by: Eric Blake <ebl...@redhat.com> -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature