[PATCH v2 6/7] qapi/block-core: derpecate block-job-change

Vladimir Sementsov-Ogievskiy posted 7 patches 5 months ago
Maintainers: John Snow <jsnow@redhat.com>, Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>, Kevin Wolf <kwolf@redhat.com>, Hanna Reitz <hreitz@redhat.com>, Eric Blake <eblake@redhat.com>, Markus Armbruster <armbru@redhat.com>
There is a newer version of this series
[PATCH v2 6/7] qapi/block-core: derpecate block-job-change
Posted by Vladimir Sementsov-Ogievskiy 5 months ago
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() do check, is found job a block-job. This OK
   when moving to more generic API, no needs to document this change.

2. find_block_job_locked() reports DeviceNotActive on failure, when
   find_job_locked() reports GenericError. Still, for block-job-change
   errors are not documented at all, so be silent in deprecated.txt as
   well.

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 ff3da68208..0ddced0781 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -134,6 +134,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.1)
+''''''''''''''''''''''''''''''''
+
+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 9087ce300c..064cad0b64 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -3099,9 +3099,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 v2 6/7] qapi/block-core: derpecate block-job-change
Posted by Markus Armbruster 3 months, 3 weeks ago
Typo in subject: it's "deprecate".
Re: [PATCH v2 6/7] qapi/block-core: derpecate block-job-change
Posted by Markus Armbruster 4 months, 1 week ago
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() do check, is found job a block-job. This OK

Do you mean something like find_block_job_locked() finds only block
jobs, whereas find_job_locked() finds any kind of job?

>    when moving to more generic API, no needs to document this change.
>
> 2. find_block_job_locked() reports DeviceNotActive on failure, when
>    find_job_locked() reports GenericError. Still, for block-job-change
>    errors are not documented at all, so be silent in deprecated.txt as
>    well.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Re: [PATCH v2 6/7] qapi/block-core: derpecate block-job-change
Posted by Vladimir Sementsov-Ogievskiy 3 months, 3 weeks ago
On 18.07.24 14:01, Markus Armbruster wrote:
> 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() do check, is found job a block-job. This OK
> 
> Do you mean something like find_block_job_locked() finds only block
> jobs, whereas find_job_locked() finds any kind of job?

Yes

> 
>>     when moving to more generic API, no needs to document this change.
>>
>> 2. find_block_job_locked() reports DeviceNotActive on failure, when
>>     find_job_locked() reports GenericError. Still, for block-job-change
>>     errors are not documented at all, so be silent in deprecated.txt as
>>     well.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> 

-- 
Best regards,
Vladimir
Re: [PATCH v2 6/7] qapi/block-core: derpecate block-job-change
Posted by Markus Armbruster 3 months, 3 weeks ago
Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> writes:

> On 18.07.24 14:01, Markus Armbruster wrote:
>> 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() do check, is found job a block-job. This OK
>>
>> Do you mean something like find_block_job_locked() finds only block
>> jobs, whereas find_job_locked() finds any kind of job?
>
> Yes

Thanks!

>>>     when moving to more generic API, no needs to document this change.
>>>
>>> 2. find_block_job_locked() reports DeviceNotActive on failure, when
>>>    find_job_locked() reports GenericError. Still, for block-job-change
>>>    errors are not documented at all, so be silent in deprecated.txt as
>>>    well.
>>>
>>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>

Suggest:

    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.