8:34 p.m.
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 -=|
8:37 p.m.
New subject: [Libvir] PATCH 1/20: Provide a generic event loop
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 -=|
5:17 a.m.
New subject: [Libvir] PATCH 1/20: Provide a generic event loop
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(a)redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
8:38 p.m.
New subject: [Libvir] PATCH 2/20: Move driver capabilities code
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 -=|
8:40 p.m.
New subject: [Libvir] PATCH 2/20: Move driver capabilities code
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 -=|
5:19 a.m.
New subject: [Libvir] PATCH 2/20: Move driver capabilities code
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(a)redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
8:40 p.m.
New subject: [Libvir] PATCH 3/20: domain lifecycle changes
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 -=|
8:41 p.m.
New subject: [Libvir] PATCH 3/20: domain lifecycle changes
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 -=|
5:23 a.m.
New subject: [Libvir] PATCH 3/20: domain lifecycle changes
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(a)redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
8:43 p.m.
New subject: [Libvir] PATCH 4/20: daemon startup/shutdown code move
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 -=|
5:45 a.m.
New subject: [Libvir] PATCH 4/20: daemon startup/shutdown code move
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(a)redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
8:45 p.m.
New subject: [Libvir] PATCH 5/20: reuse main libvirt error API
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 -=|
5:46 a.m.
New subject: [Libvir] PATCH 5/20: reuse main libvirt error API
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(a)redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
8:48 p.m.
New subject: [Libvir] PATCH 6/20: split up qemud_server struct
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 -=|
6:16 a.m.
New subject: [Libvir] PATCH 6/20: split up qemud_server struct
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(a)redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
7:16 a.m.
New subject: [Libvir] PATCH 6/20: split up qemud_server struct
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 -=|
8:49 p.m.
New subject: [Libvir] PATCH 7/20 add a UUID module & rename funcs
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 -=|
6:16 a.m.
New subject: [Libvir] PATCH 7/20 add a UUID module & rename funcs
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(a)redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
8:51 p.m.
New subject: [Libvir] PATCH 8/20 rename buffer functions
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 -=|
6:17 a.m.
New subject: [Libvir] PATCH 8/20 rename buffer functions
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(a)redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
8:53 p.m.
New subject: [Libvir] PATCH 9/20: unify the two buffer implemenations
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 -=|
7:36 a.m.
New subject: [Libvir] PATCH 9/20: unify the two buffer implemenations
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(a)redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
10:03 a.m.
New subject: [Libvir] PATCH 9/20: unify the two buffer implemenations
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
8:56 p.m.
New subject: [Libvir] PATCH 10/20: switch QEMU to using the driver API
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 -=|
7:45 a.m.
New subject: [Libvir] PATCH 10/20: switch QEMU to using the driver API
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(a)redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
7:52 a.m.
New subject: [Libvir] PATCH 10/20: switch QEMU to using the driver API
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 -=|
8:59 p.m.
New subject: [Libvir] PATCH 11/20: shuffles code from driver to conf file
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 -=|
7:47 a.m.
New subject: [Libvir] PATCH 11/20: shuffles code from driver to conf file
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(a)redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
9:02 p.m.
New subject: [Libvir] PATCH 12/20: create a internal driver API for events
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 -=|
7:54 a.m.
New subject: [Libvir] PATCH 12/20: create a internal driver API for events
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(a)redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
9:07 p.m.
New subject: [Libvir] PATCH 13/20: create internal stateful driver API
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 -=|
8:10 a.m.
New subject: [Libvir] PATCH 13/20: create internal stateful driver API
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(a)redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
9:08 p.m.
New subject: [Libvir] PATCH 14/20: disable test driver for remote URIs
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 -=|
9:12 p.m.
New subject: [Libvir] PATCH 15/20: remove use of libsysfs in bridge code
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 -=|
9:17 p.m.
New subject: [Libvir] PATCH 15/20: remove use of libsysfs in bridge code
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(a)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 -=|
8:19 a.m.
New subject: [Libvir] PATCH 15/20: remove use of libsysfs in bridge code
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(a)redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
8:21 a.m.
New subject: [Libvir] PATCH 15/20: remove use of libsysfs in bridge code
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 -=|
9:16 p.m.
New subject: [Libvir] PATCH 16/20: add a generic logging API
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 -=|
9:01 a.m.
New subject: [Libvir] PATCH 16/20: add a generic logging API
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
9:12 a.m.
New subject: [Libvir] PATCH 16/20: add a generic logging API
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(a)redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
9:19 p.m.
New subject: [Libvir] PATCH 17/20: rename the init script file
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 -=|
8:28 a.m.
New subject: [Libvir] PATCH 17/20: rename the init script file
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(a)redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
9:20 p.m.
New subject: [Libvir] PATCH 18/20: rename the daemon
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 -=|
8:31 a.m.
New subject: [Libvir] PATCH 18/20: rename the daemon
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(a)redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
9:21 p.m.
New subject: [Libvir] PATCH 19/20: fix remote driver read only socket
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 -=|
8:32 a.m.
New subject: [Libvir] PATCH 19/20: fix remote driver read only socket
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(a)redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
9:35 p.m.
New subject: [Libvir] PATCH 20/20: move the QEMU driver into libvirt.so
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 -=|
9:27 a.m.
New subject: [Libvir] PATCH 20/20: move the QEMU driver into libvirt.so
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 -=|
10:28 a.m.
New subject: [Libvir] PATCH 20/20: move the QEMU driver into libvirt.so
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,
"(a)%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(a)redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
9:55 p.m.
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 -=|
7:03 p.m.
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 -=|
1:52 a.m.
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(a)redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
3:27 a.m.
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/*
5:23 a.m.
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(a)redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
8:30 a.m.
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 -=|
6360
days inactive
6366
days old
54 comments
3 participants
participants (3)
-
Daniel P. Berrange -
Daniel Veillard -
Richard W.M. Jones