Re: [PATCH v2 44/44] qapi: convert trivial intro sections for block-core.json

[Markus Armbruster <armbru@redhat.com>]

John Snow <jsnow@redhat.com> writes:

> (Trivial)
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  qapi/block-core.json | 646 +++++++++++++++++--------------------------
>  1 file changed, 255 insertions(+), 391 deletions(-)
>
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 1f87b078505..bd98af2f298 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json

[...]

> @@ -422,10 +417,9 @@
>  
>  ##
>  # @BlockGraphInfo:
> -#
> -# Information about all nodes in a block (sub)graph in the form of
> -# `BlockNodeInfo` data.  The base `BlockNodeInfo` struct contains the
> -# information for the (sub)graph's root node.
> +#     Information about all nodes in a block (sub)graph in the form of
> +#     `BlockNodeInfo` data.  The base `BlockNodeInfo` struct contains
> +#     the information for the (sub)graph's root node.
>  #
>  # @children: Array of links to this node's child nodes' information
>  #

The first sentence is clearly overview, but the second caught my eye.
What would we want the inliner to do with it?

Looks like the inliner isn't going to do anything with it right now,
because BlockGraphInfo is only used as member type.

[...]

> @@ -1378,10 +1355,9 @@
>  
>  ##
>  # @BlockdevOnError:
> -#
> -# An enumeration of possible behaviors for errors on I/O operations.
> -# The exact meaning depends on whether the I/O was initiated by a
> -# guest or by a block job
> +#     An enumeration of possible behaviors for errors on I/O
> +#     operations.  The exact meaning depends on whether the I/O was
> +#     initiated by a guest or by a block job
>  #
>  # @report: for guest operations, report the error to the guest; for
>  #     jobs, cancel the job

The first sentence is clearly overview, but the second caught my eye.

Because this is an enum type, and we're not going to inline these, we
can leave it as is.

[...]

> @@ -1636,9 +1605,8 @@
>  
>  ##
>  # @BackupPerf:
> -#
> -# Optional parameters for backup.  These parameters don't affect
> -# functionality, but may significantly affect performance.
> +#     Optional parameters for backup.  These parameters don't affect
> +#     functionality, but may significantly affect performance.

The first sentence is clearly overview.  What would we want the inliner
to do with the second?

Looks like the inliner isn't going to do anything with it right now,
because BackupPerf is only used as member type.

Aside: this only user BackupCommon member x-perf has been @unstable for
more than five years.  That's a long time to sit on a fence.

>  #
>  # @use-copy-range: Use copy offloading.  Default false.
>  #

[...]

> @@ -1845,12 +1812,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.

This is an example of a first paragraph I might flag for review if it
was a struct or union type.

>  #
>  # @image-node-name: The name of the block driver state node of the
>  #     image to modify.  The "device" argument is used to verify

[...]

> @@ -2746,11 +2700,10 @@
>  
>  ##
>  # @ThrottleLimits:
> -#
> -# Limit parameters for throttling.  Since some limit combinations are
> -# illegal, limits should always be set in one transaction.  All fields
> -# are optional.  When setting limits, if a field is missing the
> -# current value is not changed.
> +#     Limit parameters for throttling.  Since some limit combinations
> +#     are illegal, limits should always be set in one transaction.
> +#     All fields are optional.  When setting limits, if a field is
> +#     missing the current value is not changed.

The first sentence is clearly overview, but what would we want the
inliner to do with the remainder?

Looks like the inliner isn't going to do anything with it right now,
because ThrottleLimits is only used as member type.

>  #
>  # @iops-total: limit total I/O operations per second
>  #

[...]

> @@ -3548,12 +3486,11 @@
>  
>  ##
>  # @Qcow2OverlapCheckFlags:
> -#
> -# Structure of flags for each metadata structure.  Setting a field to
> -# 'true' makes QEMU guard that Qcow2 format structure against
> -# unintended overwriting.  See Qcow2 format specification for detailed
> -# information on these structures.  The default value is chosen
> -# according to the template given.
> +#     Structure of flags for each metadata structure.  Setting a field
> +#     to 'true' makes QEMU guard that Qcow2 format structure against
> +#     unintended overwriting.  See Qcow2 format specification for
> +#     detailed information on these structures.  The default value is
> +#     chosen according to the template given.
>  #
>  # @template: Specifies a template mode which can be adjusted using the
>  #     other flags, defaults to 'cached'

The first sentence is clearly overview, but what would we want the
inliner to do with the remainder?

