Amit Shah <amit.s...@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 <yunzh...@redhat.com>
> Signed-off-by: Amit Shah <amit.s...@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
