- Devel - libvirt List Archives

Re: [libvirt] [PATCH] cygwin: fix abort of virsh on my system
by Stefan Berger 06 May '10

06 May '10

Stefan Berger/Watson/IBM wrote on 05/06/2010 02:55:31 PM: > Chris Lalancette <clalance(a)redhat.com> wrote on 05/06/2010 02:28:18 PM: > > > > On 05/06/2010 02:11 PM, Stefan Berger wrote: > > > This strange patch fixes aborts of virsh on my system. It seems that the > > > string returned by xmlSaveUri is owned by the xml library?? > > > [...] > > > > > So something else might be going on. What version of libxml2 > > are you currently building against? > > Cygwin picks up what I have installed in /cydrive/c/GTK/ and there I > have version 2.6.32. > > xmlversion.h:#define LIBXML_DOTTED_VERSION "2.6.32" > > The abort occurs upon the VIR_FREE(name). That was also the problem. Since the includes weren't missing I didn't have cygwin's libvirt-devel package installed and so it took the wrong includes... Stefan

1 0

[libvirt] [PATCH] cgroup: Enable memory.use_hierarchy of cgroup for domain
by Ryota Ozaki 06 May '10

06 May '10

Through conversation with Kumar L Srikanth-B22348, I found that the function of getting memory usage (e.g., virsh dominfo) doesn't work for lxc with ns subsystem of cgroup enabled. This is because of features of ns and memory subsystems. Ns creates child cgroup on every process fork and as a result processes in a container are not assigned in a cgroup for domain (e.g., libvirt/lxc/test1/). For example, libvirt_lxc and init (or somewhat specified in XML) are assigned into libvirt/lxc/test1/8839/ and libvirt/lxc/test1/8839/8849/, respectively. On the other hand, memory subsystem accounts memory usage within a group of processes by default, i.e., it does not take any child (and descendent) groups into account. With the two features, virsh dominfo which just checks memory usage of a cgroup for domain always returns zero because the cgroup has no process. Setting memory.use_hierarchy of a group allows to account (and limit) memory usage of every descendent groups of the group. By setting it of a cgroup for domain, we can get proper memory usage of lxc with ns subsystem enabled. (To be exact, the setting is required only when memory and ns subsystems are enabled at the same time, e.g., mount -t cgroup none /cgroup.) --- src/util/cgroup.c | 63 +++++++++++++++++++++++++++++++++++++++++++++++++--- 1 files changed, 59 insertions(+), 4 deletions(-) diff --git a/src/util/cgroup.c b/src/util/cgroup.c index b8b2eb5..f7d6b41 100644 --- a/src/util/cgroup.c +++ b/src/util/cgroup.c @@ -443,7 +443,38 @@ static int virCgroupCpuSetInherit(virCgroupPtr parent, virCgroupPtr group) return rc; } -static int virCgroupMakeGroup(virCgroupPtr parent, virCgroupPtr group, int create) +static int virCgroupSetMemoryUseHierarchy(virCgroupPtr group) +{ + int rc = 0; + unsigned long long value; + const char *filename = "memory.use_hierarchy"; + + rc = virCgroupGetValueU64(group, + VIR_CGROUP_CONTROLLER_MEMORY, + filename, &value); + if (rc != 0) { + VIR_ERROR("Failed to read %s/%s (%d)", group->path, filename, rc); + return rc; + } + + /* Setting twice causes error, so if already enabled, skip setting */ + if (value == 1) + return 0; + + VIR_DEBUG("Setting up %s/%s", group->path, filename); + rc = virCgroupSetValueU64(group, + VIR_CGROUP_CONTROLLER_MEMORY, + filename, 1); + + if (rc != 0) { + VIR_ERROR("Failed to set %s/%s (%d)", group->path, filename, rc); + } + + return rc; +} + +static int virCgroupMakeGroup(virCgroupPtr parent, virCgroupPtr group, + int create, int memory_hierarchy) { int i; int rc = 0; @@ -477,6 +508,20 @@ static int virCgroupMakeGroup(virCgroupPtr parent, virCgroupPtr group, int creat break; } } + /* + * Note that virCgroupSetMemoryUseHierarchy should always be + * called prior to creating subcgroups and attaching tasks. + */ + if (memory_hierarchy && + group->controllers[VIR_CGROUP_CONTROLLER_MEMORY].mountPoint != NULL && + (i == VIR_CGROUP_CONTROLLER_MEMORY || + STREQ(group->controllers[i].mountPoint, group->controllers[VIR_CGROUP_CONTROLLER_MEMORY].mountPoint))) { + rc = virCgroupSetMemoryUseHierarchy(group); + if (rc != 0) { + VIR_FREE(path); + break; + } + } } VIR_FREE(path); @@ -553,7 +598,7 @@ static int virCgroupAppRoot(int privileged, if (rc != 0) goto cleanup; - rc = virCgroupMakeGroup(rootgrp, *group, create); + rc = virCgroupMakeGroup(rootgrp, *group, create, 0); cleanup: virCgroupFree(&rootgrp); @@ -653,7 +698,7 @@ int virCgroupForDriver(const char *name, VIR_FREE(path); if (rc == 0) { - rc = virCgroupMakeGroup(rootgrp, *group, create); + rc = virCgroupMakeGroup(rootgrp, *group, create, 0); if (rc != 0) virCgroupFree(group); } @@ -703,7 +748,17 @@ int virCgroupForDomain(virCgroupPtr driver, VIR_FREE(path); if (rc == 0) { - rc = virCgroupMakeGroup(driver, *group, create); + /* + * Create a cgroup with memory.use_hierarchy enabled to + * surely account memory usage of lxc with ns subsystem + * enabled. (To be exact, memory and ns subsystems are + * enabled at the same time.) + * + * The reason why doing it here, not a upper group, say + * a group for driver, is to avoid overhead to track + * cumulative usage that we don't need. + */ + rc = virCgroupMakeGroup(driver, *group, create, 1); if (rc != 0) virCgroupFree(group); } -- 1.6.5.2

1 0

[libvirt] [PATCH] cgroup: Enable memory.use_hierarchy of cgroup for domain
by Ryota Ozaki 06 May '10

06 May '10

