On Tue, Mar 30, 2021 at 12:17:40PM +0200, Greg Kurz wrote:
> On Mon, 29 Mar 2021 18:10:57 +0100
> Stefan Hajnoczi <stefanha@redhat.com> wrote:
> > On Thu, Mar 25, 2021 at 04:07:30PM +0100, Greg Kurz wrote:
> > > @@ -315,6 +338,10 @@ static void virtio_bus_unset_and_cleanup_host_notifiers(VirtioBusState *bus,
> > >  
> > >      for (i = 0; i < nvqs; i++) {
> > >          virtio_bus_set_host_notifier(bus, i + n_offset, false);
> > > +    }
> > > +    /* Let address_space_update_ioeventfds() run before closing ioeventfds */
> > 
> > assert(memory_region_transaction_depth == 0)?
> > 
> 
> Hmm... appart from the fact that memory_region_transaction_depth is
> a memory internal thing that shouldn't be exposed here, it seems to
> me that memory_region_transaction_depth can be != 0 when, e.g. when
> batching is used... or I'm missing something ?
> 
> I was actually thinking of adding some asserts for that in the
> memory_region_*_eventfd_full() functions introduced by patch 1.
> 
>     if (!transaction) {
>         memory_region_transaction_begin();
>     }
>     assert(memory_region_transaction_depth != 0);

In that case is it safe to call virtio_bus_cleanup_host_notifier()
below? I thought it depends on the transaction committing first.

> 
> > > +    virtio_bus_set_host_notifier_commit(bus);
> > > +    for (i = 0; i < nvqs; i++) {
> > >          virtio_bus_cleanup_host_notifier(bus, i + n_offset);
> > >      }
> > >  }