Re: [PATCH v2 08/31] qapi/qom: Add ObjectOptions for rng-*, deprecate 'opened'

[Eric Blake <eblake@redhat.com>]

On 2/24/21 7:52 AM, Kevin Wolf wrote:
> This adds a QAPI schema for the properties of the rng-* objects.
> 
> The 'opened' property doesn't seem to make sense as an external
> interface: It is automatically set to true in ucc->complete, and
> explicitly setting it to true earlier just means that trying to set
> additional options will result in an error. After the property has once
> been set to true (i.e. when the object construction has completed), it
> can never be reset to false. In other words, the 'opened' property is
> useless. Mark it as deprecated in the schema from the start.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  qapi/qom.json              | 56 ++++++++++++++++++++++++++++++++++++--
>  docs/system/deprecated.rst |  9 ++++++
>  2 files changed, 63 insertions(+), 2 deletions(-)
> 
> diff --git a/qapi/qom.json b/qapi/qom.json
> index 1a869006a1..73f28f9608 100644
> --- a/qapi/qom.json
> +++ b/qapi/qom.json
> @@ -380,6 +380,52 @@
>              '*hugetlbsize': 'size',
>              '*seal': 'bool' } }
>  
> +##
> +# @RngProperties:
> +#
> +# Properties for objects of classes derived from rng.
> +#
> +# @opened: if true, the device is opened immediately when applying this option
> +#          and will probably fail when processing the next option. Don't use;
> +#          only provided for compatibility. (default: false)
> +#
> +# Features:
> +# @deprecated: Member @opened is deprecated.  Setting true doesn't make sense,
> +#              and false is already the default.
> +#
> +# Since: 1.3
> +##
> +{ 'struct': 'RngProperties',
> +  'data': { '*opened': { 'type': 'bool', 'features': ['deprecated'] } } }

Matches backends/rng.c:rng_backend_class_init(), and I concur with the
deprecation.

> +
> +##
> +# @RngEgdProperties:
> +#
> +# Properties for rng-egd objects.
> +#
> +# @chardev: the name of a character device backend that provides the connection
> +#           to the RNG daemon
> +#
> +# Since: 1.3
> +##
> +{ 'struct': 'RngEgdProperties',
> +  'base': 'RngProperties',
> +  'data': { 'chardev': 'str' } }

Matches backends/rng-egd.c:rng_egd_class_init().

> +
> +##
> +# @RngRandomProperties:
> +#
> +# Properties for rng-random objects.
> +#
> +# @filename: the filename of the device on the host to obtain entropy from
> +#            (default: "/dev/urandom")
> +#
> +# Since: 1.3
> +##
> +{ 'struct': 'RngRandomProperties',
> +  'base': 'RngProperties',
> +  'data': { '*filename': 'str' } }

Matches backends/rng-random.c:rng_random_class_init().

> +
>  ##
>  # @ObjectType:
>  #
> @@ -398,7 +444,10 @@
>      'iothread',
>      'memory-backend-file',
>      'memory-backend-memfd',
> -    'memory-backend-ram'
> +    'memory-backend-ram',
> +    'rng-builtin',
> +    'rng-egd',
> +    'rng-random'
>    ] }
>  
>  ##
> @@ -428,7 +477,10 @@
>        'iothread':                   'IothreadProperties',
>        'memory-backend-file':        'MemoryBackendFileProperties',
>        'memory-backend-memfd':       'MemoryBackendMemfdProperties',
> -      'memory-backend-ram':         'MemoryBackendProperties'
> +      'memory-backend-ram':         'MemoryBackendProperties',
> +      'rng-builtin':                'RngProperties',
> +      'rng-egd':                    'RngEgdProperties',
> +      'rng-random':                 'RngRandomProperties'
>    } }
>  
>  ##
> diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
> index 00b694e053..79991c2893 100644
> --- a/docs/system/deprecated.rst
> +++ b/docs/system/deprecated.rst
> @@ -146,6 +146,15 @@ library enabled as a cryptography provider.
>  Neither the ``nettle`` library, or the built-in cryptography provider are
>  supported on FIPS enabled hosts.
>  
> +``opened`` property of ``rng-*`` objects (since 6.0.0)
> +''''''''''''''''''''''''''''''''''''''''''''''''''''''
> +
> +The only effect of specifying ``opened=on`` in the command line or QMP
> +``object-add`` is that the device is opened immediately, possibly before all
> +other options have been processed.  This will either have no effect (if
> +``opened`` was the last option) or cause errors.  The property is therefore
> +useless and should not be specified.

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org