Am 22.07.2016 um 10:17 hat Denis V. Lunev geschrieben:
> From: Pavel Butsykin <[email protected]>
>
> The idea is simple - backup is "written-once" data. It is written block
> by block and it is large enough. It would be nice to save storage
> space and compress it.
>
> The patch adds a flag to the qmp/hmp drive-backup command which enables
> block compression. Compression should be implemented in the format driver
> to enable this feature.
>
> There are some limitations of the format driver to allow compressed writes.
> We can write data only once. Though for backup this is perfectly fine.
> These limitations are maintained by the driver and the error will be
> reported if we are doing something wrong.
>
> Signed-off-by: Pavel Butsykin <[email protected]>
> Reviewed-by: Stefan Hajnoczi <[email protected]>
> Signed-off-by: Denis V. Lunev <[email protected]>
> CC: Jeff Cody <[email protected]>
> CC: Markus Armbruster <[email protected]>
> CC: Eric Blake <[email protected]>
> CC: John Snow <[email protected]>
> CC: Stefan Hajnoczi <[email protected]>
> CC: Kevin Wolf <[email protected]>
> @@ -154,7 +155,8 @@ static int coroutine_fn backup_do_cow(BackupBlockJob *job,
> bounce_qiov.size, BDRV_REQ_MAY_UNMAP);
> } else {
> ret = blk_co_pwritev(job->target, start * job->cluster_size,
> - bounce_qiov.size, &bounce_qiov, 0);
> + bounce_qiov.size, &bounce_qiov,
> + job->compress ? BDRV_REQ_WRITE_COMPRESSED :
> 0);
I love this hunk. Looks so simple. :-)
> @@ -3242,8 +3245,8 @@ static void do_drive_backup(DriveBackup *backup,
> BlockJobTxn *txn, Error **errp)
> }
>
> backup_start(backup->job_id, bs, target_bs, backup->speed, backup->sync,
> - bmap, backup->on_source_error, backup->on_target_error,
> - block_job_cb, bs, txn, &local_err);
> + bmap, backup->compress, backup->on_source_error,
> + backup->on_target_error, block_job_cb, bs, txn, &local_err);
> bdrv_unref(target_bs);
> if (local_err != NULL) {
> error_propagate(errp, local_err);
> @@ -3317,7 +3320,7 @@ void do_blockdev_backup(BlockdevBackup *backup,
> BlockJobTxn *txn, Error **errp)
> }
> }
> backup_start(backup->job_id, bs, target_bs, backup->speed, backup->sync,
> - NULL, backup->on_source_error, backup->on_target_error,
> + NULL, false, backup->on_source_error,
> backup->on_target_error,
> block_job_cb, bs, txn, &local_err);
> if (local_err != NULL) {
> error_propagate(errp, local_err);
blockdev-backup is supposed to be the primary interface in the future,
so I would like to avoid introducing a feature gap where drive-backup is
more powerful. Can you please post a small diff to squash in so that
this patch adds the option to both commands? Wiring this up should be
trivial.
Kevin