On 11/03/2010 05:47 PM, Adam Litke wrote:
On Wed, 2010-11-03 at 10:27 -0500, Michael Roth wrote:
+/* mirror qemu I/O-related code for standalone daemon */
+typedef struct IOHandlerRecord {
+    int fd;
+    IOCanReadHandler *fd_read_poll;
+    IOHandler *fd_read;
+    IOHandler *fd_write;
+    int deleted;
+    void *opaque;
+    /* temporary data */
+    struct pollfd *ufd;
+    QLIST_ENTRY(IOHandlerRecord) next;
+} IOHandlerRecord;

As you say, this is exactly the same structure as defined in vl.c.  If
you copy and paste code for a new feature you should be looking for a
way to share it instead.  Why not create a separate patch that defines
this structure in vl.h which can then be included by vl.c and virtproxy
code.

+static QLIST_HEAD(, IOHandlerRecord) io_handlers =
+    QLIST_HEAD_INITIALIZER(io_handlers);
+
+int vp_set_fd_handler2(int fd,
+                         IOCanReadHandler *fd_read_poll,
+                         IOHandler *fd_read,
+                         IOHandler *fd_write,
+                         void *opaque)
+{
+    IOHandlerRecord *ioh;
+
+    if (!fd_read&&  !fd_write) {
+        QLIST_FOREACH(ioh,&io_handlers, next) {
+            if (ioh->fd == fd) {
+                ioh->deleted = 1;
+                break;
+            }
+        }
+    } else {
+        QLIST_FOREACH(ioh,&io_handlers, next) {
+            if (ioh->fd == fd)
+                goto found;
+        }
+        ioh = qemu_mallocz(sizeof(IOHandlerRecord));
+        QLIST_INSERT_HEAD(&io_handlers, ioh, next);
+    found:
+        ioh->fd = fd;
+        ioh->fd_read_poll = fd_read_poll;
+        ioh->fd_read = fd_read;
+        ioh->fd_write = fd_write;
+        ioh->opaque = opaque;
+        ioh->deleted = 0;
+    }
+    return 0;
+}

Copied from vl.c -- export it instead.

+int vp_set_fd_handler(int fd,
+                        IOHandler *fd_read,
+                        IOHandler *fd_write,
+                        void *opaque)
+{
+    return vp_set_fd_handler2(fd, NULL, fd_read, fd_write, opaque);
+}
+
+int vp_send_all(int fd, const void *buf, int len1)
+{
+    int ret, len;
+
+    len = len1;
+    while (len>  0) {
+        ret = write(fd, buf, len);
+        if (ret<  0) {
+            if (errno != EINTR&&  errno != EAGAIN) {
+                warn("write() failed");
+                return -1;
+            }
+        } else if (ret == 0) {
+            break;
+        } else {
+            buf += ret;
+            len -= ret;
+        }
+    }
+    return len1 - len;
+}

Copied from qemu-char.c -- Share please.

+static void main_loop_wait(int nonblocking)
+{
+    IOHandlerRecord *ioh;
+    fd_set rfds, wfds, xfds;
+    int ret, nfds;
+    struct timeval tv;
+    int timeout = 1000;
+
+    if (nonblocking) {
+        timeout = 0;
+    }
+
+    /* poll any events */
+    nfds = -1;
+    FD_ZERO(&rfds);
+    FD_ZERO(&wfds);
+    FD_ZERO(&xfds);
+    QLIST_FOREACH(ioh,&io_handlers, next) {
+        if (ioh->deleted)
+            continue;
+        if (ioh->fd_read&&
+            (!ioh->fd_read_poll ||
+             ioh->fd_read_poll(ioh->opaque) != 0)) {
+            FD_SET(ioh->fd,&rfds);
+            if (ioh->fd>  nfds)
+                nfds = ioh->fd;
+        }
+        if (ioh->fd_write) {
+            FD_SET(ioh->fd,&wfds);
+            if (ioh->fd>  nfds)
+                nfds = ioh->fd;
+        }
+    }
+
+    tv.tv_sec = timeout / 1000;
+    tv.tv_usec = (timeout % 1000) * 1000;
+
+    ret = select(nfds + 1,&rfds,&wfds,&xfds,&tv);
+
+    if (ret>  0) {
+        IOHandlerRecord *pioh;
+
+        QLIST_FOREACH_SAFE(ioh,&io_handlers, next, pioh) {
+            if (ioh->deleted) {
+                QLIST_REMOVE(ioh, next);
+                qemu_free(ioh);
+                continue;
+            }
+            if (ioh->fd_read&&  FD_ISSET(ioh->fd,&rfds)) {
+                ioh->fd_read(ioh->opaque);
+            }
+            if (ioh->fd_write&&  FD_ISSET(ioh->fd,&wfds)) {
+                ioh->fd_write(ioh->opaque);
+            }
+        }
+    }
+

This resembles vl.c's main_loop_wait() but I can see why you might want
your own.  There is opportunity for sharing the select logic and ioh
callbacks but I think that could be addressed later.


Yup these are all basically copy/pastes from vl.c. It feels a bit dirty, but I modeled things after the other qemu tools like qemu-nbd/qemu-io, which don't link against vl.c (and adding a target for tools to do so looks like it'd be a bit hairy since vl.c touches basically everything).

It might still make sense to share things like structs...but the ones I'm stealing here are specific to reproducing the main_loop_wait logic. So I guess the real question is whether main_loop_wait and friends make sense to expose as a utility function of some sort, and virtproxy seems to be the only use case so far.

Reply via email to