From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:52842) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Si4AW-0002Ls-Vy for qemu-devel@nongnu.org; Fri, 22 Jun 2012 09:45:05 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Si4AU-0005bv-Uw for qemu-devel@nongnu.org; Fri, 22 Jun 2012 09:45:00 -0400 Received: from mail-pb0-f45.google.com ([209.85.160.45]:39515) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Si4AU-0005bM-Lj for qemu-devel@nongnu.org; Fri, 22 Jun 2012 09:44:58 -0400 Received: by pbbro12 with SMTP id ro12so3687324pbb.4 for ; Fri, 22 Jun 2012 06:44:55 -0700 (PDT) Message-ID: <4FE476D4.7090300@codemonkey.ws> Date: Fri, 22 Jun 2012 08:44:52 -0500 From: Anthony Liguori MIME-Version: 1.0 References: <4FE240B2.4070907@codemonkey.ws> <4FE4639B.5070805@codemonkey.ws> <20120622123123.GJ10128@redhat.com> <4FE46C0D.5070404@codemonkey.ws> <20120622133425.GK10128@redhat.com> In-Reply-To: <20120622133425.GK10128@redhat.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v3 1/1] virtio-rng: hardware random number generator device List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Daniel P. Berrange" Cc: Amit Shah , Markus Armbruster , qemu list On 06/22/2012 08:34 AM, Daniel P. Berrange wrote: > On Fri, Jun 22, 2012 at 07:58:53AM -0500, Anthony Liguori wrote: >> On 06/22/2012 07:31 AM, Daniel P. Berrange wrote: >>> On Fri, Jun 22, 2012 at 07:22:51AM -0500, Anthony Liguori wrote: >>>> On 06/22/2012 07:12 AM, Markus Armbruster wrote: >>>>> Anthony Liguori writes: >>>>>> Nack. >>>>>> >>>>>> Use a protocol. This is not what QMP events are designed for! >>>>>> >>>>>> No human is going to launch nc to a unix domain socket to launch QEMU. >>>>>> That's a silly use-case to design for. >>>>> >>>>> To be honest, I'm a bit surprised to see working code that got an ACK >>>> >from the guys with the problem it solves rejected out of hand over >>>>> something that feels like artistic license to me. >>>> >>>> This is an ABI! We have to support it for the rest of time. >>>> Everything else is a detail that is fixable but ABIs need to not >>>> suck from the beginning. >>>> >>>> And using a QMP event here is sucks. It disappoints me that this is >>>> even something I need to explain. >>>> >>>> QMP events occur over a single socket. If you trigger them from >>>> guest initiated activities (that have no intrinsic rate limit), you >>>> run into a situation where the guest could flood the management tool >>>> and/or queue infinite amounts of memory (because events have to be >>>> queued before they're sent). So we have rate limiting for QMP >>>> events. >>>> >>>> That means QMP events (like this one) are unreliable. >>> >>> No it doesn't. As it stands currently, the only events that are >>> rate limited, are those where there is no state information to >>> loose. ie, the new event completely superceeds the old event >>> without loosing any information. >>> >>>> But since QMP >>>> events aren't acked, there's no way for the management tool to know >>>> whether a QMP event was dropped or not. So you can run into the >>>> following scenario: >>>> >>>> - Guest sends randomness request for 10 bytes >>>> - QMP event gets sent for 10 bytes >>>> - Guest sends randomness request for 4 bytes >>>> - QMP is dropped >>>> >>>> Now what happens? With the current virtio-rng, nothing. It gets >>>> stuck in read() for ever. Now what do we do? >>> >>> The RNG event will not be able to use the generic rate limiting >>> since it has state associated with it. The rate limiting of the >>> RNG QMP event will need to take account of this state, ie it >>> will have to accumulate the byte count of any events dropped for >>> rate limiting: >>> >>> - 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 >> >> BTW, in the current design, there's no way to tell *which* >> virtio-rng device needs entropy if you have multiple virtio-rng >> devices. > > Oh, that's a good point. > >> All of these problems are naturally solved using a protocol over a CharDriverState. > > 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); Do: 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. > As mentioned in the previous thread, I see no issue with > later implementing an alternate protocol on the chardev > backend eg as we have raw vs telnet mode for serial ports, > we ought to be able to have a choice of raw vs egd mode for > virtio-rng backends I don't really understand how raw mode works other than reading as much from /dev/urandom as possible and filling the socket buffer buffer with it. I think the only two modes that make sense are EGD over a socket and direct open of /dev/urandom. But I think the EGD mode is the more important of the two. Regards, Anthony Liguori > Daniel