[Qemu-devel] [PATCH v4 0/3] rng-builtin: add an RNG backend that uses qemu_guest_getrandom()

[Qemu-devel] [PATCH v4 0/3] rng-builtin: add an RNG backend that uses qemu_guest_getrandom()

[Laurent Vivier]

Add a new RNG backend using QEMU builtin getrandom function.

This patch applies on top of
    "[PATCH v6 00/24] Add qemu_getrandom and ARMv8.5-RNG etc"
Based-on: 20190510173049.28171-1-richard.henderson@linaro.org

v4: update PATCH 1 commit message

v3: Include Kashyap's patch in the series
    Add a patch to change virtio-rng default backend to rng-builtin

v2: Update qemu-options.hx
    describe the new backend and specify virtio-rng uses the
    rng-random by default

Kashyap Chamarthy (1):
  VirtIO-RNG: Update default entropy source to `/dev/urandom`

Laurent Vivier (2):
  rng-builtin: add an RNG backend that uses qemu_guest_getrandom()
  virtio-rng: change default backend to rng-builtin

 backends/Makefile.objs         |  2 +-
 backends/rng-builtin.c         | 54 ++++++++++++++++++++++++++++++++++
 backends/rng-random.c          |  2 +-
 hw/virtio/virtio-rng.c         |  2 +-
 include/hw/virtio/virtio-rng.h |  4 +--
 include/sysemu/rng-builtin.h   | 17 +++++++++++
 qemu-options.hx                |  9 +++++-
 7 files changed, 84 insertions(+), 6 deletions(-)
 create mode 100644 backends/rng-builtin.c
 create mode 100644 include/sysemu/rng-builtin.h

-- 
2.20.1

[Qemu-devel] [PATCH v4 1/3] VirtIO-RNG: Update default entropy source to `/dev/urandom`

[Laurent Vivier]

From: Kashyap Chamarthy <kchamart@redhat.com>

When QEMU exposes a VirtIO-RNG device to the guest, that device needs a
source of entropy, and that source needs to be "non-blocking", like
`/dev/urandom`.  However, currently QEMU defaults to the problematic
`/dev/random`, which on linux is "blocking" (as in, it waits until
sufficient entropy is available).

Why prefer `/dev/urandom` over `/dev/random`?
---------------------------------------------

The man pages of urandom(4) and random(4) state:

    "The /dev/random device is a legacy interface which dates back to a
    time where the cryptographic primitives used in the implementation
    of /dev/urandom were not widely trusted.  It will return random
    bytes only within the estimated number of bits of fresh noise in the
    entropy pool, blocking if necessary.  /dev/random is suitable for
    applications that need high quality randomness, and can afford
    indeterminate delays."

Further, the "Usage" section of the said man pages state:

    "The /dev/random interface is considered a legacy interface, and
    /dev/urandom is preferred and sufficient in all use cases, with the
    exception of applications which require randomness during early boot
    time; for these applications, getrandom(2) must be used instead,
    because it will block until the entropy pool is initialized.

    "If a seed file is saved across reboots as recommended below (all
    major Linux distributions have done this since 2000 at least), the
    output is cryptographically secure against attackers without local
    root access as soon as it is reloaded in the boot sequence, and
    perfectly adequate for network encryption session keys.  Since reads
    from /dev/random may block, users will usually want to open it in
    nonblocking mode (or perform a read with timeout), and provide some
    sort of user notification if the desired entropy is not immediately
    available."

And refer to random(7) for a comparison of `/dev/random` and
`/dev/urandom`.

What about other OSes?
----------------------

`/dev/urandom` exists and works on OS-X, FreeBSD, DragonFlyBSD, NetBSD
and OpenBSD, which cover all the non-Linux platforms we explicitly
support, aside from Windows.

On Windows `/dev/random` doesn't work either so we don't regress.
This is actually another argument in favour of using the newly
proposed 'rng-builtin' backend by default, as that will work on
Windows.

    - - -

Given the above, change the entropy source for VirtIO-RNG device to
`/dev/urandom`.

Related discussion in these[1][2] past threads.

[1] https://lists.nongnu.org/archive/html/qemu-devel/2018-06/msg08335.html
    -- "RNG: Any reason QEMU doesn't default to `/dev/urandom`?"
[2] https://lists.nongnu.org/archive/html/qemu-devel/2018-09/msg02724.html
    -- "[RFC] Virtio RNG: Consider changing the default entropy source to
       /dev/urandom"

Signed-off-by: Kashyap Chamarthy <kchamart@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
 backends/rng-random.c | 2 +-
 qemu-options.hx       | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/backends/rng-random.c b/backends/rng-random.c
