Re: [PATCH v14 04/13] qapi: add filter-node-name to block-stream

[Max Reitz]

On 04.12.20 23:07, Vladimir Sementsov-Ogievskiy wrote:
> From: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
>
> Provide the possibility to pass the 'filter-node-name' parameter to the
> block-stream job as it is done for the commit block job.
>
> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>   qapi/block-core.json           | 6 ++++++
>   include/block/block_int.h      | 7 ++++++-
>   block/monitor/block-hmp-cmds.c | 4 ++--
>   block/stream.c                 | 4 +++-
>   blockdev.c                     | 4 +++-
>   5 files changed, 20 insertions(+), 5 deletions(-)
>
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 04ad80bc1e..8ef3df6767 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -2543,6 +2543,11 @@
>   #            'stop' and 'enospc' can only be used if the block device
>   #            supports io-status (see BlockInfo).  Since 1.3.
>   #
> +# @filter-node-name: the node name that should be assigned to the
> +#                    filter driver that the stream job inserts into the graph
> +#                    above @device. If this option is not given, a node name is
> +#                    autogenerated. (Since: 5.2)

*6.0

> +#
>   # @auto-finalize: When false, this job will wait in a PENDING state after it 
> has
>   #                 finished its work, waiting for @block-job-finalize before
>   #                 making any block graph changes.

[...]

> diff --git a/include/block/block_int.h b/include/block/block_int.h
> index 95d9333be1..c05fa1eb6b 100644
> --- a/include/block/block_int.h
> +++ b/include/block/block_int.h
> @@ -1134,6 +1134,9 @@ int is_windows_drive(const char *filename);
>    *                  See @BlockJobCreateFlags
>    * @speed: The maximum speed, in bytes per second, or 0 for unlimited.
>    * @on_error: The action to take upon error.
> + * @filter_node_name: The node name that should be assigned to the filter
> + * driver that the commit job inserts into the graph above @bs. NULL means
> + * that a node name should be autogenerated.

Personally, I find such descriptions to be more easily readable (or 
rather more easily visually separable from other parameters) if they’re 
indented.  I understand that two other parameters’ descriptions aren’t 
indented either (but one is), so in the end it’s your choice.  (But I 
thought a little nudging couldn’t hurt.)

So either way (with *6.0):

Reviewed-by: Max Reitz <mreitz@redhat.com>