On Mon, Jun 18, 2007 at 05:49:59AM -0400, Daniel Veillard wrote:
On Sun, Jun 17, 2007 at 10:52:55PM +0100, Daniel P. Berrange wrote:
The following patch adds a qemud/event.c & qemud/event.h file providing a general purpose event loop built around poll. Users register file handles and associated callbacks, and / or timers. The qemud.c file is changed to make use of these APIs for dealing with server, client, and VM file handles and/or sockets. This decouples much of the QEMU VM I/O code from the main qemud.c daemon code.
qemud/event.c | 311 +++++++++++++++++++++++++++++++++++++++++ qemud/event.h | 13 + qemud/Makefile.am | 3 qemud/qemud.c | 401 +++++++++++++++++++++++++++++------------------------- qemud/remote.c | 5 5 files changed, 546 insertions(+), 187 deletions(-)
As I understand taht patch could be applied alone without disturbing operations, right ?
Assuming it is bug free, then yes :-)
diff -r 93de958458cb qemud/Makefile.am --- a/qemud/Makefile.am Fri Jun 15 14:27:39 2007 +0000 +++ b/qemud/Makefile.am Sun Jun 17 16:26:41 2007 -0400 @@ -16,7 +16,8 @@ libvirt_qemud_SOURCES = \ buf.c buf.h \ protocol.h protocol.c \ remote_protocol.h remote_protocol.c \ - remote.c + remote.c \ + event.c event.h #-D_XOPEN_SOURCE=600 -D_XOPEN_SOURCE_EXTENDED=1 -D_POSIX_C_SOURCE=199506L libvirt_qemud_CFLAGS = \ -I$(top_srcdir)/include -I$(top_builddir)/include $(LIBXML_CFLAGS) \ diff -r 93de958458cb qemud/event.c --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/qemud/event.c Sun Jun 17 16:26:34 2007 -0400 @@ -0,0 +1,311 @@
In general a severe lack of comment, I assume it's an early review version.
Yep, needs documentation.
+int virEventAddHandle(int fd, int events, virEventHandleCallback cb, void *opaque) { + struct virEventHandle *tmp; + + printf("Add handle %d %d %p %p\n", fd, events, cb, opaque); + tmp = realloc(eventLoop.handles, sizeof(struct virEventHandle) * (eventLoop.nhandles+1)); + if (!tmp) { + return -1; + }
Hum, realloc( , n+1) always leave me with a unpleasant feeling. I really prefer a current usage size, an allocated size and growing by doubling the block only when needed.
Based on my debugging this is probably a good idea - with the TLS stuff we tend to addd & remove handles *alot* because the direction of traffic is switching between TX & RX all the time.
-static int qemudDispatchSignal(struct qemud_server *server) -{ +static void qemudDispatchSignalEvent(int fd ATTRIBUTE_UNUSED, + int events ATTRIBUTE_UNUSED, + void *opaque) { + struct qemud_server *server = (struct qemud_server *)opaque; unsigned char sigc; struct qemud_vm *vm; struct qemud_network *network; @@ -194,7 +204,7 @@ static int qemudDispatchSignal(struct qe if (read(server->sigread, &sigc, 1) != 1) { qemudLog(QEMUD_ERR, "Failed to read from signal pipe: %s", strerror(errno)); - return -1; + return; }
ret = 0; @@ -266,7 +276,8 @@ static int qemudDispatchSignal(struct qe break; }
- return ret; + if (ret != 0) + server->shutdown = 1; }
is asynchronous error handling really better there ?
It is unavoidable in this context actually, since this is a callback invoked from within the event loop, there isn't any meaningful caller to deal with a return value. The event loop itself checks on the server->shutdown flag on each iteration, so the end result is the same as previous code.
static int qemudSetCloseExec(int fd) { @@ -474,19 +485,16 @@ static int qemudListenUnix(struct qemud_ } [...] diff -r 93de958458cb qemud/remote.c --- a/qemud/remote.c Fri Jun 15 14:27:39 2007 +0000 +++ b/qemud/remote.c Sun Jun 17 16:26:54 2007 -0400 @@ -691,7 +691,7 @@ remoteDispatchDomainGetInfo (struct qemu remoteDispatchError (client, req, "domain not found"); return -2; } - + printf("------------------------------------- %d %s\n", dom->id, dom->name);
I assume those are debugging left over, right ?
Opps, yes, wrong version of the patch. Dan. -- |=- 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 -=|