On Tue, Feb 18, 2020 at 01:07:07PM +0800, Coiby Xu wrote:
> +static bool vu_message_read(VuDev *dev, int conn_fd, VhostUserMsg *vmsg)
> +{
> + vu_read_msg_cb read_msg;
> + if (dev->co_iface) {
> + read_msg = dev->co_iface->read_msg;
> + } else {
> + read_msg = vu_message_read_;
> + }
> + return read_msg(dev, conn_fd, vmsg);
> +}
libvhost-user is already partially asynchronous (->set/remove_watch()
are used for eventfds).
Can you make the vhost-user socket I/O asynchronous too?
> +
> static bool
> vu_message_write(VuDev *dev, int conn_fd, VhostUserMsg *vmsg)
> {
> @@ -1075,9 +1084,14 @@ vu_set_vring_kick_exec(VuDev *dev, VhostUserMsg *vmsg)
> }
>
> if (dev->vq[index].kick_fd != -1 && dev->vq[index].handler) {
> + if (dev->set_watch_packed_data) {
> + dev->set_watch_packed_data(dev, dev->vq[index].kick_fd,
> VU_WATCH_IN,
> + dev->co_iface->kick_callback,
> + (void *)(long)index);
> + } else {
> dev->set_watch(dev, dev->vq[index].kick_fd, VU_WATCH_IN,
> vu_kick_cb, (void *)(long)index);
Why is it necessary to replace vu_kick_cb()?
The user can set a custom vq->handler() function with
vu_set_queue_handler().
Coroutines aren't needed for this part since eventfd_read() always
returns right away.
> @@ -1627,6 +1647,12 @@ vu_deinit(VuDev *dev)
> }
>
> if (vq->kick_fd != -1) {
> + /* remove watch for kick_fd
> + * When client process is running in gdb and
> + * quit command is run in gdb, QEMU will still dispatch the event
> + * which will cause segment fault in the callback function
> + */
Code and comments in libvhost-user should not refer to QEMU specifics.
Removing the watch is a good idea regardless of the application or event
loop implementation. No comment is needed here.
> + dev->remove_watch(dev, vq->kick_fd);
> close(vq->kick_fd);
> vq->kick_fd = -1;
> }
> @@ -1682,7 +1708,7 @@ vu_init(VuDev *dev,
>
> assert(max_queues > 0);
> assert(socket >= 0);
> - assert(set_watch);
> + /* assert(set_watch); */
?
> @@ -372,7 +389,8 @@ struct VuDev {
> /* @set_watch: add or update the given fd to the watch set,
> * call cb when condition is met */
> vu_set_watch_cb set_watch;
> -
> + /* AIO dispatch will only one data pointer to callback function */
I don't understand what this comment is saying.
signature.asc
Description: PGP signature
