Re: PATCH: 4/10: Refactor xenfb event handlers

["Daniel P. Berrange" <berrange@redhat.com>]

This patch is a simple code re-factoring to move the event loop
integration directly into the xenfb.c file. It is to facilitate
the patches which follow.

 xen_machine_pv.c |   29 ------
 xenfb.c          |  249 +++++++++++++++++++++++--------------------------------
 xenfb.h          |    7 -
 3 files changed, 106 insertions(+), 179 deletions(-)

   Signed-off-by: Daniel P. Berrange <berrange@redhat.com>

Dan.

diff -r bd1bddd42929 tools/ioemu/hw/xen_machine_pv.c
--- a/tools/ioemu/hw/xen_machine_pv.c	Tue Aug 14 14:38:28 2007 -0400
+++ b/tools/ioemu/hw/xen_machine_pv.c	Tue Aug 14 15:04:55 2007 -0400
@@ -136,21 +136,6 @@ void xen_pvfb_invalidate(void *opaque)
 /* Screen dump is not used in Xen, so no need to impl this ? */
 void xen_pvfb_screen_dump(void *opaque, const char *name) { }
 
-void xen_pvfb_dispatch_store(void *opaque) {
-    int ret;
-    if ((ret = xenfb_dispatch_store((struct xenfb *)opaque)) < 0) {
-        fprintf(stderr, "Failure while dispatching store: %d\n", ret);
-        exit(1);
-    }
-}
-
-void xen_pvfb_dispatch_channel(void *opaque) {
-    int ret;
-    if ((ret = xenfb_dispatch_channel((struct xenfb *)opaque)) < 0) {
-        fprintf(stderr, "Failure while dispatching store: %d\n", ret);
-        exit(1);
-    }
-}
 
 /* The Xen PV machine currently provides
  *   - a virtual framebuffer
@@ -165,7 +150,6 @@ static void xen_init_pv(uint64_t ram_siz
 {
     struct xenfb *xenfb;
     extern int domid;
-    int fd;
 
     /* Prepare PVFB state */
     xenfb = xenfb_new();
@@ -197,19 +181,6 @@ static void xen_init_pv(uint64_t ram_siz
                                  xenfb->abs_pointer_wanted,
                                  "Xen PVFB Mouse");
 
-    /* Listen for events from xenstore */
-    fd = xenfb_get_store_fd(xenfb);
-    if (qemu_set_fd_handler2(fd, NULL, xen_pvfb_dispatch_store, NULL, xenfb) < 0) {
-        fprintf(stderr, "Could not register event handler (%s)\n",
-                strerror(errno));
-    }
-
-    /* Listen for events from the event channel */
-    fd = xenfb_get_channel_fd(xenfb);
-    if (qemu_set_fd_handler2(fd, NULL, xen_pvfb_dispatch_channel, NULL, xenfb) < 0) {
-        fprintf(stderr, "Could not register event handler (%s)\n",
-                strerror(errno));
-    }
 
     /* Setup QEMU display */
     dpy_resize(ds, xenfb->width, xenfb->height);
diff -r bd1bddd42929 tools/ioemu/hw/xenfb.c
--- a/tools/ioemu/hw/xenfb.c	Tue Aug 14 14:38:28 2007 -0400
+++ b/tools/ioemu/hw/xenfb.c	Tue Aug 14 15:10:27 2007 -0400
@@ -8,7 +8,6 @@
 #include <xen/io/fbif.h>
 #include <xen/io/kbdif.h>
 #include <xen/io/protocols.h>
-#include <sys/select.h>
 #include <stdbool.h>
 #include <xen/event_channel.h>
 #include <sys/mman.h>
@@ -18,6 +17,7 @@
 #include <time.h>
 #include <xs.h>
 
+#include "vl.h"
 #include "xenfb.h"
 
 // FIXME defend against malicious frontend?
@@ -511,96 +511,6 @@ static void xenfb_dev_fatal(struct xenfb
 	xenfb_switch_state(dev, XenbusStateClosing);
 }
 
