From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46828) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aRboH-0007My-PS for qemu-devel@nongnu.org; Fri, 05 Feb 2016 03:32:10 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aRboC-0005Bh-Ot for qemu-devel@nongnu.org; Fri, 05 Feb 2016 03:32:09 -0500 Received: from mail-wm0-x22f.google.com ([2a00:1450:400c:c09::22f]:38866) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aRboC-0005BQ-HT for qemu-devel@nongnu.org; Fri, 05 Feb 2016 03:32:04 -0500 Received: by mail-wm0-x22f.google.com with SMTP id p63so16309972wmp.1 for ; Fri, 05 Feb 2016 00:32:04 -0800 (PST) Sender: Paolo Bonzini References: <1453465198-11000-1-git-send-email-lprosek@redhat.com> <20160203123639.GA20527@grmbl.mre> <56B24A9C.9010104@redhat.com> <725955967.32271042.1454606645414.JavaMail.zimbra@redhat.com> <65843372.32316615.1454609255745.JavaMail.zimbra@redhat.com> From: Paolo Bonzini Message-ID: <56B45E01.3090203@redhat.com> Date: Fri, 5 Feb 2016 09:32:01 +0100 MIME-Version: 1.0 In-Reply-To: <65843372.32316615.1454609255745.JavaMail.zimbra@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH] rng-random: implement request queue List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Ladi Prosek Cc: Amit Shah , pagupta@redhat.com, qemu-devel@nongnu.org >>> It is already migrated as part of virtio_rng_save's call to virtio_save. >>> On the loading side, virtio_rng_process condenses all requests into one >>> and chr_read fills in as many virtqueue buffers as possible from the >>> single request. >> >> Thanks! So this looks broken. The contract between virtio-rng and the rng >> backend is "one chr_read callback per one rng_backend_request_entropy", >> regardless of the request size. It guarantees that chr_read will get at >> least one byte, nothing else. If the one condensed request is bigger than >> what the backend is able to give at the moment, there may be unsatisfied >> virtio buffers left in the queue. >> >> One way of fixing this would be to have chr_read call virtio_rng_process >> if it ran out of backend data but not out of virtio buffers. Otherwise >> those buffers will be slowly drained by the rate limit timer, which is >> not optimal (especially in the absence of rate limit :) Nice observation! And the commit you mention below seems to be the right way to fix it indeed. Amit, do you recall why this was done? > Basically, we could just revert this commit: > > commit 4621c1768ef5d12171cca2aa1473595ecb9f1c9e > Author: Amit Shah > Date: Wed Nov 21 11:21:19 2012 +0530 > > virtio-rng: remove extra request for entropy > > If we got fewer bytes from the backend than requested, don't poke the > backend for more bytes; the guest will ask for more (or if the guest has > already asked for more, the backend knows about it via handle_input() I've now reviewed your patch more carefully, I will make some comments in reply to the patch directly. Paolo