index e2a49b0571d7..eff36ef14084 100644
--- a/backends/rng-random.c
+++ b/backends/rng-random.c
@@ -112,7 +112,7 @@ static void rng_random_init(Object *obj)
                             rng_random_set_filename,
                             NULL);
 
-    s->filename = g_strdup("/dev/random");
+    s->filename = g_strdup("/dev/urandom");
     s->fd = -1;
 }
 
diff --git a/qemu-options.hx b/qemu-options.hx
index 0191ef8b1eb7..4df0ea3aed5c 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -4286,7 +4286,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/random}.
+entropy from and if omitted defaults to @option{/dev/urandom}.
 
 @item -object rng-egd,id=@var{id},chardev=@var{chardevid}
 
-- 
2.20.1

[Qemu-devel] [PATCH v4 2/3] rng-builtin: add an RNG backend that uses qemu_guest_getrandom()

[Laurent Vivier]

Add a new RNG backend using QEMU builtin getrandom function.

It can be created and used with something like:

    ... -object rng-builtin,id=rng0 -device virtio-rng,rng=rng0 ...

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
 backends/Makefile.objs |  2 +-
 backends/rng-builtin.c | 56 ++++++++++++++++++++++++++++++++++++++++++
 qemu-options.hx        | 10 +++++++-
 3 files changed, 66 insertions(+), 2 deletions(-)
 create mode 100644 backends/rng-builtin.c

diff --git a/backends/Makefile.objs b/backends/Makefile.objs
index 981e8e122f2c..f0691116e86e 100644
--- a/backends/Makefile.objs
+++ b/backends/Makefile.objs
@@ -1,4 +1,4 @@
-common-obj-y += rng.o rng-egd.o
+common-obj-y += rng.o rng-egd.o rng-builtin.o
 common-obj-$(CONFIG_POSIX) += rng-random.o
 
 common-obj-$(CONFIG_TPM) += tpm.o
diff --git a/backends/rng-builtin.c b/backends/rng-builtin.c
new file mode 100644
index 000000000000..b1264b745407
--- /dev/null
+++ b/backends/rng-builtin.c
@@ -0,0 +1,56 @@
+/*
+ * 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.
+ */
+
+#include "qemu/osdep.h"
+#include "sysemu/rng.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 {
+    RngBackend parent;
+} RngBuiltin;
+
+static void rng_builtin_request_entropy(RngBackend *b, RngRequest *req)
+{
+    RngBuiltin *s = RNG_BUILTIN(b);
+
+    while (!QSIMPLEQ_EMPTY(&s->parent.requests)) {
+        RngRequest *req = QSIMPLEQ_FIRST(&s->parent.requests);
+
+        qemu_guest_getrandom_nofail(req->data, req->size);
+
+        req->receive_entropy(req->opaque, req->data, req->size);
+
+        rng_backend_finalize_request(&s->parent, req);
+    }
+}
+
+static void rng_builtin_class_init(ObjectClass *klass, void *data)
+{
+    RngBackendClass *rbc = RNG_BACKEND_CLASS(klass);
+
+    rbc->request_entropy = rng_builtin_request_entropy;
+}
+
+static const TypeInfo rng_builtin_info = {
+    .name = TYPE_RNG_BUILTIN,
+    .parent = TYPE_RNG_BACKEND,
+    .instance_size = sizeof(RngBuiltin),
+    .class_init = rng_builtin_class_init,
+};
+
+static void register_types(void)
+{
+    type_register_static(&rng_builtin_info);
+}
+
+type_init(register_types);
diff --git a/qemu-options.hx b/qemu-options.hx
index 4df0ea3aed5c..6ab920f12be4 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -4280,13 +4280,21 @@ other options.
 
 The @option{share} boolean option is @var{on} by default with memfd.
 
+@item -object rng-builtin,id=@var{id}
+
+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.
+
 @item -object rng-random,id=@var{id},filename=@var{/dev/random}
 
 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}.
+entropy from and if omitted defaults to @option{/dev/urandom}. By default,
+the @option{virtio-rng} device uses this RNG backend.
 
 @item -object rng-egd,id=@var{id},chardev=@var{chardevid}
 
-- 
2.20.1

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

[Laurent Vivier]

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);
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;
 };
 
 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.
 
 @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}
 
-- 
2.20.1

Re: [Qemu-devel] [PATCH v4 1/3] VirtIO-RNG: Update default entropy source to `/dev/urandom`

[Kashyap Chamarthy]

