[libvirt] [PATCH] let gcc's -Wformat do its job; avoid make syntax-check failure
I first noticed that make syntax-check failed.
Then I saw that virAsprintf's prototype lacked ATTRIBUTE_FORMAT.
This fixes both and updates HACKING.
From 5bae37c505738ed4223625f8e3cc88ad6f4bf68c Mon Sep 17 00:00:00 2001
From: Jim Meyering meyer...@redhat.com
Date: Wed, 17 Dec 2008 08:54:46 +0100
Subject: [PATCH] let gcc's -Wformat do its job; avoid make syntax-check
failure
* src/util.c (virAsprintf): Remove trailing space.
* src/util.h (virAsprintf): Use ATTRIBUTE_FORMAT.
* HACKING (Printf-style functions): New section.
---
HACKING| 16
src/util.c |2 +-
src/util.h |3 ++-
3 files changed, 19 insertions(+), 2 deletions(-)
diff --git a/HACKING b/HACKING
index 3945833..e088da8 100644
--- a/HACKING
+++ b/HACKING
@@ -247,6 +247,22 @@ are some special reasons why you cannot include these files
explicitly.
+Printf-style functions
+==
+
+Whenever you add a new printf-style function, i.e., one with a format
+string argument and following ... in its prototype, be sure to use
+gcc's printf attribute directive in the prototype. For example, here's
+the one for virAsprintf, in util.h:
+
+int virAsprintf(char **strp, const char *fmt, ...)
+ ATTRIBUTE_FORMAT(printf, 2, 3);
+
+This makes it so gcc's -Wformat and -Wformat-security options can do
+their jobs and cross-check format strings with the number and types
+of arguments.
+
+
Libvirt commiters guidelines
diff --git a/src/util.c b/src/util.c
index 12097d4..9eda378 100644
--- a/src/util.c
+++ b/src/util.c
@@ -1158,7 +1158,7 @@ virParseNumber(const char **str)
*
* like asprintf but makes sure *strp == NULL on failure
*/
-int
+int
virAsprintf(char **strp, const char *fmt, ...)
{
va_list ap;
diff --git a/src/util.h b/src/util.h
index 3d603dc..0475bd3 100644
--- a/src/util.h
+++ b/src/util.h
@@ -112,7 +112,8 @@ int virMacAddrCompare (const char *mac1, const char *mac2);
void virSkipSpaces(const char **str);
int virParseNumber(const char **str);
-int virAsprintf(char **strp, const char *fmt, ...);
+int virAsprintf(char **strp, const char *fmt, ...)
+ATTRIBUTE_FORMAT(printf, 2, 3);
#define VIR_MAC_BUFLEN 6
#define VIR_MAC_PREFIX_BUFLEN 3
--
1.6.1.rc2.316.geb2f0
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] don't assume builddir == srcdir
On Sat, Dec 13, 2008 at 04:52:10PM +, Daniel P. Berrange wrote: On Sat, Dec 13, 2008 at 12:23:30AM +0100, Guido G?nther wrote: Hi, Makefile.maint assumes this in some places. This doesn't make the world perfect but maybe a bit better? Seems reasonable to me. Applied now. -- Guido -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix GCC hard-coding in python/
john.le...@sun.com wrote: Fix GCC hard-coding in python/ ... diff --git a/acinclude.m4 b/acinclude.m4 --- a/acinclude.m4 +++ b/acinclude.m4 @@ -75,6 +75,11 @@ AC_DEFUN([LIBVIRT_COMPILE_WARNINGS],[ WARN_CFLAGS=$COMPILER_FLAGS $complCFLAGS AC_SUBST(WARN_CFLAGS) + +COMPILER_FLAGS= +gl_COMPILER_FLAGS(-Wno-redundant-decls) +NO_RDECLS_CFLAGS=$COMPILER_FLAGS +AC_SUBST(NO_RDECLS_CFLAGS) ]) diff --git a/python/Makefile.am b/python/Makefile.am --- a/python/Makefile.am +++ b/python/Makefile.am @@ -35,7 +35,7 @@ python_LTLIBRARIES = libvirtmod.la libvirtmod_la_SOURCES = libvir.c types.c libvirt-py.c libvirt-py.h # Python header files contain a redundant decl, hence: -libvirtmod_la_CFLAGS = -Wno-redundant-decls +libvirtmod_la_CFLAGS = @NO_RDECLS_CFLAGS@ That -Wno-redundant-decls option is no longer necessary, at least on rawhide. When I remove it, compiling with make -C python CFLAGS='-Wredundant-decls -Werror' still succeeds. So how about simply removing it? That also avoids adding the relatively distant python/Makefile.am - acinclude.m4 dependency. -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Makefile portability in qemud
john.le...@sun.com wrote: # HG changeset patch # User john.le...@sun.com # Date 1229399267 28800 # Node ID 35512df785342cd15214790e17e0c1f4d2182b32 # Parent 57c76efe37988edc64beb12ca5dc1ff5863f0085 Makefile portability in qemud Neither uuidgen nor sed -i are portable - only make the edit on platforms that can do so. Signed-off-by: John Levon john.le...@sun.com diff --git a/qemud/Makefile.am b/qemud/Makefile.am --- a/qemud/Makefile.am +++ b/qemud/Makefile.am @@ -56,7 +56,7 @@ remote_protocol.c: remote_protocol.h if WITH_LIBVIRTD -UUID=$(shell uuidgen) +UUID=$(shell type uuidgen /dev/null 21 uuidgen) ACK, but please use this smaller change: UUID=$(shell uuidgen 2/dev/null) sbin_PROGRAMS = libvirtd @@ -142,8 +142,9 @@ install-data-local: install-init install mkdir -p $(DESTDIR)$(sysconfdir)/libvirt/qemu/networks/autostart $(INSTALL_DATA) $(srcdir)/default-network.xml \ $(DESTDIR)$(sysconfdir)/$(default_xml_dest) - sed -i -e s,/name,/name\n uuid$(UUID)/uuid, \ - $(DESTDIR)$(sysconfdir)/$(default_xml_dest) + test -z $(UUID) || \ + sed -i -e s,/name,/name\n uuid$(UUID)/uuid, \ + $(DESTDIR)$(sysconfdir)/$(default_xml_dest) test -e $(DESTDIR)$(sysconfdir)/libvirt/qemu/networks/autostart/default.xml || \ ln -s ../default.xml \ $(DESTDIR)$(sysconfdir)/libvirt/qemu/networks/autostart/default.xml -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix qemud link
john.le...@sun.com wrote: # HG changeset patch # User john.le...@sun.com # Date 1229399267 28800 # Node ID 57c76efe37988edc64beb12ca5dc1ff5863f0085 # Parent 1f7707a3d49b20397011897cdc04c347322ad0c5 Fix qemud link Order the LDADDs such that symbols are correctly resolved (namely asprintf() and getdelim()). Signed-off-by: John Levon john.le...@sun.com diff --git a/qemud/Makefile.am b/qemud/Makefile.am --- a/qemud/Makefile.am +++ b/qemud/Makefile.am @@ -95,6 +95,8 @@ libvirtd_LDADD =\ $(SASL_LIBS)\ $(POLKIT_LIBS) +libvirtd_LDADD += ../src/libvirt.la + if ! WITH_DRIVER_MODULES if WITH_QEMU libvirtd_LDADD += ../src/libvirt_driver_qemu.la @@ -121,7 +123,7 @@ endif endif endif -libvirtd_LDADD += ../src/libvirt.la +libvirtd_LDADD += ../gnulib/lib/libgnu.la This is no longer needed, since the change that added this: # This must be added last, since functions it provides/replaces # are used by nearly every other library. libvirtd_LDADD += ../gnulib/lib/libgnu.la http://git.et.redhat.com/?p=libvirt.git;a=commitdiff;h=c29b4ff7a7b3c01a7c9 -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix compiler error when !HAVE_POLKIT
On Tue, Dec 16, 2008 at 06:52:38PM -0800, john.le...@sun.com wrote: # HG changeset patch # User john.le...@sun.com # Date 1229399439 28800 # Node ID f622c9de5e2488de8ae16bc7a0b50d8fa76af364 # Parent db36391b739c117f5887388f65f31e6a9d2d361b Fix compiler error when !HAVE_POLKIT Signed-off-by: John Levon john.le...@sun.com ACK Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- 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
Re: [libvirt] [PATCH] 1/8 detect syslog.h
On Wed, Dec 17, 2008 at 03:57:59PM +, Daniel P. Berrange wrote: On Wed, Dec 17, 2008 at 04:08:48PM +0100, Daniel Veillard wrote: the ability to syslog gets into libvirt.c so we need to detect its presence at configure time to compile it out on platform without support, ACK since it's standalone, applied, thanks, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] 3/8 convert libvirt.c to the new logging infrastructure
On Wed, Dec 17, 2008 at 04:15:38PM +0100, Daniel Veillard wrote:
Basically there is the initialization part which looks for the
environment variables, and all DEBUG calls are modified to show
the entry point, this was lost over time just the arguments of
the functions wasn't great for debugging,
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
Index: src/libvirt.c
===
RCS file: /data/cvs/libxen/src/libvirt.c,v
retrieving revision 1.182
diff -u -r1.182 libvirt.c
--- src/libvirt.c 4 Dec 2008 21:46:34 - 1.182
+++ src/libvirt.c 17 Dec 2008 14:25:57 -
@@ -257,8 +257,22 @@
#ifdef ENABLE_DEBUG
debugEnv = getenv(LIBVIRT_DEBUG);
-if (debugEnv *debugEnv *debugEnv != '0')
-debugFlag = 1;
+if (debugEnv *debugEnv *debugEnv != '0') {
+if (STREQ(debugEnv, 2) || STREQ(debugEnv, info))
+virLogSetDefaultPriority(VIR_LOG_INFO);
+else if (STREQ(debugEnv, 3) || STREQ(debugEnv, warning))
+virLogSetDefaultPriority(VIR_LOG_WARN);
+else if (STREQ(debugEnv, 4) || STREQ(debugEnv, error))
+virLogSetDefaultPriority(VIR_LOG_ERROR);
+else
+virLogSetDefaultPriority(VIR_LOG_DEBUG);
+}
+debugEnv = getenv(LIBVIRT_LOG_FILTERS);
+if (debugEnv)
+virLogParseFilters(debugEnv);
+debugEnv = getenv(LIBVIRT_LOG_OUTPUTS);
+if (debugEnv)
+virLogParseOutputs(debugEnv);
#endif
DEBUG0(register drivers);
@@ -734,7 +748,7 @@
virGetVersion(unsigned long *libVer, const char *type,
unsigned long *typeVer)
{
-DEBUG(libVir=%p, type=%s, typeVer=%p, libVer, type, typeVer);
+DEBUG(%s libVir=%p, type=%s, typeVer=%p, __FUNCTION__, libVer, type,
typeVer);
This is the bit which makes me thing we should pull __func__
and __line__ into the VIR_DEBUG macro all the time. I think
it could be generally useful to have this kind of info.
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- 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
Re: [libvirt] [PATCH] 4/8 convert virterror to the logging infrastructure
On Wed, Dec 17, 2008 at 04:19:47PM +0100, Daniel Veillard wrote: Basically having the debug information in one place and the errors in another place is not very helpful, so this plugs the virterror handling into the logging routine. mostly a bit of refactoring, and calling virLogMessage. Just that we pass an extra flag to virLogMessage stating it comes from the error handling level to avoid outputting the error twice on stderr in case where there is no output defined. Yep, makes sense. @@ -392,6 +411,12 @@ } /* + * Hook up the error or warning to the logging facility + */ +virLogMessage(virErrorDomainName(domain), virErrorLevelPriority(level), + 1, %s, str); + +/* * Save the information about the error */ virResetError(to); Hmm, but later on down the file we still call virDefaultErrorFunc() which would log the message a second time surely ? Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- 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
Re: [libvirt] [PATCH] 4/8 convert virterror to the logging infrastructure
On Wed, Dec 17, 2008 at 04:25:55PM +, Daniel P. Berrange wrote: On Wed, Dec 17, 2008 at 04:19:47PM +0100, Daniel Veillard wrote: Basically having the debug information in one place and the errors in another place is not very helpful, so this plugs the virterror handling into the logging routine. mostly a bit of refactoring, and calling virLogMessage. Just that we pass an extra flag to virLogMessage stating it comes from the error handling level to avoid outputting the error twice on stderr in case where there is no output defined. Yep, makes sense. @@ -392,6 +411,12 @@ } /* + * Hook up the error or warning to the logging facility + */ +virLogMessage(virErrorDomainName(domain), virErrorLevelPriority(level), + 1, %s, str); + +/* * Save the information about the error */ virResetError(to); Hmm, but later on down the file we still call virDefaultErrorFunc() which would log the message a second time surely ? To the application using libvirt, but not to the logs file. The potential annoyance is when you get both going to stderr for example if using virsh. Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [SECURITY] PATCH: Fix missing read-only access checks (CVE-2008-5086)
The following methods in libvirt.c are missing a check against the
read-only connection flag:
virDomainMigrate
virDomainMigratePrepare
virDomainMigratePerform
virDomainMigrateFinish
virDomainMigratePrepare2
virDomainMigrateFinish2
virDomainBlockPeek
virDomainMemoryPeek
virDomainSetAutostart
virNetworkSetAutostart
virConnectFindStoragePoolSources
virStoragePoolSetAutostart
If using PolicyKit auth, the default policy will allow any local user
to make a read-only connection to the libvirtd daemon without needing
authentication.
If not using PolicyKit, the default libvirtd.conf configuration settings
will allow an unprivileged user to make a read-only connection to the
libvirtd daemon without needing authentication.
Thus out of the box unprivileged local users may be able to migrate VMs,
set or unset the autostart flag for domains, networks storage pools,
and access privileged data in the VM memory, or disks.
All TCP remote connections are read-write, and default settings require
full authentication, thus remote access is not impacted by this flaw.
Administrators can apply a workaround by editting /etc/libvirt/libvirtd.conf
to explicitly set 'unix_sock_ro_perms' parameter to '0700'. Restart the
libvirtd daemon after making this change.
The first vulnerable release was 0.3.2, where the virDomainMigrate API
was added for the Xen driver. Other APIs were added in various subsequent
releases depending on the hypervisor driver in question.
The attached patch has been committed to CVS, and OS distributors are
recommended to apply this patch to all existing releases shipped. It
was diff'd against current CVS head, and applies against 0.5.1, and
is trivially re-diffable for all earlier releases.
This flaw has been assigned the identifier CVE-2008-5086
Daniel
diff --git a/src/libvirt.c b/src/libvirt.c
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -2296,6 +2296,16 @@ virDomainMigrate (virDomainPtr domain,
conn = domain-conn;/* Source connection. */
if (!VIR_IS_CONNECT (dconn)) {
virLibConnError (conn, VIR_ERR_INVALID_CONN, __FUNCTION__);
+return NULL;
+}
+
+if (domain-conn-flags VIR_CONNECT_RO) {
+virLibDomainError(domain, VIR_ERR_OPERATION_DENIED, __FUNCTION__);
+return NULL;
+}
+if (dconn-flags VIR_CONNECT_RO) {
+/* NB, delibrately report error against source object, not dest here */
+virLibDomainError(domain, VIR_ERR_OPERATION_DENIED, __FUNCTION__);
return NULL;
}
@@ -2426,6 +2436,11 @@ virDomainMigratePrepare (virConnectPtr d
return -1;
}
+if (dconn-flags VIR_CONNECT_RO) {
+virLibConnError(dconn, VIR_ERR_OPERATION_DENIED, __FUNCTION__);
+return -1;
+}
+
if (dconn-driver-domainMigratePrepare)
return dconn-driver-domainMigratePrepare (dconn, cookie, cookielen,
uri_in, uri_out,
@@ -2457,6 +2472,11 @@ virDomainMigratePerform (virDomainPtr do
}
conn = domain-conn;
+if (domain-conn-flags VIR_CONNECT_RO) {
+virLibDomainError(domain, VIR_ERR_OPERATION_DENIED, __FUNCTION__);
+return -1;
+}
+
if (conn-driver-domainMigratePerform)
return conn-driver-domainMigratePerform (domain, cookie, cookielen,
uri,
@@ -2482,6 +2502,11 @@ virDomainMigrateFinish (virConnectPtr dc
if (!VIR_IS_CONNECT (dconn)) {
virLibConnError (NULL, VIR_ERR_INVALID_CONN, __FUNCTION__);
+return NULL;
+}
+
+if (dconn-flags VIR_CONNECT_RO) {
+virLibConnError(dconn, VIR_ERR_OPERATION_DENIED, __FUNCTION__);
return NULL;
}
@@ -2517,6 +2542,11 @@ virDomainMigratePrepare2 (virConnectPtr
return -1;
}
+if (dconn-flags VIR_CONNECT_RO) {
+virLibConnError(dconn, VIR_ERR_OPERATION_DENIED, __FUNCTION__);
+return -1;
+}
+
if (dconn-driver-domainMigratePrepare2)
return dconn-driver-domainMigratePrepare2 (dconn, cookie, cookielen,
uri_in, uri_out,
@@ -2547,6 +2577,11 @@ virDomainMigrateFinish2 (virConnectPtr d
return NULL;
}
+if (dconn-flags VIR_CONNECT_RO) {
+virLibConnError(dconn, VIR_ERR_OPERATION_DENIED, __FUNCTION__);
+return NULL;
+}
+
if (dconn-driver-domainMigrateFinish2)
return dconn-driver-domainMigrateFinish2 (dconn, dname,
cookie, cookielen,
@@ -2905,6 +2940,11 @@ virDomainBlockPeek (virDomainPtr dom,
}
conn = dom-conn;
+if (dom-conn-flags VIR_CONNECT_RO) {
+virLibDomainError(dom, VIR_ERR_OPERATION_DENIED, __FUNCTION__);
+return (-1);
+}
+
if (!path) {
virLibDomainError (dom, VIR_ERR_INVALID_ARG,
_(path is NULL));
@@ -2980,6 +3020,11 @@
[libvirt] [PATCH] 4/8 convert virterror to the logging infrastructure
Basically having the debug information in one place and the errors
in another place is not very helpful, so this plugs the virterror
handling into the logging routine. mostly a bit of refactoring,
and calling virLogMessage. Just that we pass an extra flag to
virLogMessage stating it comes from the error handling level to avoid
outputting the error twice on stderr in case where there is no
output defined.
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
Index: src/virterror.c
===
RCS file: /data/cvs/libxen/src/virterror.c,v
retrieving revision 1.55
diff -u -r1.55 virterror.c
--- src/virterror.c 25 Nov 2008 10:44:53 - 1.55
+++ src/virterror.c 17 Dec 2008 14:25:57 -
@@ -17,6 +17,7 @@
#include virterror_internal.h
#include datatypes.h
+#include logging.h
virError virLastErr = /* the last error */
{ .code = 0, .domain = 0, .message = NULL, .level = VIR_ERR_NONE,
@@ -62,6 +63,97 @@
}} \
}
+static virLogPriority virErrorLevelPriority(virErrorLevel level) {
+switch (level) {
+case VIR_ERR_NONE:
+return(VIR_LOG_INFO);
+case VIR_ERR_WARNING:
+return(VIR_LOG_WARN);
+case VIR_ERR_ERROR:
+return(VIR_LOG_ERROR);
+}
+return(VIR_LOG_ERROR);
+}
+
+static const char *virErrorDomainName(virErrorDomain domain) {
+const char *dom = unknown;
+switch (domain) {
+case VIR_FROM_NONE:
+dom = ;
+break;
+case VIR_FROM_XEN:
+dom = Xen ;
+break;
+case VIR_FROM_XML:
+dom = XML ;
+break;
+case VIR_FROM_XEND:
+dom = Xen Daemon ;
+break;
+case VIR_FROM_XENSTORE:
+dom = Xen Store ;
+break;
+case VIR_FROM_XEN_INOTIFY:
+dom = Xen Inotify ;
+break;
+case VIR_FROM_DOM:
+dom = Domain ;
+break;
+case VIR_FROM_RPC:
+dom = XML-RPC ;
+break;
+case VIR_FROM_QEMU:
+dom = QEMU ;
+break;
+case VIR_FROM_NET:
+dom = Network ;
+break;
+case VIR_FROM_TEST:
+dom = Test ;
+break;
+case VIR_FROM_REMOTE:
+dom = Remote ;
+break;
+case VIR_FROM_SEXPR:
+dom = S-Expr ;
+break;
+case VIR_FROM_PROXY:
+dom = PROXY ;
+break;
+case VIR_FROM_CONF:
+dom = Config ;
+break;
+case VIR_FROM_OPENVZ:
+dom = OpenVZ ;
+break;
+case VIR_FROM_XENXM:
+dom = Xen XM ;
+break;
+case VIR_FROM_STATS_LINUX:
+dom = Linux Stats ;
+break;
+case VIR_FROM_LXC:
+dom = Linux Container ;
+break;
+case VIR_FROM_STORAGE:
+dom = Storage ;
+break;
+case VIR_FROM_NETWORK:
+dom = Network Config ;
+break;
+case VIR_FROM_DOMAIN:
+dom = Domain Config ;
+break;
+case VIR_FROM_NODEDEV:
+dom = Node Device ;
+break;
+case VIR_FROM_UML:
+dom = UML ;
+break;
+}
+return(dom);
+}
+
/*
* virGetLastError:
*
@@ -246,80 +338,7 @@
lvl = _(error);
break;
}
-switch (err-domain) {
-case VIR_FROM_NONE:
-dom = ;
-break;
-case VIR_FROM_XEN:
-dom = Xen ;
-break;
-case VIR_FROM_XML:
-dom = XML ;
-break;
-case VIR_FROM_XEND:
-dom = Xen Daemon ;
-break;
-case VIR_FROM_XENSTORE:
-dom = Xen Store ;
-break;
-case VIR_FROM_XEN_INOTIFY:
-dom = Xen Inotify ;
-break;
-case VIR_FROM_DOM:
-dom = Domain ;
-break;
-case VIR_FROM_RPC:
-dom = XML-RPC ;
-break;
-case VIR_FROM_QEMU:
-dom = QEMU ;
-break;
-case VIR_FROM_NET:
-dom = Network ;
-break;
-case VIR_FROM_TEST:
-dom = Test ;
-break;
-case VIR_FROM_REMOTE:
-dom = Remote ;
-break;
-case VIR_FROM_SEXPR:
-dom = S-Expr ;
-break;
-case VIR_FROM_PROXY:
-dom = PROXY ;
-break;
-case VIR_FROM_CONF:
-dom = Config ;
-break;
-case VIR_FROM_OPENVZ:
-dom = OpenVZ ;
-break;
-
Re: [libvirt] [PATCH] Fix _FILE_OFFSET_BITS re-definition
Daniel P. Berrange berra...@redhat.com wrote: ... diff --git a/qemud/Makefile.am b/qemud/Makefile.am index b8dae88..28fd84a 100644 --- a/qemud/Makefile.am +++ b/qemud/Makefile.am @@ -33,11 +33,16 @@ EXTRA_DIST = \ if RPCGEN SUFFIXES = .x +# The perl -ne subshell ensures that remote_protocol.c ends up +# including config.h before remote_protocol.h. .x.c: -rm -f $@ $...@-t $...@-t2 +rm -f $@ $...@-t $...@-t1 $...@-t2 rpcgen -c -o $...@-t $ +(echo '#include config.h';\ + perl -ne '/^#include config.h/ or print' $...@-t) $...@-t1 if GLIBC_RPCGEN -perl -w rpcgen_fix.pl $...@-t $...@-t2 +perl -w rpcgen_fix.pl $...@-t1 $...@-t2 +rm $...@-t1 chmod 444 $...@-t2 mv $...@-t2 $@ endif ... Rather than filtering out the bogus 'config.h' from remote_protocol.c in the Makefile.am rule, just kill this line from the original protocol definition: %#include config.h Then the bogus placed include wouldn't be added in the first place. Good idea. Committed with that change, which induced the removal of #include config.h from remote_protocol.h, too. But that's fine, because that .h file never needed it in the first place. -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] 4/8 convert virterror to the logging infrastructure
On Wed, Dec 17, 2008 at 05:49:43PM +0100, Daniel Veillard wrote: On Wed, Dec 17, 2008 at 04:25:55PM +, Daniel P. Berrange wrote: On Wed, Dec 17, 2008 at 04:19:47PM +0100, Daniel Veillard wrote: Basically having the debug information in one place and the errors in another place is not very helpful, so this plugs the virterror handling into the logging routine. mostly a bit of refactoring, and calling virLogMessage. Just that we pass an extra flag to virLogMessage stating it comes from the error handling level to avoid outputting the error twice on stderr in case where there is no output defined. Yep, makes sense. @@ -392,6 +411,12 @@ } /* + * Hook up the error or warning to the logging facility + */ +virLogMessage(virErrorDomainName(domain), virErrorLevelPriority(level), + 1, %s, str); + +/* * Save the information about the error */ virResetError(to); Hmm, but later on down the file we still call virDefaultErrorFunc() which would log the message a second time surely ? To the application using libvirt, but not to the logs file. The potential annoyance is when you get both going to stderr for example if using virsh. Oh yes, of course. In any case the output would only be duplicated if the user had turned on logging to stderr, and the app had not already disabled the default error printing function. So no real problem. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- 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
[libvirt] Re: [PATCH] portability: don't include endian.h
John Levon john.le...@sun.com wrote: On Wed, Dec 17, 2008 at 01:55:59PM +0100, Jim Meyering wrote: Adding the inclusions of string.h and mntent.h look fine, They were just moved. but rather than testing for endian.h, it seems we can do away with it altogether. Even the __LITTLE_ENDIAN and __BIG_ENDIAN constants aren't really required. Rather, all you need is Fine by me, if you prefer it this way. Of course. Avoiding an #ifdef like that is worth an iteration. I've committed the patch I proposed, with one change: also remove the unnecessary inclusion of byteswap.h. -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Fix pool define crash
There's a null dereference in the storage driver when defining a pool.
Attached patch fixes it for me.
Thanks,
Cole
diff --git a/src/storage_driver.c b/src/storage_driver.c
index 2432a9a..ac5e443 100644
--- a/src/storage_driver.c
+++ b/src/storage_driver.c
@@ -546,7 +546,7 @@ storagePoolDefine(virConnectPtr conn,
goto cleanup;
def = NULL;
-if (virStoragePoolObjSaveDef(conn, driver, pool, def) 0) {
+if (virStoragePoolObjSaveDef(conn, driver, pool, pool-def) 0) {
virStoragePoolObjRemove(driver-pools, pool);
goto cleanup;
}
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Unify pool-create-as and pool-define-as xml building
The attached patch unifies the xml building and command line option
registering for the virsh pool-*-as commands, fixing a few bugs in the
process:
- pool-define-as was creating host without a name attribute
(rhbz 476708)
- pool-create-as wasn't adding a closing tag to its host block
- pool-create-as didn't have the source-name option
All seem like a good justification for not duplicating the logic. The
patch ends up looking a bit bizarre, but it's mostly just moving the xml
building from create-as into a separate function buildPoolXML, and then
wiring it up.
Thanks,
Cole
src/virsh.c | 130 --
1 files changed, 45 insertions(+), 85 deletions(-)
diff --git a/src/virsh.c b/src/virsh.c
index 1a5b42f..1f154ee 100644
--- a/src/virsh.c
+++ b/src/virsh.c
@@ -2864,38 +2864,27 @@ cmdPoolCreate(vshControl *ctl, const vshCmd *cmd)
return ret;
}
+
/*
- * pool-create-as command
+ * XML Building helper for pool-define-as and pool-create-as
*/
-static const vshCmdInfo info_pool_create_as[] = {
-{help, gettext_noop(create a pool from a set of args)},
-{desc, gettext_noop(Create a pool.)},
-{NULL, NULL}
-};
-
-static const vshCmdOptDef opts_pool_create_as[] = {
+static const vshCmdOptDef opts_pool_X_as[] = {
{name, VSH_OT_DATA, VSH_OFLAG_REQ, gettext_noop(name of the pool)},
{type, VSH_OT_DATA, VSH_OFLAG_REQ, gettext_noop(type of the pool)},
{source-host, VSH_OT_DATA, 0, gettext_noop(source-host for underlying storage)},
{source-path, VSH_OT_DATA, 0, gettext_noop(source path for underlying storage)},
{source-dev, VSH_OT_DATA, 0, gettext_noop(source device for underlying storage)},
+{source-name, VSH_OT_DATA, 0, gettext_noop(source name for underlying storage)},
{target, VSH_OT_DATA, 0, gettext_noop(target for underlying storage)},
{NULL, 0, 0, NULL}
};
+static int buildPoolXML(const vshCmd *cmd, char **retname, char **xml) {
-static int
-cmdPoolCreateAs(vshControl *ctl, const vshCmd *cmd)
-{
-virStoragePoolPtr pool;
int found;
-char *xml;
-char *name, *type, *srcHost, *srcPath, *srcDev, *target;
+char *name, *type, *srcHost, *srcPath, *srcDev, *srcName, *target;
virBuffer buf = VIR_BUFFER_INITIALIZER;
-if (!vshConnectionUsability(ctl, ctl-conn, TRUE))
-return FALSE;
-
name = vshCommandOptString(cmd, name, found);
if (!found)
goto cleanup;
@@ -2906,20 +2895,22 @@ cmdPoolCreateAs(vshControl *ctl, const vshCmd *cmd)
srcHost = vshCommandOptString(cmd, source-host, found);
srcPath = vshCommandOptString(cmd, source-path, found);
srcDev = vshCommandOptString(cmd, source-dev, found);
+srcName = vshCommandOptString(cmd, source-name, found);
target = vshCommandOptString(cmd, target, found);
virBufferVSprintf(buf, pool type='%s'\n, type);
virBufferVSprintf(buf, name%s/name\n, name);
if (srcHost || srcPath || srcDev) {
virBufferAddLit(buf, source\n);
-if (srcHost)
-virBufferVSprintf(buf, host name='%s'\n, srcHost);
+if (srcHost)
+virBufferVSprintf(buf, host name='%s'/\n, srcHost);
if (srcPath)
virBufferVSprintf(buf, dir path='%s'/\n, srcPath);
-
if (srcDev)
virBufferVSprintf(buf, device path='%s'/\n, srcDev);
+if (srcName)
+virBufferVSprintf(buf, name%s/name\n, srcName);
virBufferAddLit(buf, /source\n);
}
@@ -2934,7 +2925,36 @@ cmdPoolCreateAs(vshControl *ctl, const vshCmd *cmd)
vshPrint(ctl, %s, _(Failed to allocate XML buffer));
return FALSE;
}
-xml = virBufferContentAndReset(buf);
+
+*xml = virBufferContentAndReset(buf);
+*retname = name;
+return TRUE;
+
+cleanup:
+free(virBufferContentAndReset(buf));
+return FALSE;
+}
+
+/*
+ * pool-create-as command
+ */
+static const vshCmdInfo info_pool_create_as[] = {
+{help, gettext_noop(create a pool from a set of args)},
+{desc, gettext_noop(Create a pool.)},
+{NULL, NULL}
+};
+
+static int
+cmdPoolCreateAs(vshControl *ctl, const vshCmd *cmd)
+{
+virStoragePoolPtr pool;
+char *xml, *name;
+
+if (!vshConnectionUsability(ctl, ctl-conn, TRUE))
+return FALSE;
+
+if (!buildPoolXML(cmd, name, xml))
+return FALSE;
pool = virStoragePoolCreateXML(ctl-conn, xml, 0);
free (xml);
@@ -2945,11 +2965,8 @@ cmdPoolCreateAs(vshControl *ctl, const vshCmd *cmd)
return TRUE;
} else {
vshError(ctl, FALSE, _(Failed to create pool %s), name);
-return FALSE;
}
- cleanup:
-free(virBufferContentAndReset(buf));
return FALSE;
}
@@ -3010,71 +3027,17 @@ static const vshCmdInfo info_pool_define_as[] = {
{NULL, NULL}
};
-static const vshCmdOptDef opts_pool_define_as[] = {
-{name, VSH_OT_DATA, VSH_OFLAG_REQ, gettext_noop(name of the pool)},
-{type, VSH_OT_DATA, VSH_OFLAG_REQ,
Re: [libvirt] PATCH 0/5: connection cloning support (WIP)
Ok, with that fix, my asynchronous Java event implementation seems to be
working (though not fully tested) with these patches.
But I'm not using connection cloning (yet). It's your concurrency
control in the remote driver that makes this work.
On the Java side, domain event callbacks from another thread work
because of the (Java) locking of Connect objects. I don't think I
should remove that, since libvirt still prohibits concurrent use of the
same virConnect object (right?)
I suppose I could clone the virConnect object before delivering an event
to a Java Domain (so the Domain's Connect would wrap the clone). I
don't think this does much in the remote case (since the underlying RPC
pipe it still shared, albeit safely). But I suppose it might allow
greater concurrency in the non-remote cases. Is this what you had in
mind?
Dave
On Wed, 2008-12-17 at 11:21 -0500, David Lively wrote:
On Wed, 2008-12-17 at 13:58 +, Daniel P. Berrange wrote:
On Wed, Dec 17, 2008 at 07:44:15AM -0500, David Lively wrote:
Hi Daniel -
When I apply these patches, I'm seeing segfaults on event delivery
when just running the existing synchronous
examples/domain-events/events-c/event-test.c (using the remote driver).
I've not come across that specific problem, but there are a definitely
some locking bugs refcounting bugs inthe patches I've posted so far.
I'll post an updated series of patches which may address this.
Daniel
It turned out to be a double-free. Trivial fix below.
Dave
diff --git a/src/remote_internal.c b/src/remote_internal.c
index 9245a2a..d6b94ff 100644
--- a/src/remote_internal.c
+++ b/src/remote_internal.c
@@ -6423,10 +6423,10 @@ remoteDomainQueueEvent(virConnectPtr conn, XDR
*xdr)
return;
if (virDomainEventQueuePush(priv-domainEvents,
-event) 0)
+event) 0) {
DEBUG0(Error adding event to queue);
-
-virDomainEventFree(event);
+virDomainEventFree(event);
+}
}
/** remoteDomainEventFired:
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix _FILE_OFFSET_BITS re-definition
john.le...@sun.com wrote:
# HG changeset patch
# User john.le...@sun.com
# Date 1229399267 28800
# Node ID db36391b739c117f5887388f65f31e6a9d2d361b
# Parent 020f8b8e9340287a6ab3d869b359e39b905cd0ff
Fix _FILE_OFFSET_BITS re-definition
Since config.h contains the _FILE_OFFSET_BITS setting (a little dubious
in itself), it must be the first header included, otherwise system
headers can define _FILE_OFFSET_BITS differently themselves.
Signed-off-by: John Levon john.le...@sun.com
diff --git a/src/node_device_hal.c b/src/node_device_hal.c
--- a/src/node_device_hal.c
+++ b/src/node_device_hal.c
@@ -21,10 +21,11 @@
* Author: David F. Lively dliv...@virtualiron.com
*/
+#include config.h
+
#include stdio.h
#include stdlib.h
-#include config.h
#include libhal.h
#include node_device_conf.h
ACK
FYI, there are 3 other cases where the include config.h first
rule is broken:
docs/examples/info1.c
docs/examples/suspend.c
qemud/remote_protocol.c
so I've just written a new syntax-check rule to enforce this.
It fixes only the last one. Since the two examples are not
necessarily built using libvirt's own build system, so they
are exempted.
So with your patch above and the following one,
the new make sc_require_config_h_first part of make syntax-check
passes:
From 15afb090ac28ab6b9274fb827eb1c1c939db4104 Mon Sep 17 00:00:00 2001
From: Jim Meyering meyer...@redhat.com
Date: Wed, 17 Dec 2008 10:49:04 +0100
Subject: [PATCH] enforce the include config.h first rule
* qemud/Makefile.am: Ensure that the generated remote_protocol.c
includes config.h first.
* qemud/remote_protocol.c: Regenerate.
* Makefile.maint (sc_require_config_h_first): New rule, so that
make syntax-check enforces this.
* .x-sc_require_config_h_first: New file.
* Makefile.am (.x-sc_require_config_h_first): Add it.
---
.x-sc_require_config_h_first |2 ++
ChangeLog| 11 +++
Makefile.am |1 +
Makefile.maint | 15 +++
qemud/Makefile.am|9 +++--
qemud/remote_protocol.c |2 +-
6 files changed, 37 insertions(+), 3 deletions(-)
create mode 100644 .x-sc_require_config_h_first
diff --git a/.x-sc_require_config_h_first b/.x-sc_require_config_h_first
new file mode 100644
index 000..58a8878
--- /dev/null
+++ b/.x-sc_require_config_h_first
@@ -0,0 +1,2 @@
+^docs/examples/info1\.c$
+^docs/examples/suspend\.c$
diff --git a/ChangeLog b/ChangeLog
index 08a1cb5..36c1278 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,14 @@
+2008-12-17 Jim Meyering meyer...@redhat.com
+
+ enforce the include config.h first rule
+ * qemud/Makefile.am: Ensure that the generated remote_protocol.c
+ includes config.h first.
+ * qemud/remote_protocol.c: Regenerate.
+ * Makefile.maint (sc_require_config_h_first): New rule, so that
+ make syntax-check enforces this.
+ * .x-sc_require_config_h_first: New file.
+ * Makefile.am (.x-sc_require_config_h_first): Add it.
+
Wed Dec 17 08:02:01 +0100 2008 Jim Meyering meyer...@redhat.com
fix numa-related (and kernel-dependent) test failures
diff --git a/Makefile.am b/Makefile.am
index d40a151..758ad50 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -14,6 +14,7 @@ EXTRA_DIST = \
libvirt.pc libvirt.pc.in \
$(man_MANS) autobuild.sh \
.x-sc_avoid_if_before_free \
+ .x-sc_require_config_h_first \
.x-sc_prohibit_strcmp \
.x-sc_require_config_h \
autogen.sh
diff --git a/Makefile.maint b/Makefile.maint
index 5758215..b7bb680 100644
--- a/Makefile.maint
+++ b/Makefile.maint
@@ -135,6 +135,21 @@ sc_require_config_h:
else :; \
fi
+# You must include config.h before including any other header file.
+sc_require_config_h_first:
+ @if $(VC_LIST_EXCEPT) | grep '\.c$$' /dev/null; then \
+ fail=0; \
+ for i in $$($(VC_LIST_EXCEPT) | grep '\.c$$'); do \
+ grep '^# *include\' $$i | sed 1q \
+ | grep '^# *include config\.h' /dev/null\
+ || { echo $$i; fail=1; }; \
+ done; \
+ test $$fail = 1 \
+ { echo '$(ME): the above files include some other header' \
+ 'before config.h' 12; exit 1; } || :; \
+ else :; \
+ fi
+
# To use this command macro, you must first define two shell variables:
# h: the header, enclosed in or
# re: a regular expression that matches IFF something provided by $h is used.
diff --git a/qemud/Makefile.am b/qemud/Makefile.am
index b8dae88..28fd84a 100644
--- a/qemud/Makefile.am
+++ b/qemud/Makefile.am
@@ -33,11 +33,16 @@
[libvirt] [PATCH] 5/8 converts a few others modules to use the logging
just a few redefines and using the new values,
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
Index: src/node_device_devkit.c
===
RCS file: /data/cvs/libxen/src/node_device_devkit.c,v
retrieving revision 1.4
diff -u -r1.4 node_device_devkit.c
--- src/node_device_devkit.c4 Dec 2008 21:48:31 - 1.4
+++ src/node_device_devkit.c17 Dec 2008 14:25:57 -
@@ -41,9 +41,6 @@
static virDeviceMonitorStatePtr driverState;
-#define DEBUG(fmt,...) VIR_DEBUG(__FILE__, fmt, __VA_ARGS__)
-#define DEBUG0(msg) VIR_DEBUG(__FILE__, %s, msg)
-
#define CONN_DRV_STATE(conn) \
((virDeviceMonitorStatePtr)((conn)-devMonPrivateData))
#define DRV_STATE_DKCLIENT(ds) ((DevkitClient *)((ds)-privateData))
Index: src/uml_conf.c
===
RCS file: /data/cvs/libxen/src/uml_conf.c,v
retrieving revision 1.3
diff -u -r1.3 uml_conf.c
--- src/uml_conf.c 12 Dec 2008 10:39:19 - 1.3
+++ src/uml_conf.c 17 Dec 2008 14:25:57 -
@@ -49,10 +49,8 @@
#include memory.h
#include verify.h
-
-#define umlLog(level, msg...) fprintf(stderr, msg)
-
-
+#define umlLog(level, msg, ...) \
+virLogMessage(__FILE__, level, 0, msg, __VA_ARGS__)
#if HAVE_NUMACTL
#define MAX_CPUS 4096
Index: src/uml_driver.c
===
RCS file: /data/cvs/libxen/src/uml_driver.c,v
retrieving revision 1.10
diff -u -r1.10 uml_driver.c
--- src/uml_driver.c10 Dec 2008 16:35:01 - 1.10
+++ src/uml_driver.c17 Dec 2008 14:25:57 -
@@ -62,17 +62,15 @@
#include uuid.h
#include domain_conf.h
#include datatypes.h
+#include logging.h
/* For storing short-lived temporary files. */
#define TEMPDIR LOCAL_STATE_DIR /cache/libvirt
static int umlShutdown(void);
-/* umlDebug statements should be changed to use this macro instead. */
-#define DEBUG(fmt,...) VIR_DEBUG(__FILE__, fmt, __VA_ARGS__)
-#define DEBUG0(msg) VIR_DEBUG(__FILE__, %s, msg)
-
-#define umlLog(level, msg...) fprintf(stderr, msg)
+#define umlLog(level, msg, ...) \
+virLogMessage(__FILE__, level, 0, msg, __VA_ARGS__)
static void umlDriverLock(struct uml_driver *driver)
{
@@ -100,7 +98,7 @@
goto error;
return 0;
error:
-umlLog(UML_ERR,
+umlLog(VIR_LOG_ERROR,
%s, _(Failed to set close-on-exec file descriptor flag\n));
return -1;
}
@@ -141,7 +139,7 @@
!virDomainIsActive(driver-domains.objs[i])
umlStartVMDaemon(conn, driver, driver-domains.objs[i]) 0) {
virErrorPtr err = virGetLastError();
-umlLog(UML_ERR, _(Failed to autostart VM '%s': %s\n),
+umlLog(VIR_LOG_ERROR, _(Failed to autostart VM '%s': %s\n),
driver-domains.objs[i]-def-name, err-message);
}
}
@@ -323,7 +321,7 @@
uml_driver-nextvmid = 1;
if (!(pw = getpwuid(uid))) {
-umlLog(UML_ERR, _(Failed to find user record for uid '%d': %s\n),
+umlLog(VIR_LOG_ERROR, _(Failed to find user record for uid '%d':
%s\n),
uid, strerror(errno));
goto error;
}
@@ -368,12 +366,12 @@
if ((uml_driver-inotifyFD = inotify_init()) 0) {
-umlLog(UML_ERR, %s, _(cannot initialize inotify));
+umlLog(VIR_LOG_ERROR, %s, _(cannot initialize inotify));
goto error;
}
if (virFileMakePath(uml_driver-monitorDir) 0) {
-umlLog(UML_ERR, _(Failed to create monitor directory %s: %s),
+umlLog(VIR_LOG_ERROR, _(Failed to create monitor directory %s: %s),
uml_driver-monitorDir, strerror(errno));
goto error;
}
@@ -403,7 +401,7 @@
return 0;
out_of_memory:
-umlLog (UML_ERR,
+umlLog (VIR_LOG_ERROR,
%s, _(umlStartup: out of memory\n));
error:
@@ -784,25 +782,25 @@
tmp = progenv;
while (*tmp) {
if (safewrite(logfd, *tmp, strlen(*tmp)) 0)
-umlLog(UML_WARN, _(Unable to write envv to logfile %d: %s\n),
+umlLog(VIR_LOG_WARN, _(Unable to write envv to logfile %d: %s\n),
errno, strerror(errno));
if (safewrite(logfd, , 1) 0)
-umlLog(UML_WARN, _(Unable to write envv to logfile %d: %s\n),
+umlLog(VIR_LOG_WARN, _(Unable to write envv to logfile %d: %s\n),
errno, strerror(errno));
tmp++;
}
tmp = argv;
while (*tmp) {
if (safewrite(logfd, *tmp, strlen(*tmp)) 0)
-umlLog(UML_WARN, _(Unable to write argv to logfile %d: %s\n),
+umlLog(VIR_LOG_WARN, _(Unable to write argv to logfile %d: %s\n),
errno,
Re: [libvirt] [PATCH] 0/8 logging infrastructure for libvirt[d]
Daniel P. Berrange wrote: On Wed, Dec 17, 2008 at 04:06:38PM +0100, Daniel Veillard wrote: In practice export LIBVIRT_DEBUG=1 export LIBVIRT_LOG_OUTPUTS=0:file:virsh.log and then running virsh will accumulate all logging to the virsh.log file in the current directory. This looks great. One thing which I feel is somewhat incomplete is that it's impossible to remotely get debugging output from the libvirt daemon serving the requests. Currently all logs are also accumulated in a cyclic logging buffer, I would associate a dump function later to be hooked for example to a signal handler in the daemon. But I'm unsure we should allow dumping logging information to the remote end, probably not the whole set. I've been wondering whether we should create an explicit tool for admin of the libvirtd daemon itself. Basically something for querying state, and performing operations wrt to the daemon for things outside the scope of the libvirt API. For example, something to get details of active client connections, and forceably drop a client. Being able to have an API to configure the log level / setup of the dameon would be a useful thing. We can easily layer this kind of thing in as another RPC protocol in parallel with the existing protocol, and have simple CLI tool libvirtd-admin. SQUID has a 'squidclient' tool and a CGI script for administration of the server itself. Daniel I think a daemon control interface is a great idea. Being able to change things like loglevel without restarting the daemon is a real plus for administrators. Dave -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] 0/8 logging infrastructure for libvirt[d]
On Wed, Dec 17, 2008 at 04:06:38PM +0100, Daniel Veillard wrote: In practice export LIBVIRT_DEBUG=1 export LIBVIRT_LOG_OUTPUTS=0:file:virsh.log and then running virsh will accumulate all logging to the virsh.log file in the current directory. This looks great. One thing which I feel is somewhat incomplete is that it's impossible to remotely get debugging output from the libvirt daemon serving the requests. Currently all logs are also accumulated in a cyclic logging buffer, I would associate a dump function later to be hooked for example to a signal handler in the daemon. But I'm unsure we should allow dumping logging information to the remote end, probably not the whole set. I've been wondering whether we should create an explicit tool for admin of the libvirtd daemon itself. Basically something for querying state, and performing operations wrt to the daemon for things outside the scope of the libvirt API. For example, something to get details of active client connections, and forceably drop a client. Being able to have an API to configure the log level / setup of the dameon would be a useful thing. We can easily layer this kind of thing in as another RPC protocol in parallel with the existing protocol, and have simple CLI tool libvirtd-admin. SQUID has a 'squidclient' tool and a CGI script for administration of the server itself. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- 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
Re: [libvirt] [PATCH] rpcgen fixes
On Tue, Dec 16, 2008 at 06:25:26PM -0800, john.le...@sun.com wrote: # HG changeset patch # User john.le...@sun.com # Date 1229399266 28800 # Node ID 12c9b283b11f5e20b9dcc91e3d10a862da5d6e81 # Parent 1ba824db09286ebc758e035211da2c533bbd1e0f rpcgen fixes quad_t is not a portable type, but rather than force rpcgen every build, we just patch in the fixes needed. IIRC, this will also help on Mac OS-X which lacked this. Also, since we ship the rpcgen-derived files in CVS, don't leave Makefile rules that could trigger during a normal build: make a special maintainer target for refreshing the derived sources. ACK, good change. The automatic refresh was broken anyway, because the generated files are needed from ../src before the build even gets to qemud/. These generated files / wire protocol define ABI and should never change for ordinary builds, so its safer to keep generation of them an explicit maintainer task. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- 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
Re: [libvirt] [PATCH] don't assume builddir == srcdir
Guido Günther a...@sigxcpu.org wrote: Makefile.maint assumes this in some places. This doesn't make the world perfect but maybe a bit better? Um, not really, since a pathological $(abs_srcdir) may contain ; rm -rf /; Best to avoid it and use $(srcdir) instead. Were you able to construct a situation in which this change helped? I wonder, since in a non-srcdir builds, GNUmakefile and Makefile.maint don't exist, so it should be impossible to run their rules without either copying them into the build directory or using make's -f option. [copying the files into place, to test this ...] It looks like if you use VC_LIST's -C option, then that invalidates the exempted names in the .x-sc* files. FWIW, I did make distclean mkdir .j cd .j ./configure. From there, none of the Makefile.maint rules are available. Eventually, I'll probably sync from coreutils/gnulib, where this file is now named maint.mk. -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] kvm: save / restore
Hallo, I use kvm-81 and libvirt 0.5.1. I can save a kvm donain but if I want to restore it I get the following error in the log file: unknown migration protocol: stdio I tried it using only kvm and got the same error. How can I save / restore a VM to / from a file? Kind regards Matthias Pfafferodt -- Matthias Pfafferodt - http://www.mapfa.de Matthias.Pfafferodt at mapfa.de -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix _FILE_OFFSET_BITS re-definition
On Wed, Dec 17, 2008 at 10:55:41AM +0100, Jim Meyering wrote: FYI, there are 3 other cases where the include config.h first rule is broken: docs/examples/info1.c docs/examples/suspend.c qemud/remote_protocol.c One of my other patches fixes this last one. so I've just written a new syntax-check rule to enforce this. Cool, thanks! regards john -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] rpcgen fixes
John Levon john.le...@sun.com wrote: ... + perl -w @top_srcdir@/qemud/rpcgen_fix.pl rp.h-t \ + @top_srcdir@/qemud/remote_protocol.h Please don't redirect directly to something used as a build source. Instead, redirect to a temporary file, and rename that into place upon successful completion. That makes it less likely you'll ever end up with corrupted sources. Sorry I'm missing why this helps any? In rules where you're redirecting to $@, it's essential so that you don't end up with a corrupt-yet-up-to-date target. Here, it'd be nice. Otherwise, if the command fails with a disk full or I/O error, you're left with an incomplete file, and you might have missed the failure and/or you might then forget to rerun it, which leaves you to figure out why things don't work. Not a terribly big deal in this case; since it's version-controlled, you'll probably notice the diffs pretty quickly. Oh, also, since you've added the new rpcgen target, please declare it phony to GNU make with this: .PHONY: rpcgen otherwise, if you run make -t (or otherwise create a file name rpcgen), that rule will do nothing. -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix GCC hard-coding in python/
Daniel P. Berrange berra...@redhat.com wrote: On Wed, Dec 17, 2008 at 09:57:23AM +0100, Jim Meyering wrote: john.le...@sun.com wrote: Fix GCC hard-coding in python/ ... diff --git a/acinclude.m4 b/acinclude.m4 --- a/acinclude.m4 +++ b/acinclude.m4 @@ -75,6 +75,11 @@ AC_DEFUN([LIBVIRT_COMPILE_WARNINGS],[ WARN_CFLAGS=$COMPILER_FLAGS $complCFLAGS AC_SUBST(WARN_CFLAGS) + +COMPILER_FLAGS= +gl_COMPILER_FLAGS(-Wno-redundant-decls) +NO_RDECLS_CFLAGS=$COMPILER_FLAGS +AC_SUBST(NO_RDECLS_CFLAGS) ]) diff --git a/python/Makefile.am b/python/Makefile.am --- a/python/Makefile.am +++ b/python/Makefile.am @@ -35,7 +35,7 @@ python_LTLIBRARIES = libvirtmod.la libvirtmod_la_SOURCES = libvir.c types.c libvirt-py.c libvirt-py.h # Python header files contain a redundant decl, hence: -libvirtmod_la_CFLAGS = -Wno-redundant-decls +libvirtmod_la_CFLAGS = @NO_RDECLS_CFLAGS@ That -Wno-redundant-decls option is no longer necessary, at least on rawhide. When I remove it, compiling with make -C python CFLAGS='-Wredundant-decls -Werror' still succeeds. What about with older RHEL-5 python though ? Python 2.4 had rather a large number of flaws in its public header file that have caused plenty of compile warning problems in past. It's no trouble for me either way. I find that it is a losing battle to try to enable aggressive compiler warnings on older systems, but don't feel strongly about this particular case. I mentioned it because there are new gcc-warning-related macros in gnulib, and it'd be slightly easier to use them if the ones libvirt uses don't have tendrils reaching into relatively distant Makefile.am files. -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix register/deregister driver method declaration
On Tue, Dec 16, 2008 at 06:26:46PM -0800, john.le...@sun.com wrote: # HG changeset patch # User john.le...@sun.com # Date 1229399267 28800 # Node ID 357ed6194907962940e31382fe988f3b64bd1968 # Parent 1d913a11d0b4df41c6a749ea34a5a2f37cb2c903 Fix register/deregister driver method declaration Callback parameter should be of type virConnectDomainEventCallback. ACK. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- 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
Re: [libvirt] proposal: allow use of bool: HACKING: discuss C types
Daniel P. Berrange berra...@redhat.com wrote: On Fri, Dec 12, 2008 at 09:58:32AM +0100, Jim Meyering wrote: I propose to allow (encourage, even) the use of the standard C99 type, bool, in libvirt, but not in public interfaces[1]. There are already uses in cgroup.c and xmlrpc.c, as well as those in the gnulib c-ctype.h header which is included from many of libvirt's .c files. The motivation is to make the code as readable as possible. When you see the declaration of an int variable, member, or function, that type gives you no clue whether it is used as a boolean. If you see a few uses that treat it as boolean, that's still no guarantee, and it may be non-trivial to ensure that there isn't some non-boolean value with a special meaning. However, if you see a bool variable, you do have that guarantee. I don't particularly like the idea of using the bool type - No system header files use it Of course. Can't use it there for the same reason that we wouldn't use it in libvirt's own public interfaces, as I explained. But public interfaces make up such a small fraction of the code in question that it'd be a shame to impose its restrictions on all the rest. - Library header files which use a boolean type have nearly all defined their own custom bool types, not using stdbool.h and there's no guarentee that stdbool's idea of 'true' matches the other apps' This is the same fundamental restriction. bool cannot be used in public headers. - We don't use it in the public API, or on the wire for the remote protocol, since it has undefined size. Same point. - The GNULIB bool emulation is unable to provide equivlance between C's idea of true (any non-zero value) and the defined 'true' constant (whose value is 1) That sounds scary, but isn't an issue in practice. The uses of true and false are typically only for initialization. IMHO, one should never compare a bool-declared variable to true or false. - Bitfields are more size efficient than bools. It's not about size, but rather readability. In a language like python where there's a fundamental builtin type in the language for 'bool' this it makes sense. In the C world with many many[1] different definitions of bool, true and false it just feels The number of definitions is because people find it useful to expose the semantics of the type, yet realize that they cannot use that precise name in a header file. like a world of hurt. The only thing you can ultimately rely on is that a true value is non-zero. There is no hurt in my experience. It's used extensively in gnulib-using projects, and -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH 0/5: connection cloning support (WIP)
On Wed, 2008-12-17 at 13:58 +, Daniel P. Berrange wrote:
On Wed, Dec 17, 2008 at 07:44:15AM -0500, David Lively wrote:
Hi Daniel -
When I apply these patches, I'm seeing segfaults on event delivery
when just running the existing synchronous
examples/domain-events/events-c/event-test.c (using the remote driver).
I've not come across that specific problem, but there are a definitely
some locking bugs refcounting bugs inthe patches I've posted so far.
I'll post an updated series of patches which may address this.
Daniel
It turned out to be a double-free. Trivial fix below.
Dave
diff --git a/src/remote_internal.c b/src/remote_internal.c
index 9245a2a..d6b94ff 100644
--- a/src/remote_internal.c
+++ b/src/remote_internal.c
@@ -6423,10 +6423,10 @@ remoteDomainQueueEvent(virConnectPtr conn, XDR
*xdr)
return;
if (virDomainEventQueuePush(priv-domainEvents,
-event) 0)
+event) 0) {
DEBUG0(Error adding event to queue);
-
-virDomainEventFree(event);
+virDomainEventFree(event);
+}
}
/** remoteDomainEventFired:
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] __FUNCTION__ is not portable
On Tue, Dec 16, 2008 at 06:50:44PM -0800, john.le...@sun.com wrote: # HG changeset patch # User john.le...@sun.com # Date 1229399267 28800 # Node ID ee6ccfc062f49f514d7ae8924d5a596b18441fee # Parent b105e5a7cd6190e0b952a1587d7e80f39d02e63c __FUNCTION__ is not portable __func__ is. Some places aren't covered by the back-compat define (why is that there?) Not sure why we added that. Seems reasonable to just use __func__ everywhere since that has broader compiler support Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- 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
Re: [libvirt] [PATCH] 6/8 Convert qemud to the logging infrastructure
On Wed, Dec 17, 2008 at 04:28:04PM +, Daniel P. Berrange wrote: On Wed, Dec 17, 2008 at 04:26:25PM +0100, Daniel Veillard wrote: This is a bit more massive because qemud already had a limited logging primitive, so we remove the old one, put the new one, initialize the logging at the beginning of remoteReadConfigFile (maybe that should be renamed, I think it's launched in all cases). The painful part is to convert the whole set of existing log calls to the new macro, ACK. I just noticed I forgot to take into account --verbose, so in qemudSetLogging I will also add if ((verbose) (log_level = VIR_LOG_WARN)) log_level = VIR_LOG_INFO; to allow the command line option to override the default level. Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] libvir: error could not connect to qemu://localhost/system
I downloaded libvirt on December 14 and did the installation procedure (autogen, make, make install). I installed it on host which worked fine with a previous version. However, now the libvirtd is not accepting remote connections. What has changed in this version? Am I doing something wrong? Details: I brought up libvirtd in listen mode: [r...@arenal kvm1]# ps -ef | grep libvirtd root 1443 803 0 15:34 pts/300:00:00 grep libvirtd? root 32258 5093 0 15:12 pts/100:00:00 libvirtd --listen I ran virsh and it works fine as long as the connection is not remote. virsh # version Compiled against library: libvir 0.5.1 Using library: libvir 0.5.1 Using API: QEMU 0.5.1 Running hypervisor: QEMU 0.9.0 virsh # connect test:///default virsh # connect qemu:///system I then tried to connect to itself (the host has credentials for both client and host). But it fails: virsh # connect qemu://arenal/system libvir: error : could not connect to qemu://arenal/system error: Failed to connect to the hypervisor virsh # connect test://arenal/default libvir: error : could not connect to test://arenal/default error: Failed to connect to the hypervisor Kenneth Nagin -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Re: [PATCH] portability: don't include endian.h
On Wed, Dec 17, 2008 at 01:55:59PM +0100, Jim Meyering wrote: Adding the inclusions of string.h and mntent.h look fine, They were just moved. but rather than testing for endian.h, it seems we can do away with it altogether. Even the __LITTLE_ENDIAN and __BIG_ENDIAN constants aren't really required. Rather, all you need is Fine by me, if you prefer it this way. regards john -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] __FUNCTION__ is not portable
Daniel P. Berrange berra...@redhat.com wrote: On Tue, Dec 16, 2008 at 06:50:44PM -0800, john.le...@sun.com wrote: # HG changeset patch # User john.le...@sun.com # Date 1229399267 28800 # Node ID ee6ccfc062f49f514d7ae8924d5a596b18441fee # Parent b105e5a7cd6190e0b952a1587d7e80f39d02e63c __FUNCTION__ is not portable __func__ is. Some places aren't covered by the back-compat define (why is that there?) Not sure why we added that. Seems reasonable to just use __func__ everywhere since that has broader compiler support ACK from me, too. Along with that, can you add a syntax-check rule to Makefile.maint that will ensure that no uses of __FUNCTION__ sneak back in? -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] portability: don't include endian.h
john.le...@sun.com wrote:
...
diff --git a/configure.in b/configure.in
--- a/configure.in
+++ b/configure.in
@@ -75,7 +75,7 @@ AC_CHECK_FUNCS([cfmakeraw regexec uname
AC_CHECK_FUNCS([cfmakeraw regexec uname sched_getaffinity getuid getgid])
dnl Availability of various common headers (non-fatal if missing).
-AC_CHECK_HEADERS([pwd.h paths.h regex.h sys/syslimits.h sys/utsname.h
sys/wait.h winsock2.h sched.h termios.h sys/poll.h])
+AC_CHECK_HEADERS([pwd.h paths.h regex.h sys/syslimits.h sys/utsname.h
sys/wait.h winsock2.h sched.h termios.h sys/poll.h endian.h])
dnl Where are the XDR functions?
dnl If portablexdr is installed, prefer that.
diff --git a/src/storage_backend_fs.c b/src/storage_backend_fs.c
--- a/src/storage_backend_fs.c
+++ b/src/storage_backend_fs.c
@@ -31,10 +31,14 @@
#include errno.h
#include fcntl.h
#include unistd.h
+#include string.h
+
+#ifdef HAVE_ENDIAN_H
#include endian.h
-#include byteswap.h
-#include mntent.h
-#include string.h
+#else
+#define __LITTLE_ENDIAN 1234
+#define __BIG_ENDIAN 4321
+#endif
#include libxml/parser.h
#include libxml/tree.h
@@ -240,6 +244,9 @@ static int virStorageBackendProbeFile(vi
}
#if WITH_STORAGE_FS
+
+#include mntent.h
Adding the inclusions of string.h and mntent.h look fine,
but rather than testing for endian.h, it seems we can do away with
it altogether. Even the __LITTLE_ENDIAN and __BIG_ENDIAN
constants aren't really required. Rather, all you need is
an indication (dare I say boolean?) of whether a fileTypeInfo
entry is big- or little-endian. However, even though the
current code treats this field as effectively boolean
(testing a single value and then else for the rest),
we can leave it as a wider enum.
I propose this:
From 65074e8d636a2eae10ceacf2619a6be77f45ee2a Mon Sep 17 00:00:00 2001
From: Jim Meyering meyer...@redhat.com
Date: Wed, 17 Dec 2008 13:54:04 +0100
Subject: [PATCH] portability: don't include endian.h
* src/storage_backend_fs.c: Don't include endian.h: not needed.
(LV_BIG_ENDIAN, LV_LITTLE_ENDIAN): Define.
Use those instead of __BIG_ENDIAN and __LITTLE_ENDIAN.
---
src/storage_backend_fs.c | 29 -
1 files changed, 16 insertions(+), 13 deletions(-)
diff --git a/src/storage_backend_fs.c b/src/storage_backend_fs.c
index 91131e5..8bc77fc 100644
--- a/src/storage_backend_fs.c
+++ b/src/storage_backend_fs.c
@@ -31,7 +31,6 @@
#include errno.h
#include fcntl.h
#include unistd.h
-#include endian.h
#include byteswap.h
#include mntent.h
#include string.h
@@ -47,6 +46,10 @@
#include memory.h
#include xml.h
+enum lv_endian {
+LV_LITTLE_ENDIAN = 1, /* 1234 */
+LV_BIG_ENDIAN /* 4321 */
+};
/* Either 'magic' or 'extension' *must* be provided */
struct FileTypeInfo {
@@ -54,7 +57,7 @@ struct FileTypeInfo {
const char *magic; /* Optional string of file magic
* to check at head of file */
const char *extension; /* Optional file extension to check */
-int endian; /* Endianness of file format */
+enum lv_endian endian; /* Endianness of file format */
int versionOffset;/* Byte offset from start of file
* where we find version number,
* -1 to skip version test */
@@ -69,16 +72,16 @@ const struct FileTypeInfo const fileTypeInfo[] = {
/* Bochs */
/* XXX Untested
{ VIR_STORAGE_VOL_BOCHS, Bochs Virtual HD Image, NULL,
- __LITTLE_ENDIAN, 64, 0x2,
+ LV_LITTLE_ENDIAN, 64, 0x2,
32+16+16+4+4+4+4+4, 8, 1 },*/
/* CLoop */
/* XXX Untested
{ VIR_STORAGE_VOL_CLOOP, #!/bin/sh\n#V2.0 Format\nmodprobe cloop file=$0
mount -r -t iso9660 /dev/cloop $1\n, NULL,
- __LITTLE_ENDIAN, -1, 0,
+ LV_LITTLE_ENDIAN, -1, 0,
-1, 0, 0 }, */
/* Cow */
{ VIR_STORAGE_VOL_FILE_COW, OOOM, NULL,
- __BIG_ENDIAN, 4, 2,
+ LV_BIG_ENDIAN, 4, 2,
4+4+1024+4, 8, 1 },
/* DMG */
/* XXX QEMU says there's no magic for dmg, but we should check... */
@@ -92,31 +95,31 @@ const struct FileTypeInfo const fileTypeInfo[] = {
/* Parallels */
/* XXX Untested
{ VIR_STORAGE_VOL_FILE_PARALLELS, WithoutFreeSpace, NULL,
- __LITTLE_ENDIAN, 16, 2,
+ LV_LITTLE_ENDIAN, 16, 2,
16+4+4+4+4, 4, 512 },
*/
/* QCow */
{ VIR_STORAGE_VOL_FILE_QCOW, QFI, NULL,
- __BIG_ENDIAN, 4, 1,
+ LV_BIG_ENDIAN, 4, 1,
4+4+8+4+4, 8, 1 },
/* QCow 2 */
{ VIR_STORAGE_VOL_FILE_QCOW2, QFI, NULL,
- __BIG_ENDIAN, 4, 2,
+ LV_BIG_ENDIAN, 4, 2,
4+4+8+4+4, 8, 1 },
/* VMDK 3 */
/* XXX Untested
{ VIR_STORAGE_VOL_FILE_VMDK, COWD, NULL,
- __LITTLE_ENDIAN, 4, 1,
+ LV_LITTLE_ENDIAN, 4, 1,
4+4+4, 4, 512 },
*/
/* VMDK 4 */
{ VIR_STORAGE_VOL_FILE_VMDK, KDMV, NULL,
- __LITTLE_ENDIAN, 4, 1,
+ LV_LITTLE_ENDIAN, 4, 1,
4+4+4, 8, 512 },
/* Connectix / VirtualPC */
/* XXX
Re: [libvirt] [PATCH] Fix pool define crash
Cole Robinson wrote:
There's a null dereference in the storage driver when defining a pool.
Attached patch fixes it for me.
diff --git a/src/storage_driver.c b/src/storage_driver.c
index 2432a9a..ac5e443 100644
--- a/src/storage_driver.c
+++ b/src/storage_driver.c
@@ -546,7 +546,7 @@ storagePoolDefine(virConnectPtr conn,
goto cleanup;
def = NULL;
-if (virStoragePoolObjSaveDef(conn, driver, pool, def) 0) {
+if (virStoragePoolObjSaveDef(conn, driver, pool, pool-def) 0) {
virStoragePoolObjRemove(driver-pools, pool);
goto cleanup;
}
Hm, I definitely see what you are getting at, and why the crash is there, but
I'm not sure this is totally correct. virStoragePoolObjAssignDef() only assigns
this def to pool-def iff the storage pool is not running. That means that if
the pool is running, and we make this change, for running pools you would always
get the old def, not the updated one. I think we need to move the def = NULL
further down, but that of course will require other changes so we still have the
unified cleanup target.
--
Chris Lalancette
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] 1/8 detect syslog.h
On Wed, Dec 17, 2008 at 04:08:48PM +0100, Daniel Veillard wrote: the ability to syslog gets into libvirt.c so we need to detect its presence at configure time to compile it out on platform without support, ACK Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- 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
Re: [libvirt] [PATCH] don't assume builddir == srcdir
Daniel P. Berrange berra...@redhat.com wrote: On Sat, Dec 13, 2008 at 12:23:30AM +0100, Guido G?nther wrote: Hi, Makefile.maint assumes this in some places. This doesn't make the world perfect but maybe a bit better? Seems reasonable to me. I think it is about time I changed the 'autobuild.sh' to do a VPATH build in the nightly builds, so we don't keep making regressions for the srcdir != builddir scenario FYI, running make distcheck does such a build. -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] let gcc's -Wformat do its job; avoid make syntax-check failure
Daniel Veillard veill...@redhat.com wrote: On Wed, Dec 17, 2008 at 09:01:24AM +0100, Jim Meyering wrote: I first noticed that make syntax-check failed. Then I saw that virAsprintf's prototype lacked ATTRIBUTE_FORMAT. This fixes both and updates HACKING. +1 I hit some of those too, Thanks. committed -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] proposal: allow use of bool: HACKING: discuss C types
On Fri, Dec 12, 2008 at 09:58:32AM +0100, Jim Meyering wrote:
Hello,
I don't want to make waves, but I do care about certain aspects
of code quality, so...
I propose to allow (encourage, even) the use of the standard C99 type,
bool, in libvirt, but not in public interfaces[1]. There are already
uses in cgroup.c and xmlrpc.c, as well as those in the gnulib c-ctype.h
header which is included from many of libvirt's .c files.
The motivation is to make the code as readable as possible.
When you see the declaration of an int variable, member, or function,
that type gives you no clue whether it is used as a boolean. If you
see a few uses that treat it as boolean, that's still no guarantee,
and it may be non-trivial to ensure that there isn't some non-boolean
value with a special meaning.
However, if you see a bool variable, you do have that guarantee.
I don't particularly like the idea of using the bool type
- No system header files use it
- Library header files which use a boolean type have nearly all defined
their own custom bool types, not using stdbool.h and there's no guarentee
that stdbool's idea of 'true' matches the other apps'
- We don't use it in the public API, or on the wire for the remote
protocol, since it has undefined size.
- The GNULIB bool emulation is unable to provide equivlance between
C's idea of true (any non-zero value) and the defined 'true'
constant (whose value is 1)
- Bitfields are more size efficient than bools.
In a language like python where there's a fundamental builtin type in
the language for 'bool' this it makes sense. In the C world with many
many[1] different definitions of bool, true and false it just feels
like a world of hurt. The only thing you can ultimately rely on is that
a true value is non-zero.
Daniel
[1] Picking a *tiny* selection of the bool's in my dev box headers
/usr/include/xulrunner-sdk-1.9/stable/jritypes.h:typedef enum JRIBoolean {
/usr/include/xulrunner-sdk-1.9/stable/nptypes.h:typedef int bool;
/usr/include/xulrunner-sdk-1.9/stable/npapi.h:typedef unsigned char NPBool;
/usr/include/xulrunner-sdk-1.9/stable/jri_md.h:typedef unsigned charjbool;
/usr/include/wine/windows/wtypes.idl:typedef long BOOL;
/usr/include/cups/raster.h:typedef enum cups_bool_e / Boolean
type /
/usr/include/lcms.h:typedef int LCMSBOOL;
/usr/include/mp4.h:typedef int bool;
/usr/include/tss/tpm.h:typedef BYTE TPM_BOOL;
/usr/include/tss/platform.h: typedef int8_t TSS_BOOL;
/usr/include/tss/platform.h: typedef signed char TSS_BOOL;
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- 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
Re: [libvirt] [PATCH] Fix pool define crash
Cole Robinson crobi...@redhat.com wrote:
There's a null dereference in the storage driver when defining a pool.
Attached patch fixes it for me.
Thanks,
Cole
diff --git a/src/storage_driver.c b/src/storage_driver.c
index 2432a9a..ac5e443 100644
--- a/src/storage_driver.c
+++ b/src/storage_driver.c
@@ -546,7 +546,7 @@ storagePoolDefine(virConnectPtr conn,
goto cleanup;
def = NULL;
-if (virStoragePoolObjSaveDef(conn, driver, pool, def) 0) {
+if (virStoragePoolObjSaveDef(conn, driver, pool, pool-def) 0) {
virStoragePoolObjRemove(driver-pools, pool);
goto cleanup;
}
Looks right, and passes this test:
qemud/libvirtd
sleep 1
src/virsh --connect qemu:///session pool-define-as b dir c d e /f j
src/virsh --connect qemu:///session pool-dumpxml b
Whereas before the patch, running pool-define-as would
cause libvirtd to segfault.
So ACK.
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix pool define crash
Chris Lalancette wrote:
Cole Robinson wrote:
There's a null dereference in the storage driver when defining a pool.
Attached patch fixes it for me.
diff --git a/src/storage_driver.c b/src/storage_driver.c
index 2432a9a..ac5e443 100644
--- a/src/storage_driver.c
+++ b/src/storage_driver.c
@@ -546,7 +546,7 @@ storagePoolDefine(virConnectPtr conn,
goto cleanup;
def = NULL;
-if (virStoragePoolObjSaveDef(conn, driver, pool, def) 0) {
+if (virStoragePoolObjSaveDef(conn, driver, pool, pool-def) 0) {
virStoragePoolObjRemove(driver-pools, pool);
goto cleanup;
}
Hm, I definitely see what you are getting at, and why the crash is there, but
I'm not sure this is totally correct. virStoragePoolObjAssignDef() only
assigns
this def to pool-def iff the storage pool is not running. That means that if
the pool is running, and we make this change, for running pools you would
always
get the old def, not the updated one. I think we need to move the def =
NULL
further down, but that of course will require other changes so we still have
the
unified cleanup target.
Ahh, I didn't realize the AssignDef caveat. Thanks for pointing that
out. Something like this should work then?
- Cole
diff --git a/src/storage_driver.c b/src/storage_driver.c
index 2432a9a..330317c 100644
--- a/src/storage_driver.c
+++ b/src/storage_driver.c
@@ -544,12 +544,13 @@ storagePoolDefine(virConnectPtr conn,
if (!(pool = virStoragePoolObjAssignDef(conn, driver-pools, def)))
goto cleanup;
-def = NULL;
if (virStoragePoolObjSaveDef(conn, driver, pool, def) 0) {
virStoragePoolObjRemove(driver-pools, pool);
+def = NULL;
goto cleanup;
}
+def = NULL;
ret = virGetStoragePool(conn, pool-def-name, pool-def-uuid);
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix compiler error when !HAVE_POLKIT
john.le...@sun.com wrote: # HG changeset patch # User john.le...@sun.com # Date 1229399439 28800 # Node ID f622c9de5e2488de8ae16bc7a0b50d8fa76af364 # Parent db36391b739c117f5887388f65f31e6a9d2d361b Fix compiler error when !HAVE_POLKIT ACK, but the title is misleading. s/error/warning/ Signed-off-by: John Levon john.le...@sun.com diff --git a/qemud/remote.c b/qemud/remote.c --- a/qemud/remote.c +++ b/qemud/remote.c @@ -3116,7 +3116,7 @@ static int static int remoteDispatchAuthPolkit (struct qemud_server *server ATTRIBUTE_UNUSED, struct qemud_client *client ATTRIBUTE_UNUSED, - virConnectPtr conn, + virConnectPtr conn ATTRIBUTE_UNUSED, -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix compiler warnings in test code
john.le...@sun.com wrote: # HG changeset patch # User john.le...@sun.com # Date 1229399267 28800 # Node ID 1b81ac255ad765ab782829e7fe0a55dc7bab72ab # Parent ee6ccfc062f49f514d7ae8924d5a596b18441fee Fix compiler warnings in test code I suppose the latter was a warning from Sun's cc about code after the exit(77) being unreachable. ACK. diff --git a/tests/nodeinfotest.c b/tests/nodeinfotest.c ... -#ifndef __linux__ -exit (77); /* means 'test skipped' for automake */ -#endif abs_srcdir = getenv(abs_srcdir); if (!abs_srcdir) @@ -102,4 +109,7 @@ mymain(int argc, char **argv) return(ret==0 ? EXIT_SUCCESS : EXIT_FAILURE); } ... -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] rpcgen fixes
On Wed, Dec 17, 2008 at 02:24:29PM +0100, Jim Meyering wrote: This sounds like an argument for not putting derived sources in CVS, since derived-for-one-system (linux/gnu) doesn't work for e.g., Solaris. If you're OK with requiring rpcgen, that's most likely fine by me (though you'd want to tighten up that glibc rpcgen check!) + rpcgen -h -o rp.h-t @top_srcdir@/qemud/remote_protocol.x + rpcgen -c -o rp.c-t @top_srcdir@/qemud/remote_protocol.x Use $(srcdir), in place of @top_srcdir@/qemud (and others below). The latter @var@ syntax is old, if not deprecated, Coo, been a while - will do. + perl -w @top_srcdir@/qemud/rpcgen_fix.pl rp.h-t \ + @top_srcdir@/qemud/remote_protocol.h Please don't redirect directly to something used as a build source. Instead, redirect to a temporary file, and rename that into place upon successful completion. That makes it less likely you'll ever end up with corrupted sources. Sorry I'm missing why this helps any? regards john -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH 0/5: connection cloning support (WIP)
On Wed, Dec 17, 2008 at 01:24:11PM -0500, David Lively wrote: Ok, with that fix, my asynchronous Java event implementation seems to be working (though not fully tested) with these patches. But I'm not using connection cloning (yet). It's your concurrency control in the remote driver that makes this work. That's good, but i just threw away the connection cloning patch. We're going to make asingle virConnectPtr object properly threadsafe. This will also mean you don't need 'synchronized' annotations on any java APis On the Java side, domain event callbacks from another thread work because of the (Java) locking of Connect objects. I don't think I should remove that, since libvirt still prohibits concurrent use of the same virConnect object (right?) Until 0.6.0.. I suppose I could clone the virConnect object before delivering an event to a Java Domain (so the Domain's Connect would wrap the clone). I don't think this does much in the remote case (since the underlying RPC pipe it still shared, albeit safely). But I suppose it might allow greater concurrency in the non-remote cases. Is this what you had in mind? I vote for not support events in another thread in Java until 0.6.0, when we can remove al the limitations on thread usage. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- 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
Re: [libvirt] [PATCH] Fix pool define crash
Cole Robinson wrote:
Ahh, I didn't realize the AssignDef caveat. Thanks for pointing that
out. Something like this should work then?
diff --git a/src/storage_driver.c b/src/storage_driver.c
index 2432a9a..330317c 100644
--- a/src/storage_driver.c
+++ b/src/storage_driver.c
@@ -544,12 +544,13 @@ storagePoolDefine(virConnectPtr conn,
if (!(pool = virStoragePoolObjAssignDef(conn, driver-pools, def)))
goto cleanup;
-def = NULL;
if (virStoragePoolObjSaveDef(conn, driver, pool, def) 0) {
virStoragePoolObjRemove(driver-pools, pool);
+def = NULL;
goto cleanup;
}
+def = NULL;
ret = virGetStoragePool(conn, pool-def-name, pool-def-uuid);
Yeah, that looks better. Seems to be good to me, so ACK.
--
Chris Lalancette
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH 0/5: connection cloning support (WIP)
On Wed, Dec 17, 2008 at 07:44:15AM -0500, David Lively wrote: Hi Daniel - When I apply these patches, I'm seeing segfaults on event delivery when just running the existing synchronous examples/domain-events/events-c/event-test.c (using the remote driver). I've added a little debug. Apparently event-name is being NULLed out sometime after the event is put on the queue: DEBUG: remote_internal.c: remoteDomainEventFired (Event fired 0 3 1 1) DEBUG: remote_internal.c: processCallRecv (Do 4 0) DEBUG: remote_internal.c: processCallRecvLen (Got length, now need 64 total (60 more)) DEBUG: remote_internal.c: processCallRecv (Do 64 4) DEBUG: remote_internal.c: processCallAsyncEvent (Encountered an event while waiting for a response) DEBUG: remote_internal.c: get_nonnull_domain (domain.name: dsl) DEBUG: datatypes.c: virGetDomain (New hash entry 0x804c728) DEBUG: domain_event.c: virDomainEventNew (event: 0x804c770 -name: dsl) DEBUG: libvirt.c: virDomainFree (domain=0x804c728) DEBUG: datatypes.c: virUnrefDomain (unref domain 0x804c728 dsl 1) DEBUG: datatypes.c: virReleaseDomain (release domain 0x804c728 dsl) DEBUG: datatypes.c: virReleaseDomain (unref connection 0x804b040 4) DEBUG: domain_event.c: virDomainEventQueuePush (event: 0x804c770 -name: dsl) DEBUG: remote_internal.c: processCallRecv (Do 0 0) DEBUG: remote_internal.c: remoteDomainEventQueueFlush () DEBUG: domain_event.c: virDomainEventDispatchDefaultFunc (event: 0x804c770 -name: (null)) virGetDomain: name is NULL Segmentation fault I'll continue looking into it. But please let me know if you're familiar with the problem ... I've not come across that specific problem, but there are a definitely some locking bugs refcounting bugs inthe patches I've posted so far. I'll post an updated series of patches which may address this. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- 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
[libvirt] [PATCH] 7/8 conver the remaining modules from qemud
Rename of the macros where available, or remapping of the qemudLog
to the new macros.
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
Index: qemud/event.c
===
RCS file: /data/cvs/libxen/qemud/event.c,v
retrieving revision 1.16
diff -u -r1.16 event.c
--- qemud/event.c 4 Dec 2008 22:14:15 - 1.16
+++ qemud/event.c 17 Dec 2008 14:25:56 -
@@ -35,7 +35,7 @@
#include memory.h
#include util.h
-#define EVENT_DEBUG(fmt, ...) qemudDebug(EVENT: fmt, __VA_ARGS__)
+#define EVENT_DEBUG(fmt, ...) VIR_DEBUG(event, fmt, __VA_ARGS__)
static int virEventInterruptLocked(void);
Index: qemud/mdns.c
===
RCS file: /data/cvs/libxen/qemud/mdns.c,v
retrieving revision 1.12
diff -u -r1.12 mdns.c
--- qemud/mdns.c19 Nov 2008 16:24:01 - 1.12
+++ qemud/mdns.c17 Dec 2008 14:25:56 -
@@ -44,7 +44,7 @@
#include remote_internal.h
#include memory.h
-#define AVAHI_DEBUG(fmt, ...) qemudDebug(AVAHI: fmt, __VA_ARGS__)
+#define AVAHI_DEBUG(fmt, ...) VIR_DEBUG(avahi, fmt, __VA_ARGS__)
struct libvirtd_mdns_entry {
char *type;
Index: qemud/remote.c
===
RCS file: /data/cvs/libxen/qemud/remote.c,v
retrieving revision 1.52
diff -u -r1.52 remote.c
--- qemud/remote.c 4 Dec 2008 22:16:40 - 1.52
+++ qemud/remote.c 17 Dec 2008 14:25:57 -
@@ -53,7 +53,7 @@
#include qemud.h
#include memory.h
-#define REMOTE_DEBUG(fmt,...) qemudDebug(REMOTE: fmt, __VA_ARGS__)
+#define REMOTE_DEBUG(fmt, ...) VIR_DEBUG(remote, fmt, __VA_ARGS__)
static void remoteDispatchFormatError (remote_error *rerr,
const char *fmt, ...)
@@ -2543,7 +2543,7 @@
REMOTE_DEBUG(Initialize SASL auth %d, client-fd);
if (client-auth != REMOTE_AUTH_SASL ||
client-saslconn != NULL) {
-qemudLog(QEMUD_ERR, %s, _(client tried invalid SASL init request));
+ERROR0(_(client tried invalid SASL init request));
goto authfail;
}
@@ -2583,7 +2583,7 @@
VIR_FREE(localAddr);
VIR_FREE(remoteAddr);
if (err != SASL_OK) {
-qemudLog(QEMUD_ERR, _(sasl context setup failed %d (%s)),
+ERROR(_(sasl context setup failed %d (%s)),
err, sasl_errstring(err, NULL, NULL));
client-saslconn = NULL;
goto authfail;
@@ -2596,7 +2596,7 @@
cipher = gnutls_cipher_get(client-tlssession);
if (!(ssf = (sasl_ssf_t)gnutls_cipher_get_key_size(cipher))) {
-qemudLog(QEMUD_ERR, %s, _(cannot TLS get cipher size));
+ERROR0(_(cannot TLS get cipher size));
sasl_dispose(client-saslconn);
client-saslconn = NULL;
goto authfail;
@@ -2605,7 +2605,7 @@
err = sasl_setprop(client-saslconn, SASL_SSF_EXTERNAL, ssf);
if (err != SASL_OK) {
-qemudLog(QEMUD_ERR, _(cannot set SASL external SSF %d (%s)),
+ERROR(_(cannot set SASL external SSF %d (%s)),
err, sasl_errstring(err, NULL, NULL));
sasl_dispose(client-saslconn);
client-saslconn = NULL;
@@ -2633,7 +2633,7 @@
err = sasl_setprop(client-saslconn, SASL_SEC_PROPS, secprops);
if (err != SASL_OK) {
-qemudLog(QEMUD_ERR, _(cannot set SASL security props %d (%s)),
+ERROR(_(cannot set SASL security props %d (%s)),
err, sasl_errstring(err, NULL, NULL));
sasl_dispose(client-saslconn);
client-saslconn = NULL;
@@ -2649,7 +2649,7 @@
NULL,
NULL);
if (err != SASL_OK) {
-qemudLog(QEMUD_ERR, _(cannot list SASL mechanisms %d (%s)),
+ERROR(_(cannot list SASL mechanisms %d (%s)),
err, sasl_errdetail(client-saslconn));
sasl_dispose(client-saslconn);
client-saslconn = NULL;
@@ -2658,7 +2658,7 @@
REMOTE_DEBUG(Available mechanisms for client: '%s', mechlist);
ret-mechlist = strdup(mechlist);
if (!ret-mechlist) {
-qemudLog(QEMUD_ERR, %s, _(cannot allocate mechlist));
+ERROR0(_(cannot allocate mechlist));
sasl_dispose(client-saslconn);
client-saslconn = NULL;
goto authfail;
@@ -2689,7 +2689,7 @@
err = sasl_getprop(client-saslconn, SASL_SSF, val);
if (err != SASL_OK) {
-qemudLog(QEMUD_ERR, _(cannot query SASL ssf on connection %d (%s)),
+ERROR(_(cannot query SASL ssf on connection %d (%s)),
err, sasl_errstring(err, NULL, NULL));
remoteDispatchAuthError(rerr);
sasl_dispose(client-saslconn);
@@ -2699,7 +2699,7 @@
ssf = *(const int *)val;
Re: [libvirt] [PATCH] qemud compile fixes
On Tue, Dec 16, 2008 at 06:47:47PM -0800, john.le...@sun.com wrote: # HG changeset patch # User john.le...@sun.com # Date 1229399267 28800 # Node ID a6c317b5fe1b4081ccb40ba90af7d31298cf6b44 # Parent a8109ee7d505787957468ebd03b09c613e3372cf qemud compile fixes Let qemud/ compile with Sun cc. ACK, Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- 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
Re: [libvirt] [PATCH] 7/8 conver the remaining modules from qemud
On Wed, Dec 17, 2008 at 04:28:49PM +0100, Daniel Veillard wrote: Rename of the macros where available, or remapping of the qemudLog to the new macros. ACK Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- 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
Re: [libvirt] [PATCH] Fix pool define crash
On Wed, Dec 17, 2008 at 01:24:14PM -0500, Cole Robinson wrote: There's a null dereference in the storage driver when defining a pool. Attached patch fixes it for me. ACK Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- 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
Re: [libvirt] [PATCH] Improve compiler flag checking
On Tue, Dec 16, 2008 at 06:48:56PM -0800, john.le...@sun.com wrote: # HG changeset patch # User john.le...@sun.com # Date 1229399267 28800 # Node ID 1f7707a3d49b20397011897cdc04c347322ad0c5 # Parent a6c317b5fe1b4081ccb40ba90af7d31298cf6b44 Improve compiler flag checking Some compilers (including GCC) don't set the return value consistently if an erroneous option is passed on the command line. Account for that. This isn't quite correct. We do explicitly need to do a AC_TRY_LINK test, because on Debian there are flags which are accepted by the compiler OK, but will then fail to link. So we should keep the AC_TRY_LINK check, and upon success of that, then also do your additional new check for zero- length stderr output. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- 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
[libvirt] [PATCH] 0/8 logging infrastructure for libvirt[d]
the following set of patches implement the logging infrastructure
based on the discussion about it last month. It is based on the
following interfaces, and in a large part reflects the log4j principles.
4 level of logging priorities:
typedef enum {
VIR_LOG_DEBUG = 1,
VIR_LOG_INFO,
VIR_LOG_WARN,
VIR_LOG_ERROR,
} virLogPriority;
An output is a channel to output logging informations and a filter is a
rule to allow or not log information to flow through.
At any given time a set of output and filters can be defined though
a public API (but I didn't put them yet in libvirt.h
that's still confined in logging.h ATM)
/**
* virLogOutputFunc:
* @data: extra output logging data
* @category: the category for the message
* @priority: the priority for the message
* @msg: the message to log, preformatted and zero terminated
* @len: the lenght of the message in bytes without the terminating zero
*
* Callback function used to output messages
*
* Returns the number of bytes written or -1 in case of error
*/
typedef int (*virLogOutputFunc) (void *data, const char *category,
int priority, const char *str, int len);
/**
* virLogCloseFunc:
* @data: extra output logging data
*
* Callback function used to close a log output
*/
typedef void (*virLogCloseFunc) (void *data);
extern int virLogSetDefaultPriority(int priority);
extern int virLogDefineFilter(const char *match, int priority, int
flags);
extern int virLogDefineOutput(virLogOutputFunc f, virLogCloseFunc c,
void *data, int priority, int flags);
The default priority allows to set a default level of logging,
individual logs matching filters will follow the rules defined for the
filters, i.e. if the logged data is of a lower level than what the
first filter matching it then it is dropped.
The internal APIs defines a function to log a message:
extern void virLogMessage(const char *category, int priority, int flags,
const char *fmt, ...);
the category is in general the file name as defined by __FILE__
The matching is done against the category based on a substring,
for example xen would match all logs emitted by
xend_internal.c xen_inotify.c xen_internal.c xen_unified.c
Something more complex could be defined for example matching based on
regexp or based on the full message content and not just the category
but this gets heavier and could be done later using specific flags
in virLogDefineFilter.
The preferred user interface for setting the output and filters
is through environment variables for application linking to libvirt
LIBVIRT_DEBUG defines the default logging level from 1 to 4
LIBVIRT_LOG_FILTERS defines the set filters
LIBVIRT_LOG_OUTPUTS defines the set outputs
and though the configuration file libvirtd.conf for the daemon with
the corresponding values:
log_level
log_filters
log_outputs
The convenience internal functions virLogParseFilters and
virLogParseOutputs parse the format of the values for filters and
outputs, so they share the same format explained in libvirtd.conf:
--
# Logging filters:
# A filter allows to select a different logging level for a given
category
# of logs
# The format for a filter is:
#x:name
# where name is a match string e.g. remote or qemu
# the x prefix is the minimal level where matching messages should be
logged
#1: DEBUG
#2: INFO
#3: WARNING
#4: ERROR
#
# Multiple filter can be defined in a single @filters, they just need
to be
# separated by spaces.
#
# e.g:
# log_filters=3:remote 4:event
# to only get warning or errors from the remote layer and only errors
from
# the event layer.
# Logging outputs:
# An output is one of the places to save logging informations
# The format for an output can be:
#x:stderr
# output goes to stderr
#x:syslog:name
# use syslog for the output and use the given name as the ident
#x:file:file_path
# output to a file, with the given filepath
# In all case the x prefix is the minimal level, acting as a filter
#0: everything
#1: DEBUG
#2: INFO
#3: WARNING
#4: ERROR
#
# Multiple output can be defined , they just need to be separated by
spaces.
# e.g.:
# log_outputs=3:syslog:libvirtd
# to log all warnings and errors to syslog under the libvirtd ident
---
In practice
export LIBVIRT_DEBUG=1
export LIBVIRT_LOG_OUTPUTS=0:file:virsh.log
and then running virsh will accumulate all logging to the virsh.log
file in the current directory.
One thing which I feel is somewhat incomplete is that it's impossible
to remotely get debugging output from the libvirt daemon serving the
requests. Currently all logs are also accumulated in a cyclic logging
buffer, I would associate a dump function later to be hooked for example
to a signal handler in
Re: [libvirt] [PATCH] qemud compile fixes
john.le...@sun.com wrote: qemud compile fixes Let qemud/ compile with Sun cc. Signed-off-by: John Levon john.le...@sun.com diff --git a/qemud/qemud.c b/qemud/qemud.c --- a/qemud/qemud.c +++ b/qemud/qemud.c @@ -28,7 +28,6 @@ #include sys/stat.h #include unistd.h #include fcntl.h -#include paths.h #include limits.h #include sys/socket.h #include sys/un.h @@ -801,7 +800,8 @@ static struct qemud_server *qemudInitial #ifdef WITH_STORAGE_DIR storageRegister(); #endif -#if defined(HAVE_HAL) || defined(HAVE_DEVKIT) +#if defined(WITH_NODE_DEVICES) \ +(defined(HAVE_HAL) || defined(HAVE_DEVKIT)) nodedevRegister(); #endif #ifdef WITH_QEMU diff --git a/qemud/remote.c b/qemud/remote.c --- a/qemud/remote.c +++ b/qemud/remote.c @@ -27,7 +27,6 @@ #include sys/stat.h #include unistd.h #include fcntl.h -#include paths.h #include limits.h #include sys/socket.h #include sys/un.h ACK. Looks fine to me. -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix pool define crash
On Wed, Dec 17, 2008 at 09:00:11PM +0100, Chris Lalancette wrote:
Cole Robinson wrote:
There's a null dereference in the storage driver when defining a pool.
Attached patch fixes it for me.
diff --git a/src/storage_driver.c b/src/storage_driver.c
index 2432a9a..ac5e443 100644
--- a/src/storage_driver.c
+++ b/src/storage_driver.c
@@ -546,7 +546,7 @@ storagePoolDefine(virConnectPtr conn,
goto cleanup;
def = NULL;
-if (virStoragePoolObjSaveDef(conn, driver, pool, def) 0) {
+if (virStoragePoolObjSaveDef(conn, driver, pool, pool-def) 0) {
virStoragePoolObjRemove(driver-pools, pool);
goto cleanup;
}
Hm, I definitely see what you are getting at, and why the crash is there, but
I'm not sure this is totally correct. virStoragePoolObjAssignDef() only
assigns
this def to pool-def iff the storage pool is not running. That means that if
the pool is running, and we make this change, for running pools you would
always
get the old def, not the updated one. I think we need to move the def =
NULL
further down, but that of course will require other changes so we still have
the
unified cleanup target.
Opps, yes you are correct.
IMHO, the API contract here is the problem
Instead of being given a 'virStoragePoolDefPtr' instance, it should be
given the parent 'virStoragePoolObjPtr', along with a flag indicating
whether to save the live or inactive config.
That way the lookup of the correct 'def' is in one place, not in all
callers, and it makes the API contract clear
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- 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
Re: [libvirt] [PATHC] 8/8 extra settings in libvirtd.conf
On Wed, Dec 17, 2008 at 04:29:25PM +, Daniel P. Berrange wrote: On Wed, Dec 17, 2008 at 04:30:13PM +0100, Daniel Veillard wrote: Basically documents the 3 new configuration variables, Also need adding to the Augeas rules qemud/libvirtd.aug and qemud/test_libvirtd.aug, so that they're not lost when editing the config file using Augeas Oh right ... And general documentation under docs/ thanks for the quick feedback ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] pid_t portability in virExec()
On Tue, Dec 16, 2008 at 06:51:49PM -0800, john.le...@sun.com wrote: # HG changeset patch # User john.le...@sun.com # Date 1229399267 28800 # Node ID 020f8b8e9340287a6ab3d869b359e39b905cd0ff # Parent 1b81ac255ad765ab782829e7fe0a55dc7bab72ab pid_t portability in virExec() Use the right type for handling process IDs. ACK, we'd talked about fixing this several times in the past. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- 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
Re: [libvirt] [PATCH] Fix pool define crash
On Wed, Dec 17, 2008 at 09:26:51PM +0100, Chris Lalancette wrote:
Cole Robinson wrote:
Ahh, I didn't realize the AssignDef caveat. Thanks for pointing that
out. Something like this should work then?
diff --git a/src/storage_driver.c b/src/storage_driver.c
index 2432a9a..330317c 100644
--- a/src/storage_driver.c
+++ b/src/storage_driver.c
@@ -544,12 +544,13 @@ storagePoolDefine(virConnectPtr conn,
if (!(pool = virStoragePoolObjAssignDef(conn, driver-pools, def)))
goto cleanup;
-def = NULL;
if (virStoragePoolObjSaveDef(conn, driver, pool, def) 0) {
virStoragePoolObjRemove(driver-pools, pool);
+def = NULL;
goto cleanup;
}
+def = NULL;
ret = virGetStoragePool(conn, pool-def-name, pool-def-uuid);
Yeah, that looks better. Seems to be good to me, so ACK.
Functionally correct, so fine to commit, but I still think we should
change the virStoragePoolObjSaveDef method params, but not urgent unless
you want to change it now.
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- 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
Re: [libvirt] [PATCH] Portability fixes for directory storage backend
On Tue, Dec 16, 2008 at 06:28:44PM -0800, john.le...@sun.com wrote: # HG changeset patch # User john.le...@sun.com # Date 1229399267 28800 # Node ID 53a18080ea210168c044d7ade8dd8db0881d5c85 # Parent cd2fb266824178c4d63296b587eeedb2360f9f1c Portability fixes for directory storage backend Signed-off-by: John Levon john.le...@sun.com ACK. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- 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
Re: [libvirt] [PATCH] Fix compiler warnings in test code
On Tue, Dec 16, 2008 at 06:51:14PM -0800, john.le...@sun.com wrote:
# HG changeset patch
# User john.le...@sun.com
# Date 1229399267 28800
# Node ID 1b81ac255ad765ab782829e7fe0a55dc7bab72ab
# Parent ee6ccfc062f49f514d7ae8924d5a596b18441fee
Fix compiler warnings in test code
Signed-off-by: John Levon john.le...@sun.com
diff --git a/src/test.c b/src/test.c
--- a/src/test.c
+++ b/src/test.c
@@ -1299,7 +1299,7 @@ static unsigned long testGetMaxMemory(vi
static unsigned long testGetMaxMemory(virDomainPtr domain) {
testConnPtr privconn = domain-conn-privateData;
virDomainObjPtr privdom;
-unsigned long ret = -1;
+unsigned long ret = (unsigned long)-1;
The original code was actually bogus. It should have been
initialized to '0' as the error return value, rather than
the current '-1'.
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- 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
Re: [libvirt] [PATCH] Fix compiler warnings in test code
On Wed, Dec 17, 2008 at 02:36:44PM +0100, Jim Meyering wrote: I suppose the latter was a warning from Sun's cc about code after the exit(77) being unreachable. No, there was linux-only code before that point. regards john -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] 2/8 logging header and core implementation
This exports the internal function from logging.h (later the public
part should be moved to libvirt.h) augments the set of convenience
macros. libvirt_sym.version.in is also extended to export internally
all the header functions,
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
Index: src/logging.h
===
RCS file: /data/cvs/libxen/src/logging.h,v
retrieving revision 1.1
diff -u -r1.1 logging.h
--- src/logging.h 6 Nov 2008 16:36:07 - 1.1
+++ src/logging.h 17 Dec 2008 14:25:57 -
@@ -30,16 +30,87 @@
* defined at runtime of from the libvirt daemon configuration file
*/
#ifdef ENABLE_DEBUG
-extern int debugFlag;
#define VIR_DEBUG(category, fmt,...)\
-do { if (debugFlag) fprintf (stderr, DEBUG: %s: %s ( fmt )\n,
category, __func__, __VA_ARGS__); } while (0)
+virLogMessage(category, VIR_LOG_DEBUG, 0, fmt, __VA_ARGS__)
+#define VIR_INFO(category, fmt,...)\
+virLogMessage(category, VIR_LOG_INFO, 0, fmt, __VA_ARGS__)
+#define VIR_WARN(category, fmt,...)\
+virLogMessage(category, VIR_LOG_WARN, 0, fmt, __VA_ARGS__)
+#define VIR_ERROR(category, fmt,...)\
+virLogMessage(category, VIR_LOG_ERROR, 0, fmt, __VA_ARGS__)
#else
#define VIR_DEBUG(category, fmt,...) \
do { } while (0)
+#define VIR_INFO(category, fmt,...) \
+do { } while (0)
+#define VIR_WARN(category, fmt,...) \
+do { } while (0)
+#define VIR_ERROR(category, fmt,...) \
+do { } while (0)
#endif /* !ENABLE_DEBUG */
#define DEBUG(fmt,...) VIR_DEBUG(__FILE__, fmt, __VA_ARGS__)
#define DEBUG0(msg) VIR_DEBUG(__FILE__, %s, msg)
+#define INFO(fmt,...) VIR_INFO(__FILE__, fmt, __VA_ARGS__)
+#define INFO0(msg) VIR_INFO(__FILE__, %s, msg)
+#define WARN(fmt,...) VIR_WARN(__FILE__, fmt, __VA_ARGS__)
+#define WARN0(msg) VIR_WARN(__FILE__, %s, msg)
+#define ERROR(fmt,...) VIR_ERROR(__FILE__, fmt, __VA_ARGS__)
+#define ERROR0(msg) VIR_ERROR(__FILE__, %s, msg)
+
+
+/*
+ * To be made public
+ */
+typedef enum {
+VIR_LOG_DEBUG = 1,
+VIR_LOG_INFO,
+VIR_LOG_WARN,
+VIR_LOG_ERROR,
+} virLogPriority;
+
+/**
+ * virLogOutputFunc:
+ * @data: extra output logging data
+ * @category: the category for the message
+ * @priority: the priority for the message
+ * @msg: the message to log, preformatted and zero terminated
+ * @len: the lenght of the message in bytes without the terminating zero
+ *
+ * Callback function used to output messages
+ *
+ * Returns the number of bytes written or -1 in case of error
+ */
+typedef int (*virLogOutputFunc) (void *data, const char *category,
+ int priority, const char *str, int len);
+
+/**
+ * virLogCloseFunc:
+ * @data: extra output logging data
+ *
+ * Callback function used to close a log output
+ */
+typedef void (*virLogCloseFunc) (void *data);
+
+extern int virLogSetDefaultPriority(int priority);
+extern int virLogDefineFilter(const char *match, int priority, int flags);
+extern int virLogDefineOutput(virLogOutputFunc f, virLogCloseFunc c,
+ void *data, int priority, int flags);
+
+#if 0
+extern char *virLogGetDump(int flags);
+#endif
+
+/*
+ * Internal logging API
+ */
+extern int virLogStartup(void);
+extern int virLogReset(void);
+extern void virLogShutdown(void);
+extern int virLogParseFilters(const char *filters);
+extern int virLogParseOutputs(const char *output);
+extern void virLogMessage(const char *category, int priority, int flags,
+ const char *fmt, ...) ATTRIBUTE_FORMAT(printf, 4, 5);
#endif
Index: src/logging.c
===
RCS file: /data/cvs/libxen/src/logging.c,v
retrieving revision 1.1
diff -u -r1.1 logging.c
--- src/logging.c 6 Nov 2008 16:36:07 - 1.1
+++ src/logging.c 17 Dec 2008 14:25:57 -
@@ -21,10 +21,747 @@
#include config.h
+#include stdio.h
+#include stdarg.h
+#include stdlib.h
+#include time.h
+#include sys/time.h
+#include sys/types.h
+#include sys/stat.h
+#include fcntl.h
+#if HAVE_SYSLOG_H
+#include syslog.h
+#endif
+
#include logging.h
+#include memory.h
+#include util.h
#ifdef ENABLE_DEBUG
int debugFlag = 0;
+
+/*
+ * Macro used to format the message as a string in virLogMessage
+ * and borrowed from libxml2 (also used in virRaiseError)
+ */
+#define VIR_GET_VAR_STR(msg, str) {\
+int size, prev_size = -1;\
+int chars; \
+char *larger; \
+va_list ap; \
+
Re: [libvirt] [PATCH] Fix _FILE_OFFSET_BITS re-definition
On Wed, Dec 17, 2008 at 10:55:41AM +0100, Jim Meyering wrote:
john.le...@sun.com wrote:
# HG changeset patch
# User john.le...@sun.com
# Date 1229399267 28800
# Node ID db36391b739c117f5887388f65f31e6a9d2d361b
# Parent 020f8b8e9340287a6ab3d869b359e39b905cd0ff
Fix _FILE_OFFSET_BITS re-definition
Since config.h contains the _FILE_OFFSET_BITS setting (a little dubious
in itself), it must be the first header included, otherwise system
headers can define _FILE_OFFSET_BITS differently themselves.
Signed-off-by: John Levon john.le...@sun.com
diff --git a/src/node_device_hal.c b/src/node_device_hal.c
--- a/src/node_device_hal.c
+++ b/src/node_device_hal.c
@@ -21,10 +21,11 @@
* Author: David F. Lively dliv...@virtualiron.com
*/
+#include config.h
+
#include stdio.h
#include stdlib.h
-#include config.h
#include libhal.h
#include node_device_conf.h
ACK
FYI, there are 3 other cases where the include config.h first
rule is broken:
docs/examples/info1.c
docs/examples/suspend.c
qemud/remote_protocol.c
so I've just written a new syntax-check rule to enforce this.
It fixes only the last one. Since the two examples are not
necessarily built using libvirt's own build system, so they
are exempted.
Yep, the examples shouldn't really need config.h at all,
using the
So with your patch above and the following one,
the new make sc_require_config_h_first part of make syntax-check
passes:
From 15afb090ac28ab6b9274fb827eb1c1c939db4104 Mon Sep 17 00:00:00 2001
From: Jim Meyering meyer...@redhat.com
Date: Wed, 17 Dec 2008 10:49:04 +0100
Subject: [PATCH] enforce the include config.h first rule
* qemud/Makefile.am: Ensure that the generated remote_protocol.c
includes config.h first.
* qemud/remote_protocol.c: Regenerate.
* Makefile.maint (sc_require_config_h_first): New rule, so that
make syntax-check enforces this.
* .x-sc_require_config_h_first: New file.
* Makefile.am (.x-sc_require_config_h_first): Add it.
ACK
---
.x-sc_require_config_h_first |2 ++
ChangeLog| 11 +++
Makefile.am |1 +
Makefile.maint | 15 +++
qemud/Makefile.am|9 +++--
qemud/remote_protocol.c |2 +-
6 files changed, 37 insertions(+), 3 deletions(-)
create mode 100644 .x-sc_require_config_h_first
diff --git a/.x-sc_require_config_h_first b/.x-sc_require_config_h_first
new file mode 100644
index 000..58a8878
--- /dev/null
+++ b/.x-sc_require_config_h_first
@@ -0,0 +1,2 @@
+^docs/examples/info1\.c$
+^docs/examples/suspend\.c$
diff --git a/ChangeLog b/ChangeLog
index 08a1cb5..36c1278 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,14 @@
+2008-12-17 Jim Meyering meyer...@redhat.com
+
+ enforce the include config.h first rule
+ * qemud/Makefile.am: Ensure that the generated remote_protocol.c
+ includes config.h first.
+ * qemud/remote_protocol.c: Regenerate.
+ * Makefile.maint (sc_require_config_h_first): New rule, so that
+ make syntax-check enforces this.
+ * .x-sc_require_config_h_first: New file.
+ * Makefile.am (.x-sc_require_config_h_first): Add it.
+
Wed Dec 17 08:02:01 +0100 2008 Jim Meyering meyer...@redhat.com
fix numa-related (and kernel-dependent) test failures
diff --git a/Makefile.am b/Makefile.am
index d40a151..758ad50 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -14,6 +14,7 @@ EXTRA_DIST = \
libvirt.pc libvirt.pc.in \
$(man_MANS) autobuild.sh \
.x-sc_avoid_if_before_free \
+ .x-sc_require_config_h_first \
.x-sc_prohibit_strcmp \
.x-sc_require_config_h \
autogen.sh
diff --git a/Makefile.maint b/Makefile.maint
index 5758215..b7bb680 100644
--- a/Makefile.maint
+++ b/Makefile.maint
@@ -135,6 +135,21 @@ sc_require_config_h:
else :; \
fi
+# You must include config.h before including any other header file.
+sc_require_config_h_first:
+ @if $(VC_LIST_EXCEPT) | grep '\.c$$' /dev/null; then \
+ fail=0; \
+ for i in $$($(VC_LIST_EXCEPT) | grep '\.c$$'); do \
+ grep '^# *include\' $$i | sed 1q \
+ | grep '^# *include config\.h' /dev/null\
+ || { echo $$i; fail=1; }; \
+ done; \
+ test $$fail = 1 \
+ { echo '$(ME): the above files include some other header' \
+ 'before config.h' 12; exit 1; } || :; \
+ else :; \
+ fi
+
# To use this command macro, you must first define two shell variables:
# h: the header, enclosed in or
# re:
[libvirt] [PATCH] 3/8 convert libvirt.c to the new logging infrastructure
Basically there is the initialization part which looks for the
environment variables, and all DEBUG calls are modified to show
the entry point, this was lost over time just the arguments of
the functions wasn't great for debugging,
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
Index: src/libvirt.c
===
RCS file: /data/cvs/libxen/src/libvirt.c,v
retrieving revision 1.182
diff -u -r1.182 libvirt.c
--- src/libvirt.c 4 Dec 2008 21:46:34 - 1.182
+++ src/libvirt.c 17 Dec 2008 14:25:57 -
@@ -257,8 +257,22 @@
#ifdef ENABLE_DEBUG
debugEnv = getenv(LIBVIRT_DEBUG);
-if (debugEnv *debugEnv *debugEnv != '0')
-debugFlag = 1;
+if (debugEnv *debugEnv *debugEnv != '0') {
+if (STREQ(debugEnv, 2) || STREQ(debugEnv, info))
+virLogSetDefaultPriority(VIR_LOG_INFO);
+else if (STREQ(debugEnv, 3) || STREQ(debugEnv, warning))
+virLogSetDefaultPriority(VIR_LOG_WARN);
+else if (STREQ(debugEnv, 4) || STREQ(debugEnv, error))
+virLogSetDefaultPriority(VIR_LOG_ERROR);
+else
+virLogSetDefaultPriority(VIR_LOG_DEBUG);
+}
+debugEnv = getenv(LIBVIRT_LOG_FILTERS);
+if (debugEnv)
+virLogParseFilters(debugEnv);
+debugEnv = getenv(LIBVIRT_LOG_OUTPUTS);
+if (debugEnv)
+virLogParseOutputs(debugEnv);
#endif
DEBUG0(register drivers);
@@ -734,7 +748,7 @@
virGetVersion(unsigned long *libVer, const char *type,
unsigned long *typeVer)
{
-DEBUG(libVir=%p, type=%s, typeVer=%p, libVer, type, typeVer);
+DEBUG(%s libVir=%p, type=%s, typeVer=%p, __FUNCTION__, libVer, type,
typeVer);
if (!initialized)
if (virInitialize() 0)
@@ -980,7 +994,7 @@
if (virInitialize() 0)
return NULL;
-DEBUG(name=%s, name);
+DEBUG(%s name=%s, __FUNCTION__, name);
return do_open (name, NULL, 0);
}
@@ -1003,7 +1017,7 @@
if (virInitialize() 0)
return NULL;
-DEBUG(name=%s, name);
+DEBUG(%s name=%s, __FUNCTION__, name);
return do_open (name, NULL, VIR_CONNECT_RO);
}
@@ -1030,7 +1044,7 @@
if (virInitialize() 0)
return NULL;
-DEBUG(name=%s, auth=%p, flags=%d, name, auth, flags);
+DEBUG(%s name=%s, auth=%p, flags=%d, __FUNCTION__, name, auth, flags);
return do_open (name, auth, flags);
}
@@ -1048,7 +1062,7 @@
int
virConnectClose(virConnectPtr conn)
{
-DEBUG(conn=%p, conn);
+DEBUG(%s conn=%p, __FUNCTION__, conn);
if (!VIR_IS_CONNECT(conn))
return (-1);
@@ -1073,7 +1087,7 @@
int
virDrvSupportsFeature (virConnectPtr conn, int feature)
{
-DEBUG(conn=%p, feature=%d, conn, feature);
+DEBUG(%s conn=%p, feature=%d, __FUNCTION__, conn, feature);
if (!VIR_IS_CONNECT(conn))
return (-1);
@@ -1096,7 +1110,7 @@
virConnectGetType(virConnectPtr conn)
{
const char *ret;
-DEBUG(conn=%p, conn);
+DEBUG(%s conn=%p, __FUNCTION__, conn);
if (!VIR_IS_CONNECT(conn)) {
virLibConnError(NULL, VIR_ERR_INVALID_CONN, __FUNCTION__);
@@ -1126,7 +1140,7 @@
int
virConnectGetVersion(virConnectPtr conn, unsigned long *hvVer)
{
-DEBUG(conn=%p, hvVer=%p, conn, hvVer);
+DEBUG(%s conn=%p, hvVer=%p, __FUNCTION__, conn, hvVer);
if (!VIR_IS_CONNECT(conn)) {
virLibConnError(NULL, VIR_ERR_INVALID_CONN, __FUNCTION__);
@@ -1160,7 +1174,7 @@
char *
virConnectGetHostname (virConnectPtr conn)
{
-DEBUG(conn=%p, conn);
+DEBUG(%s conn=%p, __FUNCTION__, conn);
if (!VIR_IS_CONNECT(conn)) {
virLibConnError(NULL, VIR_ERR_INVALID_CONN, __FUNCTION__);
@@ -1194,7 +1208,7 @@
{
char *name;
-DEBUG(conn=%p, conn);
+DEBUG(%s conn=%p, __FUNCTION__, conn);
if (!VIR_IS_CONNECT(conn)) {
virLibConnError(NULL, VIR_ERR_INVALID_CONN, __FUNCTION__);
@@ -1230,7 +1244,7 @@
virConnectGetMaxVcpus(virConnectPtr conn,
const char *type)
{
-DEBUG(conn=%p, type=%s, conn, type);
+DEBUG(%s conn=%p, type=%s, __FUNCTION__, conn, type);
if (!VIR_IS_CONNECT(conn)) {
virLibConnError(NULL, VIR_ERR_INVALID_CONN, __FUNCTION__);
@@ -1257,7 +1271,7 @@
int
virConnectListDomains(virConnectPtr conn, int *ids, int maxids)
{
-DEBUG(conn=%p, ids=%p, maxids=%d, conn, ids, maxids);
+DEBUG(%s conn=%p, ids=%p, maxids=%d, __FUNCTION__, conn, ids, maxids);
if (!VIR_IS_CONNECT(conn)) {
virLibConnError(NULL, VIR_ERR_INVALID_CONN, __FUNCTION__);
@@ -1287,7 +1301,7 @@
int
virConnectNumOfDomains(virConnectPtr conn)
{
-DEBUG(conn=%p, conn);
+DEBUG(%s conn=%p, __FUNCTION__, conn);
if (!VIR_IS_CONNECT(conn)) {
virLibConnError(NULL, VIR_ERR_INVALID_CONN,
Re: [libvirt] PATCH 0/5: connection cloning support (WIP)
Hi Daniel - When I apply these patches, I'm seeing segfaults on event delivery when just running the existing synchronous examples/domain-events/events-c/event-test.c (using the remote driver). I've added a little debug. Apparently event-name is being NULLed out sometime after the event is put on the queue: DEBUG: remote_internal.c: remoteDomainEventFired (Event fired 0 3 1 1) DEBUG: remote_internal.c: processCallRecv (Do 4 0) DEBUG: remote_internal.c: processCallRecvLen (Got length, now need 64 total (60 more)) DEBUG: remote_internal.c: processCallRecv (Do 64 4) DEBUG: remote_internal.c: processCallAsyncEvent (Encountered an event while waiting for a response) DEBUG: remote_internal.c: get_nonnull_domain (domain.name: dsl) DEBUG: datatypes.c: virGetDomain (New hash entry 0x804c728) DEBUG: domain_event.c: virDomainEventNew (event: 0x804c770 -name: dsl) DEBUG: libvirt.c: virDomainFree (domain=0x804c728) DEBUG: datatypes.c: virUnrefDomain (unref domain 0x804c728 dsl 1) DEBUG: datatypes.c: virReleaseDomain (release domain 0x804c728 dsl) DEBUG: datatypes.c: virReleaseDomain (unref connection 0x804b040 4) DEBUG: domain_event.c: virDomainEventQueuePush (event: 0x804c770 -name: dsl) DEBUG: remote_internal.c: processCallRecv (Do 0 0) DEBUG: remote_internal.c: remoteDomainEventQueueFlush () DEBUG: domain_event.c: virDomainEventDispatchDefaultFunc (event: 0x804c770 -name: (null)) virGetDomain: name is NULL Segmentation fault I'll continue looking into it. But please let me know if you're familiar with the problem ... Thanks, Dave On Tue, 2008-12-16 at 10:24 -0500, David Lively wrote: Hi Daniels - Sorry for the delay. Life's been crazy. But now I'm finally looking into using this series of patches with my Java event implementation (which I've now redone via the java.nio.Select mechanism; ready to submit pending resolution of the RPC concurrency issues). I should have some feedback for you later today ... Thanks, Dave On Tue, 2008-12-09 at 12:08 +, Daniel P. Berrange wrote: This series is a work-in-progress set of patches implementing connection cloning. The idea is that if you want to use a connection form multiple threads, you could do virConnectPtr copy = virConnectClone(conn) and use 'copy' from the other thread. This avoids the problem of having to make all the virError handling stuff thread-local whic is the blocker for allowing real mutlti-thread access to a single virConnectPtr object. I believe this cloning support should be sufficient for the Java bindings need to use a thread for its event loop. The idea being that if you wanted to use an event loop in a background thread, you'd create a cloned object for use event loop triggered callbacks like domain events notifications. I'd still like to do some experiments later making the single virConnectPtr fully thread safe, but that'll take a little more time. I'm hoping this cloning will address the Java needs right now... Daniel -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATHC] 8/8 extra settings in libvirtd.conf
Basically documents the 3 new configuration variables, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ Index: qemud/libvirtd.conf === RCS file: /data/cvs/libxen/qemud/libvirtd.conf,v retrieving revision 1.8 diff -u -r1.8 libvirtd.conf --- qemud/libvirtd.conf 4 Dec 2008 22:18:44 - 1.8 +++ qemud/libvirtd.conf 17 Dec 2008 14:25:56 - @@ -247,3 +247,52 @@ #min_workers = 5 #max_workers = 20 +# +# +# Logging controls +# + +# Logging level: 0 none, 4 errors, 3 warnings, 2 informations, 1 debug +# basically 1 will log everything possible +#log_level = 3 + +# Logging filters: +# A filter allows to select a different logging level for a given category +# of logs +# The format for a filter is: +#x:name +# where name is a match string e.g. remote or qemu +# the x prefix is the minimal level where matching messages should be logged +#1: DEBUG +#2: INFO +#3: WARNING +#4: ERROR +# +# Multiple filter can be defined in a single @filters, they just need to be +# separated by spaces. +# +# e.g: +# log_filters=3:remote 4:event +# to only get warning or errors from the remote layer and only errors from +# the event layer. + +# Logging outputs: +# An output is one of the places to save logging informations +# The format for an output can be: +#x:stderr +# output goes to stderr +#x:syslog:name +# use syslog for the output and use the given name as the ident +#x:file:file_path +# output to a file, with the given filepath +# In all case the x prefix is the minimal level, acting as a filter +#0: everything +#1: DEBUG +#2: INFO +#3: WARNING +#4: ERROR +# +# Multiple output can be defined , they just need to be separated by spaces. +# e.g.: +# log_outputs=3:syslog:libvirtd +# to log all warnings and errors to syslog under the libvirtd ident -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] fix numa-related (and kernel-dependent) test failures
On Wed, Dec 17, 2008 at 08:39:04AM +0100, Jim Meyering wrote: Daniel Veillard veill...@redhat.com wrote: ... All tests pass for me with that patch. Looks good. Same for me, +1 ! Thanks. Pushed with this comment: fix numa-related (and kernel-dependent) test failures This change is required on some kernels due to the way a change in the kernel's CONFIG_NR_CPUS propagates through the numa library. * src/qemu_conf.c (qemudCapsInitNUMA): Pass numa_all_cpus_ptr-size/8 as the buffer-length-in-bytes in the call to numa_node_to_cpus, since that's what is required on second and subseqent calls. * src/uml_conf.c (umlCapsInitNUMA): Likewise. This change has broken the compile on Fedora 9 and earlier where the numa_all_cpus_ptr symbol does not exist. So it needs to have a test in configure.ac added, and if not present, go with our original code of a fixed mask size. Fortunately on Fedora 9's libnuma, they don't have the annoying mask size checks - that's a new Fedora 10 thing I also just noticed that its only touching the size param passed into the numa_node_to_cpus, but not the actual size that's allocated for the array. This is fairly harmlessuntil someone does a kernel build with NR_CPUS 4096 Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- 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
Re: [libvirt] [PATCH] 6/8 Convert qemud to the logging infrastructure
On Wed, Dec 17, 2008 at 04:26:25PM +0100, Daniel Veillard wrote: This is a bit more massive because qemud already had a limited logging primitive, so we remove the old one, put the new one, initialize the logging at the beginning of remoteReadConfigFile (maybe that should be renamed, I think it's launched in all cases). The painful part is to convert the whole set of existing log calls to the new macro, ACK. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- 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
Re: [libvirt] [PATCH] Fix statement not reached compiler error
On Tue, Dec 16, 2008 at 06:27:54PM -0800, john.le...@sun.com wrote: # HG changeset patch # User john.le...@sun.com # Date 1229399267 28800 # Node ID 77dceb31cae41057c4274517f5c3246e47545de3 # Parent 8b69f84c9498d05ae03f5389eb83290aaeab23e6 Fix statement not reached compiler error Signed-off-by: John Levon john.le...@sun.com ACK Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- 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
Re: [libvirt] [PATCH] pid_t portability in virExec()
john.le...@sun.com wrote: pid_t portability in virExec() Use the right type for handling process IDs. ... -int veid, pid; +int veid; +pid_t pid; ACK. All of those look fine. -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATHC] 8/8 extra settings in libvirtd.conf
On Wed, Dec 17, 2008 at 04:30:13PM +0100, Daniel Veillard wrote: Basically documents the 3 new configuration variables, Also need adding to the Augeas rules qemud/libvirtd.aug and qemud/test_libvirtd.aug, so that they're not lost when editing the config file using Augeas Regards, Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- 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
Re: [libvirt] proposal: allow use of bool: HACKING: discuss C types
On Wed, Dec 17, 2008 at 01:29:02PM +0100, Jim Meyering wrote: Daniel P. Berrange berra...@redhat.com wrote: On Fri, Dec 12, 2008 at 09:58:32AM +0100, Jim Meyering wrote: I propose to allow (encourage, even) the use of the standard C99 type, bool, in libvirt, but not in public interfaces[1]. There are already uses in cgroup.c and xmlrpc.c, as well as those in the gnulib c-ctype.h header which is included from many of libvirt's .c files. The motivation is to make the code as readable as possible. When you see the declaration of an int variable, member, or function, that type gives you no clue whether it is used as a boolean. If you see a few uses that treat it as boolean, that's still no guarantee, and it may be non-trivial to ensure that there isn't some non-boolean value with a special meaning. However, if you see a bool variable, you do have that guarantee. I don't particularly like the idea of using the bool type - No system header files use it Of course. Can't use it there for the same reason that we wouldn't use it in libvirt's own public interfaces, as I explained. But public interfaces make up such a small fraction of the code in question that it'd be a shame to impose its restrictions on all the rest. - Library header files which use a boolean type have nearly all defined their own custom bool types, not using stdbool.h and there's no guarentee that stdbool's idea of 'true' matches the other apps' This is the same fundamental restriction. bool cannot be used in public headers. - We don't use it in the public API, or on the wire for the remote protocol, since it has undefined size. Same point. - The GNULIB bool emulation is unable to provide equivlance between C's idea of true (any non-zero value) and the defined 'true' constant (whose value is 1) That sounds scary, but isn't an issue in practice. The uses of true and false are typically only for initialization. IMHO, one should never compare a bool-declared variable to true or false. Ok, if you want to re-post the HACKING file also mentioning that 'bool' shouldn't be used in our public APIs wire protocol, and that 'true' / 'false' should only be used for initialization I'm fine with the rest of the docs. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- 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
Re: [libvirt] [PATCH] Fix GCC hard-coding in python/
On Wed, Dec 17, 2008 at 09:57:23AM +0100, Jim Meyering wrote: john.le...@sun.com wrote: Fix GCC hard-coding in python/ ... diff --git a/acinclude.m4 b/acinclude.m4 --- a/acinclude.m4 +++ b/acinclude.m4 @@ -75,6 +75,11 @@ AC_DEFUN([LIBVIRT_COMPILE_WARNINGS],[ WARN_CFLAGS=$COMPILER_FLAGS $complCFLAGS AC_SUBST(WARN_CFLAGS) + +COMPILER_FLAGS= +gl_COMPILER_FLAGS(-Wno-redundant-decls) +NO_RDECLS_CFLAGS=$COMPILER_FLAGS +AC_SUBST(NO_RDECLS_CFLAGS) ]) diff --git a/python/Makefile.am b/python/Makefile.am --- a/python/Makefile.am +++ b/python/Makefile.am @@ -35,7 +35,7 @@ python_LTLIBRARIES = libvirtmod.la libvirtmod_la_SOURCES = libvir.c types.c libvirt-py.c libvirt-py.h # Python header files contain a redundant decl, hence: -libvirtmod_la_CFLAGS = -Wno-redundant-decls +libvirtmod_la_CFLAGS = @NO_RDECLS_CFLAGS@ That -Wno-redundant-decls option is no longer necessary, at least on rawhide. When I remove it, compiling with make -C python CFLAGS='-Wredundant-decls -Werror' still succeeds. What about with older RHEL-5 python though ? Python 2.4 had rather a large number of flaws in its public header file that have caused plenty of compile warning problems in past. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- 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
Re: [libvirt] [PATCH] rpcgen fixes
john.le...@sun.com wrote: # HG changeset patch # User john.le...@sun.com # Date 1229399266 28800 # Node ID 12c9b283b11f5e20b9dcc91e3d10a862da5d6e81 # Parent 1ba824db09286ebc758e035211da2c533bbd1e0f rpcgen fixes quad_t is not a portable type, but rather than force rpcgen every build, we just patch in the fixes needed. Also, since we ship the rpcgen-derived files in CVS, don't leave Makefile rules that could trigger during a normal build: make a special maintainer target for refreshing the derived sources. This sounds like an argument for not putting derived sources in CVS, since derived-for-one-system (linux/gnu) doesn't work for e.g., Solaris. Signed-off-by: John Levon john.le...@sun.com diff --git a/qemud/Makefile.am b/qemud/Makefile.am --- a/qemud/Makefile.am +++ b/qemud/Makefile.am @@ -32,23 +32,23 @@ EXTRA_DIST = \ $(DAEMON_SOURCES) if RPCGEN -SUFFIXES = .x -.x.c: - rm -f $@ $...@-t $...@-t2 - rpcgen -c -o $...@-t $ +# +# Maintainer-only target for re-generating the derived .c/.h source +# files, which are actually derived from the .x file. +# +rpcgen: + rm -f rp.c-t rp.h-t + rpcgen -h -o rp.h-t @top_srcdir@/qemud/remote_protocol.x + rpcgen -c -o rp.c-t @top_srcdir@/qemud/remote_protocol.x Use $(srcdir), in place of @top_srcdir@/qemud (and others below). The latter @var@ syntax is old, if not deprecated, and $(srcdir) is guaranteed to be safe, while $(top_srcdir) may be quite nasty (arbitrary abs. dir name) in the worst case. if GLIBC_RPCGEN - perl -w rpcgen_fix.pl $...@-t $...@-t2 - chmod 444 $...@-t2 - mv $...@-t2 $@ -endif - -.x.h: - rm -f $@ $...@-t - rpcgen -h -o $...@-t $ -if GLIBC_RPCGEN - perl -pi -e 's/\t//g' $...@-t - chmod 444 $...@-t - mv $...@-t $@ + perl -w @top_srcdir@/qemud/rpcgen_fix.pl rp.h-t \ + @top_srcdir@/qemud/remote_protocol.h Please don't redirect directly to something used as a build source. Instead, redirect to a temporary file, and rename that into place upon successful completion. That makes it less likely you'll ever end up with corrupted sources. + perl -w @top_srcdir@/qemud/rpcgen_fix.pl rp.c-t \ + @top_srcdir@/qemud/remote_protocol.c + rm -f rp.c-t rp.h-t +else + mv rp.h-t @top_srcdir@/remote_protocol.h + mv rp.c-t @top_srcdir@/remote_protocol.c endif endif ... -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] let gcc's -Wformat do its job; avoid make syntax-check failure
On Wed, Dec 17, 2008 at 09:01:24AM +0100, Jim Meyering wrote: I first noticed that make syntax-check failed. Then I saw that virAsprintf's prototype lacked ATTRIBUTE_FORMAT. This fixes both and updates HACKING. +1 I hit some of those too, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] [OpenVZ] Segmentation fault
On Tue, Dec 16, 2008 at 05:11:10PM +0300, Anton Protopopov wrote: As a general rule the public virDomainPtr objects should not be passed down into internal methods. Likewise, when creating / defining a VM, the virGetDomain call should be the last one in the normal execution path of the method. Thus, just pass the 'virConnectPtr' instance to openvzDomainSetVcpusInternal as suggested above, and keep the setVcpusInternal calls above the call to virGetDomain. OK, done. Thanks I've comited this patch Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- 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
RE: [libvirt] [OpenVZ]
Guys, is it really an issue to fix also mac generation for OpenVZ? Vzctl generates different macs for host's and domain's eth devices. Libvirt creates them with equal macs. In this case there is a bridge support for OpenVZ in libvirt but it doesn't work ... :-( And it has to implement additional steps for changing macs after container creation. This message (including attachments) is private and confidential. If you have received this message in error, please notify us and remove it from your system. -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Makefile portability in qemud
On Tue, Dec 16, 2008 at 06:49:48PM -0800, john.le...@sun.com wrote: # HG changeset patch # User john.le...@sun.com # Date 1229399267 28800 # Node ID 35512df785342cd15214790e17e0c1f4d2182b32 # Parent 57c76efe37988edc64beb12ca5dc1ff5863f0085 Makefile portability in qemud Neither uuidgen nor sed -i are portable - only make the edit on platforms that can do so. ACK Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- 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
[libvirt] [PATCH] 1/8 detect syslog.h
the ability to syslog gets into libvirt.c so we need to detect its presence at configure time to compile it out on platform without support, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ Index: configure.in === RCS file: /data/cvs/libxen/configure.in,v retrieving revision 1.191 diff -u -r1.191 configure.in --- configure.in5 Dec 2008 15:05:48 - 1.191 +++ configure.in17 Dec 2008 14:25:56 - @@ -70,7 +70,7 @@ AC_CHECK_FUNCS([cfmakeraw regexec uname sched_getaffinity getuid getgid]) dnl Availability of various common headers (non-fatal if missing). -AC_CHECK_HEADERS([pwd.h paths.h regex.h sys/syslimits.h sys/utsname.h sys/wait.h winsock2.h sched.h termios.h sys/poll.h]) +AC_CHECK_HEADERS([pwd.h paths.h regex.h sys/syslimits.h sys/utsname.h sys/wait.h winsock2.h sched.h termios.h sys/poll.h syslog.h]) dnl Where are the XDR functions? dnl If portablexdr is installed, prefer that. -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] 6/8 Convert qemud to the logging infrastructure
This is a bit more massive because qemud already had a limited
logging primitive, so we remove the old one, put the new one, initialize
the logging at the beginning of remoteReadConfigFile (maybe that should
be renamed, I think it's launched in all cases).
The painful part is to convert the whole set of existing log calls
to the new macro,
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
Index: qemud/qemud.h
===
RCS file: /data/cvs/libxen/qemud/qemud.h,v
retrieving revision 1.9
diff -u -r1.9 qemud.h
--- qemud/qemud.h 4 Dec 2008 22:16:40 - 1.9
+++ qemud/qemud.h 17 Dec 2008 14:25:57 -
@@ -45,6 +45,7 @@
#include rpc/types.h
#include rpc/xdr.h
#include remote_protocol.h
+#include logging.h
#ifdef __GNUC__
#ifdef HAVE_ANSIDECL_H
@@ -61,15 +62,7 @@
#define ATTRIBUTE_FORMAT(...)
#endif
-typedef enum {
-QEMUD_ERR,
-QEMUD_WARN,
-QEMUD_INFO,
-#ifdef ENABLE_DEBUG
-QEMUD_DEBUG
-#endif
-} qemudLogPriority;
-
+#define qemudDebug DEBUG
enum qemud_mode {
QEMUD_MODE_RX_HEADER, /* Receiving the fixed length RPC header data
*/
@@ -183,11 +176,6 @@
void qemudLog(int priority, const char *fmt, ...)
ATTRIBUTE_FORMAT(printf,2,3);
-#ifdef ENABLE_DEBUG
-#define qemudDebug(...) qemudLog(QEMUD_DEBUG, __VA_ARGS__)
-#else
-#define qemudDebug(fmt, ...) do {} while(0)
-#endif
int qemudSetCloseExec(int fd);
int qemudSetNonBlock(int fd);
Index: qemud/qemud.c
===
RCS file: /data/cvs/libxen/qemud/qemud.c,v
retrieving revision 1.123
diff -u -r1.123 qemud.c
--- qemud/qemud.c 12 Dec 2008 12:19:21 - 1.123
+++ qemud/qemud.c 17 Dec 2008 14:25:57 -
@@ -91,6 +91,11 @@
static int sigwrite = -1; /* Signal handler pipe */
static int ipsock = 0; /* -l Listen for TCP/IP */
+/* Defaults for logging */
+static int log_level = 3;
+static char *log_filters = NULL;
+static char *log_outputs = NULL;
+
/* Defaults for configuration file elements */
static int listen_tls = 1;
static int listen_tcp = 0;
@@ -167,7 +172,7 @@
{
struct stat sb;
if (stat(file, sb) 0) {
-qemudLog (QEMUD_ERR, _(Cannot access %s '%s': %s (%d)),
+ERROR(_(Cannot access %s '%s': %s (%d)),
type, file, strerror(errno), errno);
return -1;
}
@@ -184,7 +189,7 @@
err = gnutls_certificate_allocate_credentials (x509_cred);
if (err) {
-qemudLog (QEMUD_ERR, _(gnutls_certificate_allocate_credentials: %s),
+ERROR(_(gnutls_certificate_allocate_credentials: %s),
gnutls_strerror (err));
return -1;
}
@@ -197,7 +202,7 @@
err = gnutls_certificate_set_x509_trust_file (x509_cred, ca_file,
GNUTLS_X509_FMT_PEM);
if (err 0) {
-qemudLog (QEMUD_ERR, _(gnutls_certificate_set_x509_trust_file:
%s),
+ERROR(_(gnutls_certificate_set_x509_trust_file: %s),
gnutls_strerror (err));
return -1;
}
@@ -207,11 +212,11 @@
if (remoteCheckCertFile(CA revocation list, crl_file) 0)
return -1;
-qemudDebug (loading CRL from %s, crl_file);
+DEBUG(loading CRL from %s, crl_file);
err = gnutls_certificate_set_x509_crl_file (x509_cred, crl_file,
GNUTLS_X509_FMT_PEM);
if (err 0) {
-qemudLog (QEMUD_ERR, _(gnutls_certificate_set_x509_crl_file: %s),
+ERROR(_(gnutls_certificate_set_x509_crl_file: %s),
gnutls_strerror (err));
return -1;
}
@@ -222,14 +227,13 @@
return -1;
if (remoteCheckCertFile(server key, key_file) 0)
return -1;
-qemudDebug (loading cert and key from %s and %s,
-cert_file, key_file);
+DEBUG(loading cert and key from %s and %s, cert_file, key_file);
err =
gnutls_certificate_set_x509_key_file (x509_cred,
cert_file, key_file,
GNUTLS_X509_FMT_PEM);
if (err 0) {
-qemudLog (QEMUD_ERR, _(gnutls_certificate_set_x509_key_file: %s),
+ERROR(_(gnutls_certificate_set_x509_key_file: %s),
gnutls_strerror (err));
return -1;
}
@@ -242,14 +246,12 @@
*/
err = gnutls_dh_params_init (dh_params);
if (err 0) {
-qemudLog (QEMUD_ERR, _(gnutls_dh_params_init: %s),
- gnutls_strerror (err));
+ERROR(_(gnutls_dh_params_init: %s), gnutls_strerror (err));
return -1;
Re: [libvirt] [PATCH] 2/8 logging header and core implementation
On Wed, Dec 17, 2008 at 04:12:20PM +0100, Daniel Veillard wrote:
diff -u -r1.1 logging.h
--- src/logging.h 6 Nov 2008 16:36:07 - 1.1
+++ src/logging.h 17 Dec 2008 14:25:57 -
@@ -30,16 +30,87 @@
* defined at runtime of from the libvirt daemon configuration file
*/
#ifdef ENABLE_DEBUG
-extern int debugFlag;
#define VIR_DEBUG(category, fmt,...)\
-do { if (debugFlag) fprintf (stderr, DEBUG: %s: %s ( fmt )\n,
category, __func__, __VA_ARGS__); } while (0)
+virLogMessage(category, VIR_LOG_DEBUG, 0, fmt, __VA_ARGS__)
+#define VIR_INFO(category, fmt,...)\
+virLogMessage(category, VIR_LOG_INFO, 0, fmt, __VA_ARGS__)
+#define VIR_WARN(category, fmt,...)\
+virLogMessage(category, VIR_LOG_WARN, 0, fmt, __VA_ARGS__)
+#define VIR_ERROR(category, fmt,...)\
+virLogMessage(category, VIR_LOG_ERROR, 0, fmt, __VA_ARGS__)
#else
#define VIR_DEBUG(category, fmt,...) \
do { } while (0)
+#define VIR_INFO(category, fmt,...) \
+do { } while (0)
+#define VIR_WARN(category, fmt,...) \
+do { } while (0)
+#define VIR_ERROR(category, fmt,...) \
+do { } while (0)
#endif /* !ENABLE_DEBUG */
#define DEBUG(fmt,...) VIR_DEBUG(__FILE__, fmt, __VA_ARGS__)
#define DEBUG0(msg) VIR_DEBUG(__FILE__, %s, msg)
+#define INFO(fmt,...) VIR_INFO(__FILE__, fmt, __VA_ARGS__)
+#define INFO0(msg) VIR_INFO(__FILE__, %s, msg)
+#define WARN(fmt,...) VIR_WARN(__FILE__, fmt, __VA_ARGS__)
+#define WARN0(msg) VIR_WARN(__FILE__, %s, msg)
+#define ERROR(fmt,...) VIR_ERROR(__FILE__, fmt, __VA_ARGS__)
+#define ERROR0(msg) VIR_ERROR(__FILE__, %s, msg)
I think we should add a prefix to the filename to give a
little scope to the namespace. This would let people use
non-filename based namespaces without risk of clashing,
eg, perhaps
#define ERROR(fmt,...) VIR_ERROR(file. __FILE__, fmt, __VA_ARGS__)
thus turning intofile.libvirt.c
+/**
+ * virLogOutputFunc:
+ * @data: extra output logging data
+ * @category: the category for the message
+ * @priority: the priority for the message
+ * @msg: the message to log, preformatted and zero terminated
+ * @len: the lenght of the message in bytes without the terminating zero
+ *
+ * Callback function used to output messages
+ *
+ * Returns the number of bytes written or -1 in case of error
+ */
+typedef int (*virLogOutputFunc) (void *data, const char *category,
+ int priority, const char *str, int len);
I have a preference for 'void *data' parameters to callbacks being
at end of the param list.
+extern int virLogStartup(void);
+extern int virLogReset(void);
+extern void virLogShutdown(void);
+extern int virLogParseFilters(const char *filters);
+extern int virLogParseOutputs(const char *output);
+extern void virLogMessage(const char *category, int priority, int flags,
+ const char *fmt, ...) ATTRIBUTE_FORMAT(printf, 4,
5);
I think it would be a good idea to have virLogMesage take a
function name, and line number too. Likewise for the
virLogOutputFunc() function.
The VIR_ERROR/WARN/INFO/DEBUG macros would automatically include
the __func__ and __line__ macros. eg
#define VIR_INFO(category, fmt,...)\
virLogMessage(category, __func__, __line__, VIR_LOG_INFO, 0, fmt,
__VA_ARGS__)
So when we process the 'char fmt,' into an actual string we could
have some flag to turn on/off inclusion of the function line data.
Cole did a similar thing for error reporting internal APIs when he
added this to virterror_internal.h:
void virReportErrorHelper(virConnectPtr conn, int domcode, int errcode,
const char *filename,
const char *funcname,
long long linenr,
const char *fmt, ...)
ATTRIBUTE_FORMAT(printf, 7, 8);
Though, the actual implementation currently ignores filename, funcname
and linenr.
+
+/*
+ * Macro used to format the message as a string in virLogMessage
+ * and borrowed from libxml2 (also used in virRaiseError)
+ */
+#define VIR_GET_VAR_STR(msg, str) { \
+int size, prev_size = -1; \
+int chars; \
+char *larger; \
+va_list ap;\
+\
+str = (char *) malloc(150); \
+if (str != NULL) { \
+\
+size = 150; \
+
Re: [libvirt] Re: [RFC] sVirt 0.20
On Wed, Dec 17, 2008 at 11:04:17AM +1100, James Morris wrote: On Mon, 15 Dec 2008, James Morris wrote: On Thu, 11 Dec 2008, Daniel P. Berrange wrote: * a virNodeInfo is a structure filled by virNodeGetInfo() and providing @@ -504,6 +567,10 @@ int virDomainSetMaxMemory (virDomainPtr domain, int virDomainSetMemory (virDomainPtr domain, unsigned long memory); int virDomainGetMaxVcpus(virDomainPtr domain); +int virDomainGetSecLabel (virDomainPtr domain, + virDomainSecLabelPtr seclabel); +int virDomainGetSecModel (virDomainPtr domain, +virDomainSecModelPtr secmodel); I'm leaning two ways on this. On the one hand I could see the virDomainGetSecModel being done against the node to match the fact that we record it in the node capabilities XML, so perhaps virNodeGetSecurityModel(virConnectPtr). Actually, this is a call to get the node information, so I think the name should be changed. Btw, is 'Node' the correct placement for this information? IIUC, a node is the physical system, whereas, the security model is a property of the hypervisor, and there can be multiple hypervisors running on a node. All the virNode calls take a virConnectPtr instance, which thus provides the hypervisor context. So consider the virNode calls to be providing info about the (hypervisor,host) tuple. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- 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
[libvirt] how to connect LXC via libvirt
Hi Daniel, I followed the instructions in: https://lists.linux-foundation.org/pipermail/containers/2008-September/013237.html to try to connect with LXC which is built in my VM (ubuntu7.10 + kernel 2.6.27-rc8). Unfortunately, I fail. The container can work when I use its user-space interfaces, however I can not execute the virsh --connect lxc:/// define mycontainer.xml after I have installed the libvirt (downloaded from CVS). I checked the libvirt.c and found there are only the xen:/// support, not any codes to support LXC. Therefore, I am wondering whether LXC has been supported in libvirt mainstream and what should I do to get the result as your statement in maillist. Thanks, Ian -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] fix numa-related (and kernel-dependent) test failures
Daniel P. Berrange berra...@redhat.com wrote: On Wed, Dec 17, 2008 at 08:39:04AM +0100, Jim Meyering wrote: Daniel Veillard veill...@redhat.com wrote: ... All tests pass for me with that patch. Looks good. Same for me, +1 ! Thanks. Pushed with this comment: fix numa-related (and kernel-dependent) test failures This change is required on some kernels due to the way a change in the kernel's CONFIG_NR_CPUS propagates through the numa library. * src/qemu_conf.c (qemudCapsInitNUMA): Pass numa_all_cpus_ptr-size/8 as the buffer-length-in-bytes in the call to numa_node_to_cpus, since that's what is required on second and subseqent calls. * src/uml_conf.c (umlCapsInitNUMA): Likewise. This change has broken the compile on Fedora 9 and earlier where the numa_all_cpus_ptr symbol does not exist. So it needs to have a test in configure.ac added, and if not present, go with our original code of a fixed mask size. Fortunately on Fedora 9's libnuma, they don't have the annoying mask size checks - that's a new Fedora 10 thing Thanks for the heads-up. While normally I'd prefer an autoconf test, it might make sense to use an #if in this case. Maybe this will do it: #if LIBNUMA_API_VERSION = 1 use old code #else use numa_all_cpus_ptr #endif I also just noticed that its only touching the size param passed into the numa_node_to_cpus, but not the actual size that's allocated for the array. This is fairly harmlessuntil someone does a kernel build with NR_CPUS 4096 I'll deal with this, too. -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] libvirt and 192.168.122.0/24
I'm kind of wondering why libvirt defaults to 192.168.122.0/24 by default. It is a piece of address space which is relatively likely to conflict with address space used in the environment surrounding the machine. Since libvirtd is on by default if installed, this is particularly problematic. I would like to suggest either one of the following as default address space, unless the user has explicitly configured otherwise: 192.0.2.0/24- reserved as test and example network 198.18.0.0/15 - reserved as benchmark test network See RFC 3330 and RFC 2544 for the definitions of these networks. Both of them are should not appear on the public Internet address blocks, and much less likely than the RFC 1918 address block (10.0.0.0/8, 172.16.0.0/12, and 192.168.0.0/16) to be encountered in the wild by a user. -hpa -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
