On 16/03/2016 13:42, Cornelia Huck wrote: > On Wed, 16 Mar 2016 13:32:59 +0100 > Paolo Bonzini <pbonz...@redhat.com> wrote: > >> On 16/03/2016 13:22, Cornelia Huck wrote: >>>>> Yeah, it doesn't help that the functions are underdocumented (as in the >>>>> "assign" parameter above). >>> My understanding is: >>> >>> - 'assign': set up a new notifier (true) or disable it (false) >>> - 'set_handler': use our handler (true) or have it handled elsewhere >>> (false) >> >> Right. So if we're setting up a new notifier in >> virtio_queue_aio_set_host_notifier_handler, virtio_pci_stop_ioeventfd should
... not call virtio_queue_host_notifier_read. >>>>> I don't think the ->set_host_notifiers() api really allows for this. >>>> >>>> I think it does, assign is the last argument to k->set_host_notifier(). >>> >>> This depends on whether we want 'assign' to clean up any old notifiers >>> before setting up new ones. I think we want different behaviour for >>> dataplane and vhost. >> >> I think dataplane and vhost are the same. >> >> The question is whether ioeventfd=off,vhost=on or >> ioeventfd=off,dataplane=on are valid combinations; I think they aren't. > > We should disallow that even temporary, then. > > (This would imply that we need to drop the _stop_ioeventfd() call in > ->set_host_notifier(), correct?) No, it would not. ioeventfd=off,vhost=on would mean: "when vhost is off, use vCPU thread notification". When turning on vhost you'd still stop ioeventfd (i.e. stop processing the virtqueue in QEMU's main iothread), but you don't need to do anything to the event notifier. vhost will pick it up and work on the virtqueue if necessary. Likewise for dataplane. >> If they aren't, it should be okay to remove the >> virtio_queue_host_notifier_read call in >> virtio_queue_set_host_notifier_fd_handler and >> virtio_queue_aio_set_host_notifier_handler. That's because a handler >> for the notifier will always be set _somewhere_. It could be the usual >> ioeventfd handler, the vhost handler or the dataplane handler, but one >> will be there. > > It should; but we probably need to do a final read when we stop the > ioeventfd. I was thinking of handing the final read directly to the next guy who polls the event notifier instead. So, when called from vhost or dataplane, virtio_pci_stop_ioeventfd would use assign=true/set_handler=false ("a new notifier is going to be set up by the caller"). The host notifier API unfortunately is full of indirections. I'm not sure how many of them are actually necessary. Paolo Paolo