From: Amit Shah <amit.shah@redhat.com>
To: Ladi Prosek <lprosek@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>,
qemu-devel@nongnu.org, pagupta@redhat.com
Subject: Re: [Qemu-devel] [PATCH] rng-random: implement request queue
Date: Thu, 3 Mar 2016 10:35:38 +0530 [thread overview]
Message-ID: <20160303050538.GD15443@grmbl.mre> (raw)
In-Reply-To: <65843372.32316615.1454609255745.JavaMail.zimbra@redhat.com>
On (Thu) 04 Feb 2016 [13:07:35], Ladi Prosek wrote:
> ----- Original Message -----
> > ----- Original Message -----
> > >
> > >
> > > On 03/02/2016 13:36, Amit Shah wrote:
> > > > ... and this can lead to breaking migration (the queue of requests on
> > > > the host needs to be migrated, else the new host will have no idea of
> > > > the queue).
> > >
> > > 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 :)
>
> Basically, we could just revert this commit:
>
> commit 4621c1768ef5d12171cca2aa1473595ecb9f1c9e
> Author: Amit Shah <amit.shah@redhat.com>
> 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()
OK, agreed.
It makes sense to revert this so the live migration scenario is sane.
Without live migration reverting this commit doesn't get us any
benefit, and that's why I had removed it.
Do you want to post a patch reverting this too?
Thanks,
Amit
next prev parent reply other threads:[~2016-03-03 5:05 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-01-22 12:19 [Qemu-devel] [PATCH] rng-random: implement request queue Ladi Prosek
2016-02-03 12:36 ` Amit Shah
2016-02-03 18:02 ` Ladi Prosek
2016-02-03 18:44 ` Paolo Bonzini
2016-02-04 8:53 ` Pankaj Gupta
2016-02-04 17:36 ` Ladi Prosek
2016-02-05 5:31 ` Pankaj Gupta
2016-02-04 17:24 ` Ladi Prosek
2016-02-04 18:07 ` Ladi Prosek
2016-02-05 8:32 ` Paolo Bonzini
2016-03-03 5:05 ` Amit Shah [this message]
2016-03-03 9:30 ` Ladi Prosek
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20160303050538.GD15443@grmbl.mre \
--to=amit.shah@redhat.com \
--cc=lprosek@redhat.com \
--cc=pagupta@redhat.com \
--cc=pbonzini@redhat.com \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).