Re: [PATCH 12/12] qapi: convert simple command intros for block-core.json

[Markus Armbruster <armbru@redhat.com>]

John Snow <jsnow@redhat.com> writes:

> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  qapi/block-core.json | 96 +++++++++++++++++++++-----------------------
>  1 file changed, 46 insertions(+), 50 deletions(-)
>
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 33e1147792b..29f011dc209 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -1816,12 +1816,12 @@
>  
>  ##
>  # @change-backing-file:
> -#
> -# Change the backing file in the image file metadata.  This does not
> -# cause QEMU to reopen the image file to reparse the backing filename
> -# (it may, however, perform a reopen to change permissions from r/o ->
> -# r/w -> r/o, if needed).  The new backing file string is written into
> -# the image file metadata, and the QEMU internal strings are updated.
> +#     Change the backing file in the image file metadata.  This does
> +#     not cause QEMU to reopen the image file to reparse the backing
> +#     filename (it may, however, perform a reopen to change
> +#     permissions from r/o -> r/w -> r/o, if needed).  The new backing
> +#     file string is written into the image file metadata, and the
> +#     QEMU internal strings are updated.
>  #
>  # @image-node-name: The name of the block driver state node of the
>  #     image to modify.  The "device" argument is used to verify
> @@ -1957,12 +1957,12 @@
>  
>  ##
>  # @drive-backup:
> -#
> -# Start a point-in-time copy of a block device to a new destination.
> -# The status of ongoing `drive-backup` operations can be checked with
> -# `query-block-jobs` where the `BlockJobInfo`.type field has the value
> -# 'backup'.  The operation can be stopped before it has completed
> -# using the `job-cancel` or `block-job-cancel` command.
> +#     Start a point-in-time copy of a block device to a new
> +#     destination.  The status of ongoing `drive-backup` operations
> +#     can be checked with `query-block-jobs` where the
> +#     `BlockJobInfo`.type field has the value 'backup'.  The operation
> +#     can be stopped before it has completed using the `job-cancel` or
> +#     `block-job-cancel` command.
>  #
>  # Features:
>  #
> @@ -1988,12 +1988,12 @@
>  
>  ##
>  # @blockdev-backup:
> -#
> -# Start a point-in-time copy of a block device to a new destination.
> -# The status of ongoing `blockdev-backup` operations can be checked
> -# with `query-block-jobs` where the `BlockJobInfo`.type field has the
> -# value 'backup'.  The operation can be stopped before it has
> -# completed using the `job-cancel` or `block-job-cancel` command.
> +#     Start a point-in-time copy of a block device to a new
> +#     destination.  The status of ongoing `blockdev-backup` operations
> +#     can be checked with `query-block-jobs` where the
> +#     `BlockJobInfo`.type field has the value 'backup'.  The operation
> +#     can be stopped before it has completed using the `job-cancel` or
> +#     `block-job-cancel` command.
>  #
>  # Errors:
>  #     - If @device is not a valid block device, DeviceNotFound
> @@ -2185,13 +2185,13 @@
>  
>  ##
>  # @drive-mirror:
> -#
> -# Start mirroring a block device's writes to a new destination.
> -# target specifies the target of the new image.  If the file exists,
> -# or if it is a device, it will be used as the new destination for
> -# writes.  If it does not exist, a new file will be created.  @format
> -# specifies the format of the mirror image, default is to probe if
> -# mode='existing', else the format of the source.
> +#     Start mirroring a block device's writes to a new destination.
> +#     target specifies the target of the new image.  If the file

@target, I think.

> +#     exists, or if it is a device, it will be used as the new
> +#     destination for writes.  If it does not exist, a new file will
> +#     be created.  @format specifies the format of the mirror image,
> +#     default is to probe if mode='existing', else the format of the

@mode

Where there are two, there are almost certainly more.

> +#     source.

Text that refers to arguments should probably go below the argument
descriptions.  I.e. this "intro" should probably be split.

Let's leave that for another day, just take note of future work:

1. Add missing markup to argument / member / feature references.

2. Review and improve doc comments where the intro refers to arguments /
members / features.

Not noting any of this again for this series.

>  #
>  # Errors:
>  #     - If @device is not a valid block device, GenericError
> @@ -2387,10 +2387,9 @@
>  
>  ##
>  # @block-dirty-bitmap-remove:
> -#
> -# Stop write tracking and remove the dirty bitmap that was created
> -# with `block-dirty-bitmap-add`.  If the bitmap is persistent, remove
> -# it from its storage too.
> +#     Stop write tracking and remove the dirty bitmap that was created
> +#     with `block-dirty-bitmap-add`.  If the bitmap is persistent,
> +#     remove it from its storage too.
>  #
>  # Errors:
>  #     - If @node is not a valid block device or node, DeviceNotFound
> @@ -4937,10 +4936,9 @@
>  
>  ##
>  # @blockdev-del:
> -#
> -# Deletes a block device that has been added using `blockdev-add`.
> -# The command will fail if the node is attached to a device or is
> -# otherwise being used.
> +#     Deletes a block device that has been added using `blockdev-add`.
> +#     The command will fail if the node is attached to a device or is
> +#     otherwise being used.

Perhaps the "will fail" part should be in an Errors: section.

The intros above use imperative mode, this one doesn't.  Elsewhere in
this series, I even saw "Command to <do stuff>".

More notes:

3. Review and improve doc comments where the intro talks about failure
modes.

4. Consistently use imperative mood for command intros.

Not noting any of this again for this series.

>  #
>  # @node-name: Name of the graph node to delete.
>  #

[...]