Re: [Qemu-devel] [PATCH v6 00/12] block: drive-backup live backup command

[Kevin Wolf <kwolf@redhat.com>]

Am 24.06.2013 um 17:13 hat Stefan Hajnoczi geschrieben:
> Note: These patches apply to kevin/block.  You can also grab the code from git
> here:
> git://github.com/stefanha/qemu.git block-backup-core
> 
> This series adds a new QMP command, drive-backup, which takes a point-in-time
> snapshot of a block device.  The snapshot is copied out to a target block
> device.  A simple example is:
> 
>   drive-backup device=virtio0 format=qcow2 target=backup-20130401.qcow2
> 
> The original drive-backup blockjob was written by Dietmar Maurer
> <dietmar@proxmox.com>.  He is currently busy but I feel the feature is worth
> pushing into QEMU since there has been interest.  This is my version of his
> patch, plus the QMP command and qemu-iotests test case.
> 
> QMP 'transaction' support is included since v3.  It adds support for atomic
> snapshots of multiple block devices.  I also added an 'abort' transaction to
> allow testing of the .abort()/.cleanup() code path.  Thanks to Wenchao for
> making qmp_transaction() extensible.
> 
> How is this different from block-stream and drive-mirror?
> ---------------------------------------------------------
> Both block-stream and drive-mirror do not provide immediate point-in-time
> snapshots.  Instead they copy data into a new file and then switch to it.  In
> other words, the point at which the "snapshot" is taken cannot be controlled
> directly.
> 
> drive-backup intercepts guest writes and saves data into the target block
> device before it is overwritten.  The target block device can be a raw image
> file, backing files are not used to implement this feature.
> 
> How can drive-backup be used?
> -----------------------------
> The simplest use-case is to copy a point-in-time snapshot to a local file.
> 
> More advanced users may wish to make the target an NBD URL.  The NBD server
> listening on the other side can process the backup writes any way it wishes.  I
> previously posted an RFC series with a backup server that streamed Dietmar's
> VMA backup archive format.
> 
> What's next for drive-backup?
> -----------------------------
> 1. Sync modes like drive-mirror (top, full, none).  This makes it possible to
>    preserve the backing file chain.
> 
> v6:
>   * Note: I will send HMP support as a follow-up patch
>   * Note: The 'Abort' action's name is unchanged
>   * Extract wait_until_completed() into iotests.py
>   * Populate image with non-zero data and resume job in 055 test_pause() [kwolf]
>   * Document NotifierWithReturn return value semantics [eblake]
>   * Convert DPRINTF() to trace events [pbonzini]
>   * Avoid calling bdrv_getlength() inside the loop [kwolf]
>   * Comment that BlockJob->offset is an opaque progress value [kwolf]
>   * Align '=' to same column [kwolf]
>   * Use bdrv_set_enable_write_cache(target, true) [kwolf]
>   * Use BACKUP_SECTORS_PER_CLUSTERS instead of 1 sector for clarity [kwolf]
>   * Check cancel_and_wait() 'data/type' is 'backup' in 055 [kwolf]
> 
> v5:
>  * Use bdrv_co_write_zeroes(job->target) [kwolf]
> 
>    This change means that we write zeroes over NBD again.  The optimization can
>    be reintroduced by skipping zeroes when bdrv_has_zero_init() is true and the
>    sectors are allocated.  Leave that for a future series, if we decide to do
>    this optimization again because it may require extra block driver
>    configuration to indicate that an image has zero init.
> 
>  * iostatus error handling [kwolf/pbonzini]
> 
>    Add configurable on-source-error and on-target-error actions just like
>    drive-mirror.  These are used when the block job coroutine hits an error.
>    If we are in guest write request context, return the errno and let the usual
>    guest error handling take over.
> 
>  * Allow BdrvActionOps->commit() to be NULL [eblake]
>  * Use bdrv_getlength() in qmp_drive_mirror() [kwolf]
>  * Drop redundant proto_drv check [kwolf]
>  * Fix outdated DPRINTF() function names
>  * Drop comment about non-existent bitmap coroutine race [kwolf]
>  * Rename BACKUP_SECTORS_PER_CLUSTER [kwolf]
>  * Fix completion when image is not multiple of backup cluster size [fam]
> 
> v4:
>  * Use notifier lists and BdrvTrackedRequest instead of custom callbacks [bonzini]
>  * Add drive-backup QMP example JSON [eblake]
>  * Add "Since: 1.6" to QMP schema changes [eblake]
> 
> v3:
>  * Rename to drive-backup for consistency with drive-mirror [kwolf]
>  * Add QMP transaction support [kwolf]
>  * Introduce bdrv_add_before_write_cb() to hook writes
>  * Mention 'query-block-jobs' lists job of type 'backup' [eblake]
>  * Rename rwlock to flush_rwlock [kwolf]
>  * Fix space in block/backup.c comment [kwolf]
> 
> v2:
>  * s/block_backup/block-backup/ in commit message [eblake]
>  * Avoid funny spacing in QMP docs [eblake]
>  * Document query-block-jobs and block-job-cancel usage [eblake]
> 
> Dietmar Maurer (1):
>   block: add basic backup support to block driver
> 
> Stefan Hajnoczi (11):
>   notify: add NotiferWithReturn so notifier list can abort
>   block: add bdrv_add_before_write_notifier()
>   blockdev: drop redundant proto_drv check
>   blockdev: use bdrv_getlength() in qmp_drive_mirror()
>   block: add drive-backup QMP command
>   blockdev: rename BlkTransactionStates to singular
>   blockdev: allow BdrvActionOps->commit() to be NULL
>   blockdev: add DriveBackup transaction
>   blockdev: add Abort transaction
>   qemu-iotests: extract wait_until_completed() into iotests.py
>   qemu-iotests: add 055 drive-backup test case
> 
>  block.c                       |  23 +--
>  block/Makefile.objs           |   1 +
>  block/backup.c                | 341 ++++++++++++++++++++++++++++++++++++++++++
>  blockdev.c                    | 288 ++++++++++++++++++++++++++---------
>  include/block/block_int.h     |  42 +++++-
>  include/qemu/notify.h         |  29 ++++
>  qapi-schema.json              |  97 +++++++++++-
>  qmp-commands.hx               |  46 ++++++
>  tests/qemu-iotests/041        |  14 +-
>  tests/qemu-iotests/055        | 282 ++++++++++++++++++++++++++++++++++
>  tests/qemu-iotests/055.out    |   5 +
>  tests/qemu-iotests/group      |   1 +
>  tests/qemu-iotests/iotests.py |  15 ++
>  trace-events                  |   8 +
>  util/notify.c                 |  30 ++++
>  15 files changed, 1129 insertions(+), 93 deletions(-)
>  create mode 100644 block/backup.c
>  create mode 100755 tests/qemu-iotests/055
>  create mode 100644 tests/qemu-iotests/055.out

Thanks, applied all to the block branch.

The only comment I had is the missing 'const' in patch 3, which I added
myself while applying the patch. I also sent a patch to make the same
change in the existing block jobs.

Kevin