Re: [Qemu-devel] [PATCH 1/2] qapi: deprecate drive-mirror and drive-backup

[John Snow]

On 8/14/19 6:07 AM, Vladimir Sementsov-Ogievskiy wrote:
> It's hard and not necessary to maintain outdated versions of these
> commands.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
> ---
>  qemu-deprecated.texi  |  4 ++++
>  qapi/block-core.json  |  4 ++++
>  qapi/transaction.json |  2 +-
>  blockdev.c            | 10 ++++++++++
>  4 files changed, 19 insertions(+), 1 deletion(-)
> 
> diff --git a/qemu-deprecated.texi b/qemu-deprecated.texi
> index fff07bb2a3..2753fafd0b 100644
> --- a/qemu-deprecated.texi
> +++ b/qemu-deprecated.texi
> @@ -179,6 +179,10 @@ and accurate ``query-qmp-schema'' command.
>  Character devices creating sockets in client mode should not specify
>  the 'wait' field, which is only applicable to sockets in server mode
>  
> +@subsection drive-mirror, drive-backup and drive-backup transaction action 
> (since 4.2)
> +
> +Use blockdev-mirror and blockdev-backup instead.
> +
>  @section Human Monitor Protocol (HMP) commands
>  
>  @subsection The hub_id parameter of 'hostfwd_add' / 'hostfwd_remove' (since 
> 3.1)
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 0d43d4f37c..4e35526634 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -1635,6 +1635,8 @@
>  ##
>  # @drive-backup:
>  #
> +# Command is deprecated, use blockdev-mirror instead.
> +#
>  # Start a point-in-time copy of a block device to a new destination.  The
>  # status of ongoing drive-backup operations can be checked with
>  # query-block-jobs where the BlockJobInfo.type field has the value 'backup'.
> @@ -1855,6 +1857,8 @@
>  ##
>  # @drive-mirror:
>  #
> +# Command is deprecated, use blockdev-mirror instead.
> +#
>  # Start mirroring a block device's writes to a new destination. target
>  # specifies the target of the new image. If the file exists, or if it
>  # is a device, it will be used as the new destination for writes. If
> diff --git a/qapi/transaction.json b/qapi/transaction.json
> index 95edb78227..a16a9ff8a6 100644
> --- a/qapi/transaction.json
> +++ b/qapi/transaction.json
> @@ -53,7 +53,7 @@
>  # - @blockdev-snapshot: since 2.5
>  # - @blockdev-snapshot-internal-sync: since 1.7
>  # - @blockdev-snapshot-sync: since 1.1
> -# - @drive-backup: since 1.6
> +# - @drive-backup: deprecated action, since 1.6
>  #
>  # Since: 1.1
>  ##
> diff --git a/blockdev.c b/blockdev.c
> index 4d141e9a1f..36e9368e01 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -1771,6 +1771,9 @@ static void drive_backup_prepare(BlkActionState 
> *common, Error **errp)
>      AioContext *aio_context;
>      Error *local_err = NULL;
>  
> +    warn_report("drive-backup transaction action is deprecated and will "
> +                "disappear in future. Use blockdev-backup action instead");
> +
>      assert(common->action->type == TRANSACTION_ACTION_KIND_DRIVE_BACKUP);
>      backup = common->action->u.drive_backup.data;
>  
> @@ -3591,6 +3594,10 @@ void qmp_drive_backup(DriveBackup *arg, Error **errp)
>  {
>  
>      BlockJob *job;
> +
> +    warn_report("drive-backup command is deprecated and will disappear in "
> +                "future. Use blockdev-backup instead");
> +
>      job = do_drive_backup(arg, NULL, errp);
>      if (job) {
>          job_start(&job->job);
> @@ -3831,6 +3838,9 @@ void qmp_drive_mirror(DriveMirror *arg, Error **errp)
>      const char *format = arg->format;
>      int ret;
>  
> +    warn_report("drive-mirror command is deprecated and will disappear in "
> +                "future. Use blockdev-mirror instead");
> +
>      bs = qmp_get_root_bs(arg->device, errp);
>      if (!bs) {
>          return;
> 

Hm!

I wonder if this is ever-so-slightly too soon for our friends over at
the libvirt project.

I don't think they have fully moved away from the non-blockdev
interfaces *just yet*, and I might encourage seeing the first full
libvirt release that does support and use it before we start the
deprecation clock.

(Juuuust in case.)

That's just me being very, very cautious though.

Peter Krempa, how do you feel about this?