[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v3 1/1] virtio-rng: hardware random number gener

From: Anthony Liguori
Subject: Re: [Qemu-devel] [PATCH v3 1/1] virtio-rng: hardware random number generator device
Date: Mon, 25 Jun 2012 07:22:13 -0500
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:11.0) Gecko/20120329 Thunderbird/11.0.1

On 06/25/2012 07:10 AM, Daniel P. Berrange wrote:
On Fri, Jun 22, 2012 at 02:59:13PM -0500, Anthony Liguori wrote:
On 06/22/2012 01:50 PM, Amit Shah wrote:
On (Fri) 22 Jun 2012 [08:44:52], Anthony Liguori wrote:
On 06/22/2012 08:34 AM, Daniel P. Berrange wrote:

Oh, that's a good point.

But easily fixed.

Of course, except that now we have to maintain compatibility so some
hideous hack goes in.

This is what fundamentally makes using events a bad approach.  There
are more problems lurking.  This is the fundamental complexity of
having two non-synchronized communication channels when you're
trying to synchronize some sort of activity.

BTW, despite what danpb mentioned, you are rate limiting entropy
requests in this patch series....

All of these problems are naturally solved using a protocol over a 

Can we at least agree on merging a patch which just includes the
raw chardev backend support for virtio-rng ? ie drop the QMP
event for now, so we can make some step forward.

All we need to do to support EGD is instead of doing:

+    QObject *data;
+    data = qobject_from_jsonf("{ 'bytes': %" PRId64 " }",
+                              size);
+    monitor_protocol_event(QEVENT_ENTROPY_NEEDED, data);
+    qobject_decref(data);


     while (size>   0) {
         uint8_t partial_size = MIN(255, size);
         uint8_t msg[2] = { 0x02, partial_size };

         qemu_chr_write(s->chr, msg, sizeof(msg));

         size -= partial_size;

And that's it.  It's an extremely simple protocol to support.  It
would actually reduce the total size of the patch.

So I now get what your objection is, and that is because of an
assumption you're making which is incorrect.

An OS asking for 5038 bytes of entropy is doing just that: asking for
those bytes.  That doesn't mean a hardware device can provide it with
that much entropy.  So, the hardware device here will just provide
whatever is available, and the OS has to be happy with what it got.
If it got 200 bytes from the device, the OS is then free to demand
even 7638 bytes more, but it may not get anything for quite a while.

(This is exactly how things work with /dev/random and /dev/urandom
too, btw.  And /dev/urandom was invented for apps which can't wait for
all the data they're asking to come to them.)

As it turns out, the EGD protocol also has a mechanism to ask for
ask much entropy as is available at the current moment.  It also has
a mechanism to query the amount of available entropy.

And no, you're assertion that we don't care about how much entropy
the guest is requesting is wrong.  If we lose entropy requests, then
we never know if the guest is in a state where it's expecting
entropy and we're not sending it.


- Guest requests entropy (X bytes)
- libvirt sees request
- libvirt sends X bytes to guest
- Guest requests entropy (Y bytes), QEMU filters due to rate limiting

The guest will never request entropy again and libvirt will never
send entropy again.  The device is effectively dead-locked.

WRT the impl of rate limiting Amit has used in this patch, you
are correct, however, this is not how QEMU should be rate
limiting this event. As mentioned in an earlier reply in this
thread, any rate limited /must/ work as follows:

    - Guest sends randomness request for 10 bytes
    - QMP event gets sent for 10 bytes
    - Guest sends randomness request for 4 bytes
    - QMP is dropped
    - Guest sends randomness request for 8 bytes
    - QMP event gets sent for 12 bytes

ie, the byte count for any events which are dropped, must be added to
the byte count in the next emitted event. Also as Amit mentioned in his
reply to me, it should take account of multiple devices, or we should
limit QEMU to 1 single RNG device per guest, as we do for the balloon

QMP events are not meant to be used like this. They are posted events and since we cannot know if there is something connected to the monitor, it's always possible for the backend to lose the information associated with an event.

Events really should just be used to indicate, "hey, you should go query information from QEMU now".

Even what you suggest above doesn't handle the case where a management application restarts.

If you were going to do this via QMP, you'd want to introduce a command to query the total outstanding requested entropy for a given device and then send a event whenever that value changes.

But again, it's silly to use QMP for this. Using an inline protocol simplifies things quite a bit.


Anthony Liguori


reply via email to

[Prev in Thread] Current Thread [Next in Thread]