On Tue, Apr 11, 2017 at 07:42:02PM +0200, Laurent Vivier wrote:
> On 11/04/2017 19:02, Stefan Hajnoczi wrote:
> > On Tue, Apr 11, 2017 at 03:17:33PM +0200, Laurent Vivier wrote:
> >> diff --git a/hw/virtio/virtio-rng.c b/hw/virtio/virtio-rng.c
> >> index 9639f4e..d270d56 100644
> >> --- a/hw/virtio/virtio-rng.c
> >> +++ b/hw/virtio/virtio-rng.c
> >> @@ -53,6 +53,15 @@ static void chr_read(void *opaque, const void *buf, size_t size)
> >>          return;
> >>      }
> >>  
> >> +    /* we can't modify the virtqueue until
> >> +     * our state is fully synced
> >> +     */
> >> +
> >> +    if (!runstate_check(RUN_STATE_RUNNING)) {
> >> +        trace_virtio_rng_cpu_is_stopped(vrng);
> >> +        return;
> >> +    }
> >> +
> > 
> > I'm concerned about what happens when the guest is stopped and resumed
> > (e.g. 'stop' and 'cont' monitor commands).  Since we throw away the
> > chr_read() callback the device will hang unless the guest kicks it
> > again?
> > 
> > It's not clear to me that the rate limit timer will help us...
> 
> I think you're right (even if it seems hard to generate this case)
> 
> What is the best solution:
> 
> - re-arming the timer/the backend request by calling
> virtio_rng_process() before the "return;"

This would waste CPU and throw away entropy bits.

> or
> 
> - adding a vmstate change handler to call virtio_rng_process()?

Yes, I think vm change handlers solve the problem.