[Libvir] PATCH 0/3: Refactor QEMU daemon
The 3 patches which follow are work-in-progress to re-factor the QEMU daemon / driver to eventually adhere to the main libvirt internal driver API. Once this work is complete, there will only need to be a single daemon running which can provide both remote & QEMU capabilities at once with no QEMU specific code in it. This is following some of the ideas I set out here http://www.redhat.com/archives/libvir-list/2007-May/msg00083.html 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 -=|
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(-) 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 -=|
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 ?
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.
+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.
+ eventLoop.handles = tmp; + + eventLoop.handles[eventLoop.nhandles].fd = fd; + eventLoop.handles[eventLoop.nhandles].events = events; + eventLoop.handles[eventLoop.nhandles].cb = cb; + eventLoop.handles[eventLoop.nhandles].opaque = opaque; + eventLoop.handles[eventLoop.nhandles].deleted = 0; + + eventLoop.nhandles++; + + return 0; +} + +int virEventRemoveHandle(int fd) { + int i; + printf("Remove handle %d\n", fd); + for (i = eventLoop.nhandles-1 ; i >= 0 ; i--) { + if (eventLoop.handles[i].fd == fd) { + printf("mark delete %d\n", i); + eventLoop.handles[i].deleted = 1; + return 0; + } + } + return -1; +} + + +int virEventAddTimeout(int timeout, virEventTimeoutCallback cb, void *opaque) { + struct virEventTimeout *tmp; + struct timeval tv; + + if (gettimeofday(&tv, NULL) < 0) { + return -1; + } + + tmp = realloc(eventLoop.timeouts, sizeof(struct virEventTimeout) * (eventLoop.ntimeouts+1)); + if (!tmp) { + return -1; + } + eventLoop.timeouts = tmp; + + eventLoop.timeouts[eventLoop.ntimeouts].timer = nextTimer++; + eventLoop.timeouts[eventLoop.ntimeouts].timeout = timeout; + eventLoop.timeouts[eventLoop.ntimeouts].cb = cb; + eventLoop.timeouts[eventLoop.ntimeouts].opaque = opaque; + eventLoop.timeouts[eventLoop.ntimeouts].deleted = 0; + eventLoop.timeouts[eventLoop.ntimeouts].expiresAt = + (((unsigned long long)tv.tv_sec)*1000) + + (((unsigned long long)tv.tv_usec)/1000); + + eventLoop.ntimeouts++; + + return 0; +} + +int virEventRemoveTimeout(int timer) { + int i; + for (i = eventLoop.ntimeouts-1 ; i >=0 ; i--) { + if (eventLoop.timeouts[i].timer == timer) { + eventLoop.timeouts[i].deleted = 1; + return 0; + } + } + return -1; +} + + +static int virEventCalculateTimeout(int *timeout) { + unsigned long long then = 0; + int i; + + /* Figure out if we need a timeout */ + for (i = 0 ; i < eventLoop.ntimeouts ; i++) { + if (then == 0 || + eventLoop.timeouts[i].expiresAt < then) + then = eventLoop.timeouts[i].expiresAt; + } + + /* Calculate how long we should wait for a timeout if needed */ + if (then > 0) { + struct timeval tv; + + if (gettimeofday(&tv, NULL) < 0) { + return -1; + } + + *timeout = then - + ((((unsigned long long)tv.tv_sec)*1000) + + (((unsigned long long)tv.tv_usec)/1000)); + + if (*timeout < 0) + *timeout = 1; + } else { + *timeout = -1; + } + + return 0; +} + +static struct pollfd *virEventMakePollFDs(void) { + struct pollfd *fds; + int i; + + /* Setup the poll file handle data structs */ + if (!(fds = malloc(sizeof(struct pollfd)*eventLoop.nhandles))) + return NULL; + + for (i = 0 ; i < eventLoop.nhandles ; i++) { + fds[i].fd = eventLoop.handles[i].fd; + fds[i].events = eventLoop.handles[i].events; + fds[i].revents = 0; + printf("Wait for %d %d\n", eventLoop.handles[i].fd, eventLoop.handles[i].events); + } [...] same here
+ +static int virEventCleanupTimeouts(void) { + int i; + for (i = 0 ; i < eventLoop.ntimeouts ; ) { + struct virEventTimeout *tmp; + if (!eventLoop.handles[i].deleted) { + i++; + continue; + } + + if ((i+1) < eventLoop.ntimeouts) { + memmove(eventLoop.timeouts+i, + eventLoop.timeouts+i+1, + sizeof(struct virEventTimeout)*(eventLoop.ntimeouts-(i+1))); + } + + tmp = realloc(eventLoop.timeouts, sizeof(struct virEventTimeout) * (eventLoop.ntimeouts-1)); + if (!tmp) { + return -1; + } + eventLoop.timeouts = tmp; + eventLoop.ntimeouts--; + } + return 0; +}
realloc( -1); in a loop, I would rather realloc at the end or just keep a maximum allocated block and realloc to maxtimeouts / 2 only if ntimeouts < (maxtimeouts / 2) -1)
+static int virEventCleanupHandles(void) { [...] + tmp = realloc(eventLoop.handles, sizeof(struct virEventHandle) * (eventLoop.nhandles-1)); + if (!tmp) { + return -1; + } + eventLoop.handles = tmp; + eventLoop.nhandles--; + } + return 0; +}
Same here
+int virEventRunOnce(void) { + struct pollfd *fds; + int ret, timeout; + + if (!(fds = virEventMakePollFDs())) + return -1; + + if (virEventCalculateTimeout(&timeout) < 0) { + free(fds); + return -1; + } + + retry: + printf("Poll on %d handles %p timeout %d\n", eventLoop.nhandles, fds, timeout); + ret = poll(fds, eventLoop.nhandles, timeout); + printf("Poll got %d event\n", ret); + if (ret < 0) { + if (errno == EINTR) { + goto retry; + } + free(fds); + return -1; + } + if (virEventDispatchTimeouts() < 0) { + free(fds); + return -1; + } + + if (ret > 0 && + virEventDispatchHandles(fds) < 0) { + free(fds); + return -1; + } + free(fds); + + if (virEventCleanupTimeouts() < 0) + return -1; + + if (virEventCleanupHandles() < 0) + return -1; + + return 0; +} ... diff -r 93de958458cb qemud/event.h --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/qemud/event.h Sun Jun 17 16:26:34 2007 -0400 @@ -0,0 +1,13 @@ + + +typedef void (*virEventHandleCallback)(int fd, int events, void *opaque); + +int virEventAddHandle(int fd, int events, virEventHandleCallback cb, void *opaque); +int virEventRemoveHandle(int fd); + +typedef void (*virEventTimeoutCallback)(int timer, void *opaque); + +int virEventAddTimeout(int timeout, virEventTimeoutCallback cb, void *opaque); +int virEventRemoveTimeout(int timer); + +int virEventRunOnce(void); diff -r 93de958458cb qemud/qemud.c --- a/qemud/qemud.c Fri Jun 15 14:27:39 2007 +0000 +++ b/qemud/qemud.c Sun Jun 17 17:37:07 2007 -0400 @@ -61,6 +61,7 @@ #include "driver.h" #include "conf.h" #include "iptables.h" +#include "event.h"
static int godaemon = 0; /* -d: Be a daemon */ static int verbose = 0; /* -v: Verbose mode */ @@ -109,6 +110,13 @@ static void sig_handler(int sig) { } errno = origerrno; } + +static void qemudDispatchVMEvent(int fd, int events, void *opaque); +static void qemudDispatchClientEvent(int fd, int events, void *opaque); +static void qemudDispatchServerEvent(int fd, int events, void *opaque); +static int qemudRegisterClientEvent(struct qemud_server *server, + struct qemud_client *client, + int remove);
static int remoteInitializeGnuTLS (void) @@ -184,8 +192,10 @@ remoteInitializeGnuTLS (void) return 0; }
-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 ?
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 ?
if (virDomainGetInfo (dom, &info) == -1) return -1;
@@ -862,7 +862,7 @@ remoteDispatchDomainLookupById (struct q
dom = virDomainLookupByID (client->conn, args->id); if (dom == NULL) return -1; - + printf("******************Loopu %d %s\n", dom->id, dom->name); make_nonnull_domain (&ret->dom, dom);
return 0; @@ -1539,6 +1539,7 @@ get_nonnull_domain (virConnectPtr conn, /* Should we believe the domain.id sent by the client? Maybe * this should be a check rather than an assignment? XXX */ + printf("********************* %d\n", domain.id); if (dom) dom->id = domain.id; return dom; }
My main concern is lack of comments and the allocation stategy, Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
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 -=|
On Mon, Jun 18, 2007 at 01:33:09PM +0100, Daniel P. Berrange wrote:
On Mon, Jun 18, 2007 at 05:49:59AM -0400, Daniel Veillard wrote:
- 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.
okay, thanks ! Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
Attached is a new version of the patch which allocs/frees memory for storing the callback data in blocks. It also adds copious comments and fixes a very subtle bug in adding handles during dispatch. The printfs are changed into qemudDebug()s and the nvmfds field is killed here. 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.
b/qemud/event.c | 460 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ b/qemud/event.h | 98 +++++++++++ qemud/Makefile.am | 3 qemud/internal.h | 1 qemud/qemud.c | 400 +++++++++++++++++++++++++--------------------- 5 files changed, 774 insertions(+), 188 deletions(-) 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 -=|
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.
I think we should use fork, but I won't labour the point. Rich. -- Emerging Technologies, Red Hat - http://et.redhat.com/~rjones/ Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod Street, Windsor, Berkshire, SL4 1TE, United Kingdom. Registered in England and Wales under Company Registration No. 03798903
In the main dispatcher.c file most methods simply call out into APIs in the driver.c or conf.c file. The capabilities and nodeinfo methods, however, containing their full logic in the dispatcher.c file. When the QEMU impl is switched to the generic driver model, the dispatcher.c file will be deleted, so these two methods need to be moved into the driver.c file instead. dispatch.c | 193 +++++-------------------------------------------------------- driver.c | 184 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-- driver.h | 11 ++- 3 files changed, 203 insertions(+), 185 deletions(-) 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 -=|
The main qemud.c file has alot of code which deals with starting and stopping QEMU, dnsmasq processes, setting up iptables and generally controlling the lifecycle of VMs and networks. With the advent of the general purpose event loop code, there is no need for any of this to be present in the qemud.c file. So this patch moves the code into the driver.c file. The qemud.d file is now solely responsible with client/server socket code. driver.c | 1041 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ driver.h | 14 internal.h | 14 qemud.c | 1036 ------------------------------------------------------------ 4 files changed, 1062 insertions(+), 1043 deletions(-) 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 -=|
On Sun, Jun 17, 2007 at 10:48:17PM +0100, Daniel P. Berrange wrote:
The 3 patches which follow are work-in-progress to re-factor the QEMU daemon / driver to eventually adhere to the main libvirt internal driver API. Once this work is complete, there will only need to be a single daemon running which can provide both remote & QEMU capabilities at once with no QEMU specific code in it.
This is following some of the ideas I set out here
http://www.redhat.com/archives/libvir-list/2007-May/msg00083.html
Following on from the 3 patches I've just sent out, I anticipate a handful of further patches over the next few days todo the following - Split the qemud_server struct into two structs - qemud_server to only contain data about client/server sockets - qemud_manager to only contain data about VMs & networks The conf.c, and driver.c files will need updating to deal with this - Re-implement qemudReportError in terms of virError APIs - Change contract of all public methods in qemud/driver.c to match that needed by the libvirt src/driver.h API. - Move the QEMU related code into src/ dir. This will consist of iptables.c/.h bridge.c/.h driver.c/.h conf.c/.h - Remove the legacy QEMU protocol by deleting qemud/protocol.* src/qemud_internal.* At this time, the daemon will be totally generic. BTW, I'm maintaining this series of patches as a patch queue, with the intent that we have fully-working code at every step of the re-factoring process, though at some intermediate stages I'll be doing some horrific things with the build process ;-) 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 -=|
This patch moves a little more code dealing with VMs / networks out of the qemud.c file into the driver.c file. Specifically code to load configs at startup, code to re-load config files on SIGHUP, and some code to cleanup unused inactive VM / network config file structs. driver.c | 82 ++++++++++++++++++++++++++++++++++++++++----------------------- driver.h | 2 + qemud.c | 45 ++-------------------------------- 3 files changed, 58 insertions(+), 71 deletions(-) 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 -=|
Daniel P. Berrange wrote:
This patch moves a little more code dealing with VMs / networks out of the qemud.c file into the driver.c file. Specifically code to load configs at startup, code to re-load config files on SIGHUP, and some code to cleanup unused inactive VM / network config file structs.
Looks fine except:
+static int qemudOneLoop(struct qemud_server *server ATTRIBUTE_UNUSED) {
Just delete this parameter? Rich. -- Emerging Technologies, Red Hat - http://et.redhat.com/~rjones/ Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod Street, Windsor, Berkshire, SL4 1TE, United Kingdom. Registered in England and Wales under Company Registration No. 03798903
On Mon, Jun 18, 2007 at 01:50:31PM +0100, Richard W.M. Jones wrote:
Daniel P. Berrange wrote:
This patch moves a little more code dealing with VMs / networks out of the qemud.c file into the driver.c file. Specifically code to load configs at startup, code to re-load config files on SIGHUP, and some code to cleanup unused inactive VM / network config file structs.
Looks fine except:
+static int qemudOneLoop(struct qemud_server *server ATTRIBUTE_UNUSED) {
Just delete this parameter?
Yes I think so. 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 -=|
This patch switches the driver.c, conf.c and dispatch.c files over to use the virError object and associated API instead of a custom version. Makefile.am | 5 + conf.c | 192 ++++++++++++++++++++++++++++++------------------------------ dispatch.c | 16 +++-- driver.c | 174 ++++++++++++++++++++++++++++++------------------------ driver.h | 6 + internal.h | 2 6 files changed, 213 insertions(+), 182 deletions(-) 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 -=|
And with the actual patch... On Mon, Jun 18, 2007 at 03:18:40AM +0100, Daniel P. Berrange wrote:
This patch switches the driver.c, conf.c and dispatch.c files over to use the virError object and associated API instead of a custom version.
Makefile.am | 5 + conf.c | 192 ++++++++++++++++++++++++++++++------------------------------ dispatch.c | 16 +++-- driver.c | 174 ++++++++++++++++++++++++++++++------------------------ driver.h | 6 + internal.h | 2 6 files changed, 213 insertions(+), 182 deletions(-)
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 -=|
-- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
-- |=- 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 -=|
On Mon, Jun 18, 2007 at 03:32:38AM +0100, Daniel P. Berrange wrote:
And with the actual patch...
On Mon, Jun 18, 2007 at 03:18:40AM +0100, Daniel P. Berrange wrote:
This patch switches the driver.c, conf.c and dispatch.c files over to use the virError object and associated API instead of a custom version.
Makefile.am | 5 + conf.c | 192 ++++++++++++++++++++++++++++++------------------------------ dispatch.c | 16 +++-- driver.c | 174 ++++++++++++++++++++++++++++++------------------------ driver.h | 6 + internal.h | 2 6 files changed, 213 insertions(+), 182 deletions(-)
Patch looks fine and can probably be applied independantly of others, it's mostly a mechanical change. +1 Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
Daniel P. Berrange wrote:
And with the actual patch...
On Mon, Jun 18, 2007 at 03:18:40AM +0100, Daniel P. Berrange wrote:
This patch switches the driver.c, conf.c and dispatch.c files over to use the virError object and associated API instead of a custom version.
I don't know if I like this ...
+extern void __virRaiseError(virConnectPtr conn, + virDomainPtr dom, + virNetworkPtr net, + int domain, + int code, + virErrorLevel level, + const char *str1, + const char *str2, + const char *str3, + int int1, int int2, const char *msg, ...) + ATTRIBUTE_FORMAT(printf, 12, 13);
although I can see why you did it. Would it be better to move the prototype of __virRaiseError out of src/internal.h and into include/libvirt/virterror.h? We declare and export other __internal symbols already. Rich. -- Emerging Technologies, Red Hat - http://et.redhat.com/~rjones/ Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod Street, Windsor, Berkshire, SL4 1TE, United Kingdom. Registered in England and Wales under Company Registration No. 03798903
On Mon, Jun 18, 2007 at 01:53:18PM +0100, Richard W.M. Jones wrote:
Daniel P. Berrange wrote:
And with the actual patch...
On Mon, Jun 18, 2007 at 03:18:40AM +0100, Daniel P. Berrange wrote:
This patch switches the driver.c, conf.c and dispatch.c files over to use the virError object and associated API instead of a custom version.
I don't know if I like this ...
+extern void __virRaiseError(virConnectPtr conn, + virDomainPtr dom, + virNetworkPtr net, + int domain, + int code, + virErrorLevel level, + const char *str1, + const char *str2, + const char *str3, + int int1, int int2, const char *msg, ...) + ATTRIBUTE_FORMAT(printf, 12, 13);
although I can see why you did it. Would it be better to move the prototype of __virRaiseError out of src/internal.h and into include/libvirt/virterror.h? We declare and export other __internal symbols already.
This is a temporary hack. Once the QEMU code moves into the src/ directory this duplicated declaration will go away again. Regards, 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 -=|
This is a very big patch, but actually there is no real functional change in it at all. It is splitting the qemud_server struct into 2 pieces. The qemud_server struct now only deals with client/server socket stuff. The new qemud_driver struct deals with QEMU vms and networks. The driver.c and conf.c files are now 100% independant of the QEMU daemon, and so ready for the final adaption to the libvirt driver API. conf.c | 192 ++++++++--------- conf.h | 30 +- dispatch.c | 295 +++++++++++---------------- driver.c | 657 ++++++++++++++++++++++++++++++++++--------------------------- driver.h | 335 ++++++++++++++++++++++++++----- internal.h | 237 ---------------------- qemud.c | 50 ---- 7 files changed, 909 insertions(+), 887 deletions(-) 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 -=|
Daniel P. Berrange wrote:
This is a very big patch, but actually there is no real functional change in it at all. It is splitting the qemud_server struct into 2 pieces. The qemud_server struct now only deals with client/server socket stuff. The new qemud_driver struct deals with QEMU vms and networks. The driver.c and conf.c files are now 100% independant of the QEMU daemon, and so ready for the final adaption to the libvirt driver API.
conf.c | 192 ++++++++--------- conf.h | 30 +- dispatch.c | 295 +++++++++++---------------- driver.c | 657 ++++++++++++++++++++++++++++++++++--------------------------- driver.h | 335 ++++++++++++++++++++++++++----- internal.h | 237 ---------------------- qemud.c | 50 ---- 7 files changed, 909 insertions(+), 887 deletions(-)
That's a complicated patch to follow, but the guts of it appears to be this, the rest being just code movement[1].
diff -r af6d5f54a13a qemud/driver.h --- a/qemud/driver.h Sun Jun 17 22:11:10 2007 -0400 +++ b/qemud/driver.h Sun Jun 17 22:11:17 2007 -0400 @@ -27,21 +27,266 @@ [...] +/* Main driver state */ +struct qemud_driver { + int qemuVersion; + int qemuCmdFlags; /* values from enum qemud_cmd_flags */ + int nactivevms; + int ninactivevms; + struct qemud_vm *vms; + int nextvmid; + int nactivenetworks; + int ninactivenetworks; + struct qemud_network *networks; + brControl *brctl; + iptablesContext *iptables; + char *configDir; + char *autostartDir; + char *networkConfigDir; + char *networkAutostartDir; + char logDir[PATH_MAX]; +};
@@ -333,25 +123,9 @@ struct qemud_server { struct qemud_server { int nsockets; struct qemud_socket *sockets; - int qemuVersion; - int qemuCmdFlags; /* values from enum qemud_cmd_flags */ int nclients; struct qemud_client *clients; int sigread; - int nvmfds; - int nactivevms; - int ninactivevms; - struct qemud_vm *vms; - int nextvmid; - int nactivenetworks; - int ninactivenetworks; - struct qemud_network *networks; - brControl *brctl; - iptablesContext *iptables; - char *configDir; - char *autostartDir; - char *networkConfigDir; - char *networkAutostartDir; char logDir[PATH_MAX]; unsigned int shutdown : 1; };
That did lead me to wonder what happened to server->nvmfds - there are two references to it in the code and both have been removed, but it turns out it was only ever being written to, which I guess made it less than useful. Rich. [1] Now if only there was a computer language where you define the structures and the code just followed ... -- Emerging Technologies, Red Hat - http://et.redhat.com/~rjones/ Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod Street, Windsor, Berkshire, SL4 1TE, United Kingdom. Registered in England and Wales under Company Registration No. 03798903
On Mon, Jun 18, 2007 at 02:08:34PM +0100, Richard W.M. Jones wrote:
Daniel P. Berrange wrote:
This is a very big patch, but actually there is no real functional change in it at all. It is splitting the qemud_server struct into 2 pieces. The qemud_server struct now only deals with client/server socket stuff. The new qemud_driver struct deals with QEMU vms and networks. The driver.c and conf.c files are now 100% independant of the QEMU daemon, and so ready for the final adaption to the libvirt driver API.
conf.c | 192 ++++++++--------- conf.h | 30 +- dispatch.c | 295 +++++++++++---------------- driver.c | 657 ++++++++++++++++++++++++++++++++++--------------------------- driver.h | 335 ++++++++++++++++++++++++++----- internal.h | 237 ---------------------- qemud.c | 50 ---- 7 files changed, 909 insertions(+), 887 deletions(-)
That did lead me to wonder what happened to server->nvmfds - there are two references to it in the code and both have been removed, but it turns out it was only ever being written to, which I guess made it less than useful.
The nvmfds stuff was an optimization used with the old integrated event loop, which I forgot to remove in my first patch. It just let us know how big an array to allocate for poll_fd, which is unneccessary now that we have a general purpose isolated event loop. Regards, 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 -=|
The UUID functions in qemud/uuid.c have qemud_ in the name. They will be moved into the main src/ directory and so deserve a more generic name. This patch makes them use virUUID as a prefix. No functional change. conf.c | 8 ++++---- uuid.c | 16 ++++++++-------- uuid.h | 12 ++++++------ 3 files changed, 18 insertions(+), 18 deletions(-) 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 -=|
On Mon, Jun 18, 2007 at 03:24:54AM +0100, Daniel P. Berrange wrote:
The UUID functions in qemud/uuid.c have qemud_ in the name. They will be moved into the main src/ directory and so deserve a more generic name. This patch makes them use virUUID as a prefix. No functional change.
completely uncontroversial, +1 Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
Daniel Veillard wrote:
On Mon, Jun 18, 2007 at 03:24:54AM +0100, Daniel P. Berrange wrote:
The UUID functions in qemud/uuid.c have qemud_ in the name. They will be moved into the main src/ directory and so deserve a more generic name. This patch makes them use virUUID as a prefix. No functional change.
completely uncontroversial, +1
Ditto, +1 Rich. -- Emerging Technologies, Red Hat - http://et.redhat.com/~rjones/ Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod Street, Windsor, Berkshire, SL4 1TE, United Kingdom. Registered in England and Wales under Company Registration No. 03798903
The bufferNNN functions in the qemud/buf.c file are identical those already present in the src/xml.c file, except the latter are named virBufferNNN. This patch renames bufferNNN to virBufferNNN so that the two impls can be merged into one. No functional change buf.c | 42 +++++------ buf.h | 30 ++++---- conf.c | 228 +++++++++++++++++++++++++++++++-------------------------------- driver.c | 34 ++++----- 4 files changed, 167 insertions(+), 167 deletions(-) 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 -=|
The subject was wrong last time. Doh. On Mon, Jun 18, 2007 at 03:27:03AM +0100, Daniel P. Berrange wrote:
The bufferNNN functions in the qemud/buf.c file are identical those already present in the src/xml.c file, except the latter are named virBufferNNN. This patch renames bufferNNN to virBufferNNN so that the two impls can be merged into one. No functional change
buf.c | 42 +++++------ buf.h | 30 ++++---- conf.c | 228 +++++++++++++++++++++++++++++++-------------------------------- driver.c | 34 ++++----- 4 files changed, 167 insertions(+), 167 deletions(-)
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 -=|
diff -r b8a4e065009d qemud/buf.c --- a/qemud/buf.c Sun Jun 17 22:11:26 2007 -0400 +++ b/qemud/buf.c Sun Jun 17 22:11:32 2007 -0400 @@ -1,7 +1,7 @@ /* - * buf.c: buffers for qemud - * - * Copyright (C) 2005 Red Hat, Inc. + * buf.c: buffers for libvirt + * + * Copyright (C) 2005-2007 Red Hat, Inc. * * See COPYING.LIB for the License of this software * @@ -17,7 +17,7 @@ #include "buf.h"
/** - * bufferGrow: + * virBufferGrow: * @buf: the buffer * @len: the minimum free size to allocate on top of existing used space * @@ -26,7 +26,7 @@ * Returns the new available space or -1 in case of error */ static int -bufferGrow(bufferPtr buf, unsigned int len) +virBufferGrow(virBufferPtr buf, unsigned int len) { int size; char *newbuf; @@ -46,7 +46,7 @@ bufferGrow(bufferPtr buf, unsigned int l }
/** - * bufferAdd: + * virBufferAdd: * @buf: the buffer to dump * @str: the string * @len: the number of bytes to add @@ -57,7 +57,7 @@ bufferGrow(bufferPtr buf, unsigned int l * Returns 0 successful, -1 in case of internal or API error. */ int -bufferAdd(bufferPtr buf, const char *str, int len) +virBufferAdd(virBufferPtr buf, const char *str, int len) { unsigned int needSize;
@@ -72,7 +72,7 @@ bufferAdd(bufferPtr buf, const char *str
needSize = buf->use + len + 2; if (needSize > buf->size) { - if (!bufferGrow(buf, needSize - buf->use)) { + if (!virBufferGrow(buf, needSize - buf->use)) { return (-1); } } @@ -83,10 +83,10 @@ bufferAdd(bufferPtr buf, const char *str return (0); }
-bufferPtr -bufferNew(unsigned int size) -{ - bufferPtr buf; +virBufferPtr +virBufferNew(unsigned int size) +{ + virBufferPtr buf;
if (!(buf = malloc(sizeof(*buf)))) return NULL; if (size && (buf->content = malloc(size))==NULL) { @@ -100,7 +100,7 @@ bufferNew(unsigned int size) }
void -bufferFree(bufferPtr buf) +virBufferFree(virBufferPtr buf) { if (buf) { if (buf->content) @@ -110,13 +110,13 @@ bufferFree(bufferPtr buf) }
/** - * bufferContentAndFree: + * virBufferContentAndFree: * @buf: Buffer * * Return the content from the buffer and free (only) the buffer structure. */ char * -bufferContentAndFree (bufferPtr buf) +virBufferContentAndFree (virBufferPtr buf) { char *content = buf->content;
@@ -125,7 +125,7 @@ bufferContentAndFree (bufferPtr buf) }
/** - * bufferVSprintf: + * virBufferVSprintf: * @buf: the buffer to dump * @format: the format * @argptr: the variable list of arguments @@ -135,7 +135,7 @@ bufferContentAndFree (bufferPtr buf) * Returns 0 successful, -1 in case of internal or API error. */ int -bufferVSprintf(bufferPtr buf, const char *format, ...) +virBufferVSprintf(virBufferPtr buf, const char *format, ...) { int size, count; va_list locarg, argptr; @@ -150,7 +150,7 @@ bufferVSprintf(bufferPtr buf, const char locarg)) < 0) || (count >= size - 1)) { buf->content[buf->use] = 0; va_end(locarg); - if (bufferGrow(buf, 1000) < 0) { + if (virBufferGrow(buf, 1000) < 0) { return (-1); } size = buf->size - buf->use - 1; @@ -163,7 +163,7 @@ bufferVSprintf(bufferPtr buf, const char }
/** - * bufferStrcat: + * virBufferStrcat: * @buf: the buffer to dump * @argptr: the variable list of strings, the last argument must be NULL * @@ -172,7 +172,7 @@ bufferVSprintf(bufferPtr buf, const char * Returns 0 successful, -1 in case of internal or API error. */ int -bufferStrcat(bufferPtr buf, ...) +virBufferStrcat(virBufferPtr buf, ...) { va_list ap; char *str; @@ -184,7 +184,7 @@ bufferStrcat(bufferPtr buf, ...) unsigned int needSize = buf->use + len + 2;
if (needSize > buf->size) { - if (!bufferGrow(buf, needSize - buf->use)) + if (!virBufferGrow(buf, needSize - buf->use)) return -1; } memcpy(&buf->content[buf->use], str, len); diff -r b8a4e065009d qemud/buf.h --- a/qemud/buf.h Sun Jun 17 22:11:26 2007 -0400 +++ b/qemud/buf.h Sun Jun 17 22:11:32 2007 -0400 @@ -1,37 +1,37 @@ /* - * buf.h: buffers for qemud + * buf.h: buffers for libvirt * - * Copyright (C) 2005 Red Hat, Inc. + * Copyright (C) 2005-2007 Red Hat, Inc. * * See COPYING.LIB for the License of this software * * Daniel Veillard <veillard@redhat.com> */
-#ifndef __QEMUD_BUF_H__ -#define __QEMUD_BUF_H__ +#ifndef __VIR_BUFFER_H__ +#define __VIR_BUFFER_H__
#include "internal.h"
/** - * buffer: + * virBuffer: * * A buffer structure. */ -typedef struct _buffer buffer; -typedef buffer *bufferPtr; -struct _buffer { +typedef struct _virBuffer virBuffer; +typedef virBuffer *virBufferPtr; +struct _virBuffer { char *content; /* The buffer content UTF8 */ unsigned int use; /* The buffer size used */ unsigned int size; /* The buffer size */ };
-bufferPtr bufferNew(unsigned int size); -void bufferFree(bufferPtr buf); -char *bufferContentAndFree(bufferPtr buf); -int bufferAdd(bufferPtr buf, const char *str, int len); -int bufferVSprintf(bufferPtr buf, const char *format, ...) +virBufferPtr virBufferNew(unsigned int size); +void virBufferFree(virBufferPtr buf); +char *virBufferContentAndFree(virBufferPtr buf); +int virBufferAdd(virBufferPtr buf, const char *str, int len); +int virBufferVSprintf(virBufferPtr buf, const char *format, ...) ATTRIBUTE_FORMAT(printf, 2, 3); -int bufferStrcat(bufferPtr buf, ...); +int virBufferStrcat(virBufferPtr buf, ...);
-#endif /* __QEMUD_BUF_H__ */ +#endif /* __VIR_BUFFER_H__ */ diff -r b8a4e065009d qemud/conf.c --- a/qemud/conf.c Sun Jun 17 22:11:26 2007 -0400 +++ b/qemud/conf.c Sun Jun 17 22:11:32 2007 -0400 @@ -2453,14 +2453,14 @@ char *qemudGenerateXML(struct qemud_driv struct qemud_vm *vm, struct qemud_vm_def *def, int live) { - bufferPtr buf = 0; + virBufferPtr buf = 0; unsigned char *uuid; struct qemud_vm_disk_def *disk; struct qemud_vm_net_def *net; const char *type = NULL; int n;
- buf = bufferNew (QEMUD_MAX_XML_LEN); + buf = virBufferNew (QEMUD_MAX_XML_LEN); if (!buf) goto no_memory;
@@ -2481,50 +2481,50 @@ char *qemudGenerateXML(struct qemud_driv }
if (qemudIsActiveVM(vm) && live) { - if (bufferVSprintf(buf, "<domain type='%s' id='%d'>\n", type, vm->id) < 0) + if (virBufferVSprintf(buf, "<domain type='%s' id='%d'>\n", type, vm->id) < 0) goto no_memory; } else { - if (bufferVSprintf(buf, "<domain type='%s'>\n", type) < 0) - goto no_memory; - } - - if (bufferVSprintf(buf, " <name>%s</name>\n", def->name) < 0) + if (virBufferVSprintf(buf, "<domain type='%s'>\n", type) < 0) + goto no_memory; + } + + if (virBufferVSprintf(buf, " <name>%s</name>\n", def->name) < 0) goto no_memory;
uuid = def->uuid; - if (bufferVSprintf(buf, " <uuid>%02x%02x%02x%02x-%02x%02x-%02x%02x-%02x%02x-%02x%02x%02x%02x%02x%02x</uuid>\n", + if (virBufferVSprintf(buf, " <uuid>%02x%02x%02x%02x-%02x%02x-%02x%02x-%02x%02x-%02x%02x%02x%02x%02x%02x</uuid>\n", uuid[0], uuid[1], uuid[2], uuid[3], uuid[4], uuid[5], uuid[6], uuid[7], uuid[8], uuid[9], uuid[10], uuid[11], uuid[12], uuid[13], uuid[14], uuid[15]) < 0) goto no_memory; - if (bufferVSprintf(buf, " <memory>%d</memory>\n", def->maxmem) < 0) - goto no_memory; - if (bufferVSprintf(buf, " <currentMemory>%d</currentMemory>\n", def->memory) < 0) - goto no_memory; - if (bufferVSprintf(buf, " <vcpu>%d</vcpu>\n", def->vcpus) < 0) - goto no_memory; - - if (bufferAdd(buf, " <os>\n", -1) < 0) + if (virBufferVSprintf(buf, " <memory>%d</memory>\n", def->maxmem) < 0) + goto no_memory; + if (virBufferVSprintf(buf, " <currentMemory>%d</currentMemory>\n", def->memory) < 0) + goto no_memory; + if (virBufferVSprintf(buf, " <vcpu>%d</vcpu>\n", def->vcpus) < 0) + goto no_memory; + + if (virBufferAdd(buf, " <os>\n", -1) < 0) goto no_memory;
if (def->virtType == QEMUD_VIRT_QEMU) { - if (bufferVSprintf(buf, " <type arch='%s' machine='%s'>%s</type>\n", + if (virBufferVSprintf(buf, " <type arch='%s' machine='%s'>%s</type>\n", def->os.arch, def->os.machine, def->os.type) < 0) goto no_memory; } else { - if (bufferVSprintf(buf, " <type>%s</type>\n", def->os.type) < 0) + if (virBufferVSprintf(buf, " <type>%s</type>\n", def->os.type) < 0) goto no_memory; }
if (def->os.kernel[0]) - if (bufferVSprintf(buf, " <kernel>%s</kernel>\n", def->os.kernel) < 0) + if (virBufferVSprintf(buf, " <kernel>%s</kernel>\n", def->os.kernel) < 0) goto no_memory; if (def->os.initrd[0]) - if (bufferVSprintf(buf, " <initrd>%s</initrd>\n", def->os.initrd) < 0) + if (virBufferVSprintf(buf, " <initrd>%s</initrd>\n", def->os.initrd) < 0) goto no_memory; if (def->os.cmdline[0]) - if (bufferVSprintf(buf, " <cmdline>%s</cmdline>\n", def->os.cmdline) < 0) + if (virBufferVSprintf(buf, " <cmdline>%s</cmdline>\n", def->os.cmdline) < 0) goto no_memory;
for (n = 0 ; n < def->os.nBootDevs ; n++) { @@ -2543,38 +2543,38 @@ char *qemudGenerateXML(struct qemud_driv boottype = "net"; break; } - if (bufferVSprintf(buf, " <boot dev='%s'/>\n", boottype) < 0) - goto no_memory; - } - - if (bufferAdd(buf, " </os>\n", -1) < 0) + if (virBufferVSprintf(buf, " <boot dev='%s'/>\n", boottype) < 0) + goto no_memory; + } + + if (virBufferAdd(buf, " </os>\n", -1) < 0) goto no_memory;
if (def->features & QEMUD_FEATURE_ACPI) { - if (bufferAdd(buf, " <features>\n", -1) < 0) - goto no_memory; - if (bufferAdd(buf, " <acpi/>\n", -1) < 0) - goto no_memory; - if (bufferAdd(buf, " </features>\n", -1) < 0) - goto no_memory; - } - - if (bufferAdd(buf, " <on_poweroff>destroy</on_poweroff>\n", -1) < 0) + if (virBufferAdd(buf, " <features>\n", -1) < 0) + goto no_memory; + if (virBufferAdd(buf, " <acpi/>\n", -1) < 0) + goto no_memory; + if (virBufferAdd(buf, " </features>\n", -1) < 0) + goto no_memory; + } + + if (virBufferAdd(buf, " <on_poweroff>destroy</on_poweroff>\n", -1) < 0) goto no_memory; if (def->noReboot) { - if (bufferAdd(buf, " <on_reboot>destroy</on_reboot>\n", -1) < 0) + if (virBufferAdd(buf, " <on_reboot>destroy</on_reboot>\n", -1) < 0) goto no_memory; } else { - if (bufferAdd(buf, " <on_reboot>restart</on_reboot>\n", -1) < 0) - goto no_memory; - } - if (bufferAdd(buf, " <on_crash>destroy</on_crash>\n", -1) < 0) - goto no_memory; - - if (bufferAdd(buf, " <devices>\n", -1) < 0) - goto no_memory; - - if (bufferVSprintf(buf, " <emulator>%s</emulator>\n", def->os.binary) < 0) + if (virBufferAdd(buf, " <on_reboot>restart</on_reboot>\n", -1) < 0) + goto no_memory; + } + if (virBufferAdd(buf, " <on_crash>destroy</on_crash>\n", -1) < 0) + goto no_memory; + + if (virBufferAdd(buf, " <devices>\n", -1) < 0) + goto no_memory; + + if (virBufferVSprintf(buf, " <emulator>%s</emulator>\n", def->os.binary) < 0) goto no_memory;
disk = def->disks; @@ -2592,21 +2592,21 @@ char *qemudGenerateXML(struct qemud_driv "cdrom", "floppy", }; - if (bufferVSprintf(buf, " <disk type='%s' device='%s'>\n", + if (virBufferVSprintf(buf, " <disk type='%s' device='%s'>\n", types[disk->type], devices[disk->device]) < 0) goto no_memory;
- if (bufferVSprintf(buf, " <source %s='%s'/>\n", typeAttrs[disk->type], disk->src) < 0) - goto no_memory; - - if (bufferVSprintf(buf, " <target dev='%s'/>\n", disk->dst) < 0) + if (virBufferVSprintf(buf, " <source %s='%s'/>\n", typeAttrs[disk->type], disk->src) < 0) + goto no_memory; + + if (virBufferVSprintf(buf, " <target dev='%s'/>\n", disk->dst) < 0) goto no_memory;
if (disk->readonly) - if (bufferAdd(buf, " <readonly/>\n", -1) < 0) + if (virBufferAdd(buf, " <readonly/>\n", -1) < 0) goto no_memory;
- if (bufferVSprintf(buf, " </disk>\n") < 0) + if (virBufferVSprintf(buf, " </disk>\n") < 0) goto no_memory;
disk = disk->next; @@ -2623,42 +2623,42 @@ char *qemudGenerateXML(struct qemud_driv "network", "bridge", }; - if (bufferVSprintf(buf, " <interface type='%s'>\n", + if (virBufferVSprintf(buf, " <interface type='%s'>\n", types[net->type]) < 0) goto no_memory;
- if (bufferVSprintf(buf, " <mac address='%02x:%02x:%02x:%02x:%02x:%02x'/>\n", + if (virBufferVSprintf(buf, " <mac address='%02x:%02x:%02x:%02x:%02x:%02x'/>\n", net->mac[0], net->mac[1], net->mac[2], net->mac[3], net->mac[4], net->mac[5]) < 0) goto no_memory;
switch (net->type) { case QEMUD_NET_NETWORK: - if (bufferVSprintf(buf, " <source network='%s'/>\n", net->dst.network.name) < 0) + if (virBufferVSprintf(buf, " <source network='%s'/>\n", net->dst.network.name) < 0) goto no_memory;
if (net->dst.network.ifname[0] != '\0') { - if (bufferVSprintf(buf, " <target dev='%s'/>\n", net->dst.network.ifname) < 0) + if (virBufferVSprintf(buf, " <target dev='%s'/>\n", net->dst.network.ifname) < 0) goto no_memory; } break;
case QEMUD_NET_ETHERNET: if (net->dst.ethernet.ifname[0] != '\0') { - if (bufferVSprintf(buf, " <target dev='%s'/>\n", net->dst.ethernet.ifname) < 0) + if (virBufferVSprintf(buf, " <target dev='%s'/>\n", net->dst.ethernet.ifname) < 0) goto no_memory; } if (net->dst.ethernet.script[0] != '\0') { - if (bufferVSprintf(buf, " <script path='%s'/>\n", net->dst.ethernet.script) < 0) + if (virBufferVSprintf(buf, " <script path='%s'/>\n", net->dst.ethernet.script) < 0) goto no_memory; } break;
case QEMUD_NET_BRIDGE: - if (bufferVSprintf(buf, " <source bridge='%s'/>\n", net->dst.bridge.brname) < 0) + if (virBufferVSprintf(buf, " <source bridge='%s'/>\n", net->dst.bridge.brname) < 0) goto no_memory; if (net->dst.bridge.ifname[0] != '\0') { - if (bufferVSprintf(buf, " <target dev='%s'/>\n", net->dst.bridge.ifname) < 0) + if (virBufferVSprintf(buf, " <target dev='%s'/>\n", net->dst.bridge.ifname) < 0) goto no_memory; } break; @@ -2667,17 +2667,17 @@ char *qemudGenerateXML(struct qemud_driv case QEMUD_NET_CLIENT: case QEMUD_NET_MCAST: if (net->dst.socket.address[0] != '\0') { - if (bufferVSprintf(buf, " <source address='%s' port='%d'/>\n", + if (virBufferVSprintf(buf, " <source address='%s' port='%d'/>\n", net->dst.socket.address, net->dst.socket.port) < 0) goto no_memory; } else { - if (bufferVSprintf(buf, " <source port='%d'/>\n", + if (virBufferVSprintf(buf, " <source port='%d'/>\n", net->dst.socket.port) < 0) goto no_memory; } }
- if (bufferVSprintf(buf, " </interface>\n") < 0) + if (virBufferVSprintf(buf, " </interface>\n") < 0) goto no_memory;
net = net->next; @@ -2685,20 +2685,20 @@ char *qemudGenerateXML(struct qemud_driv
switch (def->graphicsType) { case QEMUD_GRAPHICS_VNC: - if (bufferAdd(buf, " <graphics type='vnc'", -1) < 0) + if (virBufferAdd(buf, " <graphics type='vnc'", -1) < 0) goto no_memory;
if (def->vncPort && - bufferVSprintf(buf, " port='%d'", + virBufferVSprintf(buf, " port='%d'", qemudIsActiveVM(vm) && live ? def->vncActivePort : def->vncPort) < 0) goto no_memory;
- if (bufferAdd(buf, "/>\n", -1) < 0) + if (virBufferAdd(buf, "/>\n", -1) < 0) goto no_memory; break;
case QEMUD_GRAPHICS_SDL: - if (bufferAdd(buf, " <graphics type='sdl'/>\n", -1) < 0) + if (virBufferAdd(buf, " <graphics type='sdl'/>\n", -1) < 0) goto no_memory; break;
@@ -2710,19 +2710,19 @@ char *qemudGenerateXML(struct qemud_driv if (def->graphicsType == QEMUD_GRAPHICS_VNC) { }
- if (bufferAdd(buf, " </devices>\n", -1) < 0) - goto no_memory; - - - if (bufferAdd(buf, "</domain>\n", -1) < 0) - goto no_memory; - - return bufferContentAndFree (buf); + if (virBufferAdd(buf, " </devices>\n", -1) < 0) + goto no_memory; + + + if (virBufferAdd(buf, "</domain>\n", -1) < 0) + goto no_memory; + + return virBufferContentAndFree (buf);
no_memory: qemudReportError(NULL, NULL, NULL, VIR_ERR_NO_MEMORY, "xml"); cleanup: - if (buf) bufferFree (buf); + if (buf) virBufferFree (buf); return NULL; }
@@ -2730,21 +2730,21 @@ char *qemudGenerateNetworkXML(struct qem char *qemudGenerateNetworkXML(struct qemud_driver *driver, struct qemud_network *network, struct qemud_network_def *def) { - bufferPtr buf = 0; + virBufferPtr buf = 0; unsigned char *uuid;
- buf = bufferNew (QEMUD_MAX_XML_LEN); + buf = virBufferNew (QEMUD_MAX_XML_LEN); if (!buf) goto no_memory;
- if (bufferVSprintf(buf, "<network>\n") < 0) - goto no_memory; - - if (bufferVSprintf(buf, " <name>%s</name>\n", def->name) < 0) + if (virBufferVSprintf(buf, "<network>\n") < 0) + goto no_memory; + + if (virBufferVSprintf(buf, " <name>%s</name>\n", def->name) < 0) goto no_memory;
uuid = def->uuid; - if (bufferVSprintf(buf, " <uuid>%02x%02x%02x%02x-%02x%02x-%02x%02x-%02x%02x-%02x%02x%02x%02x%02x%02x</uuid>\n", + if (virBufferVSprintf(buf, " <uuid>%02x%02x%02x%02x-%02x%02x-%02x%02x-%02x%02x-%02x%02x%02x%02x%02x%02x</uuid>\n", uuid[0], uuid[1], uuid[2], uuid[3], uuid[4], uuid[5], uuid[6], uuid[7], uuid[8], uuid[9], uuid[10], uuid[11], @@ -2753,67 +2753,67 @@ char *qemudGenerateNetworkXML(struct qem
if (def->forward) { if (def->forwardDev[0]) { - bufferVSprintf(buf, " <forward dev='%s'/>\n", + virBufferVSprintf(buf, " <forward dev='%s'/>\n", def->forwardDev); } else { - bufferAdd(buf, " <forward/>\n", -1); - } - } - - bufferAdd(buf, " <bridge", -1); + virBufferAdd(buf, " <forward/>\n", -1); + } + } + + virBufferAdd(buf, " <bridge", -1); if (qemudIsActiveNetwork(network)) { - if (bufferVSprintf(buf, " name='%s'", network->bridge) < 0) + if (virBufferVSprintf(buf, " name='%s'", network->bridge) < 0) goto no_memory; } else if (def->bridge[0]) { - if (bufferVSprintf(buf, " name='%s'", def->bridge) < 0) - goto no_memory; - } - if (bufferVSprintf(buf, " stp='%s' forwardDelay='%d' />\n", + if (virBufferVSprintf(buf, " name='%s'", def->bridge) < 0) + goto no_memory; + } + if (virBufferVSprintf(buf, " stp='%s' forwardDelay='%d' />\n", def->disableSTP ? "off" : "on", def->forwardDelay) < 0) goto no_memory;
if (def->ipAddress[0] || def->netmask[0]) { - if (bufferAdd(buf, " <ip", -1) < 0) + if (virBufferAdd(buf, " <ip", -1) < 0) goto no_memory;
if (def->ipAddress[0] && - bufferVSprintf(buf, " address='%s'", def->ipAddress) < 0) + virBufferVSprintf(buf, " address='%s'", def->ipAddress) < 0) goto no_memory;
if (def->netmask[0] && - bufferVSprintf(buf, " netmask='%s'", def->netmask) < 0) - goto no_memory; - - if (bufferAdd(buf, ">\n", -1) < 0) + virBufferVSprintf(buf, " netmask='%s'", def->netmask) < 0) + goto no_memory; + + if (virBufferAdd(buf, ">\n", -1) < 0) goto no_memory;
if (def->ranges) { struct qemud_dhcp_range_def *range = def->ranges; - if (bufferAdd(buf, " <dhcp>\n", -1) < 0) + if (virBufferAdd(buf, " <dhcp>\n", -1) < 0) goto no_memory; while (range) { - if (bufferVSprintf(buf, " <range start='%s' end='%s' />\n", + if (virBufferVSprintf(buf, " <range start='%s' end='%s' />\n", range->start, range->end) < 0) goto no_memory; range = range->next; } - if (bufferAdd(buf, " </dhcp>\n", -1) < 0) + if (virBufferAdd(buf, " </dhcp>\n", -1) < 0) goto no_memory; }
- if (bufferAdd(buf, " </ip>\n", -1) < 0) - goto no_memory; - } - - if (bufferAdd(buf, "</network>\n", -1) < 0) - goto no_memory; - - return bufferContentAndFree (buf); + if (virBufferAdd(buf, " </ip>\n", -1) < 0) + goto no_memory; + } + + if (virBufferAdd(buf, "</network>\n", -1) < 0) + goto no_memory; + + return virBufferContentAndFree (buf);
no_memory: qemudReportError(NULL, NULL, NULL, VIR_ERR_NO_MEMORY, "xml"); - if (buf) bufferFree (buf); + if (buf) virBufferFree (buf); return NULL; }
diff -r b8a4e065009d qemud/driver.c --- a/qemud/driver.c Sun Jun 17 22:11:26 2007 -0400 +++ b/qemud/driver.c Sun Jun 17 22:11:32 2007 -0400 @@ -1391,7 +1391,7 @@ char *qemudGetCapabilities(struct qemud_ int i, j, r; int have_kqemu = 0; int have_kvm = 0; - bufferPtr xml; + virBufferPtr xml;
/* Really, this never fails - look at the man-page. */ uname (&utsname); @@ -1400,13 +1400,13 @@ char *qemudGetCapabilities(struct qemud_ have_kvm = access ("/dev/kvm", F_OK) == 0;
/* Construct the XML. */ - xml = bufferNew (1024); + xml = virBufferNew (1024); if (!xml) { qemudReportError(NULL, NULL, NULL, VIR_ERR_NO_MEMORY, NULL); return NULL; }
- r = bufferVSprintf (xml, + r = virBufferVSprintf (xml, "\ <capabilities>\n\ <host>\n\ @@ -1417,7 +1417,7 @@ char *qemudGetCapabilities(struct qemud_ utsname.machine); if (r == -1) { vir_buffer_failed: - bufferFree (xml); + virBufferFree (xml); qemudReportError(NULL, NULL, NULL, VIR_ERR_NO_MEMORY, NULL); return NULL; } @@ -1427,7 +1427,7 @@ char *qemudGetCapabilities(struct qemud_ else if (strcmp (utsname.machine, "x86_64") == 0) i = 1; if (i >= 0) { /* For the default (PC-like) guest, qemudArchs[0] or [1]. */ - r = bufferVSprintf (xml, + r = virBufferVSprintf (xml, "\ \n\ <guest>\n\ @@ -1442,7 +1442,7 @@ char *qemudGetCapabilities(struct qemud_ if (r == -1) goto vir_buffer_failed;
for (j = 0; qemudArchs[i].machines[j]; ++j) { - r = bufferVSprintf (xml, + r = virBufferVSprintf (xml, "\ <machine>%s</machine>\n", qemudArchs[i].machines[j]); @@ -1450,20 +1450,20 @@ char *qemudGetCapabilities(struct qemud_ }
if (have_kqemu) { - r = bufferAdd (xml, + r = virBufferAdd (xml, "\ <domain type=\"kqemu\"/>\n", -1); if (r == -1) goto vir_buffer_failed; } if (have_kvm) { - r = bufferAdd (xml, + r = virBufferAdd (xml, "\ <domain type=\"kvm\">\n\ <emulator>/usr/bin/qemu-kvm</emulator>\n\ </domain>\n", -1); if (r == -1) goto vir_buffer_failed; } - r = bufferAdd (xml, + r = virBufferAdd (xml, "\ </arch>\n\ </guest>\n", -1); @@ -1471,7 +1471,7 @@ char *qemudGetCapabilities(struct qemud_
/* The "other" PC architecture needs emulation. */ i = i ^ 1; - r = bufferVSprintf (xml, + r = virBufferVSprintf (xml, "\ \n\ <guest>\n\ @@ -1485,13 +1485,13 @@ char *qemudGetCapabilities(struct qemud_ qemudArchs[i].binary); if (r == -1) goto vir_buffer_failed; for (j = 0; qemudArchs[i].machines[j]; ++j) { - r = bufferVSprintf (xml, + r = virBufferVSprintf (xml, "\ <machine>%s</machine>\n", qemudArchs[i].machines[j]); if (r == -1) goto vir_buffer_failed; } - r = bufferAdd (xml, + r = virBufferAdd (xml, "\ </arch>\n\ </guest>\n", -1); @@ -1500,7 +1500,7 @@ char *qemudGetCapabilities(struct qemud_
/* The non-PC architectures, qemudArchs[>=2]. */ for (i = 2; qemudArchs[i].arch; ++i) { - r = bufferVSprintf (xml, + r = virBufferVSprintf (xml, "\ \n\ <guest>\n\ @@ -1514,13 +1514,13 @@ char *qemudGetCapabilities(struct qemud_ qemudArchs[i].binary); if (r == -1) goto vir_buffer_failed; for (j = 0; qemudArchs[i].machines[j]; ++j) { - r = bufferVSprintf (xml, + r = virBufferVSprintf (xml, "\ <machine>%s</machine>\n", qemudArchs[i].machines[j]); if (r == -1) goto vir_buffer_failed; } - r = bufferAdd (xml, + r = virBufferAdd (xml, "\ </arch>\n\ </guest>\n", -1); @@ -1528,12 +1528,12 @@ char *qemudGetCapabilities(struct qemud_ }
/* Finish off. */ - r = bufferAdd (xml, + r = virBufferAdd (xml, "\ </capabilities>\n", -1); if (r == -1) goto vir_buffer_failed;
- return bufferContentAndFree(xml); + return virBufferContentAndFree(xml); }
-- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
-- |=- 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 -=|
On Mon, Jun 18, 2007 at 03:27:03AM +0100, Daniel P. Berrange wrote:
The bufferNNN functions in the qemud/buf.c file are identical those already present in the src/xml.c file, except the latter are named virBufferNNN. This patch renames bufferNNN to virBufferNNN so that the two impls can be merged into one. No functional change
uncontroversial change, +1 Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
Daniel Veillard wrote:
On Mon, Jun 18, 2007 at 03:27:03AM +0100, Daniel P. Berrange wrote:
The bufferNNN functions in the qemud/buf.c file are identical those already present in the src/xml.c file, except the latter are named virBufferNNN. This patch renames bufferNNN to virBufferNNN so that the two impls can be merged into one. No functional change
uncontroversial change, +1
I thought we'd already done that ... but yes +1 Rich. -- Emerging Technologies, Red Hat - http://et.redhat.com/~rjones/ Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod Street, Windsor, Berkshire, SL4 1TE, United Kingdom. Registered in England and Wales under Company Registration No. 03798903
The virBuffer functions are currently held in src/xml.c file. This patch splits them out into a self-contained src/buf.c file. In combination with the previous patch this will make the move of QEMU code from qemud/ to src/ easier, since both dirs have identical buf.c/.h files now. b/src/buf.c | 210 ++++++++++++++++++++++++++++++++++++++++++++++++++++ b/src/buf.h | 37 +++++++++ src/Makefile.am | 1 src/conf.c | 2 src/test.c | 1 src/xen_internal.c | 2 src/xend_internal.c | 1 src/xm_internal.c | 1 src/xml.c | 172 ------------------------------------------ src/xml.h | 20 ---- 10 files changed, 254 insertions(+), 193 deletions(-) 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 -=|
And with the patch attached this time... On Mon, Jun 18, 2007 at 03:30:11AM +0100, Daniel P. Berrange wrote:
The virBuffer functions are currently held in src/xml.c file. This patch splits them out into a self-contained src/buf.c file. In combination with the previous patch this will make the move of QEMU code from qemud/ to src/ easier, since both dirs have identical buf.c/.h files now.
b/src/buf.c | 210 ++++++++++++++++++++++++++++++++++++++++++++++++++++ b/src/buf.h | 37 +++++++++ src/Makefile.am | 1 src/conf.c | 2 src/test.c | 1 src/xen_internal.c | 2 src/xend_internal.c | 1 src/xm_internal.c | 1 src/xml.c | 172 ------------------------------------------ src/xml.h | 20 ---- 10 files changed, 254 insertions(+), 193 deletions(-)
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 -=|
-- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
-- |=- 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 -=|
On Mon, Jun 18, 2007 at 03:31:46AM +0100, Daniel P. Berrange wrote:
And with the patch attached this time...
On Mon, Jun 18, 2007 at 03:30:11AM +0100, Daniel P. Berrange wrote:
The virBuffer functions are currently held in src/xml.c file. This patch splits them out into a self-contained src/buf.c file. In combination with the previous patch this will make the move of QEMU code from qemud/ to src/ easier, since both dirs have identical buf.c/.h files now.
Excellent, yes +1 ! Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
Daniel Veillard wrote:
On Mon, Jun 18, 2007 at 03:31:46AM +0100, Daniel P. Berrange wrote:
And with the patch attached this time...
On Mon, Jun 18, 2007 at 03:30:11AM +0100, Daniel P. Berrange wrote:
The virBuffer functions are currently held in src/xml.c file. This patch splits them out into a self-contained src/buf.c file. In combination with the previous patch this will make the move of QEMU code from qemud/ to src/ easier, since both dirs have identical buf.c/.h files now.
Excellent, yes +1 !
Yes, good cleanup. +1 Rich. -- Emerging Technologies, Red Hat - http://et.redhat.com/~rjones/ Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod Street, Windsor, Berkshire, SL4 1TE, United Kingdom. Registered in England and Wales under Company Registration No. 03798903
On Sun, Jun 17, 2007 at 10:48:17PM +0100, Daniel P. Berrange wrote:
The 3 patches which follow are work-in-progress to re-factor the QEMU daemon / driver to eventually adhere to the main libvirt internal driver API. Once this work is complete, there will only need to be a single daemon running which can provide both remote & QEMU capabilities at once with no QEMU specific code in it.
Just to clarify, we will still need one process to be forked per QEmu instance which is under control, right ?
This is following some of the ideas I set out here
http://www.redhat.com/archives/libvir-list/2007-May/msg00083.html
yes this is teh right time to get this fixed ! thanks, Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
On Mon, Jun 18, 2007 at 05:32:45AM -0400, Daniel Veillard wrote:
On Sun, Jun 17, 2007 at 10:48:17PM +0100, Daniel P. Berrange wrote:
The 3 patches which follow are work-in-progress to re-factor the QEMU daemon / driver to eventually adhere to the main libvirt internal driver API. Once this work is complete, there will only need to be a single daemon running which can provide both remote & QEMU capabilities at once with no QEMU specific code in it.
Just to clarify, we will still need one process to be forked per QEmu instance which is under control, right ?
Yes to be clear. This is changing from a model where we have processes: libvirtd libvirt_qemud | +- qemu +- qemu +- dnsmasq To merge the two daemons so we have libvirtd | +- qemu +- qemu +- dnsmasq The single daemon serves as both the remote daemon & QEMU daemon all in one, with no need for QEMU specific code.
This is following some of the ideas I set out here
http://www.redhat.com/archives/libvir-list/2007-May/msg00083.html
yes this is teh right time to get this fixed !
Yep, my primary motiviation is that we just broke QEMU protocol capability. So we should merge these daemons now to avoid having to break it a second time. 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 -=|
On Mon, Jun 18, 2007 at 12:02:02PM +0100, Daniel P. Berrange wrote:
On Mon, Jun 18, 2007 at 05:32:45AM -0400, Daniel Veillard wrote:
On Sun, Jun 17, 2007 at 10:48:17PM +0100, Daniel P. Berrange wrote:
The 3 patches which follow are work-in-progress to re-factor the QEMU daemon / driver to eventually adhere to the main libvirt internal driver API. Once this work is complete, there will only need to be a single daemon running which can provide both remote & QEMU capabilities at once with no QEMU specific code in it.
Just to clarify, we will still need one process to be forked per QEmu instance which is under control, right ?
Yes to be clear. This is changing from a model where we have processes:
libvirtd libvirt_qemud | +- qemu +- qemu +- dnsmasq
To merge the two daemons so we have
libvirtd | +- qemu +- qemu +- dnsmasq
The single daemon serves as both the remote daemon & QEMU daemon all in one, with no need for QEMU specific code.
okay, what I though, but thanks for the explanation !
This is following some of the ideas I set out here
http://www.redhat.com/archives/libvir-list/2007-May/msg00083.html
yes this is teh right time to get this fixed !
Yep, my primary motiviation is that we just broke QEMU protocol capability. So we should merge these daemons now to avoid having to break it a second time.
okay Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
Daniel P. Berrange wrote:
On Mon, Jun 18, 2007 at 05:32:45AM -0400, Daniel Veillard wrote:
On Sun, Jun 17, 2007 at 10:48:17PM +0100, Daniel P. Berrange wrote:
The 3 patches which follow are work-in-progress to re-factor the QEMU daemon / driver to eventually adhere to the main libvirt internal driver API. Once this work is complete, there will only need to be a single daemon running which can provide both remote & QEMU capabilities at once with no QEMU specific code in it. Just to clarify, we will still need one process to be forked per QEmu instance which is under control, right ?
Yes to be clear. This is changing from a model where we have processes:
libvirtd libvirt_qemud | +- qemu +- qemu +- dnsmasq
To merge the two daemons so we have
libvirtd | +- qemu +- qemu +- dnsmasq
The single daemon serves as both the remote daemon & QEMU daemon all in one, with no need for QEMU specific code.
I'm still looking at your first (and most complicated patch), so the answer may well be in there, but just to check anyway: does this solve the deadlock where we try to do qemu-over-remote? http://www.redhat.com/archives/libvir-list/2007-April/msg00122.html Rich. -- Emerging Technologies, Red Hat - http://et.redhat.com/~rjones/ Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod Street, Windsor, Berkshire, SL4 1TE, United Kingdom. Registered in England and Wales under Company Registration No. 03798903
On Mon, Jun 18, 2007 at 02:25:36PM +0100, Richard W.M. Jones wrote:
Daniel P. Berrange wrote:
On Mon, Jun 18, 2007 at 05:32:45AM -0400, Daniel Veillard wrote:
On Sun, Jun 17, 2007 at 10:48:17PM +0100, Daniel P. Berrange wrote:
The 3 patches which follow are work-in-progress to re-factor the QEMU daemon / driver to eventually adhere to the main libvirt internal driver API. Once this work is complete, there will only need to be a single daemon running which can provide both remote & QEMU capabilities at once with no QEMU specific code in it. Just to clarify, we will still need one process to be forked per QEmu instance which is under control, right ?
Yes to be clear. This is changing from a model where we have processes:
libvirtd libvirt_qemud | +- qemu +- qemu +- dnsmasq
To merge the two daemons so we have
libvirtd | +- qemu +- qemu +- dnsmasq
The single daemon serves as both the remote daemon & QEMU daemon all in one, with no need for QEMU specific code.
I'm still looking at your first (and most complicated patch), so the answer may well be in there, but just to check anyway: does this solve the deadlock where we try to do qemu-over-remote?
Not yet, because at this point we've still got two daemons. Once the patch set is complete, moving the QEMU stuff into libvirt.so with the real driver API, then there will only be one daemon & thus we should not have any deadlock issue. 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 -=|
participants (3)
-
Daniel P. Berrange -
Daniel Veillard -
Richard W.M. Jones