Re: [Qemu-devel] [PATCH 3/7] Replication/Blockjobs: Create replication jobs as internal

[Jeff Cody <jcody@redhat.com>]

On Thu, Oct 13, 2016 at 06:56:58PM -0400, John Snow wrote:
> Bubble up the internal interface to commit and backup jobs, then switch
> replication tasks over to using this methodology.
> 
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  block/backup.c            |  3 ++-
>  block/mirror.c            | 21 ++++++++++-----------
>  block/replication.c       | 14 +++++++-------
>  blockdev.c                | 11 +++++++----
>  include/block/block_int.h |  9 +++++++--
>  qemu-img.c                |  5 +++--
>  6 files changed, 36 insertions(+), 27 deletions(-)
> 
> diff --git a/block/backup.c b/block/backup.c
> index 5acb5c4..6a60ca8 100644
> --- a/block/backup.c
> +++ b/block/backup.c
> @@ -527,6 +527,7 @@ void backup_start(const char *job_id, BlockDriverState *bs,
>                    bool compress,
>                    BlockdevOnError on_source_error,
>                    BlockdevOnError on_target_error,
> +                  int creation_flags,
>                    BlockCompletionFunc *cb, void *opaque,
>                    BlockJobTxn *txn, Error **errp)
>  {
> @@ -596,7 +597,7 @@ void backup_start(const char *job_id, BlockDriverState *bs,
>      }
>  
>      job = block_job_create(job_id, &backup_job_driver, bs, speed,
> -                           BLOCK_JOB_DEFAULT, cb, opaque, errp);
> +                           creation_flags, cb, opaque, errp);
>      if (!job) {
>          goto error;
>      }
> diff --git a/block/mirror.c b/block/mirror.c
> index 74c03ae..15d2d10 100644
> --- a/block/mirror.c
> +++ b/block/mirror.c
> @@ -906,9 +906,9 @@ static const BlockJobDriver commit_active_job_driver = {
>  };
>  
>  static void mirror_start_job(const char *job_id, BlockDriverState *bs,
> -                             BlockDriverState *target, const char *replaces,
> -                             int64_t speed, uint32_t granularity,
> -                             int64_t buf_size,
> +                             int creation_flags, BlockDriverState *target,
> +                             const char *replaces, int64_t speed,
> +                             uint32_t granularity, int64_t buf_size,
>                               BlockMirrorBackingMode backing_mode,
>                               BlockdevOnError on_source_error,
>                               BlockdevOnError on_target_error,
> @@ -936,8 +936,8 @@ static void mirror_start_job(const char *job_id, BlockDriverState *bs,
>          buf_size = DEFAULT_MIRROR_BUF_SIZE;
>      }
>  
> -    s = block_job_create(job_id, driver, bs, speed,
> -                         BLOCK_JOB_DEFAULT, cb, opaque, errp);
> +    s = block_job_create(job_id, driver, bs, speed, creation_flags,
> +                         cb, opaque, errp);
>      if (!s) {
>          return;
>      }
> @@ -992,17 +992,16 @@ void mirror_start(const char *job_id, BlockDriverState *bs,
>      }
>      is_none_mode = mode == MIRROR_SYNC_MODE_NONE;
>      base = mode == MIRROR_SYNC_MODE_TOP ? backing_bs(bs) : NULL;
> -    mirror_start_job(job_id, bs, target, replaces,
> +    mirror_start_job(job_id, bs, BLOCK_JOB_DEFAULT, target, replaces,
>                       speed, granularity, buf_size, backing_mode,
>                       on_source_error, on_target_error, unmap, cb, opaque, errp,
>                       &mirror_job_driver, is_none_mode, base, false);
>  }
>  
>  void commit_active_start(const char *job_id, BlockDriverState *bs,
> -                         BlockDriverState *base, int64_t speed,
> -                         BlockdevOnError on_error,
> -                         BlockCompletionFunc *cb,
> -                         void *opaque, Error **errp,
> +                         BlockDriverState *base, int creation_flags,
> +                         int64_t speed, BlockdevOnError on_error,
> +                         BlockCompletionFunc *cb, void *opaque, Error **errp,
>                           bool auto_complete)
>  {
>      int64_t length, base_length;
> @@ -1041,7 +1040,7 @@ void commit_active_start(const char *job_id, BlockDriverState *bs,
>          }
>      }
>  
> -    mirror_start_job(job_id, bs, base, NULL, speed, 0, 0,
> +    mirror_start_job(job_id, bs, creation_flags, base, NULL, speed, 0, 0,
>                       MIRROR_LEAVE_BACKING_CHAIN,
>                       on_error, on_error, false, cb, opaque, &local_err,
>                       &commit_active_job_driver, false, base, auto_complete);
> diff --git a/block/replication.c b/block/replication.c
> index 3bd1cf1..d4f4a7b 100644
> --- a/block/replication.c
> +++ b/block/replication.c
> @@ -496,10 +496,11 @@ static void replication_start(ReplicationState *rs, ReplicationMode mode,
>          bdrv_op_block_all(top_bs, s->blocker);
>          bdrv_op_unblock(top_bs, BLOCK_OP_TYPE_DATAPLANE, s->blocker);
>  
> -        backup_start("replication-backup", s->secondary_disk->bs,
> -                     s->hidden_disk->bs, 0, MIRROR_SYNC_MODE_NONE, NULL, false,
> +        backup_start(NULL, s->secondary_disk->bs, s->hidden_disk->bs, 0,
> +                     MIRROR_SYNC_MODE_NONE, NULL, false,
>                       BLOCKDEV_ON_ERROR_REPORT, BLOCKDEV_ON_ERROR_REPORT,
> -                     backup_job_completed, s, NULL, &local_err);
> +                     BLOCK_JOB_INTERNAL, backup_job_completed, s,
> +                     NULL, &local_err);
>          if (local_err) {
>              error_propagate(errp, local_err);
>              backup_job_cleanup(s);
> @@ -621,10 +622,9 @@ static void replication_stop(ReplicationState *rs, bool failover, Error **errp)
>          }
>  
>          s->replication_state = BLOCK_REPLICATION_FAILOVER;
> -        commit_active_start("replication-commit", s->active_disk->bs,
> -                            s->secondary_disk->bs, 0, BLOCKDEV_ON_ERROR_REPORT,
> -                            replication_done,
> -                            bs, errp, true);
> +        commit_active_start(NULL, s->active_disk->bs, s->secondary_disk->bs,
> +                            BLOCK_JOB_INTERNAL, 0, BLOCKDEV_ON_ERROR_REPORT,
> +                            replication_done, bs, errp, true);
>          break;
>      default:
>          aio_context_release(aio_context);
> diff --git a/blockdev.c b/blockdev.c
> index 5904edb..0ce305c 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -3083,8 +3083,9 @@ void qmp_block_commit(bool has_job_id, const char *job_id, const char *device,
>                               " but 'top' is the active layer");
>              goto out;
>          }
> -        commit_active_start(has_job_id ? job_id : NULL, bs, base_bs, speed,
> -                            on_error, block_job_cb, bs, &local_err, false);
> +        commit_active_start(has_job_id ? job_id : NULL, bs, base_bs,
> +                            BLOCK_JOB_DEFAULT, speed, on_error, block_job_cb,
> +                            bs, &local_err, false);
>      } else {
>          commit_start(has_job_id ? job_id : NULL, bs, base_bs, top_bs, speed,
>                       on_error, block_job_cb, bs,
> @@ -3208,7 +3209,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->compress, backup->on_source_error,
> -                 backup->on_target_error, block_job_cb, bs, txn, &local_err);
> +                 backup->on_target_error, BLOCK_JOB_DEFAULT,
> +                 block_job_cb, bs, txn, &local_err);
>      bdrv_unref(target_bs);
>      if (local_err != NULL) {
>          error_propagate(errp, local_err);
> @@ -3278,7 +3280,8 @@ void do_blockdev_backup(BlockdevBackup *backup, BlockJobTxn *txn, Error **errp)
>      }
>      backup_start(backup->job_id, bs, target_bs, backup->speed, backup->sync,
>                   NULL, backup->compress, backup->on_source_error,
> -                 backup->on_target_error, block_job_cb, bs, txn, &local_err);
> +                 backup->on_target_error, BLOCK_JOB_DEFAULT,
> +                 block_job_cb, bs, txn, &local_err);
>      if (local_err != NULL) {
>          error_propagate(errp, local_err);
>      }
> diff --git a/include/block/block_int.h b/include/block/block_int.h
> index 3e79228..98f1c7f 100644
> --- a/include/block/block_int.h
> +++ b/include/block/block_int.h
> @@ -688,6 +688,8 @@ void commit_start(const char *job_id, BlockDriverState *bs,
>   * device name of @bs.
>   * @bs: Active block device to be committed.
>   * @base: Block device that will be written into, and become the new top.
> + * @creation_flags: Flags that control the behavior of the Job lifetime.
> + *                  See @BlockJobCreateFlags
>   * @speed: The maximum speed, in bytes per second, or 0 for unlimited.
>   * @on_error: The action to take upon error.
>   * @cb: Completion function for the job.
> @@ -697,8 +699,8 @@ void commit_start(const char *job_id, BlockDriverState *bs,
>   *
>   */
>  void commit_active_start(const char *job_id, BlockDriverState *bs,
> -                         BlockDriverState *base, int64_t speed,
> -                         BlockdevOnError on_error,
> +                         BlockDriverState *base, int creation_flags,
> +                         int64_t speed, BlockdevOnError on_error,
>                           BlockCompletionFunc *cb,
>                           void *opaque, Error **errp, bool auto_complete);
>  /*
> @@ -747,6 +749,8 @@ void mirror_start(const char *job_id, BlockDriverState *bs,
>   * @sync_bitmap: The dirty bitmap if sync_mode is MIRROR_SYNC_MODE_INCREMENTAL.
>   * @on_source_error: The action to take upon error reading from the source.
>   * @on_target_error: The action to take upon error writing to the target.
> + * @creation_flags: Flags that control the behavior of the Job lifetime.
> + *                  See @BlockJobCreateFlags
>   * @cb: Completion function for the job.
>   * @opaque: Opaque pointer value passed to @cb.
>   * @txn: Transaction that this job is part of (may be NULL).
> @@ -760,6 +764,7 @@ void backup_start(const char *job_id, BlockDriverState *bs,
>                    bool compress,
>                    BlockdevOnError on_source_error,
>                    BlockdevOnError on_target_error,
> +                  int creation_flags,
>                    BlockCompletionFunc *cb, void *opaque,
>                    BlockJobTxn *txn, Error **errp);
>  
> diff --git a/qemu-img.c b/qemu-img.c
> index 02c07b9..3897d82 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -928,8 +928,9 @@ static int img_commit(int argc, char **argv)
>          .bs   = bs,
>      };
>  
> -    commit_active_start("commit", bs, base_bs, 0, BLOCKDEV_ON_ERROR_REPORT,
> -                        common_block_job_cb, &cbi, &local_err, false);
> +    commit_active_start("commit", bs, base_bs, BLOCK_JOB_DEFAULT, 0,
> +                        BLOCKDEV_ON_ERROR_REPORT, common_block_job_cb, &cbi,
> +                        &local_err, false);
>      if (local_err) {
>          goto done;
>      }
> -- 
> 2.7.4
>

Reviewed-by: Jeff Cody <jcody@redhat.com>