[PATCH v3 0/7] introduce job-change qmp command

[PATCH v3 0/7] introduce job-change qmp command

[Vladimir Sementsov-Ogievskiy]

Hi all!

Here is new job-change command - a replacement for (becoming deprecated)
block-job-change, as a first step of my "[RFC 00/15] block job API"

v3:
01: add a-b by Markus
03: add a-b by Markus, s/9.1/9.2/ in QAPI
05: update commit message, s/9.1/9.2/ in QAPI
06: update commit message (and subject!), s/9.1/9.2/ in deprecated.rst

Vladimir Sementsov-Ogievskiy (7):
  qapi: rename BlockJobChangeOptions to JobChangeOptions
  blockjob: block_job_change_locked(): check job type
  qapi: block-job-change: make copy-mode parameter optional
  blockjob: move change action implementation to job from block-job
  qapi: add job-change
  qapi/block-core: deprecate block-job-change
  iotests/mirror-change-copy-mode: switch to job-change command

 block/mirror.c                                | 13 +++++---
 blockdev.c                                    |  4 +--
 blockjob.c                                    | 20 ------------
 docs/about/deprecated.rst                     |  5 +++
 include/block/blockjob.h                      | 11 -------
 include/block/blockjob_int.h                  |  7 -----
 include/qemu/job.h                            | 12 +++++++
 job-qmp.c                                     | 15 +++++++++
 job.c                                         | 23 ++++++++++++++
 qapi/block-core.json                          | 31 ++++++++++++++-----
 .../tests/mirror-change-copy-mode             |  2 +-
 11 files changed, 90 insertions(+), 53 deletions(-)

-- 
2.34.1

[PATCH v3 1/7] qapi: rename BlockJobChangeOptions to JobChangeOptions

[Vladimir Sementsov-Ogievskiy]

We are going to move change action from block-job to job
implementation, and then move to job-* extenral APIs, deprecating
block-job-* APIs. This commit simplifies further transition.

The commit is made by command

    git grep -l BlockJobChangeOptions | \
        xargs sed -i 's/BlockJobChangeOptions/JobChangeOptions/g'

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Acked-by: Markus Armbruster <armbru@redhat.com>
---
 block/mirror.c               |  4 ++--
 blockdev.c                   |  2 +-
 blockjob.c                   |  2 +-
 include/block/blockjob.h     |  2 +-
 include/block/blockjob_int.h |  2 +-
 qapi/block-core.json         | 12 ++++++------
 6 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/block/mirror.c b/block/mirror.c
index 61f0a717b7..2816bb1042 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -1258,11 +1258,11 @@ static bool commit_active_cancel(Job *job, bool force)
     return force || !job_is_ready(job);
 }
 