On Tue, May 14, 2019 at 09:56:00AM +0200, Laurent Vivier wrote:
> From: Kashyap Chamarthy <kchamart@redhat.com>
> 
> When QEMU exposes a VirtIO-RNG device to the guest, that device needs a
> source of entropy, and that source needs to be "non-blocking", like
> `/dev/urandom`.  However, currently QEMU defaults to the problematic
> `/dev/random`, which on linux is "blocking" (as in, it waits until

OCD nit: s/linux/Linux/

Maybe Michael can do the touch up when applying.

Thanks, Laurent, for reworking the commit message update.

> sufficient entropy is available).

[...]

-- 
/kashyap

Re: [Qemu-devel] [PATCH v4 1/3] VirtIO-RNG: Update default entropy source to `/dev/urandom`

[Laurent Vivier]

On 14/05/2019 10:08, Kashyap Chamarthy wrote:
> On Tue, May 14, 2019 at 09:56:00AM +0200, Laurent Vivier wrote:
>> From: Kashyap Chamarthy <kchamart@redhat.com>
>>
>> When QEMU exposes a VirtIO-RNG device to the guest, that device needs a
>> source of entropy, and that source needs to be "non-blocking", like
>> `/dev/urandom`.  However, currently QEMU defaults to the problematic
>> `/dev/random`, which on linux is "blocking" (as in, it waits until
> 
> OCD nit: s/linux/Linux/
> 
> Maybe Michael can do the touch up when applying.

A little reminder: this patch can be applied alone, but the followings 
need the series from Richard to be applied first.

Thanks,
Laurent

Re: [Qemu-devel] [PATCH v4 1/3] VirtIO-RNG: Update default entropy source to `/dev/urandom`

[Markus Armbruster]

Laurent Vivier <lvivier@redhat.com> writes:

> On 14/05/2019 10:08, Kashyap Chamarthy wrote:
>> On Tue, May 14, 2019 at 09:56:00AM +0200, Laurent Vivier wrote:
>>> From: Kashyap Chamarthy <kchamart@redhat.com>
>>>
>>> When QEMU exposes a VirtIO-RNG device to the guest, that device needs a
>>> source of entropy, and that source needs to be "non-blocking", like
>>> `/dev/urandom`.  However, currently QEMU defaults to the problematic
>>> `/dev/random`, which on linux is "blocking" (as in, it waits until
>>
>> OCD nit: s/linux/Linux/
>>
>> Maybe Michael can do the touch up when applying.
>
> A little reminder: this patch can be applied alone, but the followings
> need the series from Richard to be applied first.

I guess you mean
    [PATCH v6 00/25] Add qemu_getrandom and ARMv8.5-RNG etc
    Message-Id: <20190510173049.28171-1-richard.henderson@linaro.org>

You can make the dependency machine-readable by declaring

    Based-on: <20190510173049.28171-1-richard.henderson@linaro.org>

in your cover letter.  Patchew will then test your series on top of
Richard's.

Re: [Qemu-devel] [PATCH v4 1/3] VirtIO-RNG: Update default entropy source to `/dev/urandom`

[Markus Armbruster]

Laurent Vivier <lvivier@redhat.com> writes:

> From: Kashyap Chamarthy <kchamart@redhat.com>
>
> When QEMU exposes a VirtIO-RNG device to the guest, that device needs a
> source of entropy, and that source needs to be "non-blocking", like
> `/dev/urandom`.  However, currently QEMU defaults to the problematic
> `/dev/random`, which on linux is "blocking" (as in, it waits until
> sufficient entropy is available).
>
> Why prefer `/dev/urandom` over `/dev/random`?
> ---------------------------------------------
>
> The man pages of urandom(4) and random(4) state:
>
>     "The /dev/random device is a legacy interface which dates back to a
>     time where the cryptographic primitives used in the implementation
>     of /dev/urandom were not widely trusted.  It will return random
>     bytes only within the estimated number of bits of fresh noise in the
>     entropy pool, blocking if necessary.  /dev/random is suitable for
>     applications that need high quality randomness, and can afford
>     indeterminate delays."
>
> Further, the "Usage" section of the said man pages state:
>
>     "The /dev/random interface is considered a legacy interface, and
>     /dev/urandom is preferred and sufficient in all use cases, with the
>     exception of applications which require randomness during early boot
>     time; for these applications, getrandom(2) must be used instead,
>     because it will block until the entropy pool is initialized.
>
>     "If a seed file is saved across reboots as recommended below (all
>     major Linux distributions have done this since 2000 at least), the
>     output is cryptographically secure against attackers without local
>     root access as soon as it is reloaded in the boot sequence, and
>     perfectly adequate for network encryption session keys.  Since reads
>     from /dev/random may block, users will usually want to open it in
>     nonblocking mode (or perform a read with timeout), and provide some
>     sort of user notification if the desired entropy is not immediately
>     available."
>
> And refer to random(7) for a comparison of `/dev/random` and
> `/dev/urandom`.
>
> What about other OSes?
> ----------------------
>
> `/dev/urandom` exists and works on OS-X, FreeBSD, DragonFlyBSD, NetBSD
> and OpenBSD, which cover all the non-Linux platforms we explicitly
> support, aside from Windows.
>
> On Windows `/dev/random` doesn't work either so we don't regress.
> This is actually another argument in favour of using the newly
> proposed 'rng-builtin' backend by default, as that will work on
> Windows.
>
>     - - -
>
> Given the above, change the entropy source for VirtIO-RNG device to
> `/dev/urandom`.
>
> Related discussion in these[1][2] past threads.
>
> [1] https://lists.nongnu.org/archive/html/qemu-devel/2018-06/msg08335.html
>     -- "RNG: Any reason QEMU doesn't default to `/dev/urandom`?"
> [2] https://lists.nongnu.org/archive/html/qemu-devel/2018-09/msg02724.html
>     -- "[RFC] Virtio RNG: Consider changing the default entropy source to
>        /dev/urandom"
>
> Signed-off-by: Kashyap Chamarthy <kchamart@redhat.com>
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>

Reviewed-by: Markus Armbruster <armbru@redhat.com>

Re: [Qemu-devel] [PATCH v4 2/3] rng-builtin: add an RNG backend that uses qemu_guest_getrandom()

[Markus Armbruster]

Laurent Vivier <lvivier@redhat.com> writes:

> Add a new RNG backend using QEMU builtin getrandom function.
>
> It can be created and used with something like:
>
>     ... -object rng-builtin,id=rng0 -device virtio-rng,rng=rng0 ...
>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
>  backends/Makefile.objs |  2 +-
>  backends/rng-builtin.c | 56 ++++++++++++++++++++++++++++++++++++++++++
>  qemu-options.hx        | 10 +++++++-
>  3 files changed, 66 insertions(+), 2 deletions(-)
>  create mode 100644 backends/rng-builtin.c
>
> diff --git a/backends/Makefile.objs b/backends/Makefile.objs
> index 981e8e122f2c..f0691116e86e 100644
> --- a/backends/Makefile.objs
> +++ b/backends/Makefile.objs
> @@ -1,4 +1,4 @@
> -common-obj-y += rng.o rng-egd.o
> +common-obj-y += rng.o rng-egd.o rng-builtin.o
>  common-obj-$(CONFIG_POSIX) += rng-random.o
>  
>  common-obj-$(CONFIG_TPM) += tpm.o
> diff --git a/backends/rng-builtin.c b/backends/rng-builtin.c
> new file mode 100644
> index 000000000000..b1264b745407
> --- /dev/null
> +++ b/backends/rng-builtin.c
> @@ -0,0 +1,56 @@
> +/*
> + * 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.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "sysemu/rng.h"
> +#include "qapi/error.h"
> +#include "qapi/qmp/qerror.h"

Two superfluous includes, please drop them.

> +#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 {
> +    RngBackend parent;
> +} RngBuiltin;
> +
> +static void rng_builtin_request_entropy(RngBackend *b, RngRequest *req)
> +{
> +    RngBuiltin *s = RNG_BUILTIN(b);
> +
> +    while (!QSIMPLEQ_EMPTY(&s->parent.requests)) {
> +        RngRequest *req = QSIMPLEQ_FIRST(&s->parent.requests);
> +
> +        qemu_guest_getrandom_nofail(req->data, req->size);
> +
> +        req->receive_entropy(req->opaque, req->data, req->size);
> +
> +        rng_backend_finalize_request(&s->parent, req);
> +    }
> +}
> +
> +static void rng_builtin_class_init(ObjectClass *klass, void *data)
> +{
> +    RngBackendClass *rbc = RNG_BACKEND_CLASS(klass);
> +
> +    rbc->request_entropy = rng_builtin_request_entropy;
> +}
> +
> +static const TypeInfo rng_builtin_info = {
> +    .name = TYPE_RNG_BUILTIN,
> +    .parent = TYPE_RNG_BACKEND,
> +    .instance_size = sizeof(RngBuiltin),
> +    .class_init = rng_builtin_class_init,
> +};
> +
> +static void register_types(void)
> +{
> +    type_register_static(&rng_builtin_info);
> +}
> +
> +type_init(register_types);
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 4df0ea3aed5c..6ab920f12be4 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -4280,13 +4280,21 @@ other options.
>  
>  The @option{share} boolean option is @var{on} by default with memfd.
>  
> +@item -object rng-builtin,id=@var{id}
> +
> +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.
> +
>  @item -object rng-random,id=@var{id},filename=@var{/dev/random}
>  
>  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}.
> +entropy from and if omitted defaults to @option{/dev/urandom}. By default,
> +the @option{virtio-rng} device uses this RNG backend.
>  
>  @item -object rng-egd,id=@var{id},chardev=@var{chardevid}

I'd drop the change to rng-random's documentation.

With at least the includes tidied up:
Reviewed-by: Markus Armbruster <armbru@redhat.com>

[Qemu-devel] [PATCH 3.5/4] virtio-rng: Keep the default backend out of VirtIORNGConf

[Markus Armbruster]

The default backend is only used within virtio_rng_device_realize().
Replace VirtIORNGConf member default_backend by a local variable.
Adjust its type to reduce conversions.

While there, pass &error_abort instead of NULL when failure would be a
programming error.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/virtio/virtio-rng.c         | 20 +++++++++-----------
 include/hw/virtio/virtio-rng.h |  2 --
 2 files changed, 9 insertions(+), 13 deletions(-)

diff --git a/hw/virtio/virtio-rng.c b/hw/virtio/virtio-rng.c
index 30493a2586..73ffb476e0 100644
--- a/hw/virtio/virtio-rng.c
+++ b/hw/virtio/virtio-rng.c
@@ -16,6 +16,7 @@
 #include "hw/virtio/virtio.h"
 #include "hw/virtio/virtio-rng.h"
 #include "sysemu/rng.h"
+#include "sysemu/rng-random.h"
 #include "qom/object_interfaces.h"
 #include "trace.h"
 
@@ -189,27 +190,24 @@ 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));
+        Object *default_backend = object_new(TYPE_RNG_RANDOM);
 
-        user_creatable_complete(USER_CREATABLE(vrng->conf.default_backend),
+        user_creatable_complete(USER_CREATABLE(default_backend),
                                 &local_err);
         if (local_err) {
             error_propagate(errp, local_err);
-            object_unref(OBJECT(vrng->conf.default_backend));
+            object_unref(default_backend);
             return;
         }
 
-        object_property_add_child(OBJECT(dev),
-                                  "default-backend",
-                                  OBJECT(vrng->conf.default_backend),
-                                  NULL);
+        object_property_add_child(OBJECT(dev), "default-backend",
+                                  default_backend, &error_abort);
 
         /* The child property took a reference, we can safely drop ours now */
-        object_unref(OBJECT(vrng->conf.default_backend));
+        object_unref(default_backend);
 
-        object_property_set_link(OBJECT(dev),
-                                 OBJECT(vrng->conf.default_backend),
-                                 "rng", NULL);
+        object_property_set_link(OBJECT(dev), default_backend,
+                                 "rng", &error_abort);
     }
 
     vrng->rng = vrng->conf.rng;