Through conversation with Kumar L Srikanth-B22348, I found that the function of getting memory usage (e.g., virsh dominfo) doesn't work for lxc with ns subsystem of cgroup enabled. This is because of features of ns and memory subsystems. Ns creates child cgroup on every process fork and as a result processes in a container are not assigned in a cgroup for domain (e.g., libvirt/lxc/test1/). For example, libvirt_lxc and init (or somewhat specified in XML) are assigned into libvirt/lxc/test1/8839/ and libvirt/lxc/test1/8839/8849/, respectively. On the other hand, memory subsystem accounts memory usage within a group of processes by default, i.e., it does not take any child (and descendent) groups into account. With the two features, virsh dominfo which just checks memory usage of a cgroup for domain always returns zero because the cgroup has no process. Setting memory.use_hierarchy of a group allows to account (and limit) memory usage of every descendent groups of the group. By setting it of a cgroup for domain, we can get proper memory usage of lxc with ns subsystem enabled. (To be exact, the setting is required only when memory and ns subsystems are enabled at the same time, e.g., mount -t cgroup none /cgroup.) --- src/util/cgroup.c | 49 +++++++++++++++++++++++++++++++++++++++++++++---- 1 files changed, 45 insertions(+), 4 deletions(-) diff --git a/src/util/cgroup.c b/src/util/cgroup.c index b8b2eb5..93cd6a9 100644 --- a/src/util/cgroup.c +++ b/src/util/cgroup.c @@ -443,7 +443,38 @@ static int virCgroupCpuSetInherit(virCgroupPtr parent, virCgroupPtr group) return rc; } -static int virCgroupMakeGroup(virCgroupPtr parent, virCgroupPtr group, int create) +static int virCgroupSetMemoryUseHierarchy(virCgroupPtr group) +{ + int rc = 0; + unsigned long long value; + const char *filename = "memory.use_hierarchy"; + + rc = virCgroupGetValueU64(group, + VIR_CGROUP_CONTROLLER_MEMORY, + filename, &value); + if (rc != 0) { + VIR_ERROR("Failed to read %s/%s (%d)", group->path, filename, rc); + return rc; + } + + /* Setting twice causes error, so if already enabled, skip setting */ + if (value == 1) + return 0; + + VIR_DEBUG("Setting up %s/%s", group->path, filename); + rc = virCgroupSetValueU64(group, + VIR_CGROUP_CONTROLLER_MEMORY, + filename, 1); + + if (rc != 0) { + VIR_ERROR("Failed to set %s/%s (%d)", group->path, filename, rc); + } + + return rc; +} + +static int virCgroupMakeGroup(virCgroupPtr parent, virCgroupPtr group, + int create, int memory_hierarchy) { int i; int rc = 0; @@ -477,6 +508,16 @@ static int virCgroupMakeGroup(virCgroupPtr parent, virCgroupPtr group, int creat break; } } + if (memory_hierarchy && + group->controllers[VIR_CGROUP_CONTROLLER_MEMORY].mountPoint != NULL && + (i == VIR_CGROUP_CONTROLLER_MEMORY || + STREQ(group->controllers[i].mountPoint, group->controllers[VIR_CGROUP_CONTROLLER_MEMORY].mountPoint))) { + rc = virCgroupSetMemoryUseHierarchy(group); + if (rc != 0) { + VIR_FREE(path); + break; + } + } } VIR_FREE(path); @@ -553,7 +594,7 @@ static int virCgroupAppRoot(int privileged, if (rc != 0) goto cleanup; - rc = virCgroupMakeGroup(rootgrp, *group, create); + rc = virCgroupMakeGroup(rootgrp, *group, create, 0); cleanup: virCgroupFree(&rootgrp); @@ -653,7 +694,7 @@ int virCgroupForDriver(const char *name, VIR_FREE(path); if (rc == 0) { - rc = virCgroupMakeGroup(rootgrp, *group, create); + rc = virCgroupMakeGroup(rootgrp, *group, create, 0); if (rc != 0) virCgroupFree(group); } @@ -703,7 +744,7 @@ int virCgroupForDomain(virCgroupPtr driver, VIR_FREE(path); if (rc == 0) { - rc = virCgroupMakeGroup(driver, *group, create); + rc = virCgroupMakeGroup(driver, *group, create, 1); if (rc != 0) virCgroupFree(group); } -- 1.6.5.2

3 3

[libvirt] [PATCH] util: fix va_start usage bug
by Eric Blake 06 May '10

06 May '10

Detected by clang. POSIX requires that the second argument to va_start be the name of the last variable; and in some implementations, passing *path instead of path would dereference bogus memory instead of pulling arguments off the stack. * src/util/util.c (virBuildPathInternal): Use correct argument to va_start. --- I think this falls under the trivial rule, as it silences a compiler warning and is a one-line fix of a real bug, so I pushed it. src/util/util.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/util/util.c b/src/util/util.c index 2d32952..c44d012 100644 --- a/src/util/util.c +++ b/src/util/util.c @@ -2799,7 +2799,7 @@ int virBuildPathInternal(char **path, ...) va_list ap; int ret = 0; - va_start(ap, *path); + va_start(ap, path); path_component = va_arg(ap, char *); virBufferAdd(&buf, path_component, -1); -- 1.6.6.1

2 1

[libvirt] Libvirtd in listen mode.
by Kumar L Srikanth-B22348 06 May '10

06 May '10

Hi, If I start the libvirtd service using the command "service libvirtd start", it is not starting in listen mode. I am stopping the libvirtd service, and then starting in listen mode externally, using the following command: "libvirtd -listen". But, how can I start libvirtd service in listen mode by default (when I issue the command "service libvirtd start")? Regards, Srikanth.

2 1

[libvirt] [PATCH] build: silence a clang false positive
by Eric Blake 05 May '10

05 May '10