-int xenfb_attach_dom(struct xenfb *xenfb_pub, int domid)
-{
-	struct xenfb_private *xenfb = (struct xenfb_private *)xenfb_pub;
-	struct xs_handle *xsh = xenfb->xsh;
-	int val, serrno;
-	struct xenfb_page *fb_page;
-
-	xenfb_detach_dom(xenfb);
-
-	xenfb_device_set_domain(&xenfb->fb, domid);
-	xenfb_device_set_domain(&xenfb->kbd, domid);
-
-	if (xenfb_wait_for_backend_creation(&xenfb->fb) < 0)
-		goto error;
-	if (xenfb_wait_for_backend_creation(&xenfb->kbd) < 0)
-		goto error;
-
-	if (xenfb_xs_printf(xsh, xenfb->kbd.nodename, "feature-abs-pointer", "1"))
-		goto error;
-	if (xenfb_switch_state(&xenfb->fb, XenbusStateInitWait))
-		goto error;
-	if (xenfb_switch_state(&xenfb->kbd, XenbusStateInitWait))
-		goto error;
-
-	if (xenfb_hotplug(&xenfb->fb) < 0)
-		goto error;
-	if (xenfb_hotplug(&xenfb->kbd) < 0)
-		goto error;
-
-	if (!xs_watch(xsh, xenfb->fb.otherend, ""))
-		goto error;
-	if (!xs_watch(xsh, xenfb->kbd.otherend, ""))
-		goto error;
-
-	if (xenfb_wait_for_frontend_initialised(&xenfb->fb) < 0)
-		goto error;
-	if (xenfb_wait_for_frontend_initialised(&xenfb->kbd) < 0)
-		goto error;
-
-	if (xenfb_bind(&xenfb->fb) < 0)
-		goto error;
-	if (xenfb_bind(&xenfb->kbd) < 0)
-		goto error;
-
-	if (xenfb_xs_scanf1(xsh, xenfb->fb.otherend, "feature-update",
-			    "%d", &val) < 0)
-		val = 0;
-	if (!val) {
-		errno = ENOTSUP;
-		goto error;
-	}
-	if (xenfb_xs_scanf1(xsh, xenfb->fb.otherend, "protocol", "%63s",
-			    xenfb->protocol) < 0)
-		xenfb->protocol[0] = '\0';
-	xenfb_xs_printf(xsh, xenfb->fb.nodename, "request-update", "1");
-
-	/* TODO check for permitted ranges */
-	fb_page = xenfb->fb.page;
-	xenfb->pub.depth = fb_page->depth;
-	xenfb->pub.width = fb_page->width;
-	xenfb->pub.height = fb_page->height;
-	/* TODO check for consistency with the above */
-	xenfb->fb_len = fb_page->mem_length;
-	xenfb->pub.row_stride = fb_page->line_length;
-
-	if (xenfb_map_fb(xenfb, domid) < 0)
-		goto error;
-
-	if (xenfb_switch_state(&xenfb->fb, XenbusStateConnected))
-		goto error;
-	if (xenfb_switch_state(&xenfb->kbd, XenbusStateConnected))
-		goto error;
-
-	if (xenfb_wait_for_frontend_connected(&xenfb->kbd) < 0)
-		goto error;
-	if (xenfb_xs_scanf1(xsh, xenfb->kbd.otherend, "request-abs-pointer",
-			    "%d", &val) < 0)
-		val = 0;
-	xenfb->pub.abs_pointer_wanted = val;
-
-	return 0;
-
- error:
-	serrno = errno;
-	xenfb_detach_dom(xenfb);
-	xenfb_dev_fatal(&xenfb->fb, serrno, "on fire");
-	xenfb_dev_fatal(&xenfb->kbd, serrno, "on fire");
-        errno = serrno;
-        return -1;
-}
 
 static void xenfb_detach_dom(struct xenfb_private *xenfb)
 {
@@ -676,13 +586,13 @@ static int xenfb_on_state_change(struct 
 	return 0;
 }
 
-int xenfb_dispatch_channel(struct xenfb *xenfb_pub)
+static void xenfb_dispatch_channel(void *xenfb_pub)
 {
 	struct xenfb_private *xenfb = (struct xenfb_private *)xenfb_pub;
 	evtchn_port_t port;
 	port = xc_evtchn_pending(xenfb->evt_xch);
 	if (port == -1)
-		return -1;
+		exit(1);
 
 	if (port == xenfb->fb.port)
 		xenfb_on_fb_event(xenfb);
@@ -690,12 +600,10 @@ int xenfb_dispatch_channel(struct xenfb 
 		xenfb_on_kbd_event(xenfb);
 
 	if (xc_evtchn_unmask(xenfb->evt_xch, port) == -1)
-		return -1;
-
-	return 0;
-}
-
-int xenfb_dispatch_store(struct xenfb *xenfb_pub)
+		exit(1);
+}
+
+static void xenfb_dispatch_store(void *xenfb_pub)
 {
 	struct xenfb_private *xenfb = (struct xenfb_private *)xenfb_pub;
 	unsigned dummy;
@@ -708,52 +616,107 @@ int xenfb_dispatch_store(struct xenfb *x
 	if (r == 0)
 		r = xenfb_on_state_change(&xenfb->kbd);
 	if (r == -1)
-		return -2;
-
-	return 0;
-}
-
-
-/* Returns 0 normally, -1 on error, or -2 if the domain went away. */
-int xenfb_poll(struct xenfb *xenfb_pub, fd_set *readfds)
+		exit(1);
+}
+
+
+int xenfb_attach_dom(struct xenfb *xenfb_pub, int domid)
 {
 	struct xenfb_private *xenfb = (struct xenfb_private *)xenfb_pub;
-	int ret;
-
-	if (FD_ISSET(xc_evtchn_fd(xenfb->evt_xch), readfds)) {
-		if ((ret = xenfb_dispatch_channel(xenfb_pub)) < 0)
-			return ret;
-	}
-
-	if (FD_ISSET(xs_fileno(xenfb->xsh), readfds)) {
-		if ((ret = xenfb_dispatch_store(xenfb_pub)) < 0)
-			return ret;
-	}
-
-	return 0;
-}
-
-int xenfb_select_fds(struct xenfb *xenfb_pub, fd_set *readfds)
-{
-	struct xenfb_private *xenfb = (struct xenfb_private *)xenfb_pub;
-	int fd1 = xc_evtchn_fd(xenfb->evt_xch);
-	int fd2 = xs_fileno(xenfb->xsh);
-
-	FD_SET(fd1, readfds);
-	FD_SET(fd2, readfds);
-	return fd1 > fd2 ? fd1 + 1 : fd2 + 1;
-}
-
-int xenfb_get_store_fd(struct xenfb *xenfb_pub)
-{
-	struct xenfb_private *xenfb = (struct xenfb_private *)xenfb_pub;
-	return xs_fileno(xenfb->xsh);
-}
-
-int xenfb_get_channel_fd(struct xenfb *xenfb_pub)
-{
-	struct xenfb_private *xenfb = (struct xenfb_private *)xenfb_pub;
-	return xc_evtchn_fd(xenfb->evt_xch);
+	struct xs_handle *xsh = xenfb->xsh;
+	int val, serrno;
+	struct xenfb_page *fb_page;
+
+	xenfb_detach_dom(xenfb);
+
+	xenfb_device_set_domain(&xenfb->fb, domid);
+	xenfb_device_set_domain(&xenfb->kbd, domid);
+
+	if (xenfb_wait_for_backend_creation(&xenfb->fb) < 0)
+		goto error;
+	if (xenfb_wait_for_backend_creation(&xenfb->kbd) < 0)
+		goto error;
+
+	if (xenfb_xs_printf(xsh, xenfb->kbd.nodename, "feature-abs-pointer", "1"))
+		goto error;
+	if (xenfb_switch_state(&xenfb->fb, XenbusStateInitWait))
+		goto error;
+	if (xenfb_switch_state(&xenfb->kbd, XenbusStateInitWait))
+		goto error;
+
+	if (xenfb_hotplug(&xenfb->fb) < 0)
+		goto error;
+	if (xenfb_hotplug(&xenfb->kbd) < 0)
+		goto error;
+
+	if (!xs_watch(xsh, xenfb->fb.otherend, ""))
+		goto error;
+	if (!xs_watch(xsh, xenfb->kbd.otherend, ""))
+		goto error;
+
+	if (xenfb_wait_for_frontend_initialised(&xenfb->fb) < 0)
+		goto error;
+	if (xenfb_wait_for_frontend_initialised(&xenfb->kbd) < 0)
+		goto error;
+
+	if (xenfb_bind(&xenfb->fb) < 0)
+		goto error;
+	if (xenfb_bind(&xenfb->kbd) < 0)
+		goto error;
+
+	if (xenfb_xs_scanf1(xsh, xenfb->fb.otherend, "feature-update",
+			    "%d", &val) < 0)
+		val = 0;
+	if (!val) {
+		errno = ENOTSUP;
+		goto error;
+	}
+	if (xenfb_xs_scanf1(xsh, xenfb->fb.otherend, "protocol", "%63s",
+			    xenfb->protocol) < 0)
+		xenfb->protocol[0] = '\0';
+	xenfb_xs_printf(xsh, xenfb->fb.nodename, "request-update", "1");
+
+	/* TODO check for permitted ranges */
+	fb_page = xenfb->fb.page;
+	xenfb->pub.depth = fb_page->depth;
+	xenfb->pub.width = fb_page->width;
+	xenfb->pub.height = fb_page->height;
+	/* TODO check for consistency with the above */
+	xenfb->fb_len = fb_page->mem_length;
+	xenfb->pub.row_stride = fb_page->line_length;
+
+	if (xenfb_map_fb(xenfb, domid) < 0)
+		goto error;
+
+	if (xenfb_switch_state(&xenfb->fb, XenbusStateConnected))
+		goto error;
+	if (xenfb_switch_state(&xenfb->kbd, XenbusStateConnected))
+		goto error;
+
+	if (xenfb_wait_for_frontend_connected(&xenfb->kbd) < 0)
+		goto error;
+	if (xenfb_xs_scanf1(xsh, xenfb->kbd.otherend, "request-abs-pointer",
+			    "%d", &val) < 0)
+		val = 0;
+	xenfb->pub.abs_pointer_wanted = val;
+
+	/* Listen for events from xenstore */
+	if (qemu_set_fd_handler2(xs_fileno(xenfb->xsh), NULL, xenfb_dispatch_store, NULL, xenfb) < 0)
+		goto error;
+
+	/* Listen for events from the event channel */
+	if (qemu_set_fd_handler2(xc_evtchn_fd(xenfb->evt_xch), NULL, xenfb_dispatch_channel, NULL, xenfb) < 0)
+		goto error;
+
+	return 0;
+
+ error:
+	serrno = errno;
+	xenfb_detach_dom(xenfb);
+	xenfb_dev_fatal(&xenfb->fb, serrno, "on fire");
+	xenfb_dev_fatal(&xenfb->kbd, serrno, "on fire");
+        errno = serrno;
+        return -1;
 }
 
 static int xenfb_kbd_event(struct xenfb_private *xenfb,
diff -r bd1bddd42929 tools/ioemu/hw/xenfb.h
--- a/tools/ioemu/hw/xenfb.h	Tue Aug 14 14:38:28 2007 -0400
+++ b/tools/ioemu/hw/xenfb.h	Tue Aug 14 15:03:42 2007 -0400
@@ -25,13 +25,6 @@ void xenfb_teardown(struct xenfb *xenfb)
 
 int xenfb_attach_dom(struct xenfb *xenfb, int domid);
 
-int xenfb_dispatch_store(struct xenfb *xenfb_pub);
-int xenfb_dispatch_channel(struct xenfb *xenfb_pub);
-int xenfb_select_fds(struct xenfb *xenfb, fd_set *readfds);
-int xenfb_poll(struct xenfb *xenfb, fd_set *readfds);
-int xenfb_get_store_fd(struct xenfb *xenfb_pub);
-int xenfb_get_channel_fd(struct xenfb *xenfb_pub);
-
 int xenfb_send_key(struct xenfb *xenfb, bool down, int keycode);
 int xenfb_send_motion(struct xenfb *xenfb, int rel_x, int rel_y);
 int xenfb_send_position(struct xenfb *xenfb, int abs_x, int abs_y);

-- 
|=- Red Hat, Engineering, Emerging Technologies, Boston.  +1 978 392 2496 -=|
|=-           Perl modules: http://search.cpan.org/~danberr/              -=|
|=-               Projects: http://freshmeat.net/~danielpb/               -=|
|=-  GnuPG: 7D3B9505   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505  -=|