On Fri, Feb 13, 2015 at 05:08:41PM -0500, John Snow wrote:
> This series requires: [PATCH v3] blkdebug: fix "once" rule
> 
> Welcome to the "incremental backup" newsletter, where we discuss
> exciting developments in non-redundant backup technology.
> This issue is called the "Max Reitz Fixup" issue.
> 
> This patchset enables the in-memory part of the incremental backup
> feature. There are two series on the mailing list now by Vladimir
> Sementsov-Ogievskiy that enable the migration and persistence of
> dirty bitmaps.
> 
> This series and most patches were originally by Fam Zheng.

Please add docs/incremental-backup.txt explaining how the commands are
intended to be used to perform backups.  The QAPI documentation is
sparse and QMP clients would probably need to read the code to
understand how these commands work.

I'm not sure I understand the need for all the commands: add, remove,
enable, disable, clear.  Why are these commands necessary?

I think just add and remove should be enough:

Initial full backup
-------------------
Use transaction to add bitmaps to all drives atomically (i.e.
point-in-time snapshot across all drives) and launch drive-backup (full)
on all drives.

In your patch the bitmap starts disabled.  In my example bitmaps are
always enabled unless they have a successor.

Incremental backup
------------------
Use transaction to launch drive-backup (bitmap mode) on all drives.

If backup completes successfully we'll be able to run the same command
again for the next incremental backup.

If backup fails I see a problem when taking consistent incremental
backups across multiple drives:

Imagine 2 drives (A and B).  Transaction is used to launch drive-backup
on both with their respective dirty bitmaps.  drive-backup A fails and
merges successor dirty bits so nothing is lost, but what do we do with
drive-backup B?

drive-backup B could be in-progress or it could have completed before
drive-backup A failed.  Now we're in trouble because we need to take
consistent snapshots across all drives but we've thrown away the dirty
bitmap for drive B!

Stopping incremental backup
---------------------------
Use transaction to remove bitmaps on all drives.  This case is easy.

Finally, what about bdrv_truncate()?  When the disk size changes the
dirty bitmaps keep the old size.  Seems likely to cause problems.  Have
you thought about this case?

Stefan