January 2013 - Devel - libvirt List Archives

[libvirt] [PATCH] storage: Fix lvcreate parameter for backingStore.
by Atsushi Kumagai 18 Jan '13

18 Jan '13

When virStorageBackendLogicalCreateVol() creates a snapshot for a logical volume with backingStore element, it fails with the message below: 2013-01-17 03:10:18.869+0000: 1967: error : virCommandWait:2345 : internal error Child process (/sbin/lvcreate --name lvm-snapshot -L 51200K -s=/dev/lvm-pool/lvm-volume) unexpected exit status 3: /sbin/lvcreate: invalid option -- '=' Error during parsing of command line. This is because virCommandAddArgPair() uses '=' to connect the two parameters, it's unsuitable for -s option of the lvcreate. Signed-off-by: Atsushi Kumagai <kumagai-atsushi(a)mxc.nes.nec.co.jp> --- src/storage/storage_backend_logical.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c index 91db3fd..bb709df 100644 --- a/src/storage/storage_backend_logical.c +++ b/src/storage/storage_backend_logical.c @@ -736,7 +736,7 @@ virStorageBackendLogicalCreateVol(virConnectPtr conn, } virCommandAddArgFormat(cmd, "%lluK", VIR_DIV_UP(vol->capacity, 1024)); if (vol->backingStore.path) - virCommandAddArgPair(cmd, "-s", vol->backingStore.path); + virCommandAddArgList(cmd, "-s", vol->backingStore.path, NULL); else virCommandAddArg(cmd, pool->def->source.name); -- 1.7.1

2 1

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

18 Jan '13

From: Mike Frysinger <vapier(a)gentoo.org> Date: Thu, 17 Jan 2013 23:14:31 -0500 > the kernel already exports many types with a __kernel_ prefix. i changed the > kernel headers in Gentoo for a few releases (2.6.28 - 2.6.34) to do the same > thing to pretty much all the networking headers. a few packages broke, but > the number was low, and trivial to fix (a sed would do it most of the time). > > it's also trivial for userland packages to detect that they need to do this > sort of thing in a local header by using linux/version.h and a set of defines > to redirect the new structure name back to the old one. > > would be a lot cleaner to just break it and be done. We are not at liberty to break something that has legitimately compiled successfully for two decades. Your __kernel_ prefix idea breaks compilation just as equally. One thing you certainly don't have to be is happy about this header file situation, but you cannot use that dissatisfaction as justification to make the situation worse by breaking the build for people outside of the world you directly control.

1 0

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

18 Jan '13

From: Mike Frysinger <vapier(a)gentoo.org> Date: Wed, 16 Jan 2013 12:04:56 -0500 > certainly true, but the current expectation is that you don't mix your ABIs. > 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. This isn't how real life works. GLIBC itself brings in some of the kernel headers, as do various library headers for libraries other than glibc. So you can get these conflicting headers included indirectly, and it is of no fault of any of the various parties involved. We have to make them work when included at the same time somehow, and this is totally unavoidable.

3 5

[libvirt] [PATCH] build: fix build on BSD
by Eric Blake 17 Jan '13

17 Jan '13

A build on FreeBSD failed with: util/virportallocator.c:108: error: storage size of 'addr' isn't known util/virportallocator.c:123: error: 'INADDR_ANY' undeclared (first use in this function) It turns out that while POSIX allows sockaddr_in to leak in through other headers (the way Linux does it), conforming applications are required to get it through netinet/in.h. * src/util/virportallocator.c: Include header for struct sockaddr_in. --- Pushing under the build-breaker rule. src/util/virportallocator.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/util/virportallocator.c b/src/util/virportallocator.c index 35f2157..590bb57 100644 --- a/src/util/virportallocator.c +++ b/src/util/virportallocator.c @@ -23,6 +23,7 @@ #include <sys/socket.h> #include <arpa/inet.h> +#include <netinet/in.h> #include "viralloc.h" #include "virbitmap.h" -- 1.8.0.2