-static void mirror_change(BlockJob *job, BlockJobChangeOptions *opts,
+static void mirror_change(BlockJob *job, JobChangeOptions *opts,
                           Error **errp)
 {
     MirrorBlockJob *s = container_of(job, MirrorBlockJob, common);
-    BlockJobChangeOptionsMirror *change_opts = &opts->u.mirror;
+    JobChangeOptionsMirror *change_opts = &opts->u.mirror;
     MirrorCopyMode current;
 
     /*
diff --git a/blockdev.c b/blockdev.c
index 6740663fda..626f53102d 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -3251,7 +3251,7 @@ void qmp_block_job_dismiss(const char *id, Error **errp)
     job_dismiss_locked(&job, errp);
 }
 
-void qmp_block_job_change(BlockJobChangeOptions *opts, Error **errp)
+void qmp_block_job_change(JobChangeOptions *opts, Error **errp)
 {
     BlockJob *job;
 
diff --git a/blockjob.c b/blockjob.c
index d5f29e14af..8cfbb15543 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -312,7 +312,7 @@ static bool block_job_set_speed(BlockJob *job, int64_t speed, Error **errp)
     return block_job_set_speed_locked(job, speed, errp);
 }
 
-void block_job_change_locked(BlockJob *job, BlockJobChangeOptions *opts,
+void block_job_change_locked(BlockJob *job, JobChangeOptions *opts,
                              Error **errp)
 {
     const BlockJobDriver *drv = block_job_driver(job);
diff --git a/include/block/blockjob.h b/include/block/blockjob.h
index 7061ab7201..5dd1b08909 100644
--- a/include/block/blockjob.h
+++ b/include/block/blockjob.h
@@ -181,7 +181,7 @@ bool block_job_set_speed_locked(BlockJob *job, int64_t speed, Error **errp);
  *
  * Change the job according to opts.
  */
-void block_job_change_locked(BlockJob *job, BlockJobChangeOptions *opts,
+void block_job_change_locked(BlockJob *job, JobChangeOptions *opts,
                              Error **errp);
 
 /**
diff --git a/include/block/blockjob_int.h b/include/block/blockjob_int.h
index 4c3d2e25a2..d9c3b911d0 100644
--- a/include/block/blockjob_int.h
+++ b/include/block/blockjob_int.h
@@ -73,7 +73,7 @@ struct BlockJobDriver {
      *
      * Note that this can already be called before the job coroutine is running.
      */
-    void (*change)(BlockJob *job, BlockJobChangeOptions *opts, Error **errp);
+    void (*change)(BlockJob *job, JobChangeOptions *opts, Error **errp);
 
     /*
      * Query information specific to this kind of block job.
diff --git a/qapi/block-core.json b/qapi/block-core.json
index c3b0a2376b..0156762024 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -3068,18 +3068,18 @@
   'allow-preconfig': true }
 
 ##
-# @BlockJobChangeOptionsMirror:
+# @JobChangeOptionsMirror:
 #
 # @copy-mode: Switch to this copy mode.  Currently, only the switch
 #     from 'background' to 'write-blocking' is implemented.
 #
 # Since: 8.2
 ##
-{ 'struct': 'BlockJobChangeOptionsMirror',
+{ 'struct': 'JobChangeOptionsMirror',
   'data': { 'copy-mode' : 'MirrorCopyMode' } }
 
 ##
-# @BlockJobChangeOptions:
+# @JobChangeOptions:
 #
 # Block job options that can be changed after job creation.
 #
@@ -3089,10 +3089,10 @@
 #
 # Since: 8.2
 ##
-{ 'union': 'BlockJobChangeOptions',
+{ 'union': 'JobChangeOptions',
   'base': { 'id': 'str', 'type': 'JobType' },
   'discriminator': 'type',
-  'data': { 'mirror': 'BlockJobChangeOptionsMirror' } }
+  'data': { 'mirror': 'JobChangeOptionsMirror' } }
 
 ##
 # @block-job-change:
@@ -3102,7 +3102,7 @@
 # Since: 8.2
 ##
 { 'command': 'block-job-change',
-  'data': 'BlockJobChangeOptions', 'boxed': true }
+  'data': 'JobChangeOptions', 'boxed': true }
 
 ##
 # @BlockdevDiscardOptions:
-- 
2.34.1

[PATCH v3 2/7] blockjob: block_job_change_locked(): check job type

[Vladimir Sementsov-Ogievskiy]

User may specify wrong type for the job id. Let's check it.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
 blockjob.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/blockjob.c b/blockjob.c
index 8cfbb15543..788cb1e07d 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -319,6 +319,12 @@ void block_job_change_locked(BlockJob *job, JobChangeOptions *opts,
 
     GLOBAL_STATE_CODE();
 
+    if (job_type(&job->job) != opts->type) {
+        error_setg(errp, "Job '%s' is '%s' job, not '%s'", job->job.id,
+                   job_type_str(&job->job), JobType_str(opts->type));
+        return;
+    }
+
     if (job_apply_verb_locked(&job->job, JOB_VERB_CHANGE, errp)) {
         return;
     }
-- 
2.34.1

[PATCH v3 3/7] qapi: block-job-change: make copy-mode parameter optional

[Vladimir Sementsov-Ogievskiy]

We are going to add more parameters to change. We want to make possible
to change only one or any subset of available options. So all the
options should be optional.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Acked-by: Markus Armbruster <armbru@redhat.com>
---
 block/mirror.c       | 4 ++++
 qapi/block-core.json | 3 ++-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/block/mirror.c b/block/mirror.c
index 2816bb1042..60e8d83e4f 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -1272,6 +1272,10 @@ static void mirror_change(BlockJob *job, JobChangeOptions *opts,
 
     GLOBAL_STATE_CODE();
 
+    if (!change_opts->has_copy_mode) {
+        return;
+    }
+
     if (qatomic_read(&s->copy_mode) == change_opts->copy_mode) {
         return;
     }
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 0156762024..f370593037 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -3072,11 +3072,12 @@
 #
 # @copy-mode: Switch to this copy mode.  Currently, only the switch
 #     from 'background' to 'write-blocking' is implemented.
+#     If absent, copy mode remains the same.  (optional since 9.2)
 #
 # Since: 8.2
 ##
 { 'struct': 'JobChangeOptionsMirror',
-  'data': { 'copy-mode' : 'MirrorCopyMode' } }
+  'data': { '*copy-mode' : 'MirrorCopyMode' } }
 
 ##
 # @JobChangeOptions:
-- 
2.34.1

Re: [PATCH v3 3/7] qapi: block-job-change: make copy-mode parameter optional

[Kevin Wolf]

Am 02.10.2024 um 16:06 hat Vladimir Sementsov-Ogievskiy geschrieben:
> We are going to add more parameters to change. We want to make possible
> to change only one or any subset of available options. So all the
> options should be optional.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> Acked-by: Markus Armbruster <armbru@redhat.com>

This is different from blockdev-reopen, which requires repeating all
options (and can therefore reuse the type from blockdev-add). One of the
reasons why we made it like that is that we had some options for which
"option absent" was a meaningful value in itself, so it would have
become ambiguous if an absent option in blockdev-reopen should mean
"leave the existing value unchanged" or "unset the option".

Are we confident that this will never happen with job options? In case
of doubt, I would go for consistency with blockdev-reopen.

Either way, it would be good to document the reasoning for whatever
option we choose in the commit message.

Kevin

Re: [PATCH v3 3/7] qapi: block-job-change: make copy-mode parameter optional

[Vladimir Sementsov-Ogievskiy]

On 22.10.24 15:35, Kevin Wolf wrote:
> Am 02.10.2024 um 16:06 hat Vladimir Sementsov-Ogievskiy geschrieben:
>> We are going to add more parameters to change. We want to make possible
>> to change only one or any subset of available options. So all the
>> options should be optional.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
>> Acked-by: Markus Armbruster <armbru@redhat.com>
> 
> This is different from blockdev-reopen, which requires repeating all
> options (and can therefore reuse the type from blockdev-add). One of the
> reasons why we made it like that is that we had some options for which
> "option absent" was a meaningful value in itself, so it would have
> become ambiguous if an absent option in blockdev-reopen should mean
> "leave the existing value unchanged" or "unset the option".
> 

Right.. Thanks for noting this. I'll try to apply blockdev-reopen approach here.

> Are we confident that this will never happen with job options? In case
> of doubt, I would go for consistency with blockdev-reopen.
> 
> Either way, it would be good to document the reasoning for whatever
> option we choose in the commit message.
> 
> Kevin
> 

-- 
Best regards,
Vladimir

[PATCH v3 4/7] blockjob: move change action implementation to job from block-job

[Vladimir Sementsov-Ogievskiy]

Like for other block-job-* APIs we want have the actual functionality
in job layer and make block-job-change to be a deprecated duplication
of job-change in the following commit.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
 block/mirror.c               |  7 +++----
 blockdev.c                   |  2 +-
 blockjob.c                   | 26 --------------------------
 include/block/blockjob.h     | 11 -----------
 include/block/blockjob_int.h |  7 -------
 include/qemu/job.h           | 12 ++++++++++++
 job-qmp.c                    |  1 +
 job.c                        | 23 +++++++++++++++++++++++
 8 files changed, 40 insertions(+), 49 deletions(-)

diff --git a/block/mirror.c b/block/mirror.c
index 60e8d83e4f..63e35114f3 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -1258,10 +1258,9 @@ static bool commit_active_cancel(Job *job, bool force)
     return force || !job_is_ready(job);
 }
 
-static void mirror_change(BlockJob *job, JobChangeOptions *opts,
-                          Error **errp)
+static void mirror_change(Job *job, JobChangeOptions *opts, Error **errp)
 {
-    MirrorBlockJob *s = container_of(job, MirrorBlockJob, common);
+    MirrorBlockJob *s = container_of(job, MirrorBlockJob, common.job);
     JobChangeOptionsMirror *change_opts = &opts->u.mirror;
     MirrorCopyMode current;
 
@@ -1316,9 +1315,9 @@ static const BlockJobDriver mirror_job_driver = {
         .pause                  = mirror_pause,
         .complete               = mirror_complete,
         .cancel                 = mirror_cancel,
+        .change                 = mirror_change,
     },
     .drained_poll           = mirror_drained_poll,
-    .change                 = mirror_change,
     .query                  = mirror_query,
 };
 
diff --git a/blockdev.c b/blockdev.c
index 626f53102d..b1c3de3862 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -3262,7 +3262,7 @@ void qmp_block_job_change(JobChangeOptions *opts, Error **errp)
         return;
     }
 
-    block_job_change_locked(job, opts, errp);
+    job_change_locked(&job->job, opts, errp);
 }
 
 void qmp_change_backing_file(const char *device,
diff --git a/blockjob.c b/blockjob.c
index 788cb1e07d..2769722b37 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -312,32 +312,6 @@ static bool block_job_set_speed(BlockJob *job, int64_t speed, Error **errp)
     return block_job_set_speed_locked(job, speed, errp);
 }
 
-void block_job_change_locked(BlockJob *job, JobChangeOptions *opts,
-                             Error **errp)
-{
-    const BlockJobDriver *drv = block_job_driver(job);
-
-    GLOBAL_STATE_CODE();
-
-    if (job_type(&job->job) != opts->type) {
-        error_setg(errp, "Job '%s' is '%s' job, not '%s'", job->job.id,
-                   job_type_str(&job->job), JobType_str(opts->type));
-        return;
-    }
-
-    if (job_apply_verb_locked(&job->job, JOB_VERB_CHANGE, errp)) {
-        return;
-    }
-
-    if (drv->change) {
-        job_unlock();
-        drv->change(job, opts, errp);
-        job_lock();
-    } else {
-        error_setg(errp, "Job type does not support change");
-    }
-}
-
 void block_job_ratelimit_processed_bytes(BlockJob *job, uint64_t n)
 {
     IO_CODE();
diff --git a/include/block/blockjob.h b/include/block/blockjob.h
index 5dd1b08909..72e849a140 100644
--- a/include/block/blockjob.h
+++ b/include/block/blockjob.h
@@ -173,17 +173,6 @@ bool block_job_has_bdrv(BlockJob *job, BlockDriverState *bs);
  */
 bool block_job_set_speed_locked(BlockJob *job, int64_t speed, Error **errp);
 
-/**
- * block_job_change_locked:
- * @job: The job to change.
- * @opts: The new options.
- * @errp: Error object.
- *
- * Change the job according to opts.
- */
-void block_job_change_locked(BlockJob *job, JobChangeOptions *opts,
-                             Error **errp);
-
 /**
  * block_job_query_locked:
  * @job: The job to get information about.
diff --git a/include/block/blockjob_int.h b/include/block/blockjob_int.h
index d9c3b911d0..58bc7a5cea 100644
--- a/include/block/blockjob_int.h
+++ b/include/block/blockjob_int.h
@@ -68,13 +68,6 @@ struct BlockJobDriver {
 
     void (*set_speed)(BlockJob *job, int64_t speed);
 
-    /*
-     * Change the @job's options according to @opts.
-     *
-     * Note that this can already be called before the job coroutine is running.
-     */
-    void (*change)(BlockJob *job, JobChangeOptions *opts, Error **errp);
-
     /*
      * Query information specific to this kind of block job.
      */
diff --git a/include/qemu/job.h b/include/qemu/job.h
index 2b873f2576..6fa525dac3 100644
--- a/include/qemu/job.h
+++ b/include/qemu/job.h
@@ -27,6 +27,7 @@
 #define JOB_H
 
 #include "qapi/qapi-types-job.h"
+#include "qapi/qapi-types-block-core.h"
 #include "qemu/queue.h"
 #include "qemu/progress_meter.h"
 #include "qemu/coroutine.h"
@@ -307,6 +308,12 @@ struct JobDriver {
      */
     bool (*cancel)(Job *job, bool force);
 
+    /**
+     * Change the @job's options according to @opts.
+     *
+     * Note that this can already be called before the job coroutine is running.
+     */
+    void (*change)(Job *job, JobChangeOptions *opts, Error **errp);
 
     /**
      * Called when the job is freed.
@@ -705,6 +712,11 @@ void job_finalize_locked(Job *job, Error **errp);
  */
 void job_dismiss_locked(Job **job, Error **errp);
 
+/**
+ * Change the job according to opts.
+ */
+void job_change_locked(Job *job, JobChangeOptions *opts, Error **errp);
+
 /**
  * Synchronously finishes the given @job. If @finish is given, it is called to
  * trigger completion or cancellation of the job.
diff --git a/job-qmp.c b/job-qmp.c
index 9e26fa899f..c764bd3801 100644
--- a/job-qmp.c
+++ b/job-qmp.c
@@ -26,6 +26,7 @@
 #include "qemu/osdep.h"
 #include "qemu/job.h"
 #include "qapi/qapi-commands-job.h"
+#include "qapi/qapi-commands-block-core.h"
 #include "qapi/error.h"
 #include "trace/trace-root.h"
 
diff --git a/job.c b/job.c
index 660ce22c56..7b004fe12e 100644
--- a/job.c
+++ b/job.c
@@ -1262,3 +1262,26 @@ int job_finish_sync_locked(Job *job,
     job_unref_locked(job);
     return ret;
 }
+
+void job_change_locked(Job *job, JobChangeOptions *opts, Error **errp)
+{
+    GLOBAL_STATE_CODE();
+
+    if (job_type(job) != opts->type) {
+        error_setg(errp, "Job '%s' is '%s' job, not '%s'", job->id,
+                   job_type_str(job), JobType_str(opts->type));
+        return;
+    }
+
+    if (job_apply_verb_locked(job, JOB_VERB_CHANGE, errp)) {
+        return;
+    }
+
+    if (job->driver->change) {
+        job_unlock();
+        job->driver->change(job, opts, errp);
+        job_lock();
+    } else {
+        error_setg(errp, "Job type does not support change");
+    }
+}
-- 
2.34.1

Re: [PATCH v3 4/7] blockjob: move change action implementation to job from block-job

[Kevin Wolf]

Am 02.10.2024 um 16:06 hat Vladimir Sementsov-Ogievskiy geschrieben:
> Like for other block-job-* APIs we want have the actual functionality
> in job layer and make block-job-change to be a deprecated duplication
> of job-change in the following commit.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>

> diff --git a/include/qemu/job.h b/include/qemu/job.h
> index 2b873f2576..6fa525dac3 100644
> --- a/include/qemu/job.h
> +++ b/include/qemu/job.h
> @@ -27,6 +27,7 @@
>  #define JOB_H
>  
>  #include "qapi/qapi-types-job.h"
> +#include "qapi/qapi-types-block-core.h"
>  #include "qemu/queue.h"
>  #include "qemu/progress_meter.h"
>  #include "qemu/coroutine.h"
> @@ -307,6 +308,12 @@ struct JobDriver {
>       */
>      bool (*cancel)(Job *job, bool force);
>  
> +    /**
> +     * Change the @job's options according to @opts.
> +     *
> +     * Note that this can already be called before the job coroutine is running.
> +     */
> +    void (*change)(Job *job, JobChangeOptions *opts, Error **errp);
>  
>      /**
>       * Called when the job is freed.
> @@ -705,6 +712,11 @@ void job_finalize_locked(Job *job, Error **errp);
>   */
>  void job_dismiss_locked(Job **job, Error **errp);
>  
> +/**
> + * Change the job according to opts.
> + */
> +void job_change_locked(Job *job, JobChangeOptions *opts, Error **errp);

Other functions in this header document the locking behaviour. The right
one here seems to be: "Called with job_lock held, but might release it
temporarily."

Kevin

[PATCH v3 5/7] qapi: add job-change

[Vladimir Sementsov-Ogievskiy]

Add a new-style command job-change, doing same thing as
block-job-change. The aim is finally deprecate block-job-* APIs and
move to job-* APIs.

We add a new command to qapi/block-core.json, not to
qapi/job.json to avoid resolving json file including loops for now.
This all would be a lot simple to refactor when we finally drop
deprecated block-job-* APIs.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
 job-qmp.c            | 14 ++++++++++++++
 qapi/block-core.json | 10 ++++++++++
 2 files changed, 24 insertions(+)

diff --git a/job-qmp.c b/job-qmp.c
index c764bd3801..248e68f554 100644
--- a/job-qmp.c
+++ b/job-qmp.c
@@ -139,6 +139,20 @@ void qmp_job_dismiss(const char *id, Error **errp)
     job_dismiss_locked(&job, errp);
 }
 
+void qmp_job_change(JobChangeOptions *opts, Error **errp)
+{
+    Job *job;
+
+    JOB_LOCK_GUARD();
+    job = find_job_locked(opts->id, errp);
+
+    if (!job) {
+        return;
+    }
+
+    job_change_locked(job, opts, errp);
+}
+
 /* Called with job_mutex held. */
 static JobInfo *job_query_single_locked(Job *job, Error **errp)
 {
diff --git a/qapi/block-core.json b/qapi/block-core.json
index f370593037..e314734b53 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -3105,6 +3105,16 @@
 { 'command': 'block-job-change',
   'data': 'JobChangeOptions', 'boxed': true }
 
+##
+# @job-change:
+#
+# Change the block job's options.
+#
+# Since: 9.2
+##
+{ 'command': 'job-change',
+  'data': 'JobChangeOptions', 'boxed': true }
+
 ##
 # @BlockdevDiscardOptions:
 #
-- 
2.34.1

Re: [PATCH v3 5/7] qapi: add job-change

[Kevin Wolf]

Am 02.10.2024 um 16:06 hat Vladimir Sementsov-Ogievskiy geschrieben:
> Add a new-style command job-change, doing same thing as
> block-job-change. The aim is finally deprecate block-job-* APIs and
> move to job-* APIs.
> 
> We add a new command to qapi/block-core.json, not to
> qapi/job.json to avoid resolving json file including loops for now.
> This all would be a lot simple to refactor when we finally drop
> deprecated block-job-* APIs.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> ---
>  job-qmp.c            | 14 ++++++++++++++
>  qapi/block-core.json | 10 ++++++++++
>  2 files changed, 24 insertions(+)

> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index f370593037..e314734b53 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -3105,6 +3105,16 @@
>  { 'command': 'block-job-change',
>    'data': 'JobChangeOptions', 'boxed': true }
>  
> +##
> +# @job-change:
> +#
> +# Change the block job's options.

s/block job/job/

> +#
> +# Since: 9.2
> +##
> +{ 'command': 'job-change',
> +  'data': 'JobChangeOptions', 'boxed': true }

Kevin

[PATCH v3 6/7] qapi/block-core: deprecate block-job-change

[Vladimir Sementsov-Ogievskiy]

That's a first step to move on newer job-* APIs.

The difference between block-job-change and job-change is in
find_block_job_locked() vs find_job_locked() functions. What's
different?

1. find_block_job_locked() finds only block jobs, whereas
   find_job_locked() finds any kind of job.  job-change is a
   compatible extension of block-job-change.

2. find_block_job_locked() reports DeviceNotActive on failure, when
   find_job_locked() reports GenericError.  Since the kind of error
   reported isn't documented for either command, and clients
   shouldn't rely on undocumented error details, job-change is a
   compatible replacement for block-job-change.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
 docs/about/deprecated.rst | 5 +++++
 qapi/block-core.json      | 6 ++++++
 2 files changed, 11 insertions(+)

diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index 1e21fbbf77..d2461924ff 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -147,6 +147,11 @@ options are removed in favor of using explicit ``blockdev-create`` and
 ``blockdev-add`` calls. See :doc:`/interop/live-block-operations` for
 details.
 
+``block-job-change`` (since 9.2)
+''''''''''''''''''''''''''''''''
+
+Use ``job-change`` instead.
+
 Incorrectly typed ``device_add`` arguments (since 6.2)
 ''''''''''''''''''''''''''''''''''''''''''''''''''''''
 
diff --git a/qapi/block-core.json b/qapi/block-core.json
index e314734b53..ed87a9dc1e 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -3100,9 +3100,15 @@
 #
 # Change the block job's options.
 #
+# Features:
+#
+# @deprecated: This command is deprecated.  Use @job-change
+#     instead.
+#
 # Since: 8.2
 ##
 { 'command': 'block-job-change',
+  'features': ['deprecated'],
   'data': 'JobChangeOptions', 'boxed': true }
 
 ##
-- 
2.34.1

Re: [PATCH v3 6/7] qapi/block-core: deprecate block-job-change

[Markus Armbruster]

Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> writes:

> That's a first step to move on newer job-* APIs.
>
> The difference between block-job-change and job-change is in
> find_block_job_locked() vs find_job_locked() functions. What's
> different?
>
> 1. find_block_job_locked() finds only block jobs, whereas
>    find_job_locked() finds any kind of job.  job-change is a
>    compatible extension of block-job-change.
>
> 2. find_block_job_locked() reports DeviceNotActive on failure, when
>    find_job_locked() reports GenericError.  Since the kind of error
>    reported isn't documented for either command, and clients
>    shouldn't rely on undocumented error details, job-change is a
>    compatible replacement for block-job-change.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>

Acked-by: Markus Armbruster <armbru@redhat.com>

[PATCH v3 7/7] iotests/mirror-change-copy-mode: switch to job-change command

[Vladimir Sementsov-Ogievskiy]

block-job-change is deprecated, let's move test to job-change.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
 tests/qemu-iotests/tests/mirror-change-copy-mode | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/tests/mirror-change-copy-mode b/tests/qemu-iotests/tests/mirror-change-copy-mode
index 51788b85c7..e972604ebf 100755
--- a/tests/qemu-iotests/tests/mirror-change-copy-mode
+++ b/tests/qemu-iotests/tests/mirror-change-copy-mode
@@ -150,7 +150,7 @@ class TestMirrorChangeCopyMode(iotests.QMPTestCase):
         len_before_change = result[0]['len']
 
         # Change the copy mode while requests are happening.
-        self.vm.cmd('block-job-change',
+        self.vm.cmd('job-change',
                     id='mirror',
                     type='mirror',
                     copy_mode='write-blocking')
-- 
2.34.1