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

[Markus Armbruster <armbru@redhat.com>]

John Snow <jsnow@redhat.com> writes:

> On Thu, Sep 3, 2026, 7:58 AM Markus Armbruster <armbru@redhat.com> wrote:
>
>> 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.
>>
>
> You reviewed the series, does this mean changes for later?

No.  My "leave that for another day" below applies.

>> > +#     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.
>>
>
> OK.
>
>
>> >  #
>> >  # 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.
>>
>
> Are you keeping a log of future changes, or should I write them down?

Both :)

My notes so far:

    Noted in review of intro conversions:

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

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

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

    4. Consistently use imperative mood for command intros.

    5. Implement markup for non-local argument / member / feature
    references, and put it to use.

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