[PATCH] [for-10.1] qapi/block-core: derpecate some block-job- APIs
For change, pause, resume, complete, dismiss and finalize actions corresponding job- and block-job commands are almost equal. The difference 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. So, lets document this difference in deprecated.txt. Still, for dismiss and finalize errors are not documented at all, so be silent in deprecated.txt as well. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> --- Hi all! That's a continuation of my "[RFC 00/15] block job API"[1], exactly, the simplest part of it - deprecating block-job-* commands which simply duplicate job-* ones. Note that the old series was started with trying to introduce job-change command as substitution to both block-job-change (which only can change mirror copy-mode parameter), and block-job-set-speed. It was rather complicated and controversial attempt, which latest implemenation was "[PATCH v3 0/7] introduce job-change qmp command"[2]. In [2] Kevin noted, that we'd better follow existing blockdev-reopen approach of changing options (i.e. specify all options) than introduce a new one. But, on the other hand, now I'm afraid, that rewriting in third tools simple call to (old good) block-job-set-speed into job-change(_all_options_ + changed speed) is too much work just for "having nice interface". And too much for the only two options we want to change. So, let's just start from something more obvious. Finally, we can simple keep block-job-set-speed and block-job-change as is, as they (unlike other block-job-* commands) are not duplicated by similar job-* commands. [1] https://patchew.org/QEMU/20240313150907.623462-1-vsementsov@yandex-team.ru/ [2] https://patchew.org/QEMU/20241002140616.561652-1-vsementsov@yandex-team.ru/ docs/about/deprecated.rst | 31 +++++++++++++++++++++++++++++++ qapi/block-core.json | 30 ++++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+) diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst index e2b4f077d4..eed3356359 100644 --- a/docs/about/deprecated.rst +++ b/docs/about/deprecated.rst @@ -148,6 +148,37 @@ options are removed in favor of using explicit ``blockdev-create`` and ``blockdev-add`` calls. See :doc:`/interop/live-block-operations` for details. +``block-job-pause`` (since 10.1) +''''''''''''''''''''''''''''''' + +Use ``job-pause`` instead. The only difference is that ``job-pause`` +always reports GenericError on failure when ``block-job-pause`` reports +DeviceNotActive when block-job is not found. + +``block-job-resume`` (since 10.1) +'''''''''''''''''''''''''''''''' + +Use ``job-resume`` instead. The only difference is that ``job-resume`` +always reports GenericError on failure when ``block-job-resume`` reports +DeviceNotActive when block-job is not found. + +``block-job-complete`` (since 10.1) +'''''''''''''''''''''''''''''''''' + +Use ``job-complete`` instead. The only difference is that ``job-complete`` +always reports GenericError on failure when ``block-job-complete`` reports +DeviceNotActive when block-job is not found. + +``block-job-dismiss`` (since 10.1) +''''''''''''''''''''''''''''''''' + +Use ``job-dismiss`` instead. + +``block-job-finalize`` (since 10.1) +'''''''''''''''''''''''''''''''''' + +Use ``job-finalize`` instead. + ``query-migrationthreads`` (since 9.2) '''''''''''''''''''''''''''''''''''''' diff --git a/qapi/block-core.json b/qapi/block-core.json index b1937780e1..264be8413b 100644 --- a/qapi/block-core.json +++ b/qapi/block-core.json @@ -2968,6 +2968,11 @@ # the name of the parameter), but since QEMU 2.7 it can have other # values. # +# Features: +# +# @deprecated: This command is deprecated. Use @job-pause +# instead. +# # Errors: # - If no background operation is active on this device, # DeviceNotActive @@ -2975,6 +2980,7 @@ # Since: 1.3 ## { 'command': 'block-job-pause', 'data': { 'device': 'str' }, + 'features': ['deprecated'], 'allow-preconfig': true } ## @@ -2992,6 +2998,11 @@ # the name of the parameter), but since QEMU 2.7 it can have other # values. # +# Features: +# +# @deprecated: This command is deprecated. Use @job-resume +# instead. +# # Errors: # - If no background operation is active on this device, # DeviceNotActive @@ -2999,6 +3010,7 @@ # Since: 1.3 ## { 'command': 'block-job-resume', 'data': { 'device': 'str' }, + 'features': ['deprecated'], 'allow-preconfig': true } ## @@ -3023,6 +3035,11 @@ # the name of the parameter), but since QEMU 2.7 it can have other # values. # +# Features: +# +# @deprecated: This command is deprecated. Use @job-complete +# instead. +# # Errors: # - If no background operation is active on this device, # DeviceNotActive @@ -3030,6 +3047,7 @@ # Since: 1.3 ## { 'command': 'block-job-complete', 'data': { 'device': 'str' }, + 'features': ['deprecated'], 'allow-preconfig': true } ## @@ -3047,9 +3065,15 @@ # # @id: The job identifier. # +# Features: +# +# @deprecated: This command is deprecated. Use @job-dismiss +# instead. +# # Since: 2.12 ## { 'command': 'block-job-dismiss', 'data': { 'id': 'str' }, + 'features': ['deprecated'], 'allow-preconfig': true } ## @@ -3064,9 +3088,15 @@ # # @id: The job identifier. # +# Features: +# +# @deprecated: This command is deprecated. Use @job-finalize +# instead. +# # Since: 2.12 ## { 'command': 'block-job-finalize', 'data': { 'id': 'str' }, + 'features': ['deprecated'], 'allow-preconfig': true } ## -- 2.48.1
On Tue, Apr 01, 2025 at 18:57:30 +0300, Vladimir Sementsov-Ogievskiy wrote:
For change, pause, resume, complete, dismiss and finalize actions corresponding job- and block-job commands are almost equal. The difference 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. So, lets document this difference in deprecated.txt. Still, for dismiss and finalize errors are not documented at all, so be silent in deprecated.txt as well.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> ---
Hi all!
That's a continuation of my "[RFC 00/15] block job API"[1], exactly, the simplest part of it - deprecating block-job-* commands which simply duplicate job-* ones.
Note that the old series was started with trying to introduce job-change command as substitution to both block-job-change (which only can change mirror copy-mode parameter), and block-job-set-speed. It was rather complicated and controversial attempt, which latest implemenation was "[PATCH v3 0/7] introduce job-change qmp command"[2].
In [2] Kevin noted, that we'd better follow existing blockdev-reopen approach of changing options (i.e. specify all options) than introduce a new one. But, on the other hand, now I'm afraid, that rewriting in third tools simple call to (old good) block-job-set-speed into job-change(_all_options_ + changed speed) is too much work just for "having nice interface". And too much for the only two options we want to change.
So, let's just start from something more obvious. Finally, we can simple keep block-job-set-speed and block-job-change as is, as they (unlike other block-job-* commands) are not duplicated by similar job-* commands.
[1] https://patchew.org/QEMU/20240313150907.623462-1-vsementsov@yandex-team.ru/ [2] https://patchew.org/QEMU/20241002140616.561652-1-vsementsov@yandex-team.ru/
docs/about/deprecated.rst | 31 +++++++++++++++++++++++++++++++ qapi/block-core.json | 30 ++++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+)
diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst index e2b4f077d4..eed3356359 100644 --- a/docs/about/deprecated.rst +++ b/docs/about/deprecated.rst @@ -148,6 +148,37 @@ options are removed in favor of using explicit ``blockdev-create`` and ``blockdev-add`` calls. See :doc:`/interop/live-block-operations` for details.
+``block-job-pause`` (since 10.1) +''''''''''''''''''''''''''''''' + +Use ``job-pause`` instead. The only difference is that ``job-pause`` +always reports GenericError on failure when ``block-job-pause`` reports +DeviceNotActive when block-job is not found. + +``block-job-resume`` (since 10.1) +'''''''''''''''''''''''''''''''' + +Use ``job-resume`` instead. The only difference is that ``job-resume`` +always reports GenericError on failure when ``block-job-resume`` reports +DeviceNotActive when block-job is not found. + +``block-job-complete`` (since 10.1) +'''''''''''''''''''''''''''''''''' + +Use ``job-complete`` instead. The only difference is that ``job-complete`` +always reports GenericError on failure when ``block-job-complete`` reports +DeviceNotActive when block-job is not found. + +``block-job-dismiss`` (since 10.1) +''''''''''''''''''''''''''''''''' + +Use ``job-dismiss`` instead. + +``block-job-finalize`` (since 10.1) +'''''''''''''''''''''''''''''''''' + +Use ``job-finalize`` instead. +
Libvirt uses only the following two with 'block-job-' prefix: src/qemu/qemu_monitor_json.c: if (!(cmd = qemuMonitorJSONMakeCommand("block-job-cancel", src/qemu/qemu_monitor_json.c: if (!(cmd = qemuMonitorJSONMakeCommand("block-job-set-speed", thus on behalf of libvirt: ACKed-by: Peter Krempa <pkrempa@redhat.com>
Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> writes:
For change, pause, resume, complete, dismiss and finalize actions corresponding job- and block-job commands are almost equal. The difference 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. So, lets document this difference in deprecated.txt. Still, for dismiss and finalize errors are not documented at all, so be silent in deprecated.txt as well.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> ---
Hi all!
That's a continuation of my "[RFC 00/15] block job API"[1], exactly, the simplest part of it - deprecating block-job-* commands which simply duplicate job-* ones.
Note that the old series was started with trying to introduce job-change command as substitution to both block-job-change (which only can change mirror copy-mode parameter), and block-job-set-speed. It was rather complicated and controversial attempt, which latest implemenation was "[PATCH v3 0/7] introduce job-change qmp command"[2].
In [2] Kevin noted, that we'd better follow existing blockdev-reopen approach of changing options (i.e. specify all options) than introduce a new one. But, on the other hand, now I'm afraid, that rewriting in third tools simple call to (old good) block-job-set-speed into job-change(_all_options_ + changed speed) is too much work just for "having nice interface". And too much for the only two options we want to change.
So, let's just start from something more obvious. Finally, we can simple keep block-job-set-speed and block-job-change as is, as they (unlike other block-job-* commands) are not duplicated by similar job-* commands.
[1] https://patchew.org/QEMU/20240313150907.623462-1-vsementsov@yandex-team.ru/ [2] https://patchew.org/QEMU/20241002140616.561652-1-vsementsov@yandex-team.ru/
Thank you for your efforts at making the interfaces cleaner, simpler, and less redundant.
docs/about/deprecated.rst | 31 +++++++++++++++++++++++++++++++ qapi/block-core.json | 30 ++++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+)
diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst index e2b4f077d4..eed3356359 100644 --- a/docs/about/deprecated.rst +++ b/docs/about/deprecated.rst @@ -148,6 +148,37 @@ options are removed in favor of using explicit ``blockdev-create`` and ``blockdev-add`` calls. See :doc:`/interop/live-block-operations` for details.
+``block-job-pause`` (since 10.1) +''''''''''''''''''''''''''''''' + +Use ``job-pause`` instead. The only difference is that ``job-pause`` +always reports GenericError on failure when ``block-job-pause`` reports +DeviceNotActive when block-job is not found.
block-job-pause's doc comment: # The operation will pause as soon as possible. No event is emitted # when the operation is actually paused. [...] job-pause's: # The job will pause as soon as possible, which means transitioning # into the PAUSED state if it was RUNNING, or into STANDBY if it was # READY. The corresponding JOB_STATUS_CHANGE event will be emitted. Either one of them is incorrect about event emission, or there is another difference.
+ +``block-job-resume`` (since 10.1) +'''''''''''''''''''''''''''''''' + +Use ``job-resume`` instead. The only difference is that ``job-resume`` +always reports GenericError on failure when ``block-job-resume`` reports +DeviceNotActive when block-job is not found.
block-job-resume's doc comment: # This command also clears the error status of the job. Nothing like that job-resume's. Either one of them is incorrect / incomplete about the error status clearance, or there is another difference.
+ +``block-job-complete`` (since 10.1) +'''''''''''''''''''''''''''''''''' + +Use ``job-complete`` instead. The only difference is that ``job-complete`` +always reports GenericError on failure when ``block-job-complete`` reports +DeviceNotActive when block-job is not found.
block-job-complete's doc comment: # Manually trigger completion of an active background block operation. # This is supported for drive mirroring, where it also switches the # device to write to the target path only. The ability to complete is # signaled with a BLOCK_JOB_READY event. # # This command completes an active background block operation # synchronously. The ordering of this command's return with the # BLOCK_JOB_COMPLETED event is not defined. Note that if an I/O error # occurs during the processing of this command: 1) the command itself # will fail; 2) the error will be processed according to the # rerror/werror arguments that were specified when starting the # operation. job-complete's: # Manually trigger completion of an active job in the READY state. Is the latter lacking useful information?
+ +``block-job-dismiss`` (since 10.1) +''''''''''''''''''''''''''''''''' + +Use ``job-dismiss`` instead.
block-job-dismiss's doc comment: # For jobs that have already concluded, remove them from the # block-job-query list. This command only needs to be run for jobs # which were started with QEMU 2.12+ job lifetime management # semantics. I figure "started with QEMU 2.12+ job lifetime management semantics" is an awkward way to say "auto-dismiss is false". job-dismiss's doc comment: # Deletes a job that is in the CONCLUDED state. This command only # needs to be run explicitly for jobs that don't have automatic # dismiss enabled. @auto-dismiss defaults to true for jobs that support controlling it: drive-backup, blockdev-backup, block-commit, drive-mirror, blockdev-mirror, block-stream. I guess these are exactly the commands that already existed when we added "QEMU 2.12+ lifetime management". I figure it's always false for the other jobs, but that doesn't seem to be documented anywhere.
+ +``block-job-finalize`` (since 10.1) +'''''''''''''''''''''''''''''''''' + +Use ``job-finalize`` instead. +
block-job-finalize's doc comment: # Once a job that has manual=true reaches the pending state, it can be # instructed to finalize any graph changes and do any necessary # cleanup via this command. [...] There is no member @manual anywhere in the QAPI schema. I figure this should be @auto-finalize. job-finalize's doc comment: # Instructs all jobs in a transaction (or a single job if it is not # part of any transaction) to finalize any graph changes and do any # necessary cleanup. This command requires that all involved jobs are # in the PENDING state. Nothing on @auto-finalize. @auto-finalize defaults to true for jobs that support controlling it. These are exactly the ones that support @auto-dismiss. I figure @auto-dismiss is always false for the other jobs, but that doesn't seem to be documented anywhere. The only other bits related to @auto-dismiss and @auto-finalize seem to be two states in JobStatus: # @pending: The job has finished its work, but has finalization steps # that it needs to make prior to completing. These changes will # require manual intervention via @job-finalize if auto-finalize # was set to false. These pending changes may still fail. [...] # @concluded: The job has finished all work. If auto-dismiss was set # to false, the job will remain in the query list until it is # dismissed via @job-dismiss. Is it possible to observe @concluded via QMP when @auto-dismiss is on? What about @pending? Aside: getting rid of auto-dismiss and auto-finalize some day would be nice.
``query-migrationthreads`` (since 9.2) ''''''''''''''''''''''''''''''''''''''
diff --git a/qapi/block-core.json b/qapi/block-core.json index b1937780e1..264be8413b 100644 --- a/qapi/block-core.json +++ b/qapi/block-core.json @@ -2968,6 +2968,11 @@ # the name of the parameter), but since QEMU 2.7 it can have other # values. # +# Features: +# +# @deprecated: This command is deprecated. Use @job-pause +# instead. +# # Errors: # - If no background operation is active on this device, # DeviceNotActive @@ -2975,6 +2980,7 @@ # Since: 1.3 ## { 'command': 'block-job-pause', 'data': { 'device': 'str' }, + 'features': ['deprecated'], 'allow-preconfig': true }
## @@ -2992,6 +2998,11 @@ # the name of the parameter), but since QEMU 2.7 it can have other # values. # +# Features: +# +# @deprecated: This command is deprecated. Use @job-resume +# instead. +# # Errors: # - If no background operation is active on this device, # DeviceNotActive @@ -2999,6 +3010,7 @@ # Since: 1.3 ## { 'command': 'block-job-resume', 'data': { 'device': 'str' }, + 'features': ['deprecated'], 'allow-preconfig': true }
## @@ -3023,6 +3035,11 @@ # the name of the parameter), but since QEMU 2.7 it can have other # values. # +# Features: +# +# @deprecated: This command is deprecated. Use @job-complete +# instead. +# # Errors: # - If no background operation is active on this device, # DeviceNotActive @@ -3030,6 +3047,7 @@ # Since: 1.3 ## { 'command': 'block-job-complete', 'data': { 'device': 'str' }, + 'features': ['deprecated'], 'allow-preconfig': true }
## @@ -3047,9 +3065,15 @@ # # @id: The job identifier. # +# Features: +# +# @deprecated: This command is deprecated. Use @job-dismiss +# instead. +# # Since: 2.12 ## { 'command': 'block-job-dismiss', 'data': { 'id': 'str' }, + 'features': ['deprecated'], 'allow-preconfig': true }
## @@ -3064,9 +3088,15 @@ # # @id: The job identifier. # +# Features: +# +# @deprecated: This command is deprecated. Use @job-finalize +# instead. +# # Since: 2.12 ## { 'command': 'block-job-finalize', 'data': { 'id': 'str' }, + 'features': ['deprecated'], 'allow-preconfig': true }
##
On 04.04.25 09:20, Markus Armbruster wrote:
Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> writes:
For change, pause, resume, complete, dismiss and finalize actions corresponding job- and block-job commands are almost equal. The difference 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. So, lets document this difference in deprecated.txt. Still, for dismiss and finalize errors are not documented at all, so be silent in deprecated.txt as well.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> ---
Hi all!
That's a continuation of my "[RFC 00/15] block job API"[1], exactly, the simplest part of it - deprecating block-job-* commands which simply duplicate job-* ones.
Note that the old series was started with trying to introduce job-change command as substitution to both block-job-change (which only can change mirror copy-mode parameter), and block-job-set-speed. It was rather complicated and controversial attempt, which latest implemenation was "[PATCH v3 0/7] introduce job-change qmp command"[2].
In [2] Kevin noted, that we'd better follow existing blockdev-reopen approach of changing options (i.e. specify all options) than introduce a new one. But, on the other hand, now I'm afraid, that rewriting in third tools simple call to (old good) block-job-set-speed into job-change(_all_options_ + changed speed) is too much work just for "having nice interface". And too much for the only two options we want to change.
So, let's just start from something more obvious. Finally, we can simple keep block-job-set-speed and block-job-change as is, as they (unlike other block-job-* commands) are not duplicated by similar job-* commands.
[1] https://patchew.org/QEMU/20240313150907.623462-1-vsementsov@yandex-team.ru/ [2] https://patchew.org/QEMU/20241002140616.561652-1-vsementsov@yandex-team.ru/
Thank you for your efforts at making the interfaces cleaner, simpler, and less redundant.
docs/about/deprecated.rst | 31 +++++++++++++++++++++++++++++++ qapi/block-core.json | 30 ++++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+)
diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst index e2b4f077d4..eed3356359 100644 --- a/docs/about/deprecated.rst +++ b/docs/about/deprecated.rst @@ -148,6 +148,37 @@ options are removed in favor of using explicit ``blockdev-create`` and ``blockdev-add`` calls. See :doc:`/interop/live-block-operations` for details.
+``block-job-pause`` (since 10.1) +''''''''''''''''''''''''''''''' + +Use ``job-pause`` instead. The only difference is that ``job-pause`` +always reports GenericError on failure when ``block-job-pause`` reports +DeviceNotActive when block-job is not found.
block-job-pause's doc comment:
# The operation will pause as soon as possible. No event is emitted # when the operation is actually paused. [...]
job-pause's:
# The job will pause as soon as possible, which means transitioning # into the PAUSED state if it was RUNNING, or into STANDBY if it was # READY. The corresponding JOB_STATUS_CHANGE event will be emitted.
Either one of them is incorrect about event emission, or there is another difference.
First is incorrect. Both commands simply call job_user_pause_locked() function. I think, that block-job-pause documentation is older than JOB_STATUS_CHANGE event.
+ +``block-job-resume`` (since 10.1) +'''''''''''''''''''''''''''''''' + +Use ``job-resume`` instead. The only difference is that ``job-resume`` +always reports GenericError on failure when ``block-job-resume`` reports +DeviceNotActive when block-job is not found.
block-job-resume's doc comment:
# This command also clears the error status of the job.
Nothing like that job-resume's. Either one of them is incorrect / incomplete about the error status clearance, or there is another difference.
Comment is correct.. But it applies only to block-jobs. I can add it to job-resume, like # This command also clears the error status of block jobs `
+ +``block-job-complete`` (since 10.1) +'''''''''''''''''''''''''''''''''' + +Use ``job-complete`` instead. The only difference is that ``job-complete`` +always reports GenericError on failure when ``block-job-complete`` reports +DeviceNotActive when block-job is not found.
block-job-complete's doc comment:
# Manually trigger completion of an active background block operation. # This is supported for drive mirroring, where it also switches the # device to write to the target path only. The ability to complete is # signaled with a BLOCK_JOB_READY event. # # This command completes an active background block operation # synchronously. The ordering of this command's return with the # BLOCK_JOB_COMPLETED event is not defined. Note that if an I/O error # occurs during the processing of this command: 1) the command itself # will fail; 2) the error will be processed according to the # rerror/werror arguments that were specified when starting the # operation.
job-complete's:
# Manually trigger completion of an active job in the READY state.
Is the latter lacking useful information?
Actually, only mirror job supports this command. So yes, we can copy the comment to job-complete as well.
+ +``block-job-dismiss`` (since 10.1) +''''''''''''''''''''''''''''''''' + +Use ``job-dismiss`` instead.
block-job-dismiss's doc comment:
# For jobs that have already concluded, remove them from the # block-job-query list. This command only needs to be run for jobs # which were started with QEMU 2.12+ job lifetime management # semantics.
I figure "started with QEMU 2.12+ job lifetime management semantics" is an awkward way to say "auto-dismiss is false".
job-dismiss's doc comment:
# Deletes a job that is in the CONCLUDED state. This command only # needs to be run explicitly for jobs that don't have automatic # dismiss enabled.
@auto-dismiss defaults to true for jobs that support controlling it: drive-backup, blockdev-backup, block-commit, drive-mirror, blockdev-mirror, block-stream. I guess these are exactly the commands that already existed when we added "QEMU 2.12+ lifetime management".
I figure it's always false for the other jobs, but that doesn't seem to be documented anywhere.
Yes, relying on 2.12 version in modern documentation is awkward. I'll touch this up somehow.
+ +``block-job-finalize`` (since 10.1) +'''''''''''''''''''''''''''''''''' + +Use ``job-finalize`` instead. +
block-job-finalize's doc comment:
# Once a job that has manual=true reaches the pending state, it can be # instructed to finalize any graph changes and do any necessary # cleanup via this command. [...]
There is no member @manual anywhere in the QAPI schema. I figure this should be @auto-finalize.
job-finalize's doc comment:
# Instructs all jobs in a transaction (or a single job if it is not # part of any transaction) to finalize any graph changes and do any # necessary cleanup. This command requires that all involved jobs are # in the PENDING state.
Nothing on @auto-finalize.
@auto-finalize defaults to true for jobs that support controlling it. These are exactly the ones that support @auto-dismiss.
I figure @auto-dismiss is always false for the other jobs, but that doesn't seem to be documented anywhere.
The only other bits related to @auto-dismiss and @auto-finalize seem to be two states in JobStatus:
# @pending: The job has finished its work, but has finalization steps # that it needs to make prior to completing. These changes will # require manual intervention via @job-finalize if auto-finalize # was set to false. These pending changes may still fail. [...] # @concluded: The job has finished all work. If auto-dismiss was set # to false, the job will remain in the query list until it is # dismissed via @job-dismiss.
Is it possible to observe @concluded via QMP when @auto-dismiss is on?
Seems not.
What about @pending?
Hmm probably, if we have a transaction of several jobs (actually only backups may be joined to transactions), where some have auto-finalize and some not, the whole transaction would be pending, including jobs that has auto-finalize=true. Still, it's a strange case.
Aside: getting rid of auto-dismiss and auto-finalize some day would be nice.
Hmm.. You mean, deprecated "true" value, and finally drop the fields, making "false" the default? May be. I'll resend, with additional patch to touch-up the documentation. -- Best regards, Vladimir
Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> writes:
On 04.04.25 09:20, Markus Armbruster wrote:
Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> writes:
[...]
+ +``block-job-finalize`` (since 10.1) +'''''''''''''''''''''''''''''''''' + +Use ``job-finalize`` instead. +
block-job-finalize's doc comment:
# Once a job that has manual=true reaches the pending state, it can be # instructed to finalize any graph changes and do any necessary # cleanup via this command. [...]
There is no member @manual anywhere in the QAPI schema. I figure this should be @auto-finalize.
job-finalize's doc comment:
# Instructs all jobs in a transaction (or a single job if it is not # part of any transaction) to finalize any graph changes and do any # necessary cleanup. This command requires that all involved jobs are # in the PENDING state.
Nothing on @auto-finalize.
@auto-finalize defaults to true for jobs that support controlling it. These are exactly the ones that support @auto-dismiss.
I figure @auto-dismiss is always false for the other jobs, but that doesn't seem to be documented anywhere.
The only other bits related to @auto-dismiss and @auto-finalize seem to be two states in JobStatus:
# @pending: The job has finished its work, but has finalization steps # that it needs to make prior to completing. These changes will # require manual intervention via @job-finalize if auto-finalize # was set to false. These pending changes may still fail. [...] # @concluded: The job has finished all work. If auto-dismiss was set # to false, the job will remain in the query list until it is # dismissed via @job-dismiss.
Is it possible to observe @concluded via QMP when @auto-dismiss is on?
Seems not.
What about @pending?
Hmm probably, if we have a transaction of several jobs (actually only backups may be joined to transactions), where some have auto-finalize and some not, the whole transaction would be pending, including jobs that has auto-finalize=true. Still, it's a strange case.
So, auto-finalize=true is silently ignored when another job in the same transaction has auto-finalize=false?
Aside: getting rid of auto-dismiss and auto-finalize some day would be nice.
Hmm.. You mean, deprecated "true" value, and finally drop the fields, making "false" the default? May be.
May or may not be practical.
I'll resend, with additional patch to touch-up the documentation.
Thanks!
On 04.04.25 17:13, Markus Armbruster wrote:
Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> writes:
On 04.04.25 09:20, Markus Armbruster wrote:
Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> writes:
[...]
+ +``block-job-finalize`` (since 10.1) +'''''''''''''''''''''''''''''''''' + +Use ``job-finalize`` instead. +
block-job-finalize's doc comment:
# Once a job that has manual=true reaches the pending state, it can be # instructed to finalize any graph changes and do any necessary # cleanup via this command. [...]
There is no member @manual anywhere in the QAPI schema. I figure this should be @auto-finalize.
job-finalize's doc comment:
# Instructs all jobs in a transaction (or a single job if it is not # part of any transaction) to finalize any graph changes and do any # necessary cleanup. This command requires that all involved jobs are # in the PENDING state.
Nothing on @auto-finalize.
@auto-finalize defaults to true for jobs that support controlling it. These are exactly the ones that support @auto-dismiss.
I figure @auto-dismiss is always false for the other jobs, but that doesn't seem to be documented anywhere.
The only other bits related to @auto-dismiss and @auto-finalize seem to be two states in JobStatus:
# @pending: The job has finished its work, but has finalization steps # that it needs to make prior to completing. These changes will # require manual intervention via @job-finalize if auto-finalize # was set to false. These pending changes may still fail. [...] # @concluded: The job has finished all work. If auto-dismiss was set # to false, the job will remain in the query list until it is # dismissed via @job-dismiss.
Is it possible to observe @concluded via QMP when @auto-dismiss is on?
Seems not.
What about @pending?
Hmm probably, if we have a transaction of several jobs (actually only backups may be joined to transactions), where some have auto-finalize and some not, the whole transaction would be pending, including jobs that has auto-finalize=true. Still, it's a strange case.
So, auto-finalize=true is silently ignored when another job in the same transaction has auto-finalize=false?
Yes, at least, it looks like so: static void job_completed_txn_success_locked(Job *job) { [...] /* If no jobs need manual finalization, automatically do so */ if (job_txn_apply_locked(job, job_needs_finalize_locked) == 0) { job_do_finalize_locked(job); } }
Aside: getting rid of auto-dismiss and auto-finalize some day would be nice.
Hmm.. You mean, deprecated "true" value, and finally drop the fields, making "false" the default? May be.
May or may not be practical.
I'll resend, with additional patch to touch-up the documentation.
Thanks!
-- Best regards, Vladimir
Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> writes:
On 04.04.25 17:13, Markus Armbruster wrote:
[...]
So, auto-finalize=true is silently ignored when another job in the same transaction has auto-finalize=false?
Yes, at least, it looks like so:
static void job_completed_txn_success_locked(Job *job) {
[...]
/* If no jobs need manual finalization, automatically do so */ if (job_txn_apply_locked(job, job_needs_finalize_locked) == 0) { job_do_finalize_locked(job); } }
Silently ignoring what the user specified is not okay whether the user's instructions make sense or not. Fixing this UI bug would break usage that relies on it. Should we care? [...]
participants (3)
-
Markus Armbruster -
Peter Krempa -
Vladimir Sementsov-Ogievskiy