Re: [Qemu-devel] [PATCH 2/2 v3] virtio-rng: Serve pending request if any after timer bumps up quota.

[Pankaj Gupta <pagupta@redhat.com>]

----- Original Message -----
> From: "Amit Shah" <amit.shah@redhat.com>
> To: "Pankaj Gupta" <pagupta@redhat.com>
> Cc: qemu-devel@nongnu.org, mst@redhat.com
> Sent: Thursday, 16 July, 2015 11:35:36 AM
> Subject: Re: [Qemu-devel] [PATCH 2/2 v3] virtio-rng: Serve pending request if any after timer bumps up quota.
> 
> On (Wed) 15 Jul 2015 [17:46:48], Pankaj Gupta wrote:
> > We are arming timer when we get first request from guest.
> > Even if guest pulls all the data we will be serving guest
> > only when timer bumps up new quota. When timer expires
> > we check if we have a pending request from guest, we
> > serve it and re-arm the timer else we don't do any thing.
> > 
> > Signed-off-by: Pankaj Gupta <pagupta@redhat.com>
> > ---
> >  hw/virtio/virtio-rng.c | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> > 
> > diff --git a/hw/virtio/virtio-rng.c b/hw/virtio/virtio-rng.c
> > index ae04352..3d9a002 100644
> > --- a/hw/virtio/virtio-rng.c
> > +++ b/hw/virtio/virtio-rng.c
> > @@ -142,9 +142,13 @@ static int virtio_rng_load(QEMUFile *f, void *opaque,
> > int version_id)
> >  static void check_rate_limit(void *opaque)
> >  {
> >      VirtIORNG *vrng = opaque;
> > +    size_t size;
> >  
> >      vrng->quota_remaining = vrng->conf.max_bytes;
> > -    virtio_rng_process(vrng);
> > +    size = get_request_size(vrng->vq, 0);
> > +    if (size > 0) {
> > +        virtio_rng_process(vrng);
> > +    }
> >      vrng->activate_timer = true;
> 
> Ah, this isn't helping us much here; we might as well return in a
> similar way from virtio_rng_process.
> 
> What I had written earlier was slightly different than this:
> 
> > So now with your changes, here is what we can do: instead of just
> > activating the timer in check_rate_limit(), we can issue a
> > get_request_size() call.  If the return value is > 0, it means we have
> > a buffer queued up by the guest, and we can then call
> > virtio_rng_process() and set activated to true.  Else, no need to call
> > virtio_rng_process at all, and the job for the timer is done, since
> > there are no more outstanding requests from the guest.
> 
> Anyway we can look at that later, patch 1 is already a big improvement
> so I think we should just stick with that, and think about other
> things in a different series.

Sure.

Thanks,
Pankaj
> 
> Thanks,
> 
> 
>                 Amit
> 
>