diff --git a/include/hw/virtio/virtio-rng.h b/include/hw/virtio/virtio-rng.h
index 922dce7cac..28ff752c40 100644
--- a/include/hw/virtio/virtio-rng.h
+++ b/include/hw/virtio/virtio-rng.h
@@ -13,7 +13,6 @@
 #define QEMU_VIRTIO_RNG_H
 
 #include "sysemu/rng.h"
-#include "sysemu/rng-random.h"
 #include "standard-headers/linux/virtio_rng.h"
 
 #define TYPE_VIRTIO_RNG "virtio-rng-device"
@@ -26,7 +25,6 @@ struct VirtIORNGConf {
     RngBackend *rng;
     uint64_t max_bytes;
     uint32_t period_ms;
-    RngRandom *default_backend;
 };
 
 typedef struct VirtIORNG {
-- 
2.17.2

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

[Markus Armbruster]

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)
>  {

Rebase on top of the PATCH 3.5 I just posted gets rid of patch hunks
from here...

> 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);
> 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;
>  };
>  
>  typedef struct VirtIORNG {

... until here.

> 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.
>  
>  @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}

The last hunk goes away if you take my suggestion on PATCH 2.

Preferably simplified along these lines:
Reviewed-by: Markus Armbruster <armbru@redhat.com>

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

[Laurent Vivier]

On 14/05/2019 17:24, Markus Armbruster wrote:
> 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)
>>   {
> 
> Rebase on top of the PATCH 3.5 I just posted gets rid of patch hunks
> from here...
> 

I will resend the series including your patch.

Thanks,
Laurent