From 959955217f745f1ee6cbea97314efe69f2d7dc08 Mon Sep 17 00:00:00 2001 From: Sergio Lopez Pascual Date: Fri, 7 Feb 2020 11:27:43 +0000 Subject: [PATCH 10/18] blockdev: unify qmp_blockdev_backup and blockdev-backup transaction paths RH-Author: Sergio Lopez Pascual Message-id: <20200207112749.25073-4-slp@redhat.com> Patchwork-id: 93756 O-Subject: [RHEL-AV-8.2.0 qemu-kvm PATCH v2 3/9] blockdev: unify qmp_blockdev_backup and blockdev-backup transaction paths Bugzilla: 1745606 1746217 1773517 1779036 1782111 1782175 1783965 RH-Acked-by: Stefano Garzarella RH-Acked-by: Paolo Bonzini RH-Acked-by: Max Reitz RH-Acked-by: Stefan Hajnoczi Issuing a blockdev-backup from qmp_blockdev_backup takes a slightly different path than when it's issued from a transaction. In the code, this is manifested as some redundancy between do_blockdev_backup() and blockdev_backup_prepare(). This change unifies both paths, merging do_blockdev_backup() and blockdev_backup_prepare(), and changing qmp_blockdev_backup() to create a transaction instead of calling do_backup_common() direcly. As a side-effect, now qmp_blockdev_backup() is executed inside a drained section, as it happens when creating a blockdev-backup transaction. This change is visible from the user's perspective, as the job gets paused and immediately resumed before starting the actual work. Signed-off-by: Sergio Lopez Reviewed-by: Max Reitz Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf (cherry picked from commit 5b7bfe515ecbd584b40ff6e41d2fd8b37c7d5139) Signed-off-by: Sergio Lopez Signed-off-by: Danilo C. L. de Paula --- blockdev.c | 60 +++++++++++++----------------------------------------------- 1 file changed, 13 insertions(+), 47 deletions(-) diff --git a/blockdev.c b/blockdev.c index 5e85fc0..152a0f7 100644 --- a/blockdev.c +++ b/blockdev.c @@ -1940,16 +1940,13 @@ typedef struct BlockdevBackupState { BlockJob *job; } BlockdevBackupState; -static BlockJob *do_blockdev_backup(BlockdevBackup *backup, JobTxn *txn, - Error **errp); - static void blockdev_backup_prepare(BlkActionState *common, Error **errp) { BlockdevBackupState *state = DO_UPCAST(BlockdevBackupState, common, common); BlockdevBackup *backup; - BlockDriverState *bs, *target; + BlockDriverState *bs; + BlockDriverState *target_bs; AioContext *aio_context; - Error *local_err = NULL; assert(common->action->type == TRANSACTION_ACTION_KIND_BLOCKDEV_BACKUP); backup = common->action->u.blockdev_backup.data; @@ -1959,8 +1956,8 @@ static void blockdev_backup_prepare(BlkActionState *common, Error **errp) return; } - target = bdrv_lookup_bs(backup->target, backup->target, errp); - if (!target) { + target_bs = bdrv_lookup_bs(backup->target, backup->target, errp); + if (!target_bs) { return; } @@ -1971,13 +1968,10 @@ static void blockdev_backup_prepare(BlkActionState *common, Error **errp) /* Paired with .clean() */ bdrv_drained_begin(state->bs); - state->job = do_blockdev_backup(backup, common->block_job_txn, &local_err); - if (local_err) { - error_propagate(errp, local_err); - goto out; - } + state->job = do_backup_common(qapi_BlockdevBackup_base(backup), + bs, target_bs, aio_context, + common->block_job_txn, errp); -out: aio_context_release(aio_context); } @@ -3695,41 +3689,13 @@ XDbgBlockGraph *qmp_x_debug_query_block_graph(Error **errp) return bdrv_get_xdbg_block_graph(errp); } -BlockJob *do_blockdev_backup(BlockdevBackup *backup, JobTxn *txn, - Error **errp) +void qmp_blockdev_backup(BlockdevBackup *backup, Error **errp) { - BlockDriverState *bs; - BlockDriverState *target_bs; - AioContext *aio_context; - BlockJob *job; - - bs = bdrv_lookup_bs(backup->device, backup->device, errp); - if (!bs) { - return NULL; - } - - target_bs = bdrv_lookup_bs(backup->target, backup->target, errp); - if (!target_bs) { - return NULL; - } - - aio_context = bdrv_get_aio_context(bs); - aio_context_acquire(aio_context); - - job = do_backup_common(qapi_BlockdevBackup_base(backup), - bs, target_bs, aio_context, txn, errp); - - aio_context_release(aio_context); - return job; -} - -void qmp_blockdev_backup(BlockdevBackup *arg, Error **errp) -{ - BlockJob *job; - job = do_blockdev_backup(arg, NULL, errp); - if (job) { - job_start(&job->job); - } + TransactionAction action = { + .type = TRANSACTION_ACTION_KIND_BLOCKDEV_BACKUP, + .u.blockdev_backup.data = backup, + }; + blockdev_do_action(&action, errp); } /* Parameter check and block job starting for drive mirroring. -- 1.8.3.1