Re: [PATCH v4 2/3] util/main-loop: Introduce the main loop into QOM

[Markus Armbruster <armbru@redhat.com>]

Nicolas Saenz Julienne <nsaenzju@redhat.com> writes:

> 'event-loop-base' provides basic property handling for all 'AioContext'
> based event loops. So let's define a new 'MainLoopClass' that inherits
> from it. This will permit tweaking the main loop's properties through
> qapi as well as through the command line using the '-object' keyword[1].
> Only one instance of 'MainLoopClass' might be created at any time.
>
> 'EventLoopBaseClass' learns a new callback, 'can_be_deleted()' so as to
> mark 'MainLoop' as non-deletable.
>
> [1] For example:
>       -object main-loop,id=main-loop,aio-max-batch=<value>
>
> Signed-off-by: Nicolas Saenz Julienne <nsaenzju@redhat.com>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[...]

> diff --git a/qapi/qom.json b/qapi/qom.json
> index eeb5395ff3..e5f31c4469 100644
> --- a/qapi/qom.json
> +++ b/qapi/qom.json
> @@ -499,6 +499,17 @@
>              '*repeat': 'bool',
>              '*grab-toggle': 'GrabToggleKeys' } }
>  
> +##
> +# @EventLoopBaseProperties:
> +#
> +# Common properties for objects derived from EventLoopBase

This makes sense as internal documentation: QAPI type
EventLoopBaseProperties is associated with C type EventLoopBase.  Doc
comments are *external* documentation: they go into the QEMU QMP
Reference Manual.

What about "Common properties for event loops"?

> +#
> +# @aio-max-batch: maximum number of requests in a batch for the AIO engine,
> +#                 0 means that the engine will use its default.

Missing:

   # Since: 7.1

Permit me a short digression.  We add these unthinkingly, because
thinking is expensive.  Even when the type isn't really part of the
external interface.  The deeper problem is that we're trying to generate
documentation of the external interface from doc comments that are
written as documentation of the internal QAPI data structures.  Here,
for example, we document EventLoopBaseProperties even though it is a
purely internal thing: whether we factor out common members into a base
type or not is not visible in QMP.

> +##
> +{ 'struct': 'EventLoopBaseProperties',
> +  'data': { '*aio-max-batch': 'int' } }
> +
>  ##
>  # @IothreadProperties:
>  #
> @@ -516,17 +527,26 @@
>  #               algorithm detects it is spending too long polling without
>  #               encountering events. 0 selects a default behaviour (default: 0)
>  #
> -# @aio-max-batch: maximum number of requests in a batch for the AIO engine,
> -#                 0 means that the engine will use its default
> -#                 (default:0, since 6.1)
> +# The @aio-max-batch option is available since 6.1.

This separates the member's "since" information from its defintion.
Can't be helped, because its defined in the base type, but the since
only applies here.  Okay.

>  #
>  # Since: 2.0
>  ##
>  { 'struct': 'IothreadProperties',
> +  'base': 'EventLoopBaseProperties',
>    'data': { '*poll-max-ns': 'int',
>              '*poll-grow': 'int',
> -            '*poll-shrink': 'int',
> -            '*aio-max-batch': 'int' } }
> +            '*poll-shrink': 'int' } }
> +
> +##
> +# @MainLoopProperties:
> +#
> +# Properties for the main-loop object.
> +#
> +# Since: 7.1
> +##
> +{ 'struct': 'MainLoopProperties',
> +  'base': 'EventLoopBaseProperties',
> +  'data': {} }

The patch does two things:

1. Factor EventLoopBaseProperties out of IothreadProperties.

2. Create MainLoopProperties.

I'd split it.  This is not a demand.

>  
>  ##
>  # @MemoryBackendProperties:
> @@ -818,6 +838,7 @@
>      { 'name': 'input-linux',
>        'if': 'CONFIG_LINUX' },
>      'iothread',
> +    'main-loop',
>      { 'name': 'memory-backend-epc',
>        'if': 'CONFIG_LINUX' },
>      'memory-backend-file',
> @@ -883,6 +904,7 @@
>        'input-linux':                { 'type': 'InputLinuxProperties',
>                                        'if': 'CONFIG_LINUX' },
>        'iothread':                   'IothreadProperties',
> +      'main-loop':                  'MainLoopProperties',
>        'memory-backend-epc':         { 'type': 'MemoryBackendEpcProperties',
>                                        'if': 'CONFIG_LINUX' },
>        'memory-backend-file':        'MemoryBackendFileProperties',

[...]