Re: [Qemu-devel] [PATCH v3 3/3] virtio-rng: change default backend to rng-builtin

[Markus Armbruster <armbru@redhat.com>]

Laurent Vivier <lvivier@redhat.com> writes:

> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
>  backends/rng-builtin.c         |  8 +++-----
>  hw/virtio/virtio-rng.c         |  2 +-
>  include/hw/virtio/virtio-rng.h |  4 ++--
>  include/sysemu/rng-builtin.h   | 17 +++++++++++++++++
>  qemu-options.hx                |  5 ++---
>  5 files changed, 25 insertions(+), 11 deletions(-)
>  create mode 100644 include/sysemu/rng-builtin.h
>
> diff --git a/backends/rng-builtin.c b/backends/rng-builtin.c
> index b1264b745407..27675301933b 100644
> --- a/backends/rng-builtin.c
> +++ b/backends/rng-builtin.c
> @@ -7,17 +7,15 @@
>  
>  #include "qemu/osdep.h"
>  #include "sysemu/rng.h"
> +#include "sysemu/rng-builtin.h"
>  #include "qapi/error.h"
>  #include "qapi/qmp/qerror.h"
>  #include "qemu/main-loop.h"
>  #include "qemu/guest-random.h"
>  
> -#define TYPE_RNG_BUILTIN "rng-builtin"
> -#define RNG_BUILTIN(obj) OBJECT_CHECK(RngBuiltin, (obj), TYPE_RNG_BUILTIN)
> -
> -typedef struct RngBuiltin {
> +struct RngBuiltin {
>      RngBackend parent;
> -} RngBuiltin;
> +};
>  
>  static void rng_builtin_request_entropy(RngBackend *b, RngRequest *req)
>  {
> diff --git a/hw/virtio/virtio-rng.c b/hw/virtio/virtio-rng.c
> index 30493a258622..67209f63ddbc 100644
> --- a/hw/virtio/virtio-rng.c
> +++ b/hw/virtio/virtio-rng.c
> @@ -189,7 +189,7 @@ static void virtio_rng_device_realize(DeviceState *dev, Error **errp)
>      }
>  
>      if (vrng->conf.rng == NULL) {
> -        vrng->conf.default_backend = RNG_RANDOM(object_new(TYPE_RNG_RANDOM));
> +        vrng->conf.default_backend = RNG_BUILTIN(object_new(TYPE_RNG_BUILTIN));
>  
>          user_creatable_complete(USER_CREATABLE(vrng->conf.default_backend),
>                                  &local_err);
           if (local_err) {
               error_propagate(errp, local_err);
               object_unref(OBJECT(vrng->conf.default_backend));
               return;
           }

           object_property_add_child(OBJECT(dev),
                                     "default-backend",
                                     OBJECT(vrng->conf.default_backend),
                                     NULL);

           /* The child property took a reference, we can safely drop ours now */
           object_unref(OBJECT(vrng->conf.default_backend));

           object_property_set_link(OBJECT(dev),
                                    OBJECT(vrng->conf.default_backend),
                                    "rng", NULL);
       }
> diff --git a/include/hw/virtio/virtio-rng.h b/include/hw/virtio/virtio-rng.h
> index 922dce7caccf..f9b6339b19a4 100644
> --- a/include/hw/virtio/virtio-rng.h
> +++ b/include/hw/virtio/virtio-rng.h
> @@ -13,7 +13,7 @@
>  #define QEMU_VIRTIO_RNG_H
>  
>  #include "sysemu/rng.h"
> -#include "sysemu/rng-random.h"
> +#include "sysemu/rng-builtin.h"
>  #include "standard-headers/linux/virtio_rng.h"
>  
>  #define TYPE_VIRTIO_RNG "virtio-rng-device"
> @@ -26,7 +26,7 @@ struct VirtIORNGConf {
>      RngBackend *rng;
>      uint64_t max_bytes;
>      uint32_t period_ms;
> -    RngRandom *default_backend;
> +    RngBuiltin *default_backend;

This member appears to be superfluous.  As far as I can tell, it's only
used in the part of virtio-rng.c visible above.  If you replace it by a
local variable there (in a separate patch, perhaps), this patch will
become simpler.  In particular, you won't need to create rng-builtin.h.

>  };
>  
>  typedef struct VirtIORNG {
> diff --git a/include/sysemu/rng-builtin.h b/include/sysemu/rng-builtin.h
> new file mode 100644
> index 000000000000..a0f75f97dde8
> --- /dev/null
> +++ b/include/sysemu/rng-builtin.h
> @@ -0,0 +1,17 @@
> +/*
> + * QEMU Builtin Random Number Generator Backend
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +#ifndef QEMU_RNG_BUILTIN_H
> +#define QEMU_RNG_BUILTIN_H
> +
> +#include "qom/object.h"
> +
> +#define TYPE_RNG_BUILTIN "rng-builtin"
> +#define RNG_BUILTIN(obj) OBJECT_CHECK(RngBuiltin, (obj), TYPE_RNG_BUILTIN)
> +
> +typedef struct RngBuiltin RngBuiltin;
> +
> +#endif
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 6ab920f12be4..c9784be83cb5 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -4285,7 +4285,7 @@ The @option{share} boolean option is @var{on} by default with memfd.
>  Creates a random number generator backend which obtains entropy from
>  QEMU builtin functions. The @option{id} parameter is a unique ID that
>  will be used to reference this entropy backend from the @option{virtio-rng}
> -device.
> +device. By default, the @option{virtio-rng} device uses this RNG backend.

Aside: this piece of information should ideally go into virtio-rng
documentation.  If only it existed; -device virtio-rng,help is all we
got.  So you keep it here.  Fair enough.

>  
>  @item -object rng-random,id=@var{id},filename=@var{/dev/random}
>  
> @@ -4293,8 +4293,7 @@ Creates a random number generator backend which obtains entropy from
>  a device on the host. The @option{id} parameter is a unique ID that
>  will be used to reference this entropy backend from the @option{virtio-rng}
>  device. The @option{filename} parameter specifies which file to obtain
> -entropy from and if omitted defaults to @option{/dev/urandom}. By default,
> -the @option{virtio-rng} device uses this RNG backend.
> +entropy from and if omitted defaults to @option{/dev/urandom}.
>  
>  @item -object rng-egd,id=@var{id},chardev=@var{chardevid}