* src/qemu/qemu_monitor.c (qemuMonitorIOWriteWithFD): Work around recent clang shortcoming in analysis. --- False positive encountered with the latest F-13 clang :( src/qemu/qemu_monitor.c | 5 ++++- 1 files changed, 4 insertions(+), 1 deletions(-) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 443d216..abf1338 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -1,7 +1,7 @@ /* * qemu_monitor.c: interaction with QEMU monitor console * - * Copyright (C) 2006-2009 Red Hat, Inc. + * Copyright (C) 2006-2010 Red Hat, Inc. * Copyright (C) 2006 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -374,6 +374,9 @@ qemuMonitorIOWriteWithFD(qemuMonitorPtr mon, msg.msg_controllen = sizeof(control); cmsg = CMSG_FIRSTHDR(&msg); + /* Some static analyzers, like clang 2.6-0.6.pre2, fail to see + that our use of CMSG_FIRSTHDR will not return NULL. */ + sa_assert(cmsg); cmsg->cmsg_len = CMSG_LEN(sizeof(int)); cmsg->cmsg_level = SOL_SOCKET; cmsg->cmsg_type = SCM_RIGHTS; -- 1.6.6.1

2 2

[libvirt] [PATCH] cgroup: Change virCgroupRemove to remove all child groups at first
by Ryota Ozaki 05 May '10

05 May '10

As same as normal directories, a cgroup cannot be removed if it contains sub groups. This patch changes virCgroupRemove to remove all child groups (subdirectories) of a target group before removing the target group. The handling is required when we run lxc with ns subsystem of cgroup. Ns subsystem automatically creates child cgroups on every process forks, but unfortunately the groups are not removed on process exits, so we have to remove them by ourselves. With this patch, such child groups are surely removed at lxc shutdown, i.e., lxcVmCleanup which calls virCgroupRemove. --- src/util/cgroup.c | 48 ++++++++++++++++++++++++++++++++++++++++++++---- 1 files changed, 44 insertions(+), 4 deletions(-) diff --git a/src/util/cgroup.c b/src/util/cgroup.c index b8b2eb5..531c131 100644 --- a/src/util/cgroup.c +++ b/src/util/cgroup.c @@ -23,6 +23,7 @@ #include <sys/stat.h> #include <sys/types.h> #include <libgen.h> +#include <dirent.h> #include "internal.h" #include "util.h" @@ -561,11 +562,52 @@ cleanup: } #endif +static int virCgroupRemoveRecursively(char *grppath) +{ + DIR *grpdir; + struct dirent *ent; + int rc = 0; + + grpdir = opendir(grppath); + if (grpdir == NULL) { + VIR_ERROR("Unable to open %s (%d)", grppath, errno); + rc = -errno; + return rc; + } + + while ((ent = readdir(grpdir)) != NULL) { + char path[PATH_MAX]; + int ret; + + if (ent->d_name[0] == '.') continue; + if (ent->d_type != DT_DIR) continue; + + ret = snprintf(path, sizeof(path), "%s/%s", grppath, ent->d_name); + if (ret < 0) + break; + rc = virCgroupRemoveRecursively(path); + if (rc != 0) + break; + } + DEBUG("Removing cgroup %s", grppath); + if (rmdir(grppath) != 0 && errno != ENOENT) { + rc = -errno; + VIR_ERROR("Unable to remove %s (%d)", grppath, errno); + } + + return rc; +} + /** * virCgroupRemove: * * @group: The group to be removed * + * It first removes all child groups recursively + * in depth first order and then removes @group + * because the presence of the child groups + * prevents removing @group. + * * Returns: 0 on success */ int virCgroupRemove(virCgroupPtr group) @@ -585,10 +627,8 @@ int virCgroupRemove(virCgroupPtr group) &grppath) != 0) continue; - DEBUG("Removing cgroup %s", grppath); - if (rmdir(grppath) != 0 && errno != ENOENT) { - rc = -errno; - } + DEBUG("Removing cgroup %s and all child cgroups", grppath); + rc = virCgroupRemoveRecursively(grppath); VIR_FREE(grppath); } -- 1.6.5.2

1 0

[libvirt] [PATCH v2] rpmbuild: add ebtables & ip(6)tables dependency for rpm
by Stefan Berger 05 May '10

05 May '10

Add ebtables,iptables & iptables-ipv6 dependency to rpm. Changes from V1 to V2: -passing --without-libpcap to configure script, if libpcap is not to be used Signed-off-by: Stefan Berger <stefanb(a)us.ibm.com> --- libvirt.spec.in | 7 +++++++ 1 file changed, 7 insertions(+) Index: libvirt-acl/libvirt.spec.in =================================================================== --- libvirt-acl.orig/libvirt.spec.in +++ libvirt-acl/libvirt.spec.in @@ -61,6 +61,7 @@ %define with_udev 0%{!?_without_udev:0} %define with_hal 0%{!?_without_hal:0} %define with_yajl 0%{!?_without_yajl:0} +%define with_nwfilter 0%{!?_without_nwfilter:0} %define with_libpcap 0%{!?_without_libpcap:0} # Non-server/HV driver defaults which are always enabled @@ -150,6 +151,7 @@ # Enable libpcap library %if %{with_qemu} +%define with_nwfilter 0%{!?_without_nwfilter:%{server_drivers}} %define with_libpcap 0%{!?_without_libpcap:%{server_drivers}} %endif @@ -195,6 +197,11 @@ Requires: bridge-utils Requires: dnsmasq Requires: iptables %endif +%if %{with_nwfilter} +Requires: ebtables +Requires: iptables +Requires: iptables-ipv6 +%endif # needed for device enumeration %if %{with_hal} Requires: hal @@ -517,6 +524,10 @@ of recent versions of Linux (and other O %define _without_yajl --without-yajl %endif +%if ! %{with_libpcap} +%define _without_libpcap --without-libpcap +%endif + %configure %{?_without_xen} \ %{?_without_qemu} \ %{?_without_openvz} \ @@ -545,6 +556,7 @@ of recent versions of Linux (and other O %{?_without_hal} \ %{?_without_udev} \ %{?_without_yajl} \ + %{?_without_libpcap} \ --with-qemu-user=%{qemu_user} \ --with-qemu-group=%{qemu_group} \ --with-init-script=redhat \

2 2

[libvirt] [PATCH] rpmbuild: add ebtables & ip(6)tables dependency for rpm
by Stefan Berger 05 May '10

05 May '10

Add ebtables,iptables & iptables-ipv6 dependency to rpm. Signed-off-by: Stefan Berger <stefanb(a)us.ibm.com> --- libvirt.spec.in | 7 +++++++ 1 file changed, 7 insertions(+) Index: libvirt-acl/libvirt.spec.in =================================================================== --- libvirt-acl.orig/libvirt.spec.in +++ libvirt-acl/libvirt.spec.in @@ -61,6 +61,7 @@ %define with_udev 0%{!?_without_udev:0} %define with_hal 0%{!?_without_hal:0} %define with_yajl 0%{!?_without_yajl:0} +%define with_nwfilter 0%{!?_without_nwfilter:0} %define with_libpcap 0%{!?_without_libpcap:0} # Non-server/HV driver defaults which are always enabled @@ -150,6 +151,7 @@ # Enable libpcap library %if %{with_qemu} +%define with_nwfilter 0%{!?_without_nwfilter:%{server_drivers}} %define with_libpcap 0%{!?_without_libpcap:%{server_drivers}} %endif @@ -195,6 +197,11 @@ Requires: bridge-utils Requires: dnsmasq Requires: iptables %endif +%if %{with_nwfilter} +Requires: ebtables +Requires: iptables +Requires: iptables-ipv6 +%endif # needed for device enumeration %if %{with_hal} Requires: hal

3 3

[libvirt] libvirt with qemu-kvm, not recognizing NIC model
by Jonathan Hoover 05 May '10

05 May '10

Is this the right list for this question, or should I be elsewhere? I am trying to specify a network card "model type" of "pcnet" (to emulate vmware esxi's network card). No matter what I put for model type in my xml config file, it comes up as an Intel e1000. I ran "qemu-kvm -net nic,model=? /dev/null" and I got back a list of supported models including pcnet,e1000,virtio, and others as expected. No matter which I put in my xml file for /etc/libvirt/qemu/Symantec-bg.xml (a Symantec Brightmail Gateway virtual machine), I just get back that its an Intel card on boot (which doesn't work with Symantec BG). I am running Fedora 12, kernel is 2.6.31.6-166.fc12.x86_64. The version of libvirt is 0.7.1. I am running it on an Intel Core 2 Quad CPU Q8400 @ 2.66 GHz. I have the same problem under Fedora 13 Beta. Thoughts? Jonathan Hoover

2 2

[libvirt] Alternative to XML Creation and Parsing?
by Ganesh Pagade 05 May '10

05 May '10

Hello, We plan to develop a fancy GUI which would help creating and managing VMs/Domains for RHEL 5.4 KVM. However looking at the schemas provided in docs/schemas/ (Ex: domain.rng) generating the XML file from the inputs taken from the GUI and pass it on to virDomainDefineXML() turns out to be a huge tasks in itself. I was wondering if there is an better way for me to provide my configurations to the library and also to read data from it (instead of creating, parsing XMLs)? If an alternate approach exist, are there any limitations with it? Also the schema given in docs/schemas/ would be generic for all hyper visor. How can I identify the tags applicable only for a particular hyper visor? I found: virConnectDomainXMLFromNative(virConnectPtr conn, const char * nativeFormat, const char * nativeConfig, unsigned int flags) in API reference, which could solve my first problem but couldn't figure out what nativeFormat would be. Any inputs/pointers would be highly appreciated. Regards, Ganesh

2 2

[libvirt] Fwd: Re: need your help about virito vmchannel
by Matthew Booth 04 May '10

04 May '10

FYI for the list. I haven't looked at this yet. Matt -------- Original Message -------- Subject: Re: need your help about virito vmchannel Date: Tue, 4 May 2010 18:00:02 +0530 From: Amit Shah <amit.shah(a)redhat.com> To: Matthew Booth <mbooth(a)redhat.com> CC: Jianlin Liu <jialiu(a)redhat.com> On (Tue) May 04 2010 [13:18:30], Matthew Booth wrote: > On 04/05/10 12:48, Jianlin Liu wrote: > > Hi Matthew, > > > > I want to create a virtio vm channel in my guest. So I add the followiong to my guest xml file: > > <channel type='pty'> > > <target type='virtio' name='org.linux-kvm.port.0'/> > > </channel> > > Jianlin, > > While I wrote the libvirt->vmchannel glue, I'm not actually that > familiar with vmchannel itself. I've cc'd Amit, who hopefully might > recognise this problem and tell me how to fix it ;) > > Amit, > > Any idea what's going on here? Yes: > > Then I try to start the guest: > > # virsh start winxp > > error: Failed to start domain winxp > > error: internal error Process exited while reading console log output: char device redirected to /dev/pts/4 > > char device redirected to /dev/pts/5 > > qemu-kvm: -device virtio-serial-pci,id=virtio-serial0,max_ports=0,vectors=0,bus=pci.0,addr=0x5: Device 'virtio-serial-pci' could not be initialized With max_ports=0, the virtio-serial device doesn't get created and hence this error. Amit

3 3

[libvirt] [PATCH] configure.ac: Avoid uname, which breaks cross-compilation
by Matthias Bolte 04 May '10

04 May '10

When cross-compiling on Linux, configure will misdetect the target as Linux because it uses uname instead of relying on the $host variable. This results in including libvirt_linux.syms into libvirt.syms and therefore trying to export undefined symbols. Replace uname checks with $host checks to fix this. --- configure.ac | 18 ++++++++++++++---- 1 files changed, 14 insertions(+), 4 deletions(-) diff --git a/configure.ac b/configure.ac index 6ee5b90..5d68dcc 100644 --- a/configure.ac +++ b/configure.ac @@ -210,8 +210,11 @@ if test "$prefix" = "/usr" && test "$sysconfdir" = '${prefix}/etc' ; then fi dnl lxc and qemu drivers require linux headers -if test `uname -s` != "Linux" -then +case "$host" in + *-*-linux*) + # match linux here so the *) case will match anything non-linux + ;; + *) if test "x$with_lxc" != "xyes" then with_lxc=no @@ -220,7 +223,8 @@ then then with_qemu=no fi -fi + ;; +esac dnl Allow to build without Xen, QEMU/KVM, test or remote driver AC_ARG_WITH([xen], @@ -1983,7 +1987,13 @@ then fi AM_CONDITIONAL([WITH_NODE_DEVICES], [test "$with_nodedev" = "yes"]) -AM_CONDITIONAL([WITH_LINUX], [test `uname -s` = "Linux"]) +with_linux=no +case "$host" in + *-*-linux*) + with_linux=yes + ;; +esac +AM_CONDITIONAL([WITH_LINUX], [test "$with_linux" = "yes"]) AC_ARG_WITH([qemu-user], -- 1.7.0.4

3 3

[libvirt] [PATCH] Don't wipe generated iface target in active domains
by jdenemar＠redhat.com 04 May '10

04 May '10

From: Jiri Denemark <jdenemar(a)redhat.com> Wipe generated interface target only when reading configuration of inactive domains. --- src/conf/domain_conf.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 546ddf2..3e45f79 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1889,7 +1889,8 @@ virDomainNetDefParseXML(virCapsPtr caps, xmlStrEqual(cur->name, BAD_CAST "target")) { ifname = virXMLPropString(cur, "dev"); if ((ifname != NULL) && - ((STRPREFIX((const char*)ifname, "vnet")) || + (((flags & VIR_DOMAIN_XML_INACTIVE) && + (STRPREFIX((const char*)ifname, "vnet"))) || (!isValidIfname(ifname)))) { /* An auto-generated target name, blank it out */ /* blank out invalid interface names */ -- 1.7.1

3 2

[libvirt] [PATCH] Various fixes for the spec file
by Daniel Veillard 04 May '10

04 May '10

This includes various things: - fix the Requires: libvirt-client to use %{name} to allow easy renaming - when building ESX support one need libcurl-devel - remove Makefile[.in] from xml/nwfilter in the docs, as this breaks parallel install ation of i686 and x86_64 packages - don't include nwfilter config files if not building with the daemon all relatively trivial which is why I packed them together, Daniel diff --git a/libvirt.spec.in b/libvirt.spec.in index 47e3050..9045c9a 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -184,7 +184,7 @@ URL: http://libvirt.org/ BuildRequires: python-devel # The client side, i.e. shared libs and virsh are in a subpackage -Requires: libvirt-client = %{version}-%{release} +Requires: %{name}-client = %{version}-%{release} # Used by many of the drivers, so turn it on whenever the # daemon is present @@ -347,6 +347,9 @@ BuildRequires: libssh2-devel %if %{with_netcf} BuildRequires: netcf-devel >= 0.1.4 %endif +%if %{with_esx} +BuildRequires: libcurl-devel +%endif # Fedora build root suckage BuildRequires: gawk @@ -378,7 +381,7 @@ virtualization capabilities of recent versions of Linux (and other OSes). %package devel Summary: Libraries, includes, etc. to compile with the libvirt library Group: Development/Libraries -Requires: libvirt-client = %{version}-%{release} +Requires: %{name}-client = %{version}-%{release} Requires: pkgconfig %if %{with_xen} Requires: xen-devel @@ -392,7 +395,7 @@ the virtualization capabilities of recent versions of Linux (and other OSes). %package python Summary: Python bindings for the libvirt library Group: Development/Libraries -Requires: libvirt-client = %{version}-%{release} +Requires: %{name}-client = %{version}-%{release} %description python The libvirt-python package contains a module that permits applications @@ -556,7 +559,7 @@ gzip -9 ChangeLog rm -fr %{buildroot} %makeinstall -for i in domain-events/events-c dominfo domsuspend hellolibvirt python +for i in domain-events/events-c dominfo domsuspend hellolibvirt python xml/nwfilter do (cd examples/$i ; make clean ; rm -rf .deps .libs Makefile Makefile.in) done @@ -600,6 +603,10 @@ rm -rf $RPM_BUILD_ROOT%{_datadir}/doc/libvirt-python-%{version} rm -rf $RPM_BUILD_ROOT%{_datadir}/doc/libvirt-%{version} %endif +%if ! %{with_libvirtd} +rm -rf $RPM_BUILD_ROOT%{_sysconfdir}/libvirt/nwfilter +%endif + %if ! %{with_qemu} rm -rf $RPM_BUILD_ROOT%{_sysconfdir}/libvirt/qemu.conf rm -rf $RPM_BUILD_ROOT%{_sysconfdir}/logrotate.d/libvirtd.qemu -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

2 1

[libvirt] why we should use a mechanical indentation checker
by Jim Meyering 04 May '10

04 May '10

I introduced a bug with this supposedly-safe, no-semantic-change delta: http://libvirt.org/git/?p=libvirt.git;a=commitdiff;h=b6719eab9e95c5daeeea85 See if you can spot it. Here is the loop in question, as it was prior to my erroneous change: for (i = 0; i < nruleInstances; i++) switch (inst[i]->ruleType) { case RT_EBTABLES: ebiptablesInstCommand(&buf, inst[i]->commandTemplate, 'A', -1, 1); break; case RT_IPTABLES: haveIptables = true; break; case RT_IP6TABLES: haveIp6tables = true; break; } Its formatting is WRONG. Sure, it is technically correct to omit the curly braces when the for-loop body is a single *statement*, as it is above. However, the moment the construct in the body occupies more than one line, it is very risky to omit the curly braces. I demonstrated that yesterday by inserting a no-op sa_assert statement as the "first" line of that loop body, not noticing that unlike my other two additions, this one pushed the single-stmt body *out of the loop*. Nasty. For example, do not omit the curly braces even when the body is just a single-line statement but with a preceding comment. while (true) /* explanation... */ BAD! single_line_stmt (); while (true) { /* Always put braces around a multi-line body. */ /* explanation... */ single_line_stmt (); } Of course do not do this, either: if (expr) while (other_expr) { BAD! ... } Do this, instead: if (expr) { while (other_expr) { ... } } I am adding something like the following to coreutils' HACKING, and will add the equivalent (adjusting indentation/brace-positioning style in the examples) to libvirt's hacking.html.in, so if you object, speak up soon. While writing the above and below, I noticed that with the GNU formatting style, this is less of a problem, since there you do not add those oh-so-important-to-semantics curly braces at the *end* of a line where it easy to miss them, but at the beginning. In addition, with the GNU style, adding the opening curly brace changes the indentation level of the body, while in libvirt's style, it does not. In either case, an automatic indentation checker would catch the problem. This is yet another reason to enforce an indentation style. The longer we wait, the harder it will become. Note: these diffs are relative to coreutils' HACKING file, not libvirt's: diff --git a/HACKING b/HACKING index 124c666..4a6d593 100644 --- a/HACKING +++ b/HACKING @@ -233,6 +233,110 @@ Try to make the summary line fit one of the following forms: maint: change-description +Curly braces: use judiciously +============================= +Omit the curly braces around an "if", "while", "for" etc. body only when +that body occupies a single line. In every other case we require the braces. +This ensures that it is trivially easy to identify a single-*statement* loop: +each has only one *line* in its body. + +For example, do not omit the curly braces even when the body is just a +single-line statement but with a preceding comment. + +Omitting braces with a single-line body is fine: + + while (expr) + single_line_stmt (); + +However, the moment your loop body extends onto a second line, for +whatever reason (even if it's just an added comment), then you should +add braces. Otherwise, it would be too easy to insert a statement just +before that comment (without adding braces), thinking it is already a +multi-statement loop: + + while (true) + /* comment... */ // BAD: multi-line body without braces + single_line_stmt (); + +Do this instead: + + while (true) + { /* Always put braces around a multi-line body. */ + /* explanation... */ + single_line_stmt (); + } + +There is one exception: when the second body line is not +at the same indentation level as the first body line. + + if (expr) + error (0, 0, _("a diagnostic that would make this line" + " extend past the 80-column limit")); + +It seems safe not to require curly braces in this case, +since the further-indented second body line makes it obvious +that this is still a single-statement body. + +To reiterate, don't do this: + + if (expr) + while (expr_2) // BAD: multi-line body without braces + { + ... + } + +Do this, instead: + + if (expr) + { + while (expr_2) + { + ... + } + } + +However, there is one exception in the other direction, when +even a one-line block should have braces. +That occurs when that one-line, brace-less block +is an "else" block, and the corresponding "then" block *does* use braces. +In that case, either put braces around the "else" block, or negate the +"if"-condition and swap the bodies, putting the one-line block first +and making the longer, multi-line block be the "else" block. + + if (expr) + { + ... + ... + } + else + x = y; // BAD: braceless "else" with braced "then" + +This is preferred, especially when the multi-line body is more +than a few lines long, because it is easier to read and grasp +the semantics of an if-then-else block when the simpler block +occurs first, rather than after the more involved block: + + if (!expr) + x = y; /* more readable */ + else + { + ... + ... + } + +If you'd rather not negate the condition, then add braces: + + if (expr) + { + ... + ... + } + else + { + x = y; + } + + Use SPACE-only indentation in all[*] files ========================================== We use space-only indentation in nearly all files.

2 3

[libvirt] [PATCH 0/3] lxc: Fix domain lookup and error handling
by jdenemar＠redhat.com 04 May '10

04 May '10

From: Jiri Denemark <jdenemar(a)redhat.com> Jiri Denemark (3): lxc: Use virDomainFindByUUID for domain lookup lxc: Make SetMemory work for active domains only lxc: Check domain is active/inactive as required by operation src/lxc/lxc_driver.c | 138 ++++++++++++++++++++++++++++++++++---------------- 1 files changed, 94 insertions(+), 44 deletions(-)

4 10

[libvirt] qemu-kvm spending time in do_info_migrate() during virDomainSave(); 50ms polling too fast?
by Charles Duffy 04 May '10

04 May '10

...or more particularly, in ram_bytes_remaining() called by do_info_migrate(); oddly, this is much more pronounced when running with <emulator> pointing at a shim prepending -no-kvm-irqchip to the invoked command line. This VM was intended to be paused for the save event (if my software was doing its job correctly), so we shouldn't be spending time running the guest CPU and writing updates for already-once-written blocks. I'm seeing much more CPU time spent inside qemu-kvm than in the exec'd lzop process compressing and writing the data stream; on attaching gdb and taking some stack traces to sample where execution time was spent, it appeared that we were spending our time responding to requests from the monitor. The question then -- is the 50ms poll in qemuDomainWaitForMigrationComplete (called from qemudDomainSave) perhaps too frequent? Thanks! --- Below is an exchange from IRC: <nDuff> How often should libvirt be calling "info migrate" during a virDomainSave (of a qemu domain)? * nDuff is seeing his qemu-kvm spending the bulk of its time inside ram_bytes_remaining() under do_info_migrate(). <DV> nDuff: I doubt libvirt is doing this on his own, something else is asking for the information I would assume <nDuff> DV, I'm not running virt-manager or such; the only management layer on top is locally developed, and it only has a single thread that's blocked waiting for the dom.save() call [this is using the Python bindings] to complete. <DV> interesting <DV> nDuff -> send this to the list, someone need to look at it, at least raise the problem, maybe we didn't expected that to be so costly <DV> nDuff: maybe open a bugzilla <nDuff> it might be that it's not _usually_ so costly except that I'm hitting a qemu/kvm bug; it only started expressing itself when I added -no-kvm-irqchip to the commandline via a shim <DV> nDuff: I could see how trying to extract this too often could stall the migration process <nDuff> ...but yes, I'll post to the list. <DV> nDuff: maybe it's related to the capability to force migration end when the full flush will be shorter than a user defined limit

3 3

[libvirt] [PATCH] nwfilter: skip interfaces on filter update
by Stefan Berger 04 May '10

04 May '10

When a filter is updated, only those interfaces must have their old rules cleared that either reference the filter directly or indirectly through another filter. Remember between the different steps of the instantiation of the filters which interfaces must be skipped. I am using a hash map to remember the names of the interfaces and store a bogus pointer to ~0 into it that need not be freed. Signed-off-by: Stefan Berger <stefanb(a)us.ibm.com> --- src/conf/nwfilter_conf.c | 8 ++++ src/conf/nwfilter_conf.h | 1 src/nwfilter/nwfilter_gentech_driver.c | 66 ++++++++++++++++++++++++--------- src/nwfilter/nwfilter_gentech_driver.h | 3 + 4 files changed, 59 insertions(+), 19 deletions(-) Index: libvirt-acl/src/conf/nwfilter_conf.c =================================================================== --- libvirt-acl.orig/src/conf/nwfilter_conf.c +++ libvirt-acl/src/conf/nwfilter_conf.c @@ -2088,8 +2088,14 @@ virNWFilterTriggerVMFilterRebuild(virCon .conn = conn, .err = 0, .step = STEP_APPLY_NEW, + .skipInterfaces = virHashCreate(0), }; + if (!cb.skipInterfaces) { + virReportOOMError(); + return 1; + } + for (i = 0; i < nCallbackDriver; i++) { callbackDrvArray[i]->vmFilterRebuild(conn, virNWFilterDomainFWUpdateCB, @@ -2115,6 +2121,8 @@ virNWFilterTriggerVMFilterRebuild(virCon &cb); } + virHashFree(cb.skipInterfaces, NULL); + return err; } Index: libvirt-acl/src/conf/nwfilter_conf.h =================================================================== --- libvirt-acl.orig/src/conf/nwfilter_conf.h +++ libvirt-acl/src/conf/nwfilter_conf.h @@ -471,6 +471,7 @@ struct domUpdateCBStruct { virConnectPtr conn; enum UpdateStep step; int err; + virHashTablePtr skipInterfaces; }; Index: libvirt-acl/src/nwfilter/nwfilter_gentech_driver.h =================================================================== --- libvirt-acl.orig/src/nwfilter/nwfilter_gentech_driver.h +++ libvirt-acl/src/nwfilter/nwfilter_gentech_driver.h @@ -40,7 +40,8 @@ enum instCase { int virNWFilterInstantiateFilter(virConnectPtr conn, const virDomainNetDefPtr net); int virNWFilterUpdateInstantiateFilter(virConnectPtr conn, - const virDomainNetDefPtr net); + const virDomainNetDefPtr net, + bool *skipIface); int virNWFilterRollbackUpdateFilter(virConnectPtr conn, const virDomainNetDefPtr net); Index: libvirt-acl/src/nwfilter/nwfilter_gentech_driver.c =================================================================== --- libvirt-acl.orig/src/nwfilter/nwfilter_gentech_driver.c +++ libvirt-acl/src/nwfilter/nwfilter_gentech_driver.c @@ -316,7 +316,7 @@ _virNWFilterInstantiateRec(virConnectPtr virNWFilterHashTablePtr vars, int *nEntries, virNWFilterRuleInstPtr **insts, - enum instCase useNewFilter, int *foundNewFilter, + enum instCase useNewFilter, bool *foundNewFilter, virNWFilterDriverStatePtr driver) { virNWFilterPoolObjPtr obj; @@ -381,7 +381,7 @@ _virNWFilterInstantiateRec(virConnectPtr case INSTANTIATE_FOLLOW_NEWFILTER: if (obj->newDef) { next_filter = obj->newDef; - *foundNewFilter = 1; + *foundNewFilter = true; } break; case INSTANTIATE_ALWAYS: @@ -562,7 +562,7 @@ virNWFilterInstantiate(virConnectPtr con int ifindex, const char *linkdev, virNWFilterHashTablePtr vars, - enum instCase useNewFilter, int *foundNewFilter, + enum instCase useNewFilter, bool *foundNewFilter, bool teardownOld, const unsigned char *macaddr, virNWFilterDriverStatePtr driver, @@ -693,7 +693,8 @@ __virNWFilterInstantiateFilter(virConnec virNWFilterHashTablePtr filterparams, enum instCase useNewFilter, virNWFilterDriverStatePtr driver, - bool forceWithPendingReq) + bool forceWithPendingReq, + bool *foundNewFilter) { int rc; const char *drvname = EBIPTABLES_DRIVER_ID; @@ -702,7 +703,6 @@ __virNWFilterInstantiateFilter(virConnec virNWFilterHashTablePtr vars, vars1; virNWFilterDefPtr filter; char vmmacaddr[VIR_MAC_STRING_BUFLEN] = {0}; - int foundNewFilter = 0; char *str_macaddr = NULL; const char *ipaddr; char *str_ipaddr = NULL; @@ -775,7 +775,7 @@ __virNWFilterInstantiateFilter(virConnec case INSTANTIATE_FOLLOW_NEWFILTER: if (obj->newDef) { filter = obj->newDef; - foundNewFilter = 1; + *foundNewFilter = true; } break; @@ -791,7 +791,7 @@ __virNWFilterInstantiateFilter(virConnec ifindex, linkdev, vars, - useNewFilter, &foundNewFilter, + useNewFilter, foundNewFilter, teardownOld, macaddr, driver, @@ -816,7 +816,8 @@ static int _virNWFilterInstantiateFilter(virConnectPtr conn, const virDomainNetDefPtr net, bool teardownOld, - enum instCase useNewFilter) + enum instCase useNewFilter, + bool *foundNewFilter) { const char *linkdev = (net->type == VIR_DOMAIN_NET_TYPE_DIRECT) ? net->data.direct.linkdev @@ -837,7 +838,8 @@ _virNWFilterInstantiateFilter(virConnect net->filterparams, useNewFilter, conn->nwfilterPrivateData, - false); + false, + foundNewFilter); } @@ -853,6 +855,8 @@ virNWFilterInstantiateFilterLate(virConn virNWFilterDriverStatePtr driver) { int rc; + bool foundNewFilter = false; + rc = __virNWFilterInstantiateFilter(conn, 1, ifname, @@ -864,7 +868,8 @@ virNWFilterInstantiateFilterLate(virConn filterparams, INSTANTIATE_ALWAYS, driver, - true); + true, + &foundNewFilter); if (rc) { //something went wrong... 'DOWN' the interface if (ifaceCheck(false, ifname, NULL, ifindex) != 0 || @@ -881,19 +886,29 @@ int virNWFilterInstantiateFilter(virConnectPtr conn, const virDomainNetDefPtr net) { + bool foundNewFilter = false; + return _virNWFilterInstantiateFilter(conn, net, 1, - INSTANTIATE_ALWAYS); + INSTANTIATE_ALWAYS, + &foundNewFilter); } int virNWFilterUpdateInstantiateFilter(virConnectPtr conn, - const virDomainNetDefPtr net) + const virDomainNetDefPtr net, + bool *skipIface) { - return _virNWFilterInstantiateFilter(conn, net, - 0, - INSTANTIATE_FOLLOW_NEWFILTER); + bool foundNewFilter = false; + + int rc = _virNWFilterInstantiateFilter(conn, net, + 0, + INSTANTIATE_FOLLOW_NEWFILTER, + &foundNewFilter); + + *skipIface = !foundNewFilter; + return rc; } int virNWFilterRollbackUpdateFilter(virConnectPtr conn, @@ -993,6 +1008,7 @@ virNWFilterDomainFWUpdateCB(void *payloa virDomainDefPtr vm = obj->def; struct domUpdateCBStruct *cb = data; int i; + bool skipIface; virDomainObjLock(obj); @@ -1003,15 +1019,29 @@ virNWFilterDomainFWUpdateCB(void *payloa switch (cb->step) { case STEP_APPLY_NEW: cb->err = virNWFilterUpdateInstantiateFilter(cb->conn, - net); + net, + &skipIface); + if (cb->err == 0 && skipIface == true) { + // filter tree unchanged -- no update needed + cb->err = virHashAddEntry(cb->skipInterfaces, + net->ifname, + (void *)~0); + if (cb->err) + virReportOOMError(); + } break; case STEP_TEAR_NEW: - cb->err = virNWFilterRollbackUpdateFilter(cb->conn, net); + if ( !virHashLookup(cb->skipInterfaces, net->ifname)) { + cb->err = virNWFilterRollbackUpdateFilter(cb->conn, + net); + } break; case STEP_TEAR_OLD: - cb->err = virNWFilterTearOldFilter(cb->conn, net); + if ( !virHashLookup(cb->skipInterfaces, net->ifname)) { + cb->err = virNWFilterTearOldFilter(cb->conn, net); + } break; } if (cb->err)

2 2

[libvirt] [PATCH] mingw: Fix two undefined symbols
by Matthias Bolte 04 May '10

04 May '10

Add an empty body for virCondWaitUntil and move virPipeReadUntilEOF out of the '#ifndef WIN32' block, because it compiles fine with MinGW in combination with gnulib. --- src/util/threads-win32.c | 10 +++ src/util/util.c | 160 +++++++++++++++++++++++----------------------- 2 files changed, 90 insertions(+), 80 deletions(-) diff --git a/src/util/threads-win32.c b/src/util/threads-win32.c index a30bccf..e478560 100644 --- a/src/util/threads-win32.c +++ b/src/util/threads-win32.c @@ -157,6 +157,16 @@ int virCondWait(virCondPtr c, virMutexPtr m) return 0; } +int virCondWaitUntil(virCondPtr c ATTRIBUTE_UNUSED, + virMutexPtr m ATTRIBUTE_UNUSED, + unsigned long long whenms ATTRIBUTE_UNUSED) +{ + /* FIXME: this function is currently only used by the QEMU driver that + * is not compiled on Windows, so it's okay for now to just + * miss an implementation */ + return -1; +} + void virCondSignal(virCondPtr c) { virMutexLock(&c->lock); diff --git a/src/util/util.c b/src/util/util.c index 3209185..e444c97 100644 --- a/src/util/util.c +++ b/src/util/util.c @@ -782,86 +782,6 @@ int virExecDaemonize(const char *const*argv, return ret; } -int -virPipeReadUntilEOF(int outfd, int errfd, - char **outbuf, char **errbuf) { - - struct pollfd fds[2]; - int i; - int finished[2]; - - fds[0].fd = outfd; - fds[0].events = POLLIN; - finished[0] = 0; - fds[1].fd = errfd; - fds[1].events = POLLIN; - finished[1] = 0; - - while(!(finished[0] && finished[1])) { - - if (poll(fds, ARRAY_CARDINALITY(fds), -1) < 0) { - if ((errno == EAGAIN) || (errno == EINTR)) - continue; - goto pollerr; - } - - for (i = 0; i < ARRAY_CARDINALITY(fds); ++i) { - char data[1024], **buf; - int got, size; - - if (!(fds[i].revents)) - continue; - else if (fds[i].revents & POLLHUP) - finished[i] = 1; - - if (!(fds[i].revents & POLLIN)) { - if (fds[i].revents & POLLHUP) - continue; - - virUtilError(VIR_ERR_INTERNAL_ERROR, - "%s", _("Unknown poll response.")); - goto error; - } - - got = read(fds[i].fd, data, sizeof(data)); - - if (got == 0) { - finished[i] = 1; - continue; - } - if (got < 0) { - if (errno == EINTR) - continue; - if (errno == EAGAIN) - break; - goto pollerr; - } - - buf = ((fds[i].fd == outfd) ? outbuf : errbuf); - size = (*buf ? strlen(*buf) : 0); - if (VIR_REALLOC_N(*buf, size+got+1) < 0) { - virReportOOMError(); - goto error; - } - memmove(*buf+size, data, got); - (*buf)[size+got] = '\0'; - } - continue; - - pollerr: - virReportSystemError(errno, - "%s", _("poll error")); - goto error; - } - - return 0; - -error: - VIR_FREE(*outbuf); - VIR_FREE(*errbuf); - return -1; -} - /** * @argv NULL terminated argv to run * @status optional variable to return exit status in @@ -1008,6 +928,86 @@ virExecDaemonize(const char *const*argv ATTRIBUTE_UNUSED, # endif /* WIN32 */ int +virPipeReadUntilEOF(int outfd, int errfd, + char **outbuf, char **errbuf) { + + struct pollfd fds[2]; + int i; + int finished[2]; + + fds[0].fd = outfd; + fds[0].events = POLLIN; + finished[0] = 0; + fds[1].fd = errfd; + fds[1].events = POLLIN; + finished[1] = 0; + + while(!(finished[0] && finished[1])) { + + if (poll(fds, ARRAY_CARDINALITY(fds), -1) < 0) { + if ((errno == EAGAIN) || (errno == EINTR)) + continue; + goto pollerr; + } + + for (i = 0; i < ARRAY_CARDINALITY(fds); ++i) { + char data[1024], **buf; + int got, size; + + if (!(fds[i].revents)) + continue; + else if (fds[i].revents & POLLHUP) + finished[i] = 1; + + if (!(fds[i].revents & POLLIN)) { + if (fds[i].revents & POLLHUP) + continue; + + virUtilError(VIR_ERR_INTERNAL_ERROR, + "%s", _("Unknown poll response.")); + goto error; + } + + got = read(fds[i].fd, data, sizeof(data)); + + if (got == 0) { + finished[i] = 1; + continue; + } + if (got < 0) { + if (errno == EINTR) + continue; + if (errno == EAGAIN) + break; + goto pollerr; + } + + buf = ((fds[i].fd == outfd) ? outbuf : errbuf); + size = (*buf ? strlen(*buf) : 0); + if (VIR_REALLOC_N(*buf, size+got+1) < 0) { + virReportOOMError(); + goto error; + } + memmove(*buf+size, data, got); + (*buf)[size+got] = '\0'; + } + continue; + + pollerr: + virReportSystemError(errno, + "%s", _("poll error")); + goto error; + } + + return 0; + +error: + VIR_FREE(*outbuf); + VIR_FREE(*errbuf); + return -1; +} + +int virRun(const char *const*argv, int *status) { return virRunWithHook(argv, NULL, NULL, status); -- 1.7.0.4

2 2

[libvirt] [PATCH] configure.ac: avoid uname, which breaks cross-compilation
by Eric Blake 04 May '10

04 May '10

Without this patch, compiling to mingw with a $build on windows (think MSYS or cygwin) worked, but failed with a $build on Linux. * configure.ac: Fix cross-compilation from Linux to mingw. Reported by Matthias Bolte. --- configure.ac | 13 +++++++++---- 1 files changed, 9 insertions(+), 4 deletions(-) diff --git a/configure.ac b/configure.ac index 0fb8bd9..3225cfa 100644 --- a/configure.ac +++ b/configure.ac @@ -197,8 +197,8 @@ if test "$prefix" = "/usr" && test "$sysconfdir" = '${prefix}/etc' ; then fi dnl lxc and qemu drivers require linux headers -if test `uname -s` != "Linux" -then +case $host in + linux*) if test "x$with_lxc" != "xyes" then with_lxc=no @@ -207,7 +207,8 @@ then then with_qemu=no fi -fi + ;; +esac dnl Allow to build without Xen, QEMU/KVM, test or remote driver AC_ARG_WITH([xen], @@ -1970,7 +1971,11 @@ then fi AM_CONDITIONAL([WITH_NODE_DEVICES], [test "$with_nodedev" = "yes"]) -AM_CONDITIONAL([WITH_LINUX], [test `uname -s` = "Linux"]) +case $host in + linux*) with_linux=yes ;; + *) with_linux=no ;; +esac +AM_CONDITIONAL([WITH_LINUX], [test $with_linux = yes]) AC_ARG_WITH([qemu-user], -- 1.6.6.1

1 1

[libvirt] [PATCH] build: avoid compiler warning
by Eric Blake 04 May '10

04 May '10

Necessary on cygwin, where uid_t and gid_t are 4-byte long rather than int, causing gcc -Wformat warnings. * src/util/util.c (virFileOperationNoFork, virDirCreateNoFork) (virFileOperation, virDirCreate, virGetUserEnt): Cast uid_t and gid_t before passing to printf. * .gitignore: Ignore cygwin executables. --- .gitignore | 1 + src/util/util.c | 21 ++++++++++++--------- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/.gitignore b/.gitignore index 8c275f4..4faf384 100644 --- a/.gitignore +++ b/.gitignore @@ -1,6 +1,7 @@ *#*# *.#*# *.a +*.exe *.o *.orig *.rej diff --git a/src/util/util.c b/src/util/util.c index 3209185..0c5a22b 100644 --- a/src/util/util.c +++ b/src/util/util.c @@ -74,11 +74,14 @@ #include "util.h" #include "memory.h" #include "threads.h" +#include "verify.h" #ifndef NSIG # define NSIG 32 #endif +verify(sizeof (gid_t) <= sizeof (int) && sizeof (uid_t) <= sizeof (int)); + #define VIR_FROM_THIS VIR_FROM_NONE #define virUtilError(code, ...) \ @@ -1277,7 +1280,7 @@ static int virFileOperationNoFork(const char *path, int openflags, mode_t mode, && (fchown(fd, uid, gid) < 0)) { ret = errno; virReportSystemError(errno, _("cannot chown '%s' to (%u, %u)"), - path, uid, gid); + path, (int) uid, (int) gid); goto error; } if ((flags & VIR_FILE_OP_FORCE_PERMS) @@ -1328,7 +1331,7 @@ static int virDirCreateNoFork(const char *path, mode_t mode, uid_t uid, gid_t gi && (chown(path, uid, gid) < 0)) { ret = errno; virReportSystemError(errno, _("cannot chown '%s' to (%u, %u)"), - path, uid, gid); + path, (int) uid, (int) gid); goto error; } if ((flags & VIR_DIR_CREATE_FORCE_PERMS) @@ -1407,14 +1410,14 @@ parenterror: ret = errno; virReportSystemError(errno, _("cannot set gid %u creating '%s'"), - gid, path); + (int) gid, path); goto childerror; } if ((uid != 0) && (setuid(uid) != 0)) { ret = errno; virReportSystemError(errno, _("cannot set uid %u creating '%s'"), - uid, path); + (int) uid, path); goto childerror; } if ((fd = open(path, openflags, mode)) < 0) { @@ -1436,7 +1439,7 @@ parenterror: && (fchown(fd, -1, gid) < 0)) { ret = errno; virReportSystemError(errno, _("cannot chown '%s' to (%u, %u)"), - path, uid, gid); + path, (int) uid, (int) gid); goto childerror; } if ((flags & VIR_FILE_OP_FORCE_PERMS) @@ -1517,13 +1520,13 @@ parenterror: if ((gid != 0) && (setgid(gid) != 0)) { ret = errno; virReportSystemError(errno, _("cannot set gid %u creating '%s'"), - gid, path); + (int) gid, path); goto childerror; } if ((uid != 0) && (setuid(uid) != 0)) { ret = errno; virReportSystemError(errno, _("cannot set uid %u creating '%s'"), - uid, path); + (int) uid, path); goto childerror; } if (mkdir(path, mode) < 0) { @@ -1547,7 +1550,7 @@ parenterror: ret = errno; virReportSystemError(errno, _("cannot chown '%s' to group %u"), - path, gid); + path, (int) gid); goto childerror; } if ((flags & VIR_DIR_CREATE_FORCE_PERMS) @@ -2564,7 +2567,7 @@ static char *virGetUserEnt(uid_t uid, if (getpwuid_r(uid, &pwbuf, strbuf, strbuflen, &pw) != 0 || pw == NULL) { virReportSystemError(errno, _("Failed to find user record for uid '%d'"), - uid); + (int) uid); VIR_FREE(strbuf); return NULL; } -- 1.6.6.1

2 5

[libvirt] [PATCH] dnsmasqReload: avoid mingw link failure
by Eric Blake 04 May '10

04 May '10

* src/util/dnsmasq.c (dnsmasqReload): Mingw lacks kill, but is not running a dnsmasq daemon either. --- I'm not as familiar with the mingw cross-build setup, but I finally got enough pieces installed on my F-12 machine that this was the only remaining compilation failure that I ran into. Let me know if we need an alternate patch that solves dnsmasq more gracefully than just crippling one function. src/util/dnsmasq.c | 4 +++- 1 files changed, 3 insertions(+), 1 deletions(-) diff --git a/src/util/dnsmasq.c b/src/util/dnsmasq.c index 1cb5f21..d6cef40 100644 --- a/src/util/dnsmasq.c +++ b/src/util/dnsmasq.c @@ -328,14 +328,16 @@ dnsmasqDelete(const dnsmasqContext *ctx) * Reloads all the configurations associated to a context */ int -dnsmasqReload(pid_t pid) +dnsmasqReload(pid_t pid ATTRIBUTE_UNUSED) { +#ifndef __MINGW32__ if (kill(pid, SIGHUP) != 0) { virReportSystemError(errno, _("Failed to make dnsmasq (PID: %d) reload config files.\n"), pid); return -1; } +#endif /* __MINGW32__ */ return 0; } -- 1.6.6.1

2 5

[libvirt] [PATCH] nwfilter: pass info where request stems from to have rules applied
by Stefan Berger 03 May '10

03 May '10

For the decision on whether to instantiate the rules, the check for a pending IP address learn request is not sufficient since then only the thread could instantiate the rules. So, a boolean needs to be passed when the thread instantiates the filter rules late and the IP address learn request is still pending in order to override the check for the pending learn request. If the rules are to be updated while the thread is active, this will not be done immediately but the thread will do that later on. Signed-off-by: Stefan Berger <stefanb(a)us.ibm.com> --- src/nwfilter/nwfilter_gentech_driver.c | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) Index: libvirt-acl/src/nwfilter/nwfilter_gentech_driver.c =================================================================== --- libvirt-acl.orig/src/nwfilter/nwfilter_gentech_driver.c +++ libvirt-acl/src/nwfilter/nwfilter_gentech_driver.c @@ -543,6 +543,8 @@ virNWFilterRuleInstancesToArray(int nEnt * @ifname: The name of the interface to apply the rules to * @vars: A map holding variable names and values used for instantiating * the filter and its subfilters. + * @forceWithPendingReq: Ignore the check whether a pending learn request + * is active; 'true' only when the rules are applied late * * Returns 0 on success, a value otherwise. * @@ -563,7 +565,8 @@ virNWFilterInstantiate(virConnectPtr con enum instCase useNewFilter, int *foundNewFilter, bool teardownOld, const unsigned char *macaddr, - virNWFilterDriverStatePtr driver) + virNWFilterDriverStatePtr driver, + bool forceWithPendingReq) { int rc; int j, nptrs; @@ -610,7 +613,8 @@ virNWFilterInstantiate(virConnectPtr con } else if (virHashSize(missing_vars->hashTable) > 1) { rc = 1; goto err_exit; - } else if (virNWFilterLookupLearnReq(ifindex) == NULL) { + } else if (!forceWithPendingReq && + virNWFilterLookupLearnReq(ifindex) != NULL) { goto err_exit; } @@ -688,7 +692,8 @@ __virNWFilterInstantiateFilter(virConnec const char *filtername, virNWFilterHashTablePtr filterparams, enum instCase useNewFilter, - virNWFilterDriverStatePtr driver) + virNWFilterDriverStatePtr driver, + bool forceWithPendingReq) { int rc; const char *drvname = EBIPTABLES_DRIVER_ID; @@ -789,7 +794,8 @@ __virNWFilterInstantiateFilter(virConnec useNewFilter, &foundNewFilter, teardownOld, macaddr, - driver); + driver, + forceWithPendingReq); virNWFilterHashTableFree(vars); @@ -830,7 +836,8 @@ _virNWFilterInstantiateFilter(virConnect net->filter, net->filterparams, useNewFilter, - conn->nwfilterPrivateData); + conn->nwfilterPrivateData, + false); } @@ -856,7 +863,8 @@ virNWFilterInstantiateFilterLate(virConn filtername, filterparams, INSTANTIATE_ALWAYS, - driver); + driver, + true); if (rc) { //something went wrong... 'DOWN' the interface if (ifaceCheck(false, ifname, NULL, ifindex) != 0 ||

2 1

[libvirt] [PATCH] cgroup: Fix possible memory leak in virCgroupMakeGroup
by Ryota Ozaki 03 May '10

03 May '10

* src/util/cgroup.c: free temporal path string before breaking loop --- src/util/cgroup.c | 4 +++- 1 files changed, 3 insertions(+), 1 deletions(-) diff --git a/src/util/cgroup.c b/src/util/cgroup.c index b649c3c..b8b2eb5 100644 --- a/src/util/cgroup.c +++ b/src/util/cgroup.c @@ -472,8 +472,10 @@ static int virCgroupMakeGroup(virCgroupPtr parent, virCgroupPtr group, int creat (i == VIR_CGROUP_CONTROLLER_CPUSET || STREQ(group->controllers[i].mountPoint, group->controllers[VIR_CGROUP_CONTROLLER_CPUSET].mountPoint))) { rc = virCgroupCpuSetInherit(parent, group); - if (rc != 0) + if (rc != 0) { + VIR_FREE(path); break; + } } } -- 1.6.5.2

2 1