Re: [Qemu-devel] [PATCH v3 2/3] block: Add blockdev-backup to transaction

[Markus Armbruster]

Fam Zheng <address@hidden> writes:

> BTW add version info for other transaction types.
>
> Signed-off-by: Fam Zheng <address@hidden>
> ---
>  blockdev.c       | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
>  qapi-schema.json |  7 +++++++
>  2 files changed, 55 insertions(+)
>
> diff --git a/blockdev.c b/blockdev.c
> index 2e5068c..6401850 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -1495,6 +1495,49 @@ static void drive_backup_abort(BlkTransactionState 
> *common)
>      }
>  }
>  
> +typedef struct BlockdevBackupState {
> +    BlkTransactionState common;
> +    BlockDriverState *bs;
> +    BlockJob *job;
> +} BlockdevBackupState;
> +
> +static void blockdev_backup_prepare(BlkTransactionState *common, Error 
> **errp)
> +{
> +    BlockdevBackupState *state = DO_UPCAST(BlockdevBackupState, common, 
> common);
> +    BlockdevBackup *backup;
> +    Error *local_err = NULL;
> +
> +    assert(common->action->kind == TRANSACTION_ACTION_KIND_BLOCKDEV_BACKUP);
> +    backup = common->action->blockdev_backup;
> +
> +    qmp_blockdev_backup(backup->device, backup->target,
> +                        backup->sync,
> +                        backup->has_speed, backup->speed,
> +                        backup->has_on_source_error, backup->on_source_error,
> +                        backup->has_on_target_error, backup->on_target_error,
> +                        &local_err);
> +    if (local_err) {
> +        error_propagate(errp, local_err);
> +        state->bs = NULL;
> +        state->job = NULL;
> +        return;
> +    }
> +
> +    state->bs = bdrv_find(backup->device);
> +    state->job = state->bs->job;
> +}
> +
> +static void blockdev_backup_abort(BlkTransactionState *common)
> +{
> +    BlockdevBackupState *state = DO_UPCAST(BlockdevBackupState, common, 
> common);
> +    BlockDriverState *bs = state->bs;
> +
> +    /* Only cancel if it's the job we started */
> +    if (bs && bs->job && bs->job == state->job) {
> +        block_job_cancel_sync(bs->job);
> +    }
> +}
> +
>  static void abort_prepare(BlkTransactionState *common, Error **errp)
>  {
>      error_setg(errp, "Transaction aborted using Abort action");
> @@ -1517,6 +1560,11 @@ static const BdrvActionOps actions[] = {
>          .prepare = drive_backup_prepare,
>          .abort = drive_backup_abort,
>      },
> +    [TRANSACTION_ACTION_KIND_BLOCKDEV_BACKUP] = {
> +        .instance_size = sizeof(BlockdevBackupState),
> +        .prepare = blockdev_backup_prepare,
> +        .abort = blockdev_backup_abort,
> +    },
>      [TRANSACTION_ACTION_KIND_ABORT] = {
>          .instance_size = sizeof(BlkTransactionState),
>          .prepare = abort_prepare,
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 24379ab..5fc6892 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -1254,11 +1254,18 @@
>  #
>  # A discriminated record of operations that can be performed with
>  # @transaction.
> +#
> +# Since 1.1
> +# drive-backup since 1.6
> +# abort since 1.6
> +# blockdev-snapshot-internal-sync since 1.7

I'd do these belated doc updates in a separate patch preceding this one.
Up to you.

> +# blockdev-backup since 2.3
>  ##
>  { 'union': 'TransactionAction',
>    'data': {
>         'blockdev-snapshot-sync': 'BlockdevSnapshot',
>         'drive-backup': 'DriveBackup',
> +       'blockdev-backup': 'BlockdevBackup',
>         'abort': 'Abort',
>         'blockdev-snapshot-internal-sync': 'BlockdevSnapshotInternal'
>     } }