qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v9 2/5] generic vhost user server


From: Coiby Xu
Subject: Re: [PATCH v9 2/5] generic vhost user server
Date: Mon, 17 Aug 2020 16:24:53 +0800

On Fri, Jun 19, 2020 at 01:13:00PM +0100, Stefan Hajnoczi wrote:
On Mon, Jun 15, 2020 at 02:39:04AM +0800, Coiby Xu wrote:
+/*
+ * a wrapper for vu_kick_cb
+ *
+ * since aio_dispatch can only pass one user data pointer to the
+ * callback function, pack VuDev and pvt into a struct. Then unpack it
+ * and pass them to vu_kick_cb
+ */
+static void kick_handler(void *opaque)
+{
+    KickInfo *kick_info = opaque;
+    kick_info->cb(kick_info->vu_dev, 0, (void *) kick_info->index);

Where is kick_info->index assigned? It appears to be NULL in all cases.

+}
+
+
+static void
+set_watch(VuDev *vu_dev, int fd, int vu_evt,
+          vu_watch_cb cb, void *pvt)
+{
+
+    VuServer *server = container_of(vu_dev, VuServer, vu_dev);
+    g_assert(vu_dev);
+    g_assert(fd >= 0);
+    long index = (intptr_t) pvt;

The meaning of the pvt argument is not defined in the library interface.
set_watch() callbacks shouldn't interpret pvt.

You could modify libvhost-user to explicitly pass the virtqueue index
(or -1 if the fd is not associated with a virtqueue), but it's nice to
avoid libvhost-user API changes so that existing libvhost-user
applications don't require modifications.

What I would do here is to change the ->kick_info[] data struct. How
about a linked list of VuFdWatch objects? That way the code can handle
any number of fd watches and doesn't make assumptions about virtqueues.
set_watch() is a generic fd monitoring interface and doesn't need to be
tied to virtqueues.

A linked list of VuFdWatch objects has been adopted in v10. Thank you!

--
Best regards,
Coiby



reply via email to

[Prev in Thread] Current Thread [Next in Thread]