December 2008 - Devel - libvirt List Archives

[libvirt] [PATCH] libvirtd: plug a nominal leak
by Jim Meyering 12 Dec '08

12 Dec '08

Running make check through valgrind exposed this: 40 bytes in 1 blocks are definitely lost in loss record 9 of 68 at 0x4A05174: calloc (vg_replace_malloc.c:397) by 0x4C422C1: virAllocN (memory.c:128) by 0x412CB1: qemudRunLoop (qemud.c:1846) by 0x41480B: main (qemud.c:2469) Here's the fix: >From a660df67d623184001a74f5fcf88a2f58217ec29 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Thu, 11 Dec 2008 19:58:08 +0100 Subject: [PATCH] libvirtd: plug a nominal leak * qemud/qemud.c (qemudRunLoop): Free server->workers. --- qemud/qemud.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/qemud/qemud.c b/qemud/qemud.c index f35d0fd..80b8778 100644 --- a/qemud/qemud.c +++ b/qemud/qemud.c @@ -1919,6 +1919,7 @@ static int qemudRunLoop(struct qemud_server *server) { pthread_mutex_lock(&server->lock); } + free(server->workers); pthread_mutex_unlock(&server->lock); return ret; } -- 1.6.0.4.1044.g77718

2 2

[libvirt] RFC: Removing use of strerror() in favour of strerror_r()
by Daniel P. Berrange 12 Dec '08

12 Dec '08

The man page for 'strerror()' has this to say The strerror() function returns a string describ- ing the error code passed in the argument errnum, possibly using the LC_MESSAGES part of the current locale to select the appropriate language. This string must not be modified by the application, but may be modified by a subsequent call to per- ror(3) or strerror(). No library function will modify this string. ie, strerror is not even remotely safe to use if the application calling libvirt uses threads, regardless of whether libvirt itself is threaded. The replacement is strerror_r(), but it is kind of tedious to use as you can't simply pass its return value straight in as format arg. While looking at this I also noticed that we're rather inconsistent about whether we use VIR_ERR_INTERNAL_ERROR or VIR_ERR_SYSTEM_ERROR code for reporting system errors. So I'm intending to create a convenient function for reporting system errors that hides the sterrror_r horribleness and also standardizes on error code VIR_ERR_SYSTEM_ERROR. At the same time also creating a convenience function for reporting OOM, since we have wildly inconsistent reporting of that too. The API contracts for src/virterror_internal.h are intended to be: void virReportSystemErrorFull(virConnectPtr conn, int domcode, int theerrno, const char *filename, const char *funcname, long long linenr, const char *fmt, ...) ATTRIBUTE_FORMAT(printf, 7, 8); void virReportOOMFull(virConnectPtr conn, int domcode, const char *filename, const char *funcname, long long linenr); And using a macro to automatically set domcode, filename, funcname and linenr, since its tedious to make the caller supply all those #define virReportSystemError(conn, theerrno, fmt,...) \ virReportSystemErrorFull((conn), \ VIR_FROM_THIS, \ (theerrno), \ __FILE__, __FUNCTION__, __LINE__, \ (fmt), __VA_ARGS__) #define virReportOOM(conn) \ virReportOOMFull((conn), VIR_FROM_THIS, \ __FILE__, __FUNCTION__, __LINE__) \ So whereas before you might do xenXMError (conn, VIR_ERR_INTERNAL_ERROR, _("cannot stat %s: %s"), filename, strerror(errno)); xenXMError (conn, VIR_ERR_NO_MEMORY, "%s", strerror(errno)); Now you would do: virReportSystemError(conn, errno, _("cannot stat: %s"), filename); virReportOOM(conn); Before I actually go and start making this change across all 300 uses of strerror(), do people agree with this approach.... 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 :|

4 3

[libvirt] [PATCH] implement --version for libvirtd
by Dave Allan 12 Dec '08

12 Dec '08

The daemon-conf script, when run in verbose mode, tries to execute libvirtd --version. The attached patch implements --version. Dave diff --git a/qemud/qemud.c b/qemud/qemud.c index f35d0fd..b10d680 100644 --- a/qemud/qemud.c +++ b/qemud/qemud.c @@ -2272,6 +2272,13 @@ remoteReadConfigFile (struct qemud_server *server, const char *filename) return -1; } +/* Display version information. */ +static void +version (const char *argv0) +{ + printf ("%s (%s) %s\n", argv0, PACKAGE_NAME, PACKAGE_VERSION); +} + /* Print command-line usage. */ static void usage (const char *argv0) @@ -2287,6 +2294,7 @@ Options:\n\ -l | --listen Listen for TCP/IP connections.\n\ -t | --timeout <secs> Exit after timeout period.\n\ -f | --config <file> Configuration file.\n\ + | --version Display version information.\n\ -p | --pid-file <file> Change name of PID file.\n\ \n\ libvirt management daemon:\n\ @@ -2317,6 +2325,10 @@ libvirt management daemon:\n\ : "(disabled in ./configure)"); } +enum { + OPT_VERSION = 129 +}; + #define MAX_LISTEN 5 int main(int argc, char **argv) { struct qemud_server *server = NULL; @@ -2333,6 +2345,7 @@ int main(int argc, char **argv) { { "config", required_argument, NULL, 'f'}, { "timeout", required_argument, NULL, 't'}, { "pid-file", required_argument, NULL, 'p'}, + { "version", no_argument, NULL, OPT_VERSION }, { "help", no_argument, NULL, '?' }, {0, 0, 0, 0} }; @@ -2378,6 +2391,10 @@ int main(int argc, char **argv) { remote_config_file = optarg; break; + case OPT_VERSION: + version (argv[0]); + return 0; + case '?': usage (argv[0]); return 2;

3 2

[libvirt] [PATCH] fix for Bug 460510
by Dave Allan 12 Dec '08

12 Dec '08

Attached is a patch for bug 460510, which is a bug in the RPM spec file. https://bugzilla.redhat.com/show_bug.cgi?id=460510 Dave diff --git a/libvirt.spec.in b/libvirt.spec.in index cddf5c8..8638377 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -21,7 +21,7 @@ %define with_xen_proxy 0 %endif -%if "%{fedora}" +%if 0%{?fedora} %ifarch ppc64 %define with_qemu 0 %endif

2 1

[libvirt] [PATCH] retry poll() in EINTR case in virPipeReadUntilEOF
by Dave Allan 12 Dec '08

12 Dec '08

I saw a few messages: libvir: error : internal error poll error: Interrupted system call during daemon-conf tests. The messages come from virPipeReadUntilEOF. I think the intention is that this case should be retried. Patch attached. Dave diff --git a/src/util.c b/src/util.c index da26009..31a9702 100644 --- a/src/util.c +++ b/src/util.c @@ -473,7 +473,7 @@ virPipeReadUntilEOF(virConnectPtr conn, int outfd, int errfd, while(!(finished[0] && finished[1])) { if (poll(fds, ARRAY_CARDINALITY(fds), -1) < 0) { - if (errno == EAGAIN) + if (errno == EAGAIN || errno == EINTR) continue; goto pollerr; }

3 2

[libvirt] Deploying latest libvirt in rhel / centos 5.2
by Marco Sinhoreli 11 Dec '08

11 Dec '08

Hello all, I'm wanting to know if I will have some difficulty to package libvirt 0.5.x to rhel / centos 5.2. I'm needing of the latest libvirt release to tests with oVirt. Regards, -- Marco Sinhoreli

4 5

[libvirt] [PATCH] Sanitize qemu monitor output
by Cole Robinson 11 Dec '08

11 Dec '08

The attached patch cleans up output we read from the qemu monitor. This is a simplified form of a patch I posted awhile ago. From the patch: /* The monitor doesn't dump clean output after we have written to * it. Every character we write dumps a bunch of useless stuff, * so the result looks like "cXcoXcomXcommXcommaXcommanXcommand" * Try to throw away everything before the first full command * occurence, and inbetween the command and the newline starting * the response */ This extra output makes our qemu log files _huge_ if doing things like polling disk/net stats, and prevents us from returning any useful error message printed from the monitor (say for media/disk eject, disk/net stats, etc). I've been running with this patch for a while and haven't hit any issues. Thanks, Cole

3 5

[libvirt] [PATCH] build: avoid libvirtd link failure with CFLAGS=-g
by Jim Meyering 11 Dec '08

11 Dec '08

Building libvirtd with CFLAGS=-g (and not -O) failed: ../src/.libs/libvirt_driver_qemu.a(libvirt_driver_qemu_la-qemu_driver.o): In function `qemudExtractMonitorPath': /home/meyering/w/co/libvirt/src/qemu_driver.c:549: undefined reference to `c_isspace' ../src/.libs/libvirt_driver_qemu.a(libvirt_driver_qemu_la-qemu_driver.o): In function `qemudDetectVcpuPIDs': /home/meyering/w/co/libvirt/src/qemu_driver.c:702: undefined reference to `c_isspace' collect2: ld returned 1 exit status make[2]: *** [libvirtd] Error 1 make[2]: Leaving directory `/home/meyering/w/co/libvirt/qemud' make[1]: *** [all-recursive] Error 1 make[1]: Leaving directory `/home/meyering/w/co/libvirt' make: *** [all] Error 2 Here's the fix: >From 7b963a92060c4591ce5c89f85301c35655056557 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Thu, 11 Dec 2008 18:43:12 +0100 Subject: [PATCH] build: avoid libvirtd link failure with CFLAGS=-g * qemud/Makefile.am (libvirtd_LDADD): Add gnulib's libgnu.la last. Otherwise, building with -g (no inlining) would fail due to a use of c_isspace in libvirt_driver_qemu.a, which used to followed libgnu.a in the link command. --- qemud/Makefile.am | 6 ++++-- 1 files changed, 4 insertions(+), 2 deletions(-) diff --git a/qemud/Makefile.am b/qemud/Makefile.am index 8cb0847..ea89d06 100644 --- a/qemud/Makefile.am +++ b/qemud/Makefile.am @@ -93,8 +93,7 @@ libvirtd_LDADD = \ $(LIBXML_LIBS) \ $(GNUTLS_LIBS) \ $(SASL_LIBS) \ - $(POLKIT_LIBS) \ - ../gnulib/lib/libgnu.la + $(POLKIT_LIBS) if ! WITH_DRIVER_MODULES if WITH_QEMU @@ -244,6 +243,9 @@ uninstall-init: endif # DBUS_INIT_SCRIPTS_RED_HAT +# This must be added last. +libvirtd_LDADD += ../gnulib/lib/libgnu.la + endif # WITH_LIBVIRTD CLEANFILES = libvirtd.init -- 1.6.0.4.1044.g77718

2 2

[libvirt] [PATCH] Fix locking issue in testVolumeLookupByPath
by Cole Robinson 11 Dec '08

11 Dec '08

The attached patch fixes a locking issue in the test driver, specifically testStorageVolumeLookupByPath. We were only unlocking the pool object if it was running. This fixed a deadlock I was seeing while adding some new tests to virtinst's test framework. Thanks, Cole

2 2

[libvirt] [PATCH] Fix rpm building with new logrotate file
by Cole Robinson 11 Dec '08

11 Dec '08

The attached patch teaches the rpm spec about the new logrotate file, which unbreaks rpm builds. Thanks, Cole

4 5