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

[Markus Armbruster <armbru@redhat.com>]

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.
>  #