Re: [PATCH] replay: improve determinism of virtio-net

[Michael S. Tsirkin]

On Wed, Oct 20, 2021 at 02:40:18PM +0100, Alex Bennée wrote:
> 
> Pavel Dovgalyuk <pavel.dovgalyuk@ispras.ru> writes:
> 
> > On 31.05.2021 07:55, Jason Wang wrote:
> >> 在 2021/5/17 下午9:04, Pavel Dovgalyuk 写道:
> <snip>
> >>> @@ -2546,7 +2547,7 @@ static void
> >>> virtio_net_handle_tx_bh(VirtIODevice *vdev, VirtQueue *vq)
> >>>           return;
> >>>       }
> >>>       virtio_queue_set_notification(vq, 0);
> >>> -    qemu_bh_schedule(q->tx_bh);
> >>> +    replay_bh_schedule_event(q->tx_bh);
> >> Not familiar with replay but any chance to change qemu_bh_schedule()
> >> instead?
> >
> > It would be better, but sometimes qemu_bh_schedule is used for the
> > callbacks that are not related to the guest state change.
> 
> Maybe that indicates we should expand the API:
> 
>   void qemu_bh_schedule(QEMUBH *bh, bool guest_state);
> 
> or maybe explicit functions:
> 
>   void qemu_bh_schedule(QEMUBH *bh);
>   void qemu_bh_schedule_guest_update(QEMUBH *bh);
> 
> And document the cases with proper prototypes in main-loop.h.
> 
> While I was poking around I also saw qemu_bh_schedule_idle which made me
> wonder what happens if a guest change is triggered by this. Would this
> be impossible to make deterministic because we don't know when the bh
> actually get activated?
> 
> My concern with just adding replay_bh_schedule_event in the appropriate
> places is we end up with a patchwork of support for different devices
> and make it very easy to break things.

Right. In fact I think the default should be guest update,
and some kind of new API to be used for things that are
not guest visible.

We really need static analysis to enforce this kind of
constraint, too.


> -- 
> Alex Bennée