[PATCH v2 1/2] qapi: synchronize jobs and block-jobs documentation

Vladimir Sementsov-Ogievskiy posted 2 patches 8 months, 2 weeks ago
Maintainers: Eric Blake <eblake@redhat.com>, Markus Armbruster <armbru@redhat.com>, Kevin Wolf <kwolf@redhat.com>, Hanna Reitz <hreitz@redhat.com>, John Snow <jsnow@redhat.com>, Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
There is a newer version of this series
[PATCH v2 1/2] qapi: synchronize jobs and block-jobs documentation
Posted by Vladimir Sementsov-Ogievskiy 8 months, 2 weeks ago
Actualize documentation and synchronize it for commands which actually
call the same functions internally.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
 qapi/block-core.json | 59 +++++++++++++++++++++++++-------------------
 qapi/job.json        | 29 ++++++++++++++++++++--
 2 files changed, 61 insertions(+), 27 deletions(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index b1937780e1..d74a1f8b8b 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -2956,13 +2956,14 @@
 #
 # Pause an active background block operation.
 #
-# This command returns immediately after marking the active background
-# block operation for pausing.  It is an error to call this command if
-# no operation is in progress or if the job is already paused.
+# This command returns immediately after marking the active job for
+# pausing.  Pausing an already paused job is an error.
+#
+# 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.
 #
-# The operation will pause as soon as possible.  No event is emitted
-# when the operation is actually paused.  Cancelling a paused job
-# automatically resumes it.
+# Cancelling a paused job automatically resumes it.
 #
 # @device: The job identifier.  This used to be a device name (hence
 #     the name of the parameter), but since QEMU 2.7 it can have other
@@ -2982,9 +2983,8 @@
 #
 # Resume an active background block operation.
 #
-# This command returns immediately after resuming a paused background
-# block operation.  It is an error to call this command if no
-# operation is in progress or if the job is not paused.
+# This command returns immediately after resuming a paused job.
+# Resuming an already running job is an error.
 #
 # This command also clears the error status of the job.
 #
@@ -3004,10 +3004,14 @@
 ##
 # @block-job-complete:
 #
-# 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.
+# Manually trigger completion of an active job in the READY or STANDBY
+# state.  Completing the job in any other state is an error.
+#
+# This is supported only for drive mirroring (which includes
+# drive-mirror, blockdev-mirror and block-commit job (only in case of
+# "active commit", when the node being commited is used by the guest)),
+# 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
@@ -3017,8 +3021,6 @@
 # rerror/werror arguments that were specified when starting the
 # operation.
 #
-# A cancelled or paused job cannot be completed.
-#
 # @device: The job identifier.  This used to be a device name (hence
 #     the name of the parameter), but since QEMU 2.7 it can have other
 #     values.
@@ -3035,10 +3037,12 @@
 ##
 # @block-job-dismiss:
 #
-# 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.
+# 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. In turn, automatic dismiss may be enabled only
+# for jobs that have @auto-dismiss option, which are drive-backup,
+# blockdev-backup, drive-mirror, blockdev-mirror, block-commit and
+# block-stream.
 #
 # This command will refuse to operate on any job that has not yet
 # reached its terminal state, JOB_STATUS_CONCLUDED.  For jobs that
@@ -3055,12 +3059,17 @@
 ##
 # @block-job-finalize:
 #
-# 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.  For jobs in a transaction, instructing
-# one job to finalize will force ALL jobs in the transaction to
-# finalize, so it is only necessary to instruct a single member job to
-# finalize.
+# 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.
+#
+# For jobs in a transaction, instructing one job to finalize will
+# force ALL jobs in the transaction to finalize, so it is only
+# necessary to instruct a single member job to finalize.
+#
+# The command is applicable only to jobs which have @auto-finalize option
+# and only when this option is set to false.
 #
 # @id: The job identifier.
 #
diff --git a/qapi/job.json b/qapi/job.json
index cfc3beedd2..c8736f2a05 100644
--- a/qapi/job.json
+++ b/qapi/job.json
@@ -156,6 +156,9 @@
 # This command returns immediately after resuming a paused job.
 # Resuming an already running job is an error.
 #
+# This command also clears the error status for block-jobs (stream,
+# commit, mirror, backup).
+#
 # @id: The job identifier.
 #
 # Since: 3.0
@@ -184,7 +187,22 @@
 ##
 # @job-complete:
 #
-# Manually trigger completion of an active job in the READY state.
+# Manually trigger completion of an active job in the READY or STANDBY
+# state.  Completing the job in any other state is an error.
+#
+# This is supported only for drive mirroring (which includes
+# drive-mirror, blockdev-mirror and block-commit job (only in case of
+# "active commit", when the node being commited is used by the guest)),
+# 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.
 #
 # @id: The job identifier.
 #
@@ -197,7 +215,11 @@
 #
 # 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.
+# dismiss enabled. In turn, automatic dismiss may be enabled only
+# for jobs that have @auto-dismiss option, which are drive-backup,
+# blockdev-backup, drive-mirror, blockdev-mirror, block-commit and
+# block-stream. And historically it's enabled by default for these
+# jobs.
 #
 # This command will refuse to operate on any job that has not yet
 # reached its terminal state, JOB_STATUS_CONCLUDED.  For jobs that
@@ -222,6 +244,9 @@
 # force ALL jobs in the transaction to finalize, so it is only
 # necessary to instruct a single member job to finalize.
 #
+# The command is applicable only to jobs which have @auto-finalize option
+# and only when this option is set to false.
+#
 # @id: The identifier of any job in the transaction, or of a job that
 #     is not part of any transaction.
 #
-- 
2.48.1
Re: [PATCH v2 1/2] qapi: synchronize jobs and block-jobs documentation
Posted by Markus Armbruster via Devel 8 months, 2 weeks ago
Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> writes:

> Actualize documentation and synchronize it for commands which actually
> call the same functions internally.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> ---
>  qapi/block-core.json | 59 +++++++++++++++++++++++++-------------------
>  qapi/job.json        | 29 ++++++++++++++++++++--
>  2 files changed, 61 insertions(+), 27 deletions(-)
>
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index b1937780e1..d74a1f8b8b 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -2956,13 +2956,14 @@
>  #
>  # Pause an active background block operation.
>  #
> -# This command returns immediately after marking the active background
> -# block operation for pausing.  It is an error to call this command if
> -# no operation is in progress or if the job is already paused.
> +# This command returns immediately after marking the active job for
> +# pausing.  Pausing an already paused job is an error.
> +#
> +# 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.
>  #
> -# The operation will pause as soon as possible.  No event is emitted
> -# when the operation is actually paused.  Cancelling a paused job
> -# automatically resumes it.
> +# Cancelling a paused job automatically resumes it.
>  #
>  # @device: The job identifier.  This used to be a device name (hence
>  #     the name of the parameter), but since QEMU 2.7 it can have other
> @@ -2982,9 +2983,8 @@
>  #
>  # Resume an active background block operation.
>  #
> -# This command returns immediately after resuming a paused background
> -# block operation.  It is an error to call this command if no
> -# operation is in progress or if the job is not paused.
> +# This command returns immediately after resuming a paused job.
> +# Resuming an already running job is an error.
>  #
>  # This command also clears the error status of the job.
>  #
> @@ -3004,10 +3004,14 @@
>  ##
>  # @block-job-complete:
>  #
> -# 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.
> +# Manually trigger completion of an active job in the READY or STANDBY
> +# state.  Completing the job in any other state is an error.
> +#
> +# This is supported only for drive mirroring (which includes
> +# drive-mirror, blockdev-mirror and block-commit job (only in case of
> +# "active commit", when the node being commited is used by the guest)),

I agree with Eric: needs a rephrasing to avoid the nested parenthesis.

> +# 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
> @@ -3017,8 +3021,6 @@
>  # rerror/werror arguments that were specified when starting the
>  # operation.
>  #
> -# A cancelled or paused job cannot be completed.
> -#
>  # @device: The job identifier.  This used to be a device name (hence
>  #     the name of the parameter), but since QEMU 2.7 it can have other
>  #     values.
> @@ -3035,10 +3037,12 @@
>  ##
>  # @block-job-dismiss:
>  #
> -# 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.
> +# 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. In turn, automatic dismiss may be enabled only
> +# for jobs that have @auto-dismiss option, which are drive-backup,
> +# blockdev-backup, drive-mirror, blockdev-mirror, block-commit and
> +# block-stream.
>  #
>  # This command will refuse to operate on any job that has not yet
>  # reached its terminal state, JOB_STATUS_CONCLUDED.  For jobs that
> @@ -3055,12 +3059,17 @@
>  ##
>  # @block-job-finalize:
>  #
> -# 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.  For jobs in a transaction, instructing
> -# one job to finalize will force ALL jobs in the transaction to
> -# finalize, so it is only necessary to instruct a single member job to
> -# finalize.
> +# 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.
> +#
> +# For jobs in a transaction, instructing one job to finalize will
> +# force ALL jobs in the transaction to finalize, so it is only
> +# necessary to instruct a single member job to finalize.
> +#
> +# The command is applicable only to jobs which have @auto-finalize option
> +# and only when this option is set to false.
>  #
>  # @id: The job identifier.
>  #
> diff --git a/qapi/job.json b/qapi/job.json
> index cfc3beedd2..c8736f2a05 100644
> --- a/qapi/job.json
> +++ b/qapi/job.json
> @@ -156,6 +156,9 @@
>  # This command returns immediately after resuming a paused job.
>  # Resuming an already running job is an error.
>  #
> +# This command also clears the error status for block-jobs (stream,
> +# commit, mirror, backup).
> +#
>  # @id: The job identifier.
>  #
>  # Since: 3.0
> @@ -184,7 +187,22 @@
>  ##
>  # @job-complete:
>  #
> -# Manually trigger completion of an active job in the READY state.
> +# Manually trigger completion of an active job in the READY or STANDBY
> +# state.  Completing the job in any other state is an error.
> +#
> +# This is supported only for drive mirroring (which includes
> +# drive-mirror, blockdev-mirror and block-commit job (only in case of
> +# "active commit", when the node being commited is used by the guest)),
> +# 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.
>  #
>  # @id: The job identifier.
>  #
> @@ -197,7 +215,11 @@
>  #
>  # 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.
> +# dismiss enabled. In turn, automatic dismiss may be enabled only
> +# for jobs that have @auto-dismiss option, which are drive-backup,
> +# blockdev-backup, drive-mirror, blockdev-mirror, block-commit and
> +# block-stream. And historically it's enabled by default for these
> +# jobs.

Scratch "And historically"?

>  #
>  # This command will refuse to operate on any job that has not yet
>  # reached its terminal state, JOB_STATUS_CONCLUDED.  For jobs that
> @@ -222,6 +244,9 @@
>  # force ALL jobs in the transaction to finalize, so it is only
>  # necessary to instruct a single member job to finalize.
>  #
> +# The command is applicable only to jobs which have @auto-finalize option
> +# and only when this option is set to false.
> +#
>  # @id: The identifier of any job in the transaction, or of a job that
>  #     is not part of any transaction.
>  #
Re: [PATCH v2 1/2] qapi: synchronize jobs and block-jobs documentation
Posted by Eric Blake via Devel 8 months, 2 weeks ago
On Fri, Apr 04, 2025 at 10:31:53PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> Actualize documentation and synchronize it for commands which actually
> call the same functions internally.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> ---
>  qapi/block-core.json | 59 +++++++++++++++++++++++++-------------------
>  qapi/job.json        | 29 ++++++++++++++++++++--
>  2 files changed, 61 insertions(+), 27 deletions(-)
>

Overall makes sense, but one thing jumped out at me:

>  # @block-job-complete:
>  #
> -# 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.
> +# Manually trigger completion of an active job in the READY or STANDBY
> +# state.  Completing the job in any other state is an error.
> +#
> +# This is supported only for drive mirroring (which includes
> +# drive-mirror, blockdev-mirror and block-commit job (only in case of
> +# "active commit", when the node being commited is used by the guest)),
> +# where it also switches the device to write to the target path only.

The doubly-nested parenthetical feels long.  Is there a more concise
way of expressing the list of three commands, where one of the
commands only supports it in the special case of "active commit"?


-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization:  qemu.org | libguestfs.org
Re: [PATCH v2 1/2] qapi: synchronize jobs and block-jobs documentation
Posted by Vladimir Sementsov-Ogievskiy 8 months, 2 weeks ago
On 07.04.25 18:28, Eric Blake wrote:
> On Fri, Apr 04, 2025 at 10:31:53PM +0300, Vladimir Sementsov-Ogievskiy wrote:
>> Actualize documentation and synchronize it for commands which actually
>> call the same functions internally.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
>> ---
>>   qapi/block-core.json | 59 +++++++++++++++++++++++++-------------------
>>   qapi/job.json        | 29 ++++++++++++++++++++--
>>   2 files changed, 61 insertions(+), 27 deletions(-)
>>
> 
> Overall makes sense, but one thing jumped out at me:
> 
>>   # @block-job-complete:
>>   #
>> -# 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.
>> +# Manually trigger completion of an active job in the READY or STANDBY
>> +# state.  Completing the job in any other state is an error.
>> +#
>> +# This is supported only for drive mirroring (which includes
>> +# drive-mirror, blockdev-mirror and block-commit job (only in case of
>> +# "active commit", when the node being commited is used by the guest)),
>> +# where it also switches the device to write to the target path only.
> 
> The doubly-nested parenthetical feels long.  Is there a more concise
> way of expressing the list of three commands, where one of the
> commands only supports it in the special case of "active commit"?
> 

Probably, move the big "(...)" to the second sentence:

Note that drive mirroring includes [...] the guest).

-- 
Best regards,
Vladimir