- Devel - libvirt List Archives

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

18 Jan '13

On Thu, Jan 17, 2013 at 11:20 PM, Mike Frysinger <vapier(a)gentoo.org> wrote: > On Wednesday 16 January 2013 22:15:38 David Miller wrote: >> From: Carlos O'Donell <carlos(a)systemhalted.org> >> Date: Wed, 16 Jan 2013 21:15:03 -0500 >> >> > +/* If a glibc-based userspace has already included in.h, then we will >> > not + * define in6_addr (nor the defines), sockaddr_in6, or ipv6_mreq. >> > The + * ABI used by the kernel and by glibc match exactly. Neither the >> > kernel + * nor glibc should break this ABI without coordination. >> > + */ >> > +#ifndef _NETINET_IN_H >> > + >> >> I think we should shoot for a non-glibc-centric solution. >> >> I can't imagine that other libc's won't have the same exact problem >> with their netinet/in.h conflicting with the kernel's, redefining >> structures like in6_addr, that we'd want to provide a protection >> scheme for here as well. > > yes, the kernel's use of __GLIBC__ in exported headers has already caused > problems in the past. fortunately, it's been reduced down to just one case > now (stat.h). let's not balloon it back up. > -mike I also see coda.h has grown a __GLIBC__ usage. In the next revision of the patch I created a single libc-compat.h header which encompasses the logic for any libc that wants to coordinate with the kernel headers. It's simple enough to move all of the __GLIBC__ uses into libc-compat.h, then you control userspace libc coordination from one file. Cheers, Carlos.

3 5

[libvirt] [PATCH 00/12] Introduce public APIs for dealing with virTypedParameters
by Jiri Denemark 18 Jan '13

18 Jan '13

Working with virTypedParameters in clients written in C is ugly and requires all clients to duplicate the same code. This set of APIs makes this code for manipulating with virTypedParameters integral part of libvirt so that all clients may benefit from it. Jiri Denemark (12): Add virTypedParams* APIs virsh: Use virTypedParams* APIs in blkdeviotune virsh: Use virTypedParams* APIs in blkiotune virsh: Use virTypedParams* APIs in domiftune virsh: Use virTypedParams* APIs in schedinfo virsh: Use virTypedParams* APIs in domblkstat virsh: Use virTypedParams* APIs in memtune virsh: Use virTypedParams* APIs in numatune virsh: Use virTypedParams* APIs in node-memory-tune virsh: Use virTypedParams* APIs in cpu-stats Introduce virTypedParamsClear public API python: Avoid freeing uninitialized new_params pointer daemon/remote.c | 34 +- include/libvirt/libvirt.h.in | 95 ++++++ python/generator.py | 20 ++ python/libvirt-override.c | 73 ++-- src/libvirt_private.syms | 1 - src/libvirt_public.syms | 18 + src/remote/remote_driver.c | 4 +- src/rpc/gendispatch.pl | 3 +- src/util/virtypedparam.c | 782 ++++++++++++++++++++++++++++++++++++++++++- src/util/virtypedparam.h | 2 - tools/virsh-domain-monitor.c | 5 +- tools/virsh-domain.c | 542 +++++++++++------------------- tools/virsh-host.c | 78 ++--- tools/virsh.c | 20 -- 14 files changed, 1171 insertions(+), 506 deletions(-) -- 1.8.1.1

3 17

[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 18 Jan '13

18 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 18 Jan '13

18 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 18 Jan '13

18 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