- Devel - libvirt List Archives

[libvirt] iptables --physdev-out warnings
by Reinier Schoof 17 Jan '13

17 Jan '13

Hi, we've experienced some issues with starting lots of KVM based VM's with libvirt. Since I couldn't find any clues on the libvirt mailing list, I'm posting the way I fixed the issues. When starting a VM, /var/log/messages was spammed with the following message: xt_physdev: using --physdev-out in the OUTPUT, FORWARD and POSTROUTING chains for non-bridged traffic is not supported anymore. With each extra VM I start, the messages get amplified exponentially. This results in longer starting times every new VM, relative the the previously started VM. When I ran a test with starting 100 equal VM's, the first VM started in about 2 seconds, the 100th VM took 48 seconds to start. I'm running a vanilla 3.7.1 kernel, but I have the same issue on VM hosts with kernel 3.2.28 or 3.2.0, running libvirt 0.9.12 and 0.9.8 respectively. Looking into the warning, it seemed that iptables need an extra argument, --physdev-is-bridged, in commands like: iptables -A libvirt-out -m physdev --physdev-is-bridged --physdev-out vnet99 -g FP-vnet99 I patched the libvirt source (version 1.0.0) to test whether this works or not: --- src/nwfilter/nwfilter_ebiptables_driver.c.orig 2013-01-16 10:51:43.000000000 +0100 +++ src/nwfilter/nwfilter_ebiptables_driver.c 2013-01-16 10:52:07.000000000 +0100 @@ -166,7 +166,7 @@ snprintf(buf, sizeof(buf), "%c%c-%s", prefix[0], prefix[1], ifname) #define PHYSDEV_IN "--physdev-in" -#define PHYSDEV_OUT "--physdev-out" +#define PHYSDEV_OUT "--physdev-is-bridged --physdev-out" static const char *m_state_out_str = "-m state --state NEW,ESTABLISHED"; static const char *m_state_in_str = "-m state --state ESTABLISHED"; The warnings in /var/log/messages are gone and running the test again proved the 100th VM started in 3.8 seconds. It suprises me I'm the first to mention this problem on the libvirt mailing list and I wondering if I'm doing something wrong. Until then, this fix helps me a lot! Reinier Schoof -- TransIP BV | https://www.transip.nl/

2 1

[libvirt] [PATCH] Fix typo in variable name
by Guido Günther 16 Jan '13

16 Jan '13

Hi, I pushed that under the build breaker rule: that broke the build like: CC libvirt_conf_la-domain_conf.lo conf/domain_conf.c: In function 'virDomainVcpuPinAdd': conf/domain_conf.c:11920:29: error: 'vpcupin' undeclared (first use in this function) conf/domain_conf.c:11920:29: note: each undeclared identifier is reported only once for each function it appears in make[3]: *** [libvirt_conf_la-domain_conf.lo] Error 1 --- src/conf/domain_conf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 88865b5..13c14e9 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -11917,7 +11917,7 @@ int virDomainVcpuPinAdd(virDomainVcpuPinDefPtr **vcpupin_list, no_memory: virReportOOMError(); - virDomainVcpuPinDefFree(vpcupin); + virDomainVcpuPinDefFree(vcpupin); return -1; } -- 1.7.10.4

1 0

Re: [libvirt] Redefinition of struct in6_addr in <netinet/in.h> and <linux/in6.h>
by David Miller 16 Jan '13

16 Jan '13

From: Mike Frysinger <vapier(a)gentoo.org> Date: Wed, 16 Jan 2013 14:22:16 -0500 > On Wednesday 16 January 2013 13:59:59 David Miller wrote: >> This has been done for decades, wake up. > > and it's been broken for just as long. no need to be a dick. By being ignorant and having such a simplistic view of the situation, you're the one who's actually the dick.

1 0

[libvirt] [PATCH 0/2] add qemu machine type q35 support
by liguang 16 Jan '13

16 Jan '13

qemu-1.3 added machine type q35, changelog said: Added Intel Q35 chipset as a new machine type, '--machine q35'. Adds PCIe support. Requires an updated SeaBIOS (bios.bin), and '-acpitable file=/seabios-path/q35-acpi-dsdt.aml' to run. so add q35 support for libvirt. src/conf/device_conf.c | 8 +++++++- src/conf/device_conf.h | 1 + src/conf/domain_conf.c | 1 + src/conf/domain_conf.h | 1 + src/qemu/qemu_capabilities.c | 11 +++++++++++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 8 +++- 7 files changed, 29 insertions(+), 2 deletions(-)

