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

On Fri, Jul 24, 2026 at 4:38=E2=80=AFAM Markus Armbruster <[email protected]=
m> wrote:
>
> John Snow <[email protected]> writes:
>
> > (Trivial)
> >
> > Signed-off-by: John Snow <[email protected]>
> > ---
> >  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=3Dcontent1;
> >  #     name2=3Dcontent2;" 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)?

That's tricky to automate, and doing it by hand doesn't sound much fun
either... can we just power through one-by-one?

Unless you have a script or a trick you believe will help, and then I
am happy to oblige.