[Libvir] PATCH 0/20: Re-factor libvirtd / QEMU driver
The work to refactor the QEMU driver to use the regular libvirt driver API is now complete. The next 20 (!) patches provide fairly fine-grained steps refactoring the code. Aside from that which adds an event loop, and the very last one, they should all be pretty much straightforward refactoring with no functional change. With this patch series fully applied there is now only a single daemon which can serve both remote & QEMU drivers in one go, with no deadlock issues. 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 attached patch provides a generic event loop engine for the daemon. It re-factors the qemud/qemu.c to make use of this event loop implementation. NB, with this patch applied, the automatic timeout for idle shutdown is not available. This will be re-introduced in a later patch.... b/qemud/event.c | 477 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ b/qemud/event.h | 98 +++++++++++ qemud/Makefile.am | 3 qemud/internal.h | 1 qemud/qemud.c | 400 ++++++++++++++++++++++++--------------------- 5 files changed, 791 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 -=|
On Fri, Jun 22, 2007 at 02:37:26AM +0100, Daniel P. Berrange wrote:
The attached patch provides a generic event loop engine for the daemon. It re-factors the qemud/qemu.c to make use of this event loop implementation. NB, with this patch applied, the automatic timeout for idle shutdown is not available. This will be re-introduced in a later patch....
Comments and allocations in blocks, that resolves my issues with the prtevious version of the patch, excellent ! +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/
This patch moves the QEMU driver capabilities XML generation code out of the dispatch.c file and into the driver.c where the rest of the QEMU driver impls live. This is to enable the dispatch.c file to be removed later. 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 -=|
Attached this time.. On Fri, Jun 22, 2007 at 02:38:50AM +0100, Daniel P. Berrange wrote:
This patch moves the QEMU driver capabilities XML generation code out of the dispatch.c file and into the driver.c where the rest of the QEMU driver impls live. This is to enable the dispatch.c file to be removed later.
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 -=|
On Fri, Jun 22, 2007 at 02:40:55AM +0100, Daniel P. Berrange wrote:
Attached this time..
On Fri, Jun 22, 2007 at 02:38:50AM +0100, Daniel P. Berrange wrote:
This patch moves the QEMU driver capabilities XML generation code out of the dispatch.c file and into the driver.c where the rest of the QEMU driver impls live. This is to enable the dispatch.c file to be removed later.
dispatch.c | 193 +++++-------------------------------------------------------- driver.c | 184 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-- driver.h | 11 ++- 3 files changed, 203 insertions(+), 185 deletions(-)
okay, cosmetic internal interface changes and code moved around, fine +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/
This patch moves the code which deals with starting and stopping of domains and networks out of qemud.d and into the main QEMU driver.c file. driver.c | 1067 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ driver.h | 14 internal.h | 12 qemud.c | 1029 ---------------------------------------------------------- 4 files changed, 1083 insertions(+), 1039 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 -=|
Attached this time... On Fri, Jun 22, 2007 at 02:40:20AM +0100, Daniel P. Berrange wrote:
This patch moves the code which deals with starting and stopping of domains and networks out of qemud.d and into the main QEMU driver.c file.
driver.c | 1067 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ driver.h | 14 internal.h | 12 qemud.c | 1029 ---------------------------------------------------------- 4 files changed, 1083 insertions(+), 1039 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 Fri, Jun 22, 2007 at 02:41:30AM +0100, Daniel P. Berrange wrote:
Attached this time...
On Fri, Jun 22, 2007 at 02:40:20AM +0100, Daniel P. Berrange wrote:
This patch moves the code which deals with starting and stopping of domains and networks out of qemud.d and into the main QEMU driver.c file.
driver.c | 1067 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ driver.h | 14 internal.h | 12 qemud.c | 1029 ---------------------------------------------------------- 4 files changed, 1083 insertions(+), 1039 deletions(-)
okay moving code, +1 Now if we had a structured version of patch that would report things like moved blocks that would be cool :-) 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/
This patch moves the code which deals with loading QEMU config files, and autostart of domains & networks out of the qemud.c and into the driver.c file. These will later be invoked via a new libvirt internal driver API method. 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 -=|
On Fri, Jun 22, 2007 at 02:43:46AM +0100, Daniel P. Berrange wrote:
This patch moves the code which deals with loading QEMU config files, and autostart of domains & networks out of the qemud.c and into the driver.c file. These will later be invoked via a new libvirt internal driver API method.
driver.c | 82 ++++++++++++++++++++++++++++++++++++++++----------------------- driver.h | 2 + qemud.c | 45 ++-------------------------------- 3 files changed, 58 insertions(+), 71 deletions(-)
Looks fine, +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/
This patch makes the daemon re-use the main libvirt error API instead of inventing its own duplicate. As a temporary hack, the daemon compiles a copy of virterror.c directly. This will go away in a future patch Makefile.am | 5 + conf.c | 192 ++++++++++++++++++++++++++++++------------------------------ dispatch.c | 16 +++-- driver.c | 174 ++++++++++++++++++++++++++++++------------------------ driver.h | 6 + internal.h | 2 qemud.c | 4 - 7 files changed, 215 insertions(+), 184 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 Fri, Jun 22, 2007 at 02:45:39AM +0100, Daniel P. Berrange wrote:
This patch makes the daemon re-use the main libvirt error API instead of inventing its own duplicate. As a temporary hack, the daemon compiles a copy of virterror.c directly. This will go away in a future patch
Makefile.am | 5 + conf.c | 192 ++++++++++++++++++++++++++++++------------------------------ dispatch.c | 16 +++-- driver.c | 174 ++++++++++++++++++++++++++++++------------------------ driver.h | 6 + internal.h | 2 qemud.c | 4 - 7 files changed, 215 insertions(+), 184 deletions(-)
Seems I already +1 that on previous round :-) 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/
This is the 2nd biggest patch of the series. It is a major refactoring of the data structures, to split qemud_server into a smaller qemud_server and a new qemud_driver. conf.c | 251 ++++++++++---------- conf.h | 294 +++++++++++++++++++++++- dispatch.c | 295 ++++++++++-------------- driver.c | 733 ++++++++++++++++++++++++++++++++++--------------------------- driver.h | 98 +++----- internal.h | 236 ------------------- qemud.c | 50 ---- 7 files changed, 992 insertions(+), 965 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 Fri, Jun 22, 2007 at 02:48:38AM +0100, Daniel P. Berrange wrote:
This is the 2nd biggest patch of the series. It is a major refactoring of the data structures, to split qemud_server into a smaller qemud_server and a new qemud_driver.
conf.c | 251 ++++++++++---------- conf.h | 294 +++++++++++++++++++++++- dispatch.c | 295 ++++++++++-------------- driver.c | 733 ++++++++++++++++++++++++++++++++++--------------------------- driver.h | 98 +++----- internal.h | 236 ------------------- qemud.c | 50 ---- 7 files changed, 992 insertions(+), 965 deletions(-)
[...]
diff -r 4684eb84957d qemud/conf.h --- a/qemud/conf.h Thu Jun 21 16:14:19 2007 -0400 +++ b/qemud/conf.h Thu Jun 21 16:14:44 2007 -0400 @@ -24,15 +24,277 @@ #ifndef __QEMUD_CONF_H #define __QEMUD_CONF_H [...] + +static inline int +qemudIsActiveVM(struct qemud_vm *vm) +{ + return vm->id != -1; +} + +static inline int +qemudIsActiveNetwork(struct qemud_network *network) +{ + return network->active; +}
I'm not too fond of this, this assumes a compiler accepting the inlining and or having code being defined in the config file. I way prefer a simple macro, that's more portable. Not a blocker though, but that makes me feel uneasy. Let's use a real function or a macro, it's not like we need to optimize at the tenth of micro second in libvirt :-)
diff -r 4684eb84957d qemud/dispatch.c --- a/qemud/dispatch.c Thu Jun 21 16:14:19 2007 -0400 +++ b/qemud/dispatch.c Thu Jun 21 16:14:44 2007 -0400 [...] +extern struct qemud_driver *qemu_driver;
hum, is that a declaration or ar reference ? I'm alway a bit suspicious of such construct, if shared it goes in the .h as extern, and without extern in one .c , if not shared it should really be static. [...]
diff -r 4684eb84957d qemud/driver.c --- a/qemud/driver.c Thu Jun 21 16:14:19 2007 -0400 +++ b/qemud/driver.c Thu Jun 21 16:26:12 2007 -0400 @@ -23,6 +23,8 @@
#include <config.h>
+#define _GNU_SOURCE /* for asprintf */ +
please no, I understand it's painful and error prone to do the calculation of the target string lenght, but that's really not portable. [...]
+ if (asprintf (&base, "%s/.libvirt/qemu", pw->pw_dir) == -1) { + qemudLog (QEMUD_ERR, "out of memory in asprintf"); + goto out_of_memory; + } + }
plus it's for a path names, use a MAX_PATH buffer, snprintf in it and the strdup() it. It won't be that much longuer and avoid the issue
+ + /* Configuration paths are either ~/.libvirt/qemu/... (session) or + * /etc/libvirt/qemu/... (system). + */ + if (asprintf (&qemu_driver->configDir, "%s", base) == -1) + goto out_of_memory; + + if (asprintf (&qemu_driver->autostartDir, "%s/autostart", base) == -1) + goto out_of_memory; + + if (asprintf (&qemu_driver->networkConfigDir, "%s/networks", base) == -1) + goto out_of_memory; + + if (asprintf (&qemu_driver->networkAutostartDir, "%s/networks/autostart", + base) == -1) + goto out_of_memory;
[...]
@@ -499,25 +596,25 @@ int qemudStartVMDaemon(struct qemud_serv } else vm->def->vncActivePort = vm->def->vncPort;
- if ((strlen(server->logDir) + /* path */ + if ((strlen(driver->logDir) + /* path */ 1 + /* Separator */ strlen(vm->def->name) + /* basename */ 4 + /* suffix .log */ 1 /* NULL */) > PATH_MAX) { qemudReportError(NULL, NULL, NULL, VIR_ERR_INTERNAL_ERROR, "config file path too long: %s/%s.log", - server->logDir, vm->def->name); - return -1; - } - strcpy(logfile, server->logDir); + driver->logDir, vm->def->name); + return -1; + } + strcpy(logfile, driver->logDir); strcat(logfile, "/"); strcat(logfile, vm->def->name); strcat(logfile, ".log");
here too we are doing weird stuff to build paths, the erro code aren't checked a single snprintf to a MAX_PATH buffer followed by an strdup() would be cleaner IMHO. I understand it's not something added by the patch here, but probably woth fixing since similar to previous. [...]
+ if (snprintf(server->logDir, PATH_MAX, "%s/.libvirt/qemu/log", pw->pw_dir) >= PATH_MAX) + goto snprintf_error; +
So server->logDir is a PATH_MAX array embbeded in the structure, I'm sure we don't need this and can allocate the string instead, no ?
if (asprintf (&base, "%s/.libvirt/qemu", pw->pw_dir) == -1) { qemudLog (QEMUD_ERR, "out of memory in asprintf"); return -1; } }
one more here.
@@ -711,7 +691,6 @@ static struct qemud_server *qemudInitial }
/* We don't have a dom-0, so start from 1 */ - server->nextvmid = 1; server->sigread = sigread;
shouldn't the comment be removed too then ? I would say, still apply the patch but we must remember to clean up those paths allocations. Maybe a single vararg function to build up paths and return the new string would be a good separate cleanup, that we can probably do at a later stage. 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 Fri, Jun 22, 2007 at 07:16:19AM -0400, Daniel Veillard wrote:
On Fri, Jun 22, 2007 at 02:48:38AM +0100, Daniel P. Berrange wrote:
This is the 2nd biggest patch of the series. It is a major refactoring of the data structures, to split qemud_server into a smaller qemud_server and a new qemud_driver.
conf.c | 251 ++++++++++---------- conf.h | 294 +++++++++++++++++++++++- dispatch.c | 295 ++++++++++-------------- driver.c | 733 ++++++++++++++++++++++++++++++++++--------------------------- driver.h | 98 +++----- internal.h | 236 ------------------- qemud.c | 50 ---- 7 files changed, 992 insertions(+), 965 deletions(-)
[...]
diff -r 4684eb84957d qemud/conf.h --- a/qemud/conf.h Thu Jun 21 16:14:19 2007 -0400 +++ b/qemud/conf.h Thu Jun 21 16:14:44 2007 -0400 @@ -24,15 +24,277 @@ #ifndef __QEMUD_CONF_H #define __QEMUD_CONF_H [...] + +static inline int +qemudIsActiveVM(struct qemud_vm *vm) +{ + return vm->id != -1; +} + +static inline int +qemudIsActiveNetwork(struct qemud_network *network) +{ + return network->active; +}
I'm not too fond of this, this assumes a compiler accepting the inlining and or having code being defined in the config file. I way prefer a simple macro, that's more portable.
This was in the code already - I just moved it from internal.h to conf.h
diff -r 4684eb84957d qemud/dispatch.c --- a/qemud/dispatch.c Thu Jun 21 16:14:19 2007 -0400 +++ b/qemud/dispatch.c Thu Jun 21 16:14:44 2007 -0400 [...] +extern struct qemud_driver *qemu_driver;
hum, is that a declaration or ar reference ? I'm alway a bit suspicious of such construct, if shared it goes in the .h as extern, and without extern in one .c , if not shared it should really be static.
It is a reference. It is also an evil short term hack. The entire dispatch.c file is killed off in patch 20, so can ignore it.
[...]
diff -r 4684eb84957d qemud/driver.c --- a/qemud/driver.c Thu Jun 21 16:14:19 2007 -0400 +++ b/qemud/driver.c Thu Jun 21 16:26:12 2007 -0400 @@ -23,6 +23,8 @@
#include <config.h>
+#define _GNU_SOURCE /* for asprintf */ +
please no, I understand it's painful and error prone to do the calculation of the target string lenght, but that's really not portable.
Again just copying existing code here.
[...]
+ if (asprintf (&base, "%s/.libvirt/qemu", pw->pw_dir) == -1) { + qemudLog (QEMUD_ERR, "out of memory in asprintf"); + goto out_of_memory; + } + }
plus it's for a path names, use a MAX_PATH buffer, snprintf in it and the strdup() it. It won't be that much longuer and avoid the issue
[snip]
here too we are doing weird stuff to build paths, the erro code aren't checked a single snprintf to a MAX_PATH buffer followed by an strdup() would be cleaner IMHO. I understand it's not something added by the patch here, but probably woth fixing since similar to previous.
[...]
+ if (snprintf(server->logDir, PATH_MAX, "%s/.libvirt/qemu/log", pw->pw_dir) >= PATH_MAX) + goto snprintf_error; +
So server->logDir is a PATH_MAX array embbeded in the structure, I'm sure we don't need this and can allocate the string instead, no ?
I'll re-visit all these config file / log file path strings later, so we don't mix up plain re-factoring with other changes.
if (asprintf (&base, "%s/.libvirt/qemu", pw->pw_dir) == -1) { qemudLog (QEMUD_ERR, "out of memory in asprintf"); return -1; } }
one more here.
@@ -711,7 +691,6 @@ static struct qemud_server *qemudInitial }
/* We don't have a dom-0, so start from 1 */ - server->nextvmid = 1; server->sigread = sigread;
shouldn't the comment be removed too then ?
Yes - it should be moved to the new place where nextvmid is initialized. 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 pulls the UUID parsing & generation functions out into a separate code module, and renames them to be more general purpose. conf.c | 8 ++++---- uuid.c | 16 ++++++++-------- uuid.h | 14 ++++++++------ 3 files changed, 20 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 Fri, Jun 22, 2007 at 02:49:52AM +0100, Daniel P. Berrange wrote:
This patch pulls the UUID parsing & generation functions out into a separate code module, and renames them to be more general purpose.
no brainer, +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/
This patch renames the buffer functions in the qemud/buf.c file so that they match the name of those in the main src directory. This is in preparation for merging the two implementations. 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 -=|
On Fri, Jun 22, 2007 at 02:51:23AM +0100, Daniel P. Berrange wrote:
This patch renames the buffer functions in the qemud/buf.c file so that they match the name of those in the main src directory. This is in preparation for merging the two implementations.
Sure, already +1 in the last round IIRC, 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/
This patch moves the previously created qemud/buf.c file into the src/ directory and deletes the original implementation in src/xml.c. The qemud/Makefile.am is hacked to compile the code from src/buf.c directly. This hack will go away in a later patch. a/qemud/buf.c | 210 ---------------------------------------------------- a/qemud/buf.h | 37 --------- b/src/buf.c | 210 ++++++++++++++++++++++++++++++++++++++++++++++++++++ b/src/buf.h | 37 +++++++++ proxy/Makefile.am | 2 qemud/Makefile.am | 2 qemud/conf.c | 14 --- qemud/driver.c | 2 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 ---- src/xmlrpc.h | 2 tests/xmlrpctest.c | 2 18 files changed, 260 insertions(+), 458 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 Fri, Jun 22, 2007 at 02:53:23AM +0100, Daniel P. Berrange wrote:
This patch moves the previously created qemud/buf.c file into the src/ directory and deletes the original implementation in src/xml.c. The qemud/Makefile.am is hacked to compile the code from src/buf.c directly. This hack will go away in a later patch.
okay, makes sense, +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:
This patch moves the previously created qemud/buf.c file into the src/ directory and deletes the original implementation in src/xml.c. The qemud/Makefile.am is hacked to compile the code from src/buf.c directly. This hack will go away in a later patch.
No, this is wrong. The two buffer implementations aren't quite the same, because the one inside src/xml.c sets virterror, whereas the one in qemud/buf.c doesn't: - virXMLError(NULL, VIR_ERR_NO_MEMORY, _("growing buffer"), size); So there are some calls to the src/xml.c version which are doing stuff like: buf = virBufferNew(500); if (buf == NULL) return -1; // virBufferNew has set virterror I would prefer my patch (see recent email) to go in instead of 8/20 and 9/20, for this reason and also because my patch creates a separate lib/ subdir which I want to use for other library functions. 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
This is the 2nd key patch in the series. It switches the qemud/driver.c file methods to all follow the official libvirt internal driver API. NB, since we're using dummy statically declared virConnect object in the dispatch.c file, we can't use the regular virGetDomain function for constructing virDomainPtr objects at this time. Thus we have a hack to just malloc a virDomain struct & manually fill in the fields. This hack will go away in a later patch. conf.c | 2 dispatch.c | 449 ++++++++++++++++++++++----------- driver.c | 828 +++++++++++++++++++++++++++++++++++++++++++------------------ driver.h | 141 ++++------ 4 files changed, 951 insertions(+), 469 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 Fri, Jun 22, 2007 at 02:56:58AM +0100, Daniel P. Berrange wrote:
This is the 2nd key patch in the series. It switches the qemud/driver.c file methods to all follow the official libvirt internal driver API. NB, since we're using dummy statically declared virConnect object in the dispatch.c file, we can't use the regular virGetDomain function for constructing virDomainPtr objects at this time. Thus we have a hack to just malloc a virDomain struct & manually fill in the fields. This hack will go away in a later patch.
Okay, just two nitpicks, one about lack of comments, okay those functions are basically described as part of the driver API so it's a bit like cut and paste, but I still think it's nicer to have next to the code, and some lines are really long,
- for (i = 0 ; i < QEMUD_MAX_NUM_NETWORKS ; i++) { - names[i] = &out->qemud_packet_server_data_u.listNetworksReply.networks[i*QEMUD_MAX_NAME_LEN]; - }
There is a lot of place like that where the code manipulates all the dereference chain a lot of time, why not use temporary variables to make the code more readable and help the compiler w.r.t. generating good code ? +1 but maybe we need to revisit a bit that code. 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 Fri, Jun 22, 2007 at 08:45:59AM -0400, Daniel Veillard wrote:
On Fri, Jun 22, 2007 at 02:56:58AM +0100, Daniel P. Berrange wrote:
This is the 2nd key patch in the series. It switches the qemud/driver.c file methods to all follow the official libvirt internal driver API. NB, since we're using dummy statically declared virConnect object in the dispatch.c file, we can't use the regular virGetDomain function for constructing virDomainPtr objects at this time. Thus we have a hack to just malloc a virDomain struct & manually fill in the fields. This hack will go away in a later patch.
Okay, just two nitpicks, one about lack of comments, okay those functions are basically described as part of the driver API so it's a bit like cut and paste, but I still think it's nicer to have next to the code, and some lines are really long,
- for (i = 0 ; i < QEMUD_MAX_NUM_NETWORKS ; i++) { - names[i] = &out->qemud_packet_server_data_u.listNetworksReply.networks[i*QEMUD_MAX_NAME_LEN]; - }
There is a lot of place like that where the code manipulates all the dereference chain a lot of time, why not use temporary variables to make the code more readable and help the compiler w.r.t. generating good code ?
Again the dispatch.c file is killed in the last patch, so I wasn't really focusing on making nice code in dispatch.c Just functional enough for me to verify it works after each patch in the series is applied, so I didn't waste too much effort on code that is going away :-)
+1 but maybe we need to revisit a bit that code.
Yep, by deleting the entire file :-) 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 is just shuffling a little code from the driver.c file into the conf.c file. This is to give a clear separation between methods which implement QEMU driver functionality, vs those which deal with manipulating VM / network config objects. conf.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++++ conf.h | 2 + dispatch.c | 15 ++++------- driver.c | 81 ++++++++++++------------------------------------------------- driver.h | 13 --------- 5 files changed, 88 insertions(+), 87 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 Fri, Jun 22, 2007 at 02:59:39AM +0100, Daniel P. Berrange wrote:
This patch is just shuffling a little code from the driver.c file into the conf.c file. This is to give a clear separation between methods which implement QEMU driver functionality, vs those which deal with manipulating VM / network config objects.
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/
This patch provides an internal driver API to let drivers register event callbacks. It also provides a way for an application using libvirt to provide specific event loop implementations for dealing with these callbacks. Currently this is internal only - no new exported symbols except for a private symbol __virEventRegisterImpl intended for use by the libvirt daemon only. b/src/event.c | 80 ++++++++++++++++++++++++++++++++++++++ b/src/event.h | 100 ++++++++++++++++++++++++++++++++++++++++++++++++ docs/apibuild.py | 2 qemud/driver.c | 3 + qemud/event.c | 8 +-- qemud/event.h | 35 ++++------------ qemud/qemud.c | 48 +++++++++++------------ src/Makefile.am | 1 src/libvirt_sym.version | 2 9 files changed, 225 insertions(+), 54 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 Fri, Jun 22, 2007 at 03:02:52AM +0100, Daniel P. Berrange wrote:
This patch provides an internal driver API to let drivers register event callbacks. It also provides a way for an application using libvirt to provide specific event loop implementations for dealing with these callbacks. Currently this is internal only - no new exported symbols except for a private symbol __virEventRegisterImpl intended for use by the libvirt daemon only.
Okay so this is actually extracting code around select() (that could use poll too) to abstract processing if the event look, both for timers and file descriptor events availability. I'm not sure yet this is the kind of API we would need to export in libvirt, I consider this more an internal refactoring at that point, +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/
Some drivers are stateless (Xen, test), while others are stateful (QEMU). The later can only be accessed via the daemon. This patch adds a new internal driver API to allow drivers to register a set of functions for performing work on daemon startup, shutdown and reload. It makes the QEMU driver in qemud/driver.c provide implementations of these funtions. It adapts the qemud/qemud.c to call these new global driver functions. Finally it fixes the idle timeout shutdown of the daemon disabled in an earlier patch. NB this patch adds 3 new exported symbols for private use by the daemon. qemud/driver.c | 32 +++++++++++++++++--- qemud/driver.h | 5 +-- qemud/internal.h | 1 qemud/qemud.c | 51 ++++++++++++++++++++++++++++---- src/driver.h | 15 +++++++++ src/internal.h | 9 +++++ src/libvirt.c | 75 ++++++++++++++++++++++++++++++++++++++++++++++++ src/libvirt_sym.version | 5 +++ 8 files changed, 180 insertions(+), 13 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 Fri, Jun 22, 2007 at 03:07:08AM +0100, Daniel P. Berrange wrote:
Some drivers are stateless (Xen, test), while others are stateful (QEMU). The later can only be accessed via the daemon. This patch adds a new internal driver API to allow drivers to register a set of functions for performing work on daemon startup, shutdown and reload. It makes the QEMU driver in qemud/driver.c provide implementations of these funtions. It adapts the qemud/qemud.c to call these new global driver functions. Finally it fixes the idle timeout shutdown of the daemon disabled in an earlier patch. NB this patch adds 3 new exported symbols for private use by the daemon.
[...]
+typedef int (*virDrvStateInitialize) (void); +typedef int (*virDrvStateCleanup) (void); +typedef int (*virDrvStateReload) (void); +typedef int (*virDrvStateActive) (void); + +typedef struct _virStateDriver virStateDriver; +typedef virStateDriver *virStateDriverPtr; + +struct _virStateDriver { + virDrvStateInitialize initialize; + virDrvStateCleanup cleanup; + virDrvStateReload reload; + virDrvStateActive active; +};
/* * Registration @@ -324,6 +338,7 @@ struct _virNetworkDriver { */ int virRegisterDriver(virDriverPtr); int virRegisterNetworkDriver(virNetworkDriverPtr); +int virRegisterStateDriver(virStateDriverPtr);
Hum, shouldn't that be more closely associated to the virDriver itself it looks completely orthogonal, so I'm a bit surprized.
#ifdef __cplusplus } diff -r 968ca2c71e5f src/internal.h --- a/src/internal.h Thu Jun 21 21:20:57 2007 -0400 +++ b/src/internal.h Thu Jun 21 21:20:58 2007 -0400 @@ -214,6 +214,15 @@ int virFreeNetwork (virConnectPtr conn, #define virGetDomain(c,n,u) __virGetDomain((c),(n),(u)) #define virGetNetwork(c,n,u) __virGetNetwork((c),(n),(u))
+int __virStateInitialize(void); +int __virStateCleanup(void); +int __virStateReload(void); +int __virStateActive(void); +#define virStateInitialize() __virStateInitialize() +#define virStateCleanup() __virStateCleanup() +#define virStateReload() __virStateReload() +#define virStateActive() __virStateActive()
Funky, a small comment why we do this might help newbies going over the code. /* * those function are exported by the library but not supposed to be * used for normal use of libvirt. */ or something similar.
--- a/src/libvirt.c Thu Jun 21 21:20:57 2007 -0400 +++ b/src/libvirt.c Thu Jun 21 21:20:58 2007 -0400 @@ -40,6 +40,8 @@ static int virDriverTabCount = 0; static int virDriverTabCount = 0; static virNetworkDriverPtr virNetworkDriverTab[MAX_DRIVERS]; static int virNetworkDriverTabCount = 0; +static virStateDriverPtr virStateDriverTab[MAX_DRIVERS]; +static int virStateDriverTabCount = 0; static int initialized = 0;
I'm still a bit surprized this is separate from the drivers, somehow I would have expected the virStateDriverPtr to be a subfield of virDriver
diff -r 968ca2c71e5f src/libvirt_sym.version --- a/src/libvirt_sym.version Thu Jun 21 21:20:57 2007 -0400 +++ b/src/libvirt_sym.version Thu Jun 21 21:20:58 2007 -0400 @@ -101,5 +101,10 @@
__virEventRegisterImpl;
+ __virStateInitialize; + __virStateCleanup; + __virStateReload; + __virStateActive; +
Are all the __functions used by the daemon ? making a quick check in the end and adding a comment would make sense. But I think the patch is fine even if I'm a bit surprised about the way state is handled. I would have though it was a per driver property and hence associated to the driver structure. 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/
This trivial patch stops the test driver from grabbing any URIs which have a hostname set. test.c | 5 +++++ 1 file changed, 5 insertions(+) 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 current code for setting up bridges in virtual networks links against the libsysfs library. This is use to get/set the spanning-tree-protocol and forward-delay parameters on the bridge device. None of the four methods using libsysfs are ever called though. The fact that the setters are not called during network start is a bug. There is no need for getters at all since we have the config in memory all the time. The libsysfs is also not exactly an ABI stable library - we're unable to compile libvirt on FC5 for example because of this. This patch changes the bridge code to just invoke the brctl command directly which is much more portable & avoids the need for us to link libvirt.so against libsysfs.so It also fixes the network creation process to actually set STP & forward-delay parameters if specified. configure.in | 16 -- libvirt.spec.in | 4 qemud/Makefile.am | 2 qemud/bridge.c | 298 +++++++++++++++++++++++------------------------------- qemud/driver.c | 16 ++ 5 files changed, 146 insertions(+), 190 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 -=|
Sigh, missed another attachment... On Fri, Jun 22, 2007 at 03:12:11AM +0100, Daniel P. Berrange wrote:
The current code for setting up bridges in virtual networks links against the libsysfs library. This is use to get/set the spanning-tree-protocol and forward-delay parameters on the bridge device. None of the four methods using libsysfs are ever called though. The fact that the setters are not called during network start is a bug. There is no need for getters at all since we have the config in memory all the time. The libsysfs is also not exactly an ABI stable library - we're unable to compile libvirt on FC5 for example because of this. This patch changes the bridge code to just invoke the brctl command directly which is much more portable & avoids the need for us to link libvirt.so against libsysfs.so It also fixes the network creation process to actually set STP & forward-delay parameters if specified.
configure.in | 16 -- libvirt.spec.in | 4 qemud/Makefile.am | 2 qemud/bridge.c | 298 +++++++++++++++++++++++------------------------------- qemud/driver.c | 16 ++ 5 files changed, 146 insertions(+), 190 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 Fri, Jun 22, 2007 at 03:17:07AM +0100, Daniel P. Berrange wrote:
Sigh, missed another attachment...
On Fri, Jun 22, 2007 at 03:12:11AM +0100, Daniel P. Berrange wrote:
The current code for setting up bridges in virtual networks links against the libsysfs library. This is use to get/set the spanning-tree-protocol and forward-delay parameters on the bridge device. None of the four methods using libsysfs are ever called though. The fact that the setters are not called during network start is a bug. There is no need for getters at all since we have the config in memory all the time. The libsysfs is also not exactly an ABI stable library - we're unable to compile libvirt on FC5 for example because of this. This patch changes the bridge code to just invoke the brctl command directly which is much more portable & avoids the need for us to link libvirt.so against libsysfs.so It also fixes the network creation process to actually set STP & forward-delay parameters if specified.
I don't have enough expertise to really juge the change from the library to calling the system command, I like the idea of dropping the dependancy to the library though.
+#define BRCTL_PATH "/usr/sbin/brctl"
That should probably be tested in configure.in I wonder what impact it would have for example on Solaris, expert feedback would be welcome :-)
+ if ((null = open(_PATH_DEVNULL, O_RDONLY)) < 0) + return errno;
Hum probably worth raising an error. Though if /dev/null can't be opened libvirt is probably the last of your worries.
+ char **argv; + int retval = ENOMEM; + int n; + + n = 1 + /* brctl */ + 1 + /* setfd */ + 1 + /* brige name */ + 1; /* value */ 1; /* NULL */
and then calloc with n instead, let's describe fully.
+ + if (!(argv = (char **)calloc(n + 1, sizeof(char *)))) + goto error;
I'm fine with the patch to the extend I understand it implications +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/
On Fri, Jun 22, 2007 at 09:19:44AM -0400, Daniel Veillard wrote:
On Fri, Jun 22, 2007 at 03:17:07AM +0100, Daniel P. Berrange wrote:
Sigh, missed another attachment...
On Fri, Jun 22, 2007 at 03:12:11AM +0100, Daniel P. Berrange wrote:
The current code for setting up bridges in virtual networks links against the libsysfs library. This is use to get/set the spanning-tree-protocol and forward-delay parameters on the bridge device. None of the four methods using libsysfs are ever called though. The fact that the setters are not called during network start is a bug. There is no need for getters at all since we have the config in memory all the time. The libsysfs is also not exactly an ABI stable library - we're unable to compile libvirt on FC5 for example because of this. This patch changes the bridge code to just invoke the brctl command directly which is much more portable & avoids the need for us to link libvirt.so against libsysfs.so It also fixes the network creation process to actually set STP & forward-delay parameters if specified.
I don't have enough expertise to really juge the change from the library to calling the system command, I like the idea of dropping the dependancy to the library though.
+#define BRCTL_PATH "/usr/sbin/brctl"
That should probably be tested in configure.in I wonder what impact it would have for example on Solaris, expert feedback would be welcome :-)
Well the old code didn't work for Solaris, and neither will the new code :-) This code, and the iptables.c file will both need completely separate impls for Solaris vs Linux, though the top level API in the header files is probably portable enough.
+ if ((null = open(_PATH_DEVNULL, O_RDONLY)) < 0) + return errno;
Hum probably worth raising an error. Though if /dev/null can't be opened libvirt is probably the last of your worries.
+ char **argv; + int retval = ENOMEM; + int n; + + n = 1 + /* brctl */ + 1 + /* setfd */ + 1 + /* brige name */ + 1; /* value */ 1; /* NULL */
and then calloc with n instead, let's describe fully.
+ + if (!(argv = (char **)calloc(n + 1, sizeof(char *)))) + goto error;
I just copied the approach from iptables.c here. I'll do a separate patch which fixes both files at once. 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 adds a generic API for drivers to log warning / debug / info messages. The existing error APIs are not suitable for this purpose since each time you set an error, it clears the previous one. This adds a public API virSetLogFunc allowing applications to register a callback to receive log messages. If none is registered, they are sent to the big void. It adapts the QEMU driver to use this logging API instead of qemudLog and qemudDebug(). It makes the qemud/qemud.c file register a logging callback to receive the messages & send them onto syslog/stderr as needed. There are other drivers (in particular src/xm_internal.c) where this logging API will be useful too. I've not attempted to make them use it yet though. include/libvirt/virterror.h | 24 ++++++++++++++++++ qemud/conf.c | 46 ++++++++++++++++++------------------ qemud/driver.c | 56 ++++++++++++++++++++++---------------------- qemud/iptables.c | 8 +++--- qemud/qemud.c | 36 ++++++++++++++++++++++++++++ qemud/uuid.c | 9 +++---- src/internal.h | 5 +++ src/libvirt_sym.version | 1 src/virterror.c | 28 ++++++++++++++++++++++ 9 files changed, 155 insertions(+), 58 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 adds a generic API for drivers to log warning / debug / info messages. The existing error APIs are not suitable for this purpose since each time you set an error, it clears the previous one. This adds a public API virSetLogFunc allowing applications to register a callback to receive log messages. If none is registered, they are sent to the big void. It adapts the QEMU driver to use this logging API instead of qemudLog and qemudDebug(). It makes the qemud/qemud.c file register a logging callback to receive the messages & send them onto syslog/stderr as needed.
There are other drivers (in particular src/xm_internal.c) where this logging API will be useful too. I've not attempted to make them use it yet though.
So to get this straight - this not intended to replace error messages, just informational messages? You should allow more than one handler to be registered, and you should allow handlers to unregister themselves (give them a token which they can pass back to an unregister function). Should the default be to print to stderr? syslog? ...? 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 Fri, Jun 22, 2007 at 03:16:38AM +0100, Daniel P. Berrange wrote:
This patch adds a generic API for drivers to log warning / debug / info messages. The existing error APIs are not suitable for this purpose since each time you set an error, it clears the previous one. This adds a public API virSetLogFunc allowing applications to register a callback to receive log messages. If none is registered, they are sent to the big void. It adapts the QEMU driver to use this logging API instead of qemudLog and qemudDebug(). It makes the qemud/qemud.c file register a logging callback to receive the messages & send them onto syslog/stderr as needed.
There are other drivers (in particular src/xm_internal.c) where this logging API will be useful too. I've not attempted to make them use it yet though.
include/libvirt/virterror.h | 24 ++++++++++++++++++ qemud/conf.c | 46 ++++++++++++++++++------------------ qemud/driver.c | 56 ++++++++++++++++++++++---------------------- qemud/iptables.c | 8 +++--- qemud/qemud.c | 36 ++++++++++++++++++++++++++++ qemud/uuid.c | 9 +++---- src/internal.h | 5 +++ src/libvirt_sym.version | 1 src/virterror.c | 28 ++++++++++++++++++++++ 9 files changed, 155 insertions(+), 58 deletions(-)
Hum, I actually have a couple of issues with defining a new error logging API without having discussed it before at least for a bit. I'm a bit worried by by a few things about it: - it's only string level, okay that's functionally equivalent to a syslog, but I find that as an user API, having just a string is a bit problematic - it's the first callback kind of support we would actually export at a blessed API level, I would not rush this I guess we should be clear that reentrancy of libvirt API from that callback is not something we would support. Basically I would prefer if a subset version of that patch not changing virterror.h could be applied to avoid blocking the reunification work. 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/
A future patch will rename the daemon to be libvirtd instead of libvirt_qemud. The initscript file, however, is currently being created as libvirtd in this same directory which will cause a filename clash. This renames to be called libvirtd.init.in instead of libvirtd.in a/qemud/libvirtd.in | 77 ----------------------------------------------- b/qemud/libvirtd.init.in | 77 +++++++++++++++++++++++++++++++++++++++++++++++ qemud/Makefile.am | 25 ++++++++++----- 3 files changed, 94 insertions(+), 85 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 Fri, Jun 22, 2007 at 03:19:11AM +0100, Daniel P. Berrange wrote:
A future patch will rename the daemon to be libvirtd instead of libvirt_qemud. The initscript file, however, is currently being created as libvirtd in this same directory which will cause a filename clash. This renames to be called libvirtd.init.in instead of libvirtd.in
as long as the user/system visible rc.d/init.d/libvirtd file doesn't change it's name it's just internal cooking recipe and 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/
This patch renames the daemon from libvirt_qemud to libvirtd and fixes the specfile and init script to take account libvirt.spec.in | 2 +- qemud/Makefile.am | 12 ++++++------ qemud/libvirtd.init.in | 4 ++-- 3 files changed, 9 insertions(+), 9 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 Fri, Jun 22, 2007 at 03:20:16AM +0100, Daniel P. Berrange wrote:
This patch renames the daemon from libvirt_qemud to libvirtd and fixes the specfile and init script to take account
libvirt.spec.in | 2 +- qemud/Makefile.am | 12 ++++++------ qemud/libvirtd.init.in | 4 ++-- 3 files changed, 9 insertions(+), 9 deletions(-)
+1 I'm just wondering about the upgrade sequence, say you have libvirt-0.2.3 running with libvirt_qemud running, you upgrade the rpm, the %post might do a restart but the process name has changed. Will the .pid file save us there or will we need to add a release note specific to this when issuing 0.3.0 ? (there is probably enough changed to increas the minor there. 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/
The remote_internal.c file is checkign VIR_CONNECT_RO instead of VIR_DRV_OPEN_RO when determining what socket to open. Since these have different values, the readonly socket is never used for a read only connection. remote_internal.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 Fri, Jun 22, 2007 at 03:21:50AM +0100, Daniel P. Berrange wrote:
The remote_internal.c file is checkign VIR_CONNECT_RO instead of VIR_DRV_OPEN_RO when determining what socket to open. Since these have different values, the readonly socket is never used for a read only connection.
dohh +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/
This is the final step to move the QEMU code out of the daemon binary and into the main libvirt.so library. Practically all of protocol.x goes away apart from a single struct representing the header - this should probably be moved into remote_protocol.x - or vica-verca. The --remote switch to the daemon is removed since its no longer needed. A single --listen switch is added to control whether the daemon opens any TCP/IP sockets or not, and a config file in /etc/sysconfig/libvirtd is provided to toggle this. By default the daemon will only listen on a UNIX socket, unless the admin edits /etc/sysconfig/libvirtd to enable TCP - whether it uses TCP or TLS is still upto the libvirtd.conf config file. The driver.c file is switched over to using virGetDomain and virGetNetwork instead of its previous temporary / nasty hack. A qemudRegister function is added to make the new implementation be registered. The daemon now registers an event loop implementation to be used by the QEMU driver. Various bits of code relating to the QEMU protocol are killed off in the qemud.c file. The remote_internal.c file is adapted so that it will handle qemu:///sesion and qemu:///system URLs as if they were qemu+unix:///session and qemu+unix:///system respectively. It is also adapted so that in the case of qemu:///session is uses $HOME/.libvirt/libvirt-sock for the UNIX domain socket path. It is also tweaked to use the abstract namespace if the first character is @ Rather than showing a huge +10000 lines, -10000 lines diff caused by moving files from qemud/ into src/, i've applied some makefile trickery to make libvirt.so compile some files out of qemud/ directly. I intend to commit it in this format, and then do the actual file names in a second commit. This will avoid mixing functional changes, with plain renames to preserve better historical CVS logs. The files to be moved from qemud/ to src/ are: driver.c -> qemud_driver.c driver.h -> qemud_driver.h conf.c -> qemud_conf.c conf.h -> qemud_conf.h bridge.c bridge.h iptables.c iptables.h uuid.c uuid.h The files to be completely deleted are: dispatch.c dispatch.h protocol.c protocol.h protocol.x qemu_internal.c qemu_internal.h NB. one feature I've just realized is missing is the autostart of the daemon when using qemu:///session in remote_internal.c. Shouldn't be much work to move the neccessary code from qemu_internal.c across. 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 -=|
Forgot to add the diffstat for this patch. Its not nearly as scary as its size sugests b/qemud/libvirtd.sysconf | 6 libvirt.spec.in | 5 qemud/Makefile.am | 19 - qemud/conf.c | 2 qemud/conf.h | 11 qemud/driver.c | 69 +---- qemud/driver.h | 3 qemud/internal.h | 2 qemud/libvirtd.init.in | 12 qemud/protocol.x | 574 ----------------------------------------------- qemud/qemud.c | 293 +++++------------------ qemud/remote.c | 4 src/Makefile.am | 14 - src/libvirt.c | 7 src/remote_internal.c | 38 ++- src/remote_internal.h | 5 16 files changed, 171 insertions(+), 893 deletions(-) On Fri, Jun 22, 2007 at 03:35:42AM +0100, Daniel P. Berrange wrote:
This is the final step to move the QEMU code out of the daemon binary and into the main libvirt.so library. Practically all of protocol.x goes away apart from a single struct representing the header - this should probably be moved into remote_protocol.x - or vica-verca. The --remote switch to the daemon is removed since its no longer needed. A single --listen switch is added to control whether the daemon opens any TCP/IP sockets or not, and a config file in /etc/sysconfig/libvirtd is provided to toggle this. By default the daemon will only listen on a UNIX socket, unless the admin edits /etc/sysconfig/libvirtd to enable TCP - whether it uses TCP or TLS is still upto the libvirtd.conf config file.
The driver.c file is switched over to using virGetDomain and virGetNetwork instead of its previous temporary / nasty hack. A qemudRegister function is added to make the new implementation be registered.
The daemon now registers an event loop implementation to be used by the QEMU driver. Various bits of code relating to the QEMU protocol are killed off in the qemud.c file.
The remote_internal.c file is adapted so that it will handle qemu:///sesion and qemu:///system URLs as if they were qemu+unix:///session and qemu+unix:///system respectively. It is also adapted so that in the case of qemu:///session is uses $HOME/.libvirt/libvirt-sock for the UNIX domain socket path. It is also tweaked to use the abstract namespace if the first character is @
Rather than showing a huge +10000 lines, -10000 lines diff caused by moving files from qemud/ into src/, i've applied some makefile trickery to make libvirt.so compile some files out of qemud/ directly. I intend to commit it in this format, and then do the actual file names in a second commit. This will avoid mixing functional changes, with plain renames to preserve better historical CVS logs.
The files to be moved from qemud/ to src/ are:
driver.c -> qemud_driver.c driver.h -> qemud_driver.h conf.c -> qemud_conf.c conf.h -> qemud_conf.h bridge.c bridge.h iptables.c iptables.h uuid.c uuid.h
The files to be completely deleted are:
dispatch.c dispatch.h protocol.c protocol.h protocol.x qemu_internal.c qemu_internal.h
NB. one feature I've just realized is missing is the autostart of the daemon when using qemu:///session in remote_internal.c. Shouldn't be much work to move the neccessary code from qemu_internal.c across.
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 Fri, Jun 22, 2007 at 03:35:42AM +0100, Daniel P. Berrange wrote:
This is the final step to move the QEMU code out of the daemon binary and into the main libvirt.so library. Practically all of protocol.x goes away apart from a single struct representing the header - this should probably be moved into remote_protocol.x - or vica-verca. The --remote switch to the daemon is removed since its no longer needed. A single --listen switch is added to control whether the daemon opens any TCP/IP sockets or not, and a config file in /etc/sysconfig/libvirtd is provided to toggle this. By default the daemon will only listen on a UNIX socket, unless the admin edits /etc/sysconfig/libvirtd to enable TCP - whether it uses TCP or TLS is still upto the libvirtd.conf config file.
The driver.c file is switched over to using virGetDomain and virGetNetwork instead of its previous temporary / nasty hack. A qemudRegister function is added to make the new implementation be registered.
The daemon now registers an event loop implementation to be used by the QEMU driver. Various bits of code relating to the QEMU protocol are killed off in the qemud.c file.
The remote_internal.c file is adapted so that it will handle qemu:///sesion and qemu:///system URLs as if they were qemu+unix:///session and qemu+unix:///system respectively. It is also adapted so that in the case of qemu:///session is uses $HOME/.libvirt/libvirt-sock for the UNIX domain socket path. It is also tweaked to use the abstract namespace if the first character is @
Rather than showing a huge +10000 lines, -10000 lines diff caused by moving files from qemud/ into src/, i've applied some makefile trickery to make libvirt.so compile some files out of qemud/ directly. I intend to commit it in this format, and then do the actual file names in a second commit. This will avoid mixing functional changes, with plain renames to preserve better historical CVS logs.
The files to be moved from qemud/ to src/ are:
driver.c -> qemud_driver.c driver.h -> qemud_driver.h conf.c -> qemud_conf.c conf.h -> qemud_conf.h bridge.c bridge.h iptables.c iptables.h uuid.c uuid.h
The files to be completely deleted are:
dispatch.c dispatch.h protocol.c protocol.h protocol.x qemu_internal.c qemu_internal.h
NB. one feature I've just realized is missing is the autostart of the daemon when using qemu:///session in remote_internal.c. Shouldn't be much work to move the neccessary code from qemu_internal.c across.
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 c6fe9aa77e5b libvirt.spec.in --- a/libvirt.spec.in Thu Jun 21 21:21:11 2007 -0400 +++ b/libvirt.spec.in Thu Jun 21 21:21:14 2007 -0400 @@ -124,10 +124,7 @@ fi %dir %attr(0700, root, root) %{_sysconfdir}/libvirt/qemu/networks/ %dir %attr(0700, root, root) %{_sysconfdir}/libvirt/qemu/networks/autostart %{_sysconfdir}/rc.d/init.d/libvirtd -%dir %{_sysconfdir}/libvirt -%dir %{_sysconfdir}/libvirt/qemu -%dir %{_sysconfdir}/libvirt/qemu/networks -%dir %{_sysconfdir}/libvirt/qemu/networks/autostart +%config(noreplace) %{_sysconfdir}/sysconfig/libvirtd %dir %{_datadir}/libvirt/ %dir %{_datadir}/libvirt/networks/ %{_datadir}/libvirt/networks/default.xml
Hum, what happens on rpm -U of the old version w.r.t. the daemon ? [...]
--- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/qemud/libvirtd.sysconf Thu Jun 21 21:21:14 2007 -0400 @@ -0,0 +1,6 @@ +# Override the default config file +#LIBVIRTD_CONFIG=/etc/libvirt/libvirtd.conf + +# Listen for TCP/IP connections +# NB. must setup TLS/SSL keys prior to using this +#LIBVIRTD_ARGS="--listen"
Hum interesting that wasn't present before
diff -r c6fe9aa77e5b qemud/qemud.c [...] + if (snprintf(sockname, maxlen, "@%s/.libvirt/libvirt-sock", pw->pw_dir) >= maxlen) + goto snprintf_error; + + if (snprintf(server->logDir, PATH_MAX, "%s/.libvirt/log", pw->pw_dir) >= PATH_MAX) + goto snprintf_error; + + if (asprintf (&base, "%s/.libvirt", pw->pw_dir) == -1) { + qemudLog (QEMUD_ERR, "out of memory in asprintf"); + return -1;
We really need one function to cleanup those path builds.
diff -r c6fe9aa77e5b src/Makefile.am [...] @@ -42,8 +43,12 @@ CLIENT_SOURCES = \ proxy_internal.c proxy_internal.h \ conf.c conf.h \ xm_internal.c xm_internal.h \ - qemu_internal.c qemu_internal.h \ - remote_internal.c remote_internal.h + remote_internal.c remote_internal.h \ + ../qemud/bridge.c ../qemud/bridge.h \ + ../qemud/iptables.c ../qemud/iptables.h \ + ../qemud/uuid.c ../qemud/uuid.h \ + ../qemud/driver.c ../qemud/driver.h \ + ../qemud/qemu_conf.c ../qemud/conf.h
SERVER_SOURCES = \ ../qemud/protocol.h ../qemud/protocol.c \ @@ -58,6 +63,9 @@ virsh_DEPENDENCIES = $(DEPS) virsh_DEPENDENCIES = $(DEPS) virsh_LDADD = $(LDADDS) $(VIRSH_LIBS) virsh_CFLAGS = $(COVERAGE_CFLAGS) + +../qemud/qemu_conf.c: + ln -s conf.c $@
As long as it's temporary just to help review, fine :-) Okay, looks fine, +1 thanks a million ! 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 Fri, Jun 22, 2007 at 02:34:05AM +0100, Daniel P. Berrange wrote:
The work to refactor the QEMU driver to use the regular libvirt driver API is now complete. The next 20 (!) patches provide fairly fine-grained steps refactoring the code. Aside from that which adds an event loop, and the very last one, they should all be pretty much straightforward refactoring with no functional change.
With this patch series fully applied there is now only a single daemon which can serve both remote & QEMU drivers in one go, with no deadlock issues.
BTW, as a rough comparison of the old QEMU impl vs the new one in terms of performance, here are the timings for listing active guests 10000 times over Original QEMU code: time virsh --connect qemu:///system < src/data > /dev/null real 0m0.963s user 0m0.246s sys 0m0.540s New generic remote code: time ./src/virsh --connect qemu:///system < src/data > /dev/null real 0m1.142s user 0m0.470s sys 0m0.377s Which is easily close enough to ignore the difference New generic remote code using plain TCP localhost: time ./src/virsh --connect qemu+tcp://localhost/system < src/data > /dev/null real 0m1.857s user 0m0.543s sys 0m0.779s New generic remote code using TLS localhost: time ./src/virsh --connect qemu://localhost/system < src/data > /dev/null real 0m5.031s user 0m1.814s sys 0m1.303s New generic remote code using SSH localhost: time ./virsh --connect qemu+ssh://localhost/system < data > /dev/null real 0m5.987s user 0m0.621s sys 0m1.147s So there's a non-trivial hit from doing TLS/SSH. Only 0.25s is accounted for by the initial handshake / negotiation. Still, small price to pay for having data security of course... 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 Fri, Jun 22, 2007 at 02:34:05AM +0100, Daniel P. Berrange wrote:
The work to refactor the QEMU driver to use the regular libvirt driver API is now complete. The next 20 (!) patches provide fairly fine-grained steps refactoring the code. Aside from that which adds an event loop, and the very last one, they should all be pretty much straightforward refactoring with no functional change.
With this patch series fully applied there is now only a single daemon which can serve both remote & QEMU drivers in one go, with no deadlock issues.
All of this patch series with the exception of the one about the logging API is now committed to CVS. As a temporary hack,the loggin stuff is replaced by a simple fprintf, until we can decide what todo about it. The two known broken things I need to address tomorrow: - The network driver won't work for non-QEMU local access eg Xen. It will work for QEMU, or Xen via the remote daemon - The daemon for handling QEMU session connections doesn't autospawn. 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 -=|
On Wed, Jun 27, 2007 at 01:03:01AM +0100, Daniel P. Berrange wrote:
On Fri, Jun 22, 2007 at 02:34:05AM +0100, Daniel P. Berrange wrote:
The work to refactor the QEMU driver to use the regular libvirt driver API is now complete. The next 20 (!) patches provide fairly fine-grained steps refactoring the code. Aside from that which adds an event loop, and the very last one, they should all be pretty much straightforward refactoring with no functional change.
With this patch series fully applied there is now only a single daemon which can serve both remote & QEMU drivers in one go, with no deadlock issues.
All of this patch series with the exception of the one about the logging API is now committed to CVS. As a temporary hack,the loggin stuff is replaced by a simple fprintf, until we can decide what todo about it.
The two known broken things I need to address tomorrow:
- The network driver won't work for non-QEMU local access eg Xen. It will work for QEMU, or Xen via the remote daemon
- The daemon for handling QEMU session connections doesn't autospawn.
Excellent. IIRC you want to move files at some point, tell me I will try to copy the v files on the CVS back-end to not loose history. 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/
Daniel Veillard wrote:
Excellent. IIRC you want to move files at some point, tell me I will try to copy the v files on the CVS back-end to not loose history.
I have added a list of suggested file renames, into the file RENAMES in the top level directory. That's my list - I suggest than Dan edits this. The current content of this file is attached. 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 # Suggested list of file renames. # # File renames don't normally go into patches because they make # the patches much harder to read, so list them here instead. # # $Id: RENAMES,v 1.1 2007/06/11 12:25:06 rjones Exp $ # Clearer naming scheme after Xen-unified patch went in. src/xen_internal.c src/xen_internal_hv.c src/xen_internal.h src/xen_internal_hv.h src/proxy_internal.c src/xen_internal_proxy.c src/proxy_internal.h src/xen_internal_proxy.h src/xend_internal.c src/xen_internal_xend.c src/xend_internal.h src/xen_internal_xend.h src/xm_internal.c src/xen_internal_inactive.c src/xm_internal.h src/xen_internal_inactive.h src/xs_internal.c src/xen_internal_xenstore.c src/xs_internal.h src/xen_internal_xenstore.h src/xen_unified.c src/xen_internal.c src/xen_unified.h src/xen_internal.h # Test driver should really be called test_internal. src/test.c src/test_internal.c src/test.h src/test_internal.h # This would be better: src/*_internal*.c src/*_driver*.c src/*_internal*.h src/*_driver*.h # Qemud is now the qemud + remote driver. qemud/protocol.x qemud/qemud_protocol.x qemud/* remote/*
On Wed, Jun 27, 2007 at 09:27:48AM +0100, Richard W.M. Jones wrote:
Daniel Veillard wrote:
Excellent. IIRC you want to move files at some point, tell me I will try to copy the v files on the CVS back-end to not loose history.
I have added a list of suggested file renames, into the file RENAMES in the top level directory. That's my list - I suggest than Dan edits this. The current content of this file is attached.
I think I already said I wasn't found of renaming just for renaming files in the same directory, I would rather see documetation stating what is in the different files if one want to help people to look at the code. No Dan will move them to different directories, that's why I suggested this. He don't want to do it, fine. 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 Wed, Jun 27, 2007 at 02:52:23AM -0400, Daniel Veillard wrote:
On Wed, Jun 27, 2007 at 01:03:01AM +0100, Daniel P. Berrange wrote:
On Fri, Jun 22, 2007 at 02:34:05AM +0100, Daniel P. Berrange wrote:
The work to refactor the QEMU driver to use the regular libvirt driver API is now complete. The next 20 (!) patches provide fairly fine-grained steps refactoring the code. Aside from that which adds an event loop, and the very last one, they should all be pretty much straightforward refactoring with no functional change.
With this patch series fully applied there is now only a single daemon which can serve both remote & QEMU drivers in one go, with no deadlock issues.
All of this patch series with the exception of the one about the logging API is now committed to CVS. As a temporary hack,the loggin stuff is replaced by a simple fprintf, until we can decide what todo about it.
The two known broken things I need to address tomorrow:
- The network driver won't work for non-QEMU local access eg Xen. It will work for QEMU, or Xen via the remote daemon
- The daemon for handling QEMU session connections doesn't autospawn.
Excellent. IIRC you want to move files at some point, tell me I will try to copy the v files on the CVS back-end to not loose history.
I already moved the QEMU driver files from qemud -> src as part of the last patch. Copying files in the CVS back-end will totally screw up all the tags. eg, if you try to get a checkout of 'libvirt-0.1.0' tag you'll get all the moved files. CVS sucks at renames. Trying to hack the repo to deal with it just makes it worse IMHO. If its really that important we shouldn't be using CVS.... 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