7 12

[libvirt] [PATCH] safe{read,write}: Don't lie on nonblocking FD
by Michal Privoznik 16 Jan '13

16 Jan '13

Currently, whenever somebody calls saferead() on nonblocking FD (safewrite() is totally interchangeable for purpose of this message) he might get wrong return value. For instance, in the first iteration some data is read. The number of bytes read is stored into local variable 'nread'. However, in next iterations we can get -1 from read() with errno == EAGAIN, in which case the -1 is returned despite fact some data has already been read. So the caller gets confused. Moreover, the comment just above the functions says, they act like regular read() with nicer handling of EINTR. Well, they don't now. --- src/util/virutil.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/util/virutil.c b/src/util/virutil.c index b36e9d3..cd19971 100644 --- a/src/util/virutil.c +++ b/src/util/virutil.c @@ -105,6 +105,8 @@ saferead(int fd, void *buf, size_t count) size_t nread = 0; while (count > 0) { ssize_t r = read(fd, buf, count); + if (r < 0 && nread && errno == EAGAIN) + return nread; if (r < 0 && errno == EINTR) continue; if (r < 0) @@ -125,7 +127,8 @@ safewrite(int fd, const void *buf, size_t count) size_t nwritten = 0; while (count > 0) { ssize_t r = write(fd, buf, count); - + if (r < 0 && nwritten && errno == EAGAIN) + return nwritten; if (r < 0 && errno == EINTR) continue; if (r < 0) -- 1.8.0.2

2 4

[libvirt] [PATCH] conf: fix class_id bitmap leak in virNetworkObj
by Ján Tomko 16 Jan '13

16 Jan '13

Commit '07d1b6b' added class_id bitmap to virNetworkObj but never freed it. --- src/conf/network_conf.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 48639a9..9c35ea8 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -235,6 +235,7 @@ void virNetworkObjFree(virNetworkObjPtr net) virNetworkDefFree(net->def); virNetworkDefFree(net->newDef); + virBitmapFree(net->class_id); virMutexDestroy(&net->lock); -- 1.7.8.6

3 5

Re: [libvirt] Redefinition of struct in6_addr in <netinet/in.h> and <linux/in6.h>
by Ben Hutchings 16 Jan '13

16 Jan '13

