Re: [Qemu-devel] [PATCH 1/1] rng backend: open backend in blocking mode

[Anthony Liguori <anthony@codemonkey.ws>]

Amit Shah <amit.shah@redhat.com> writes:

> Opening backends in non-blocking mode isn't necessary, we don't do
> anything while waiting for data.
>
> This also excuses us from checking for EAGAIN, which for the default
> random backend, is a very common return error type.

It's not common...  It really shouldn't happen however.

>  Starting the guest
> with '-device virtio-rng-pci', issuing a 'cat /dev/hwrng' in the guest
> while also doing 'cat /dev/random' on the host causes

You are essentially cat'ing the same device twice.  What's happening is
that there is entropy available in /dev/random so a select()
notification happens but before we are able to read() it, the cat of
/dev/hwrng ends up consuming that entropy.

This would never happen with a socket, for instance.  /dev/random is
special in there are multiple readers.

>
> backends/rng-random.c:44:entropy_available: assertion failed: (len != -1)
>
> without this fix.

This fix would cause QEMU to block indefinitely which I don't think is
very good behavior.  I think a better solution would be:

diff --git a/backends/rng-random.c b/backends/rng-random.c
index acd20af..9fde566 100644
--- a/backends/rng-random.c
+++ b/backends/rng-random.c
@@ -41,6 +41,9 @@ static void entropy_available(void *opaque)
     ssize_t len;
 
     len = read(s->fd, buffer, s->size);
+    if (len == -1 && errno == EINTR) {
+        return;
+    }
     g_assert(len != -1);
 
     s->receive_func(s->opaque, buffer, len);

Since this simply ignores the extraneous select notification that occurs
because of the race above.

Regards,

Anthony Liguori

>
> Reported-by: yunpingzheng <yunzheng@redhat.com>
> Signed-off-by: Amit Shah <amit.shah@redhat.com>
> ---
>  backends/rng-random.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/backends/rng-random.c b/backends/rng-random.c
> index acd20af..252139b 100644
> --- a/backends/rng-random.c
> +++ b/backends/rng-random.c
> @@ -74,7 +74,7 @@ static void rng_random_opened(RngBackend *b, Error **errp)
>          error_set(errp, QERR_INVALID_PARAMETER_VALUE,
>                    "filename", "a valid filename");
>      } else {
> -        s->fd = qemu_open(s->filename, O_RDONLY | O_NONBLOCK);
> +        s->fd = qemu_open(s->filename, O_RDONLY);
>  
>          if (s->fd == -1) {
>              error_set(errp, QERR_OPEN_FILE_FAILED, s->filename);
> -- 
> 1.8.1.4