Looks like the inliner isn't going to do anything with it right now,
because Qcow2OverlapCheckFlags is only used as member type.


[...]

> @@ -4550,9 +4460,8 @@
>  
>  ##
>  # @BlockdevOptionsCurlHttp:
> -#
> -# Driver specific block device options for HTTP connections over the
> -# curl backend.  URLs must start with "http://".
> +#     Driver specific block device options for HTTP connections over
> +#     the curl backend.  URLs must start with "http://".
>  #
>  # @cookie: List of cookies to set; format is "name1=content1;
>  #     name2=content2;" as explained by CURLOPT_COOKIE(3).  Defaults to

The first sentence is clearly overview, but what about the second?
"URLs" are actually member @url of base type BlockdevOptionsCurlBase.
What would we want the inliner to do with it?

BlockdevOptionsCurlBase is the base type of BlockdevOptionsCurlHttps and
a branch type of BlockdevOptions, where this will get inlined.

> @@ -4576,9 +4485,8 @@
>  
>  ##
>  # @BlockdevOptionsCurlHttps:
> -#
> -# Driver specific block device options for HTTPS connections over the
> -# curl backend.  URLs must start with "https://".
> +#     Driver specific block device options for HTTPS connections over
> +#     the curl backend.  URLs must start with "https://".
>  #
>  # @sslverify: Whether to verify the SSL certificate's validity
>  #     (defaults to true)

Likewise.

> @@ -4591,9 +4499,8 @@
>  
>  ##
>  # @BlockdevOptionsCurlFtp:
> -#
> -# Driver specific block device options for FTP connections over the
> -# curl backend.  URLs must start with "ftp://".
> +#     Driver specific block device options for FTP connections over
> +#     the curl backend.  URLs must start with "ftp://".
>  #
>  # Since: 2.9
>  ##

Likewise.

> @@ -4603,9 +4510,8 @@
>  
>  ##
>  # @BlockdevOptionsCurlFtps:
> -#
> -# Driver specific block device options for FTPS connections over the
> -# curl backend.  URLs must start with "ftps://".
> +#     Driver specific block device options for FTPS connections over
> +#     the curl backend.  URLs must start with "ftps://".
>  #
>  # @sslverify: Whether to verify the SSL certificate's validity
>  #     (defaults to true)

Likewise.


[...]

> @@ -4735,14 +4637,13 @@
>  
>  ##
>  # @BlockdevOptionsCbw:
> -#
> -# Driver specific block device options for the copy-before-write
> -# driver, which does so called copy-before-write operations: when data
> -# is written to the filter, the filter first reads corresponding
> -# blocks from its file child and copies them to @target child.  After
> -# successfully copying, the write request is propagated to file child.
> -# If copying fails, the original write request is failed too and no
> -# data is written to file child.
> +#     Driver specific block device options for the copy-before-write
> +#     driver, which does so called copy-before-write operations: when
> +#     data is written to the filter, the filter first reads
> +#     corresponding blocks from its file child and copies them to
> +#     @target child.  After successfully copying, the write request is
> +#     propagated to file child.  If copying fails, the original write
> +#     request is failed too and no data is written to file child.
>  #
>  # @target: The target for copy-before-write operations.
>  #

What would we want the inliner to do with this one?

BlockdevOptionsCbw is a branch type of BlockdevOptions, where this will
get inlined.

> @@ -4778,9 +4679,8 @@
>  
>  ##
>  # @BlockdevOptions:
> -#
> -# Options for creating a block device.  Many options are available for
> -# all block devices, independent of the block driver:
> +#     Options for creating a block device.  Many options are available
> +#     for all block devices, independent of the block driver:
>  #
>  # @driver: block driver name
>  #

What would we want the inliner to do with this one?

BlockdevOptions is used as argument of blockdev-add, where it will be
inlined.


[...]

> @@ -5587,9 +5465,8 @@
>  
>  ##
>  # @BlockdevAmendOptionsQcow2:
> -#
> -# Driver specific image amend options for qcow2.  For now, only
> -# encryption options can be amended
> +#     Driver specific image amend options for qcow2.  For now, only
> +#     encryption options can be amended
>  #
>  # @encrypt: Encryption options to be amended
>  #

What would we want the inliner to do with this one?

BlockdevAmendOptionsQcow2 is used as branch of BlockdevAmendOptions,
where it will be inlined.

[...]

I believe all the problematic first paragraphs consist of more than one
sentence.  What about splitting "trivial" into "very trivial" (just one
paragraph, no sentence-ending punctuation in the middle) and "hopefully
trivial" (just one paragraph, remainder)?