On Wed, 2013-01-16 at 12:04 -0500, Mike Frysinger wrote: > On Wednesday 16 January 2013 10:47:12 Ben Hutchings wrote: > > On Wed, 2013-01-16 at 23:21 +0900, YOSHIFUJI Hideaki wrote: > > > Cong Wang wrote: > > > > (Cc'ing some glibc developers...) > > > > > > > > Hello, > > > > > > > > In glibc source file inet/netinet/in.h and kernel source file > > > > include/uapi/linux/in6.h, both define struct in6_addr, and both are > > > > visible to user applications. Thomas reported a conflict below. > > > > > > > > So, how can we handle this? /me is wondering why we didn't see this > > > > before. > > > > [...] > > > > > This is not a new issue. In addition to this, > > > netinet/in.h also conflits with linux/in.h. > > > > > > We might have > > > > > > #if !defined(__GLIBC__) || !defined(_NETINET_IN_H) > > > > > > #endif > > > > > > around those conflicting definitions in uapi/linux/in{,6}.h. > > > > This only solves half the problem, as <netinet/in.h> might be included > > after <linux/in.h>. Also, not all Linux userland uses glibc. > > certainly true, but the current expectation is that you don't mix your ABIs. Whose expectation? Which ABIs are being mixed? > if you're programming with the C library API, then use the C library headers. > if you're banging directly on the kernel, then use the kernel headers. not > saying it's a perfect solution, but it works for the vast majority of use > cases. In practice most C programs for Linux will use a mixture of thinly wrapped system calls and higher-level APIs from the C library, and never really call the kernel directly (as that requires inline assembler). Userland programmers will work around this historical mess by tweaking the #include order or splitting source files. But they shouldn't have to. Ben. -- Ben Hutchings, Staff Engineer, Solarflare Not speaking for my employer; that's the marketing department's job. They asked us to note that Solarflare product names are trademarked.

1 0

[libvirt] [PATCH] qemu: fix QEMU_CAPS_NO_ACPI detection
by Ján Tomko 16 Jan '13

16 Jan '13

In commit c4bbaaf8, caps->arch was checked uninitialized, rendering the whole check useless. This patch moves the conditional setting of QEMU_CAPS_NO_ACPI to qemuCapsInitQMP, and removes the no longer needed exception for S390. It also clears the flag for all non-x86 archs instead of just S390 in qemuCapsInitHelp. --- src/qemu/qemu_capabilities.c | 29 ++++++++--------------------- 1 files changed, 8 insertions(+), 21 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 167cdb2..e0d0c2a 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -2175,13 +2175,11 @@ qemuCapsInitHelp(qemuCapsPtr caps, uid_t runUid, gid_t runGid) if (caps->arch == VIR_ARCH_X86_64 || caps->arch == VIR_ARCH_I686) { qemuCapsSet(caps, QEMU_CAPS_PCI_MULTIBUS); - } - - /* S390 and probably other archs do not support no-acpi - - maybe the qemu option parsing should be re-thought. */ - if (caps->arch == VIR_ARCH_S390 || - caps->arch == VIR_ARCH_S390X) + } else { + /* -no-acpi is not supported on other archs + * even if qemu reports it in -help */ qemuCapsClear(caps, QEMU_CAPS_NO_ACPI); + } /* qemuCapsExtractDeviceStr will only set additional caps if qemu * understands the 0.13.0+ notion of "-device driver,". */ @@ -2260,13 +2258,6 @@ qemuCapsInitQMPBasic(qemuCapsPtr caps) qemuCapsSet(caps, QEMU_CAPS_DRIVE_CACHE_DIRECTSYNC); qemuCapsSet(caps, QEMU_CAPS_NO_SHUTDOWN); qemuCapsSet(caps, QEMU_CAPS_DRIVE_CACHE_UNSAFE); - - /* ACPI is only supported on x86, PPC or - * other platforms don't support it*/ - if (caps->arch == VIR_ARCH_I686 || - caps->arch == VIR_ARCH_X86_64) - qemuCapsSet(caps, QEMU_CAPS_NO_ACPI); - qemuCapsSet(caps, QEMU_CAPS_FSDEV_READONLY); qemuCapsSet(caps, QEMU_CAPS_VIRTIO_BLK_SG_IO); qemuCapsSet(caps, QEMU_CAPS_DRIVE_COPY_ON_READ); @@ -2402,16 +2393,12 @@ qemuCapsInitQMP(qemuCapsPtr caps, } VIR_FREE(archstr); - /* Currently only x86_64 and i686 support PCI-multibus. */ + /* Currently only x86_64 and i686 support PCI-multibus and -no-acpi. */ if (caps->arch == VIR_ARCH_X86_64 || - caps->arch == VIR_ARCH_I686) + caps->arch == VIR_ARCH_I686) { qemuCapsSet(caps, QEMU_CAPS_PCI_MULTIBUS); - - /* S390 and probably other archs do not support no-acpi - - maybe the qemu option parsing should be re-thought. */ - if (caps->arch == VIR_ARCH_S390 || - caps->arch == VIR_ARCH_S390X) - qemuCapsClear(caps, QEMU_CAPS_NO_ACPI); + qemuCapsSet(caps, QEMU_CAPS_NO_ACPI); + } if (qemuCapsProbeQMPCommands(caps, mon) < 0) goto cleanup; -- 1.7.8.6

4 11

[libvirt] [PATCH 00/14] Resolve some RESOURCE_LEAK errors found by Coverity
by John Ferlan 16 Jan '13

16 Jan '13

This patch addresses some "Error: RESOURCE_LEAK (CWE-404)" errors found by Coverity. There are 100+ remaining; however, many seem to be false positives which will be addressed in a separate patch. The esx_driver.c and hyperv_driver.c changes could be tagged as false positives; however, the changes made will avoid the Coverity error since the freeing of the resource marked isn't based soley on the result. The phyp_driver.c changes in openSSHSession() and its callers were the most involved since the code with now use connection_data->sock in order to track the socket so as to allow it to be closed once done with it. Of interest in this change is that phypExec() used connection_data->sock even though it had never been initialized. John Ferlan (14): phyp: Resolve some file descriptor leaks esx: Address resource leak found by Coverity storage: Resolve resource leak using 'vol' buffer util: Resolve resource leak for 'res' in virSetInherit error path. locking: Resolve resource leak with 'dir' on non error path rpc: Avoid resource leak of 'socks' if any object append fails uml: Avoid resource leak of event in umlInofityEvent test: Resource resource leak with 'tmp_vols' storage: Resource resource leak using 'tmp_vols' interface: Resolve resource leak wth 'tmp_iface_objs' xen: Resource resource leak with 'cpuset' parallels: Resolve some resource leaks storage: Need to also VIR_FREE(reg) hyperv: Address resource leak found by Coverityo src/esx/esx_driver.c | 8 ++++---- src/hyperv/hyperv_driver.c | 10 +++++----- src/interface/interface_backend_netcf.c | 1 + src/locking/lock_driver_sanlock.c | 1 + src/parallels/parallels_driver.c | 30 +++++++++++++++++++----------- src/phyp/phyp_driver.c | 19 ++++++++++++++----- src/rpc/virnetserverservice.c | 6 +++--- src/storage/storage_backend_logical.c | 1 + src/storage/storage_backend_rbd.c | 13 ++++++++++--- src/storage/storage_driver.c | 1 + src/test/test_driver.c | 1 + src/uml/uml_driver.c | 6 ++++-- src/util/virlockspace.c | 1 + src/xen/xend_internal.c | 12 ++++++------ 14 files changed, 71 insertions(+), 39 deletions(-) -- 1.7.11.7

6 47

[libvirt] [PATCH] Add RESUME event listener to qemu monitor.
by Andres Lagar-Cavilla 16 Jan '13

16 Jan '13

Perform all the appropriate plumbing. When qemu/KVM VMs are paused manually through a monitor not-owned by libvirt, libvirt will think of them as "paused" event after they are resumed and effectively running. With this patch the discrepancy goes away. This is meant to address bug 892791. Signed-off-by: Andres Lagar-Cavilla <andres(a)lagarcavilla.org> --- src/qemu/qemu_monitor.c | 10 ++++++++ src/qemu/qemu_monitor.h | 3 +++ src/qemu/qemu_monitor_json.c | 7 ++++++ src/qemu/qemu_process.c | 56 ++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 76 insertions(+) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 99642b6..cb5a3e2 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -1022,6 +1022,16 @@ int qemuMonitorEmitStop(qemuMonitorPtr mon) } +int qemuMonitorEmitResume(qemuMonitorPtr mon) +{ + int ret = -1; + VIR_DEBUG("mon=%p", mon); + + QEMU_MONITOR_CALLBACK(mon, ret, domainResume, mon->vm); + return ret; +} + + int qemuMonitorEmitRTCChange(qemuMonitorPtr mon, long long offset) { int ret = -1; diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index d7faa90..00ce813 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -97,6 +97,8 @@ struct _qemuMonitorCallbacks { virDomainObjPtr vm); int (*domainStop)(qemuMonitorPtr mon, virDomainObjPtr vm); + int (*domainResume)(qemuMonitorPtr mon, + virDomainObjPtr vm); int (*domainRTCChange)(qemuMonitorPtr mon, virDomainObjPtr vm, long long offset); @@ -187,6 +189,7 @@ int qemuMonitorEmitShutdown(qemuMonitorPtr mon); int qemuMonitorEmitReset(qemuMonitorPtr mon); int qemuMonitorEmitPowerdown(qemuMonitorPtr mon); int qemuMonitorEmitStop(qemuMonitorPtr mon); +int qemuMonitorEmitResume(qemuMonitorPtr mon); int qemuMonitorEmitRTCChange(qemuMonitorPtr mon, long long offset); int qemuMonitorEmitWatchdog(qemuMonitorPtr mon, int action); int qemuMonitorEmitIOError(qemuMonitorPtr mon, diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 2d2d254..de5f115 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -55,6 +55,7 @@ static void qemuMonitorJSONHandleShutdown(qemuMonitorPtr mon, virJSONValuePtr da static void qemuMonitorJSONHandleReset(qemuMonitorPtr mon, virJSONValuePtr data); static void qemuMonitorJSONHandlePowerdown(qemuMonitorPtr mon, virJSONValuePtr data); static void qemuMonitorJSONHandleStop(qemuMonitorPtr mon, virJSONValuePtr data); +static void qemuMonitorJSONHandleResume(qemuMonitorPtr mon, virJSONValuePtr data); static void qemuMonitorJSONHandleRTCChange(qemuMonitorPtr mon, virJSONValuePtr data); static void qemuMonitorJSONHandleWatchdog(qemuMonitorPtr mon, virJSONValuePtr data); static void qemuMonitorJSONHandleIOError(qemuMonitorPtr mon, virJSONValuePtr data); @@ -87,6 +88,7 @@ static qemuEventHandler eventHandlers[] = { { "DEVICE_TRAY_MOVED", qemuMonitorJSONHandleTrayChange, }, { "POWERDOWN", qemuMonitorJSONHandlePowerdown, }, { "RESET", qemuMonitorJSONHandleReset, }, + { "RESUME", qemuMonitorJSONHandleResume, }, { "RTC_CHANGE", qemuMonitorJSONHandleRTCChange, }, { "SHUTDOWN", qemuMonitorJSONHandleShutdown, }, { "SPICE_CONNECTED", qemuMonitorJSONHandleSPICEConnect, }, @@ -589,6 +591,11 @@ static void qemuMonitorJSONHandleStop(qemuMonitorPtr mon, virJSONValuePtr data A qemuMonitorEmitStop(mon); } +static void qemuMonitorJSONHandleResume(qemuMonitorPtr mon, virJSONValuePtr data ATTRIBUTE_UNUSED) +{ + qemuMonitorEmitResume(mon); +} + static void qemuMonitorJSONHandleRTCChange(qemuMonitorPtr mon, virJSONValuePtr data) { long long offset = 0; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index f5fc1b3..938c17e 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -736,6 +736,61 @@ unlock: static int +qemuProcessHandleResume(qemuMonitorPtr mon ATTRIBUTE_UNUSED, + virDomainObjPtr vm) +{ + virQEMUDriverPtr driver = qemu_driver; + virDomainEventPtr event = NULL; + + virDomainObjLock(vm); + if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_PAUSED) { + qemuDomainObjPrivatePtr priv = vm->privateData; + + if (priv->gotShutdown) { + VIR_DEBUG("Ignoring RESUME event after SHUTDOWN"); + goto unlock; + } + + VIR_DEBUG("Transitioned guest %s out of paused into resumed state", + vm->def->name); + + virDomainObjSetState(vm, VIR_DOMAIN_RUNNING, + VIR_DOMAIN_RUNNING_UNPAUSED); + event = virDomainEventNewFromObj(vm, + VIR_DOMAIN_EVENT_RESUMED, + VIR_DOMAIN_EVENT_RESUMED_UNPAUSED); + + VIR_DEBUG("Using lock state '%s' on resume event", NULLSTR(priv->lockState)); + if (virDomainLockProcessResume(driver->lockManager, driver->uri, + vm, priv->lockState) < 0) { + /* Don't free priv->lockState on error, because we need + * to make sure we have state still present if the user + * tries to resume again + */ + goto unlock; + } + VIR_FREE(priv->lockState); + + if (virDomainSaveStatus(driver->caps, driver->stateDir, vm) < 0) { + VIR_WARN("Unable to save status on vm %s after state change", + vm->def->name); + } + } + +unlock: + virDomainObjUnlock(vm); + + if (event) { + qemuDriverLock(driver); + qemuDomainEventQueue(driver, event); + qemuDriverUnlock(driver); + } + + return 0; +} + + +static int qemuProcessHandleRTCChange(qemuMonitorPtr mon ATTRIBUTE_UNUSED, virDomainObjPtr vm, long long offset) @@ -1250,6 +1305,7 @@ static qemuMonitorCallbacks monitorCallbacks = { .diskSecretLookup = qemuProcessFindVolumeQcowPassphrase, .domainShutdown = qemuProcessHandleShutdown, .domainStop = qemuProcessHandleStop, + .domainResume = qemuProcessHandleResume, .domainReset = qemuProcessHandleReset, .domainRTCChange = qemuProcessHandleRTCChange, .domainWatchdog = qemuProcessHandleWatchdog, -- 1.7.9.5

6 9