1 0

[libvirt] [PATCH v2.1 RESEND 00/11] Rework storage migration
by Michal Privoznik 17 Jan '13

17 Jan '13

This patch set re-implements migration with storage for enough new qemu. Currently, you can migrate a domain to a host without need for shared storage. This is done by setting 'blk' or 'inc' attribute (representing VIR_MIGRATE_NON_SHARED_DISK and VIR_MIGRATE_NON_SHARED_INC flags respectively) of 'migrate' monitor command. However, the qemu implementation is buggy and applications are advised to switch to new impementation which, moreover, offers some nice features, like migrating only explicitly specified disks. The new functionality is controlled via 'nbd-server-*' and 'drive-mirror' commands. The flow is meant to look like this: 1) User invokes libvirt's migrate functionality. 2) libvirt checks that no block jobs are active on the source. 3) libvirt starts the destination QEMU and sets up the NBD server using the nbd-server-start and nbd-server-add commands. 4) libvirt starts drive-mirror with a destination pointing to the remote NBD server, for example nbd:host:port:exportname=diskname (where diskname is the -drive id specified on the destination). 5) once all mirroring jobs reach steady state, libvirt invokes the migrate command. 6) once migration completed, libvirt invokes the nbd-server-stop command on the destination QEMU. If we just skip the 2nd step and there is an active block-job, qemu will fail in step 4. No big deal. Since we try to NOT break migration and keep things compatible, this feature is enabled iff both sides support it. Since there's obvious need for some data transfer between src and dst, I've put it into qemuCookieMigration: 1) src -> dest: (QEMU_MIGRATION_PHASE_BEGIN3 -> QEMU_MIGRATION_PHASE_PREPARE) <nbd> <disk size='17179869184'/> </nbd> Hey destination, I know how to use this cool new feature. Moreover, these are the disks I'll send you. Each one of them is X bytes big. It's one of the prerequisite - the file (disk->src) on dst exists and has at least the same size as on dst. 2) dst -> src: (QEMU_MIGRATION_PHASE_PREPARE -> QEMU_MIGRATION_PHASE_PERFORM3) <nbd port='X'/> Okay, I (destination) support this feature as well. I've created all files as you (src) told me to and you can start rolling data. I am listening on port X. 3) src -> dst: (QEMU_MIGRATION_PHASE_PERFORM3 -> QEMU_MIGRATION_PHASE_FINISH3) <nbd port='-1'/> Migration completed, destination, you may shut the NBD server down. If either src or dst doesn't support NBD, it is not used and whole process fall backs to old implementation. diff to v1: -Eric's and Daniel's suggestions worked in. To point out the bigger ones: don't do NBD style when TUNNELLED requested, added 'b:writable' to 'nbd-server-add' -drop '/qemu-migration/nbd/disk/@src' attribute from migration cookie. As pointed out by Jirka, disk->src can be changed during migration (e.g. by migration hook or by passed xml). So I've tried (as suggested on the list) passing disk alias. However, since qemu hasn't been started on destination yet, the aliases hasn't been generated yet. So we have to rely on ordering completely. diff to v2: -rebase to reflect changes made by offline migration patch -send initial nbd cookie only if needed diff to previous v2.1: -rebase to current HEAD The patches 1,3 and 5 has been ACKed already. Maybe, I'll need to drop 7/11 patch completely, once Dan's patches [1] are in. 1: https://www.redhat.com/archives/libvir-list/2013-January/msg00742.html Michal Privoznik (11): qemu: Introduce NBD_SERVER capability Introduce NBD migration cookie qemu: Introduce nbd-server-start command qemu: Introduce nbd-server-add command qemu: Introduce nbd-server-stop command qemu_migration: Introduce qemuMigrationStartNBDServer qemu_migration: Move port allocation to a separate func qemu_migration: Implement qemuMigrationStartNBDServer() qemu_migration: Implement qemuMigrationDriveMirror qemu_migration: Check size prerequisites qemu_migration: Stop NBD server at Finish phase src/qemu/qemu_capabilities.c | 4 +- src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_migration.c | 624 ++++++++++++++++++++++++++++++++++++++++--- src/qemu/qemu_monitor.c | 63 +++++ src/qemu/qemu_monitor.h | 7 + src/qemu/qemu_monitor_json.c | 95 +++++++ src/qemu/qemu_monitor_json.h | 7 + 7 files changed, 768 insertions(+), 33 deletions(-) -- 1.8.0.2

2 23

[libvirt] [RFC] Data in the <topology> element in the capabilities XML
by Peter Krempa 17 Jan '13

17 Jan '13

Hi everybody, a while ago there was a discussion about changing the data that is returned in the <topology> sub-element: <capabilities> <host> <cpu> <arch>x86_64</arch> <model>SandyBridge</model> <vendor>Intel</vendor> <topology sockets='1' cores='2' threads='2'/> The data provided here is as of today taken from the nodeinfo detection code and thus is really wrong when the fallback mechanisms are used. To get a useful count, the user has to multiply the data by the number of NUMA nodes in the host. With the fallback detection code used for nodeinfo the NUMA node count used to get the CPU count should be 1 instead of the actual number. As Jiri proposed, I think we should change this output to separate detection code that will not take into account NUMA nodes for this output and will rather provide data as the "lspci" command does. This change will make the data provided by the element standalone and also usable in guest XMLs to mirror host's topology. The meaning of the attributes of that element isn't really documented anywhere, so as an additional precaution we should document that. ( http://libvirt.org/formatcaps.html ) For some additional background, please refer to the original discussion here: https://www.redhat.com/archives/libvir-list/2012-March/msg01123.html Thanks in advance for your comments and suggestions. Peter

3 16

[libvirt] [PATCH v2] Avoid integer wrap on remotePortMax in QEMU driver
by Daniel P. Berrange 17 Jan '13

17 Jan '13

From: "Daniel P. Berrange" <berrange(a)redhat.com> The QEMU driver default max port is 65535, but it then increments this by 1 to 65536. This maps to 0 in an unsigned short :-( This was apparently done so that for() loops could use "< max" instead of "<= max". Remove this insanity and just make the loop do the right thing. --- src/qemu/qemu_conf.c | 4 ---- src/util/virportallocator.c | 6 +++--- tests/virportallocatortest.c | 2 +- 3 files changed, 4 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 680e8fb..b21392e 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -233,10 +233,6 @@ int qemuLoadDriverConfig(virQEMUDriverPtr driver, filename, QEMU_REMOTE_PORT_MAX); goto cleanup; } - /* increasing the value by 1 makes all the loops going through - the bitmap (i = remotePortMin; i < remotePortMax; i++), work as - expected. */ - driver->remotePortMax++; if (driver->remotePortMin > driver->remotePortMax) { virReportError(VIR_ERR_INTERNAL_ERROR, diff --git a/src/util/virportallocator.c b/src/util/virportallocator.c index 033aee4..d80347a 100644 --- a/src/util/virportallocator.c +++ b/src/util/virportallocator.c @@ -84,7 +84,7 @@ virPortAllocatorPtr virPortAllocatorNew(unsigned short start, pa->start = start; pa->end = end; - if (!(pa->bitmap = virBitmapNew(end-start))) { + if (!(pa->bitmap = virBitmapNew((end-start)+1))) { virReportOOMError(); virObjectUnref(pa); return NULL; @@ -103,7 +103,7 @@ int virPortAllocatorAcquire(virPortAllocatorPtr pa, *port = 0; virObjectLock(pa); - for (i = pa->start ; i < pa->end && !*port; i++) { + for (i = pa->start ; i <= pa->end && !*port; i++) { int reuse = 1; struct sockaddr_in addr; bool used = false; @@ -168,7 +168,7 @@ int virPortAllocatorRelease(virPortAllocatorPtr pa, virObjectLock(pa); if (port < pa->start || - port >= pa->end) { + port > pa->end) { virReportInvalidArg(port, "port %d must be in range (%d, %d)", port, pa->start, pa->end); goto cleanup; diff --git a/tests/virportallocatortest.c b/tests/virportallocatortest.c index 93577d7..3f6edcc 100644 --- a/tests/virportallocatortest.c +++ b/tests/virportallocatortest.c @@ -59,7 +59,7 @@ int bind(int sockfd ATTRIBUTE_UNUSED, static int testAllocAll(const void *args ATTRIBUTE_UNUSED) { - virPortAllocatorPtr alloc = virPortAllocatorNew(5900, 5910); + virPortAllocatorPtr alloc = virPortAllocatorNew(5900, 5909); int ret = -1; unsigned short p1, p2, p3, p4, p5, p6, p7; -- 1.8.0.2

3 2

[libvirt] [PATCH] Avoid integer wrap on remotePortMax in QEMU driver
by Daniel P. Berrange 17 Jan '13

17 Jan '13

From: "Daniel P. Berrange" <berrange(a)redhat.com> The QEMU driver default max port is 65535, but it then increments this by 1 to 65536. This maps to 0 in an unsigned short :-( This was apparently done so that for() loops could use "< max" instead of "<= max". Remove this insanity and just make the loop do the right thing. --- src/qemu/qemu_conf.c | 4 ---- src/util/virportallocator.c | 4 ++-- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 680e8fb..b21392e 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -233,10 +233,6 @@ int qemuLoadDriverConfig(virQEMUDriverPtr driver, filename, QEMU_REMOTE_PORT_MAX); goto cleanup; } - /* increasing the value by 1 makes all the loops going through - the bitmap (i = remotePortMin; i < remotePortMax; i++), work as - expected. */ - driver->remotePortMax++; if (driver->remotePortMin > driver->remotePortMax) { virReportError(VIR_ERR_INTERNAL_ERROR, diff --git a/src/util/virportallocator.c b/src/util/virportallocator.c index 033aee4..5d07dd0 100644 --- a/src/util/virportallocator.c +++ b/src/util/virportallocator.c @@ -84,7 +84,7 @@ virPortAllocatorPtr virPortAllocatorNew(unsigned short start, pa->start = start; pa->end = end; - if (!(pa->bitmap = virBitmapNew(end-start))) { + if (!(pa->bitmap = virBitmapNew((end-start)+1))) { virReportOOMError(); virObjectUnref(pa); return NULL; @@ -103,7 +103,7 @@ int virPortAllocatorAcquire(virPortAllocatorPtr pa, *port = 0; virObjectLock(pa); - for (i = pa->start ; i < pa->end && !*port; i++) { + for (i = pa->start ; i <= pa->end && !*port; i++) { int reuse = 1; struct sockaddr_in addr; bool used = false; -- 1.8.0.2

3 2

[libvirt] [PATCH 0/5] Resolve REVERSE_INULL errors found by Coverity
by John Ferlan 17 Jan '13

17 Jan '13

This patch will resolve a set of "REVERSE_INULL (CWE-476):" errors found by Coverity. John Ferlan (5): network: Resolve some issues around vlan copying xen: Remove extraneous checks in xenStoreGetDomainInfo() interface: Need to check ifacedef->mac not just ifacedef after strdup() xen: Need to check validity of domain->conn before deref privateData openvz: Need to check 'vm' first before dereferencing 'def' src/interface/interface_backend_udev.c | 2 +- src/network/bridge_driver.c | 16 +++++++++++++--- src/openvz/openvz_driver.c | 2 +- src/xen/xen_hypervisor.c | 3 ++- src/xen/xs_internal.c | 2 +- 5 files changed, 18 insertions(+), 7 deletions(-) -- 1.7.11.7

5 15

[libvirt] [PATCH] network: use bandwidth from portgroup when appropriate
by Laine Stump 17 Jan '13

17 Jan '13

The bandwidth plug and unplug functions were assuming that an interface's bandwidth setting was always specified directly in the domain's <interface> definition, but that's not necessarily true - it could have been obtained from a <portgroup> definition in the network definition. This patch fixes those functions to use virDomainNetGetActualBandwidth(), which gets the bandwidth pointer from iface->data.network.actual if it exists, otherwise returns iface->bandwidth. --- src/network/bridge_driver.c | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index c3cb63d..6f3c839 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -4518,7 +4518,7 @@ networkCheckBandwidth(virNetworkObjPtr net, { int ret = -1; virNetDevBandwidthPtr netBand = net->def->bandwidth; - virNetDevBandwidthPtr ifaceBand = iface->bandwidth; + virNetDevBandwidthPtr ifaceBand = virDomainNetGetActualBandwidth(iface); unsigned long long tmp_floor_sum = net->floor_sum; unsigned long long tmp_new_rate = 0; char ifmac[VIR_MAC_STRING_BUFLEN]; @@ -4597,6 +4597,7 @@ networkPlugBandwidth(virNetworkObjPtr net, unsigned long long new_rate = 0; ssize_t class_id = 0; char ifmac[VIR_MAC_STRING_BUFLEN]; + virNetDevBandwidthPtr ifaceBand = virDomainNetGetActualBandwidth(iface); if ((plug_ret = networkCheckBandwidth(net, iface, &new_rate)) < 0) { /* helper reported error */ @@ -4625,11 +4626,8 @@ networkPlugBandwidth(virNetworkObjPtr net, goto cleanup; } - plug_ret = virNetDevBandwidthPlug(net->def->bridge, - net->def->bandwidth, - &iface->mac, - iface->bandwidth, - class_id); + plug_ret = virNetDevBandwidthPlug(net->def->bridge, net->def->bandwidth, + &iface->mac, ifaceBand, class_id); if (plug_ret < 0) { ignore_value(virNetDevBandwidthUnplug(net->def->bridge, class_id)); goto cleanup; @@ -4638,11 +4636,11 @@ networkPlugBandwidth(virNetworkObjPtr net, /* QoS was set, generate new class ID */ iface->data.network.actual->class_id = class_id; /* update sum of 'floor'-s of attached NICs */ - net->floor_sum += iface->bandwidth->in->floor; + net->floor_sum += ifaceBand->in->floor; /* update status file */ if (virNetworkSaveStatus(NETWORK_STATE_DIR, net) < 0) { ignore_value(virBitmapClearBit(net->class_id, class_id)); - net->floor_sum -= iface->bandwidth->in->floor; + net->floor_sum -= ifaceBand->in->floor; iface->data.network.actual->class_id = 0; ignore_value(virNetDevBandwidthUnplug(net->def->bridge, class_id)); goto cleanup; @@ -4666,6 +4664,7 @@ networkUnplugBandwidth(virNetworkObjPtr net, { int ret = 0; unsigned long long new_rate; + virNetDevBandwidthPtr ifaceBand = virDomainNetGetActualBandwidth(iface); if (iface->data.network.actual && iface->data.network.actual->class_id) { @@ -4680,13 +4679,13 @@ networkUnplugBandwidth(virNetworkObjPtr net, if (ret < 0) goto cleanup; /* update sum of 'floor'-s of attached NICs */ - net->floor_sum -= iface->bandwidth->in->floor; + net->floor_sum -= ifaceBand->in->floor; /* return class ID */ ignore_value(virBitmapClearBit(net->class_id, iface->data.network.actual->class_id)); /* update status file */ if (virNetworkSaveStatus(NETWORK_STATE_DIR, net) < 0) { - net->floor_sum += iface->bandwidth->in->floor; + net->floor_sum += ifaceBand->in->floor; ignore_value(virBitmapSetBit(net->class_id, iface->data.network.actual->class_id)); goto cleanup; -- 1.7.11.7

2 2