December 2008 - Devel - libvirt List Archives

[libvirt] [PATCH] qemud/qemud.c (qemudCleanup): Plug a leak.
by Jim Meyering 01 Dec '08

01 Dec '08

Hi Dan, With your patches, "make check" hits one new leak: 40 bytes in 1 blocks are definitely lost in loss record 9 of 65 at 0x4A05174: calloc (vg_replace_malloc.c:397) by 0x447F29: virAllocN (memory.c:128) by 0x40CD82: qemudRunLoop (qemud.c:1839) by 0x40E713: main (qemud.c:2462) This fixes it: >From 9230c41d3deb00b4a0bb6800126e5cd41a350a54 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Mon, 1 Dec 2008 15:07:40 +0100 Subject: [PATCH] * qemud/qemud.c (qemudCleanup): Plug a leak. --- qemud/qemud.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/qemud/qemud.c b/qemud/qemud.c index 94f5918..8518bd5 100644 --- a/qemud/qemud.c +++ b/qemud/qemud.c @@ -1942,6 +1942,7 @@ static void qemudCleanup(struct qemud_server *server) { virStateCleanup(); + VIR_FREE(server->workers); free(server); } -- 1.6.0.4.1044.g77718

1 0

[libvirt] avoid "make check" hang on daemon-conf
by Jim Meyering 01 Dec '08

01 Dec '08

Hi Dan, With your 28-part patch, I was getting a daemon-conf failure in "make check". This fixes it: Unless you tell me otherwise, I will apply most of this right away, (and let you remove the space-before-max_clients in your patch) since no other part of this change conflicts with your series. >From 57842865f8845c788408e18033f739cf02987ca3 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Mon, 1 Dec 2008 14:12:44 +0100 Subject: [PATCH] tests: daemon-conf: accommodate new numeric-valued config params: This test would hang when failing to perturb the new numeric (and non-boolean) valued parameters, max_clients, max_workers, min_workers. * tests/daemon-conf: Require that all commented out settings in libvirtd.conf have the same form. Before, two parameters were not being tested, since a space had snuck between the leading "#" and the "param = value" parts. Apply each RHS-value-perturbing transformation separately, not in series. Let VERBOSE=yes turn on debugging. Be more verbose by default, since this is a relatively long-running test. * qemud/libvirtd.conf: Normalize the spacing around each setting that is to be perturbed by tests/daemon-conf. --- qemud/libvirtd.conf | 10 ++++++---- tests/daemon-conf | 32 ++++++++++++++++++++++++++------ 2 files changed, 32 insertions(+), 10 deletions(-) diff --git a/qemud/libvirtd.conf b/qemud/libvirtd.conf index f269766..b0962f2 100644 --- a/qemud/libvirtd.conf +++ b/qemud/libvirtd.conf @@ -1,7 +1,10 @@ # Master libvirt daemon configuration file # # For further information consult http://libvirt.org/format.html - +# +# NOTE: the tests/daemon-conf regression test script requires +# that each "PARAMETER = VALUE" line in this file have the parameter +# name just after a leading "#". ################################################################# # @@ -45,7 +48,7 @@ # Override the default configuration which binds to all network # interfaces. This can be a numeric IPv4/6 address, or hostname # -# listen_addr = "192.168.0.1" +#listen_addr = "192.168.0.1" # Flag toggling mDNS advertizement of the libvirt service. @@ -233,7 +236,7 @@ # The maximum number of concurrent client connections to allow # over all sockets combined. -# max_clients = 20 +#max_clients = 20 # The minimum limit sets the number of workers to start up @@ -243,4 +246,3 @@ # of clients allowed #min_workers = 5 #max_workers = 20 - diff --git a/tests/daemon-conf b/tests/daemon-conf index db1f0d3..03189d5 100755 --- a/tests/daemon-conf +++ b/tests/daemon-conf @@ -1,6 +1,11 @@ #!/bin/sh # Get coverage of libvirtd's config-parsing code. +if test "$VERBOSE" = yes; then + set -x + libvirtd --version +fi + # Boilerplate code to set up a test directory, cd into it, # and to ensure we remove it upon completion. this_test_() { echo "./$0" | sed 's,.*/,,'; } @@ -12,8 +17,14 @@ trap '(exit $?); exit $?' 1 2 13 15 mkdir "$t_" || fail=1 cd "$t_" || fail=1 +conf="$abs_top_srcdir/qemud/libvirtd.conf" + +# Ensure that each commented out PARAMETER = VALUE line has the expected form. +grep '[a-z_] *= *[^ ]' "$conf" | grep -vE '^#[a-z_]+ = ' \ + && { echo "$0: found unexpected lines (above) in $conf" 1>&2; exit 1; } + # Start with the sample libvirtd.conf file, uncommenting all real directives. -sed -n 's/^#$[^ #]$/\1/p' $abs_top_srcdir/qemud/libvirtd.conf > tmp.conf +sed -n 's/^#$[^ #]$/\1/p' "$conf" > tmp.conf # Iterate through that list of directives, corrupting one RHS at a # time and running libvirtd with the resulting config. Each libvirtd @@ -22,16 +33,23 @@ n=$(wc -l < tmp.conf) i=1 while :; do param_name=$(sed -n "$i"'s/ = .*//p' tmp.conf) + printf "testing with corrupted config: $param_name\n" 1>&2 rhs=$(sed -n "$i"'s/.* = $.*$/\1/p' tmp.conf) f=in$i.conf - # Change an RHS that starts with '"' or '[' to "3". - # Change an RHS that starts with 0 or 1 to the string '"foo"'. - sed "$i"'s/ = [["].*/ = 3/;'"$i"'s/ = [01].*/ = "foo"/' tmp.conf > $f + case $rhs in + # Change an RHS that starts with '"' or '[' to "3". + [[\"]*) sed "$i"'s/ = [["].*/ = 3/' tmp.conf > $f;; + # Change an RHS that starts with a digit to the string '"foo"'. + [0-9]*) sed "$i"'s/ = [0-9].*/ = "foo"/' tmp.conf > $f;; + esac + + # Run libvirtd, expecting it to fail. libvirtd --config=$f 2> err && fail=1 + case $rhs in # '"'*) msg='should be a string';; '"'*) msg='invalid type: got long; expected string';; - [01]*) msg='invalid type: got string; expected long';; + [0-9]*) msg='invalid type: got string; expected long';; '['*) msg='must be a string or list of strings';; *) echo "unexpected RHS: $rhs" 1>&2; fail=1;; esac @@ -48,8 +66,10 @@ while :; do done # Run with the unmodified config file. +sleep_secs=2 +printf "running libvirtd with a valid config file ($sleep_secs seconds)\n" 1>&2 libvirtd --config=tmp.conf > log 2>&1 & pid=$! -sleep 2 +sleep $sleep_secs kill $pid # Expect an orderly shut-down and successful exit. -- 1.6.0.4.1044.g77718

1 0

[libvirt] KVM/qemu: problems with autostart of vms with non-bridged nets
by Gerd von Egidy 01 Dec '08

01 Dec '08

Hi, I'm using libvirt 0.5.0 to manage my kvm domains. Everything works fine except autostarting vms which use a network device not directly bridged to hw. I have the following net defined (and set to autostart): <network> <name>local</name> <uuid>7ec7a457-0b8e-4670-3b4a-2d0914288daa</uuid> <bridge stp='on' forwardDelay='0' /> <ip address='192.168.100.1' netmask='255.255.255.0'> <dhcp> <range start='192.168.100.128' end='192.168.100.254' /> </dhcp> </ip> </network> When a vm using this net is autostarted I get the following error: libvir: error : invalid connection pointer in virNetworkLookupByName libvir: QEMU error : internal error Network 'local' not found Failed to autostart VM 'testlocalnet': internal error Network 'local' not found I took a dive into the code and found two problems causing this: 1. initialization order: qemuStateDriver is registered (and thus initialized) before networkStateDriver. This means the network is not up yet when qemu does it's autostart. I could fix this with the attached patch. 2. missing virConnectPtr during autostart of the vm: in src/qemu_driver.c:145, qemudAutostartConfigs() you can find this call: int ret = qemudStartVMDaemon(NULL, driver, vm, NULL); This means virConnectPtr conn is NULL within qemudStartVMDaemon(). Following the calls: qemudStartVMDaemon() -> qemudBuildCommandLine() -> qemudNetworkIfaceConnect() -> virNetworkLookupByName() virNetworkLookupByName bails out if called with invalid virConnectPtr. This means starting this vm fails. Currently I'm not familiar enough with the libvirt structures to come up with an idea how to fix this. Could anybody with a deeper knowledge of the code take a look at this? Thank you very much. Kind regards, Gerd diff -r -u libvirt-0.5.0.orig/qemud/qemud.c libvirt-0.5.0/qemud/qemud.c --- libvirt-0.5.0.orig/qemud/qemud.c 2008-11-21 13:47:32.000000000 +0100 +++ libvirt-0.5.0/qemud/qemud.c 2008-11-30 21:27:06.000000000 +0100 @@ -761,22 +761,13 @@ * If they try to use a open a connection for a module that * is not loaded they'll get a suitable error at that point */ - virDriverLoadModule("qemu"); - virDriverLoadModule("lxc"); - virDriverLoadModule("uml"); virDriverLoadModule("network"); virDriverLoadModule("storage"); virDriverLoadModule("nodedev"); + virDriverLoadModule("qemu"); + virDriverLoadModule("lxc"); + virDriverLoadModule("uml"); #else -#ifdef WITH_QEMU - qemuRegister(); -#endif -#ifdef WITH_LXC - lxcRegister(); -#endif -#ifdef WITH_UML - umlRegister(); -#endif #ifdef WITH_NETWORK networkRegister(); #endif @@ -786,6 +777,15 @@ #if defined(HAVE_HAL) || defined(HAVE_DEVKIT) nodedevRegister(); #endif +#ifdef WITH_QEMU + qemuRegister(); +#endif +#ifdef WITH_LXC + lxcRegister(); +#endif +#ifdef WITH_UML + umlRegister(); +#endif #endif virEventRegisterImpl(virEventAddHandleImpl, -- Address (better: trap) for people I really don't want to get mail from: james(a)cactusamerica.com

2 2

[libvirt] [PATCH] avoid format string warnings
by Jim Meyering 01 Dec '08

01 Dec '08

Hi Dan, I've just applied all of your changes, built, and saw some new warnings: uml_driver.c:1742: warning: format not a string literal and no format arguments uml_driver.c:1748: warning: format not a string literal and no format arguments uml_driver.c:1785: warning: format not a string literal and no format arguments openvz_conf.c:468: warning: format not a string literal and no format arguments But they were introduced before your changes. Here's the patch, on top of your changes. Note that as is, the patch does not apply to the latest in CVS, so my checking it in there would induce merge conflicts for you. So I'll hold off until after your 28-part series is committed. Jim >From fb32deb5dfb2a3c6b598de619f25c0bae4314a05 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Mon, 1 Dec 2008 11:17:08 +0100 Subject: [PATCH] avoid format string warnings * src/uml_driver.c (umlDomainBlockPeek): Put "%s" before each literal format string with no % directive. * src/openvz_conf.c (openvzGetNodeCPUs): Likewise. --- src/openvz_conf.c | 2 +- src/uml_driver.c | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/openvz_conf.c b/src/openvz_conf.c index fc4ef20..1361c53 100644 --- a/src/openvz_conf.c +++ b/src/openvz_conf.c @@ -466,7 +466,7 @@ openvzGetNodeCPUs(void) if (virNodeInfoPopulate(NULL, &nodeinfo) < 0) { openvzError(NULL, VIR_ERR_INTERNAL_ERROR, - _("Cound not read nodeinfo")); + "%s", _("Cound not read nodeinfo")); return 0; } diff --git a/src/uml_driver.c b/src/uml_driver.c index d2f2c7b..8d33c70 100644 --- a/src/uml_driver.c +++ b/src/uml_driver.c @@ -1740,13 +1740,13 @@ umlDomainBlockPeek (virDomainPtr dom, if (!vm) { umlReportError (dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN, - _("no domain with matching uuid")); + "%s", _("no domain with matching uuid")); goto cleanup; } if (!path || path[0] == '\0') { umlReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_ARG, - _("NULL or empty path")); + "%s", _("NULL or empty path")); goto cleanup; } @@ -1783,7 +1783,7 @@ umlDomainBlockPeek (virDomainPtr dom, ret = 0; } else { umlReportError (dom->conn, dom, NULL, VIR_ERR_INVALID_ARG, - _("invalid path")); + "%s", _("invalid path")); } cleanup: -- 1.6.0.4.1044.g77718

1 0