October 2013 - Devel - libvirt List Archives

[libvirt] [PATCH] virfile: safezero: fall back to writing block by block if mmap fails
by Oskari Saarenmaa 04 Oct '13

04 Oct '13

mmap can fail on 32-bit systems if we're trying to zero out a lot of data. Fall back to using block-by-block writing in that case. While we could map smaller blocks it's unlikely that this code is used a lot and its easier to just fall back to one of the existing methods. Also modified the block-by-block zeroing to not allocate a megabyte of zeroes if we're writing less than that. Signed-off-by: Oskari Saarenmaa <os(a)ohmu.fi> --- src/util/virfile.c | 34 ++++++++++++++-------------------- 1 file changed, 14 insertions(+), 20 deletions(-) diff --git a/src/util/virfile.c b/src/util/virfile.c index 16f8101..f662127 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -1032,16 +1032,18 @@ safezero(int fd, off_t offset, off_t len) errno = ret; return -1; } + #else -# ifdef HAVE_MMAP int safezero(int fd, off_t offset, off_t len) { - static long pagemask = 0; - off_t map_skip; int r; char *buf; + unsigned long long remain, bytes; +# ifdef HAVE_MMAP + static long pagemask = 0; + off_t map_skip; /* align offset and length, rounding offset down and length up */ if (pagemask == 0) @@ -1057,30 +1059,23 @@ safezero(int fd, off_t offset, off_t len) buf = mmap(NULL, len + map_skip, PROT_READ | PROT_WRITE, MAP_SHARED, fd, offset - map_skip); - if (buf == MAP_FAILED) - return -1; + if (buf != MAP_FAILED) { + memset(buf + map_skip, 0, len); + munmap(buf, len + map_skip); - memset(buf + map_skip, 0, len); - munmap(buf, len + map_skip); - - return 0; -} - -# else /* HAVE_MMAP */ + return 0; + } -int -safezero(int fd, off_t offset, off_t len) -{ - int r; - char *buf; - unsigned long long remain, bytes; + /* fall back to writing zeroes using safewrite if mmap fails (for + * example because of virtual memory limits) */ +# endif /* HAVE_MMAP */ if (lseek(fd, offset, SEEK_SET) < 0) return -1; /* Split up the write in small chunks so as not to allocate lots of RAM */ remain = len; - bytes = 1024 * 1024; + bytes = MAX(1024 * 1024, len); r = VIR_ALLOC_N(buf, bytes); if (r < 0) { @@ -1104,7 +1099,6 @@ safezero(int fd, off_t offset, off_t len) VIR_FREE(buf); return 0; } -# endif /* HAVE_MMAP */ #endif /* HAVE_POSIX_FALLOCATE */ -- 1.8.3.1

2 3

[libvirt] [PATCH v2] test: Fix coverity warnings
by Cole Robinson 04 Oct '13

04 Oct '13

--- v2: Address Eric's suggestion Address additional coverity warnings pointed out by John src/test/test_driver.c | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 255cc2b..c2e530e 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -6547,16 +6547,12 @@ testDomainSnapshotIsCurrent(virDomainSnapshotPtr snapshot, { virDomainObjPtr vm = NULL; int ret = -1; - virDomainSnapshotObjPtr snap = NULL; virCheckFlags(0, -1); if (!(vm = testDomObjFromSnapshot(snapshot))) goto cleanup; - if (!(snap = testSnapObjFromSnapshot(vm, snapshot))) - goto cleanup; - ret = (vm->current_snapshot && STREQ(snapshot->name, vm->current_snapshot->def->name)); @@ -6573,14 +6569,13 @@ testDomainSnapshotHasMetadata(virDomainSnapshotPtr snapshot, { virDomainObjPtr vm = NULL; int ret = -1; - virDomainSnapshotObjPtr snap = NULL; virCheckFlags(0, -1); if (!(vm = testDomObjFromSnapshot(snapshot))) goto cleanup; - if (!(snap = testSnapObjFromSnapshot(vm, snapshot))) + if (!testSnapObjFromSnapshot(vm, snapshot)) goto cleanup; ret = 1; @@ -6679,8 +6674,8 @@ testDomainSnapshotCreateXML(virDomainPtr domain, goto cleanup; if (redefine) { - if (!virDomainSnapshotRedefinePrep(domain, vm, &def, &snap, - &update_current, flags) < 0) + if (virDomainSnapshotRedefinePrep(domain, vm, &def, &snap, + &update_current, flags) < 0) goto cleanup; } else { if (!(def->dom = virDomainDefCopy(vm->def, @@ -7078,6 +7073,8 @@ cleanup: testDomainEventQueue(privconn, event); if (event2) testDomainEventQueue(privconn, event2); + } else { + virDomainEventFree(event2); } virObjectUnlock(vm); testDriverUnlock(privconn); -- 1.8.3.1

3 3

[libvirt] [PATCH] qemu: Use maximum guest memory size when getting NUMA placement advice
by Peter Krempa 04 Oct '13

04 Oct '13

When starting the VM the guest balloon driver is not loaded at that time. We need to ask numad for placement of the complete VM. --- src/qemu/qemu_process.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 7a30a5e..079f062 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3684,7 +3684,7 @@ int qemuProcessStart(virConnectPtr conn, (vm->def->numatune.memory.placement_mode == VIR_NUMA_TUNE_MEM_PLACEMENT_MODE_AUTO)) { nodeset = virNumaGetAutoPlacementAdvice(vm->def->vcpus, - vm->def->mem.cur_balloon); + vm->def->mem.max_balloon); if (!nodeset) goto cleanup; -- 1.8.3.2

2 2

[libvirt] [PATCH v3 0/8] test: Mock snapshot APIs and misc improvements
by Cole Robinson 04 Oct '13

04 Oct '13

This series implements snapshot and managed save APIs for the test driver, and adds some misc improvements, like specifying domain transient state in the driver XML. v3: Rebase Validate that managedsave, transient, and runstate combo make sense Use STREQ_NULLABLE to simplify a check Cole Robinson (8): test: Allow specifying object transient state in driver XML test: Wire up managed save APIs test: Implement readonly snapshot APIs test: Implement snapshot create/delete/revert APIs qemu: snapshots: Simplify REDEFINE flag check qemu: snapshot: Break out redefine preparation to shared function test: snapshot: Add REDEFINE support test: Allow specifying domainsnapshot XML src/conf/domain_conf.c | 2 +- src/conf/snapshot_conf.c | 150 ++++++ src/conf/snapshot_conf.h | 7 + src/libvirt_private.syms | 1 + src/qemu/qemu_driver.c | 145 +----- src/test/test_driver.c | 1162 +++++++++++++++++++++++++++++++++++++++++++++- tests/virshtest.c | 2 +- 7 files changed, 1328 insertions(+), 141 deletions(-) -- 1.8.3.1

3 29

[libvirt] [PATCH] nodedev: Resolve Relax-NG validity error
by John Ferlan 04 Oct '13

04 Oct '13

Commit id 'c4a4603de' added an output <path> to the nodedev xml, but did not update the schema. This resulted in the failure of the 'virt-xml-validate' on a file generated by 'virsh nodedev-dumpxml pci_0000_00_00_0' (for example). This was found/seen by running autotest on my host. --- I'm not sure if something like this needs to be backported or not, but I'm sure someone will let me know... docs/schemas/nodedev.rng | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/schemas/nodedev.rng b/docs/schemas/nodedev.rng index d2bebff..81ab4d4 100644 --- a/docs/schemas/nodedev.rng +++ b/docs/schemas/nodedev.rng @@ -13,6 +13,9 @@ and in virsh --> <element name="name"><text/></element> <optional> + <element name="path"><text/></element> + </optional> + <optional> <element name="parent"><text/></element> </optional> -- 1.8.3.1

2 2

[libvirt] [PATCH 0/5] Fixes for lxc veth handling
by Daniel P. Berrange 04 Oct '13

04 Oct '13

From: "Daniel P. Berrange" <berrange(a)redhat.com> The primary goal of this series was to address the race condition in creation of veth device names. A few other bugs were identified along the way though & fixed. Daniel P. Berrange (5): Don't set netdev offline in container cleanup Avoid reporting an error if veth device is already deleted Avoid deleting NULL veth device name Retry veth device creation on failure Use 'vnet' as prefix for veth devices src/lxc/lxc_process.c | 3 +- src/util/virnetdevveth.c | 168 ++++++++++++++++++++++++++++++----------------- 2 files changed, 107 insertions(+), 64 deletions(-) -- 1.8.3.1

3 11

[libvirt] [PATCH] test: snapshots: Fix some unused variables
by Cole Robinson 04 Oct '13

04 Oct '13

Spotted by coverity as John mentioned here: http://www.redhat.com/archives/libvir-list/2013-October/msg00078.html --- src/test/test_driver.c | 24 ++---------------------- 1 file changed, 2 insertions(+), 22 deletions(-) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 255cc2b..a09facb 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -6547,16 +6547,12 @@ testDomainSnapshotIsCurrent(virDomainSnapshotPtr snapshot, { virDomainObjPtr vm = NULL; int ret = -1; - virDomainSnapshotObjPtr snap = NULL; virCheckFlags(0, -1); if (!(vm = testDomObjFromSnapshot(snapshot))) goto cleanup; - if (!(snap = testSnapObjFromSnapshot(vm, snapshot))) - goto cleanup; - ret = (vm->current_snapshot && STREQ(snapshot->name, vm->current_snapshot->def->name)); @@ -6568,27 +6564,11 @@ cleanup: static int -testDomainSnapshotHasMetadata(virDomainSnapshotPtr snapshot, +testDomainSnapshotHasMetadata(virDomainSnapshotPtr snapshot ATTRIBUTE_UNUSED, unsigned int flags) { - virDomainObjPtr vm = NULL; - int ret = -1; - virDomainSnapshotObjPtr snap = NULL; - virCheckFlags(0, -1); - - if (!(vm = testDomObjFromSnapshot(snapshot))) - goto cleanup; - - if (!(snap = testSnapObjFromSnapshot(vm, snapshot))) - goto cleanup; - - ret = 1; - -cleanup: - if (vm) - virObjectUnlock(vm); - return ret; + return 1; } static int -- 1.8.3.1

2 1

[libvirt] [PATCH] build: fix 'make -C src check' under subset builds
by Eric Blake 04 Oct '13

04 Oct '13

Another build failure detected on cygwin, where lxc is not built. The lxc_monitor_protocol.c file requires the use of $(XDR_CFLAGS), but should only be built on platforms where WITH_LXC is true; however, 'make -C src check' was unconditionally trying to build the .o file (and with the wrong CFLAGS) in order to validate the matching -structs file. Rework the checks to only test files that are built for the current configuration. * src/Makefile.am (PROTOCOL_STRUCTS): Split list by which files are currently being compiled. (EXTRA_DIST): Ensure that -structs files are shipped even when not built. (check-protocol): Declare unconditionally, now that the condition has been moved into PROTOCOL_STRUCTS. (libvirt_driver_lxc_impl_la_CFLAGS): Add $(XDR_CFLAGS). (libvirt_net_rpc_la_SOURCES): List generated files more compactly. Signed-off-by: Eric Blake <eblake(a)redhat.com> --- This is a build-breaker, but I'm still in the middle of running a (lengthy) 'make distcheck' on several platforms before pushing, so a review wouldn't hurt. src/Makefile.am | 35 ++++++++++++++++++++++------------- 1 file changed, 22 insertions(+), 13 deletions(-) diff --git a/src/Makefile.am b/src/Makefile.am index 09311b7..20ba1c8 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -428,18 +428,35 @@ check-symsorting: EXTRA_DIST += check-symfile.pl check-symsorting.pl # Keep this list synced with RPC_PROBE_FILES -PROTOCOL_STRUCTS = \ +PROTOCOL_STRUCTS = +if WITH_REMOTE +PROTOCOL_STRUCTS += \ $(srcdir)/remote_protocol-structs \ $(srcdir)/lxc_protocol-structs \ $(srcdir)/qemu_protocol-structs \ + $(NULL) +endif WITH_REMOTE +PROTOCOL_STRUCTS += \ $(srcdir)/virnetprotocol-structs \ $(srcdir)/virkeepaliveprotocol-structs \ + $(NULL) +if WITH_LXC +PROTOCOL_STRUCTS += \ $(srcdir)/lxc_monitor_protocol-structs \ + $(NULL) +endif WITH_LXC +if WITH_LIBVIRTD +PROTOCOL_STRUCTS += \ $(srcdir)/lock_protocol-structs \ $(NULL) +endif WITH_LIBVIRTD + +# The various -structs files live in git, because they are only +# conditionally generated; ship one -structs file for each .x file. +EXTRA_DIST += $(notdir $(RPC_PROBE_FILES:.x=-structs))) -if WITH_REMOTE check-protocol: $(PROTOCOL_STRUCTS) $(PROTOCOL_STRUCTS:structs=struct) +.PHONY: check-protocol $(PROTOCOL_STRUCTS:structs=struct) # The .o file that pdwtags parses is created as a side effect of running # libtool; but from make's perspective we depend on the .lo file. @@ -458,13 +475,6 @@ $(srcdir)/lock_protocol-struct: \ $(srcdir)/%-struct: locking/lockd_la-%.lo $(PDWTAGS) -else !WITH_REMOTE -# The $(PROTOCOL_STRUCTS) files must live in git, because they cannot be -# re-generated when configured --without-remote. -check-protocol: -endif !WITH_REMOTE -EXTRA_DIST += $(PROTOCOL_STRUCTS) - check-drivername: $(AM_V_GEN)$(PERL) $(srcdir)/check-drivername.pl \ $(srcdir)/driver.h \ @@ -538,7 +548,6 @@ EXTRA_DIST += check-driverimpls.pl check-aclrules.pl check-aclperms.pl check-local: check-protocol check-symfile check-symsorting \ check-drivername check-driverimpls check-aclrules \ check-aclperms -.PHONY: check-protocol $(PROTOCOL_STRUCTS:structs=struct) # Mock driver, covering domains, storage, networks, etc TEST_DRIVER_SOURCES = \ @@ -1183,6 +1192,7 @@ endif ! WITH_DRIVER_MODULES libvirt_driver_lxc_impl_la_CFLAGS = \ $(LIBNL_CFLAGS) \ + $(XDR_CFLAGS) \ $(FUSE_CFLAGS) \ -I$(top_srcdir)/src/access \ -I$(top_srcdir)/src/conf \ @@ -2203,10 +2213,9 @@ BUILT_SOURCES += $(VIR_NET_RPC_GENERATED) libvirt_net_rpc_la_SOURCES = \ rpc/virnetmessage.h rpc/virnetmessage.c \ - rpc/virnetprotocol.h rpc/virnetprotocol.c \ rpc/virnetsocket.h rpc/virnetsocket.c \ - rpc/virkeepaliveprotocol.h rpc/virkeepaliveprotocol.c \ - rpc/virkeepalive.h rpc/virkeepalive.c + rpc/virkeepalive.h rpc/virkeepalive.c \ + $(VIR_NET_RPC_GENERATED) if WITH_SSH2 libvirt_net_rpc_la_SOURCES += \ rpc/virnetsshsession.h rpc/virnetsshsession.c -- 1.8.3.1

1 1

[libvirt] [PATCH] Fix crash in libvirtd when events are registered & ACLs active
by Daniel P. Berrange 04 Oct '13

04 Oct '13

From: "Daniel P. Berrange" <berrange(a)redhat.com> When a client disconnects from libvirtd, all event callbacks must be removed. This involves running the public API virConnectDomainEventDeregisterAny This code does not run in normal API dispatch context, so no identity was set. The result was that the access control drivers denied the attempt to deregister callbacks. The callbacks thus continued to trigger after the client was free'd causing fairly predictable use of free memory & a crash. This can be triggered by any client with readonly access when the ACL drivers are active. Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com> --- daemon/remote.c | 6 ++++++ 1 file changed, 6 insertions(+) NB, this bug is a publically reported security flaw and I'll be backporting it to other stable branches. A CVE will be assigned soon and will notify when that info is available. diff --git a/daemon/remote.c b/daemon/remote.c index 9497cc1..f3de6a0 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -666,8 +666,11 @@ void remoteClientFreeFunc(void *data) /* Deregister event delivery callback */ if (priv->conn) { + virIdentityPtr sysident = virIdentityGetSystem(); size_t i; + virIdentitySetCurrent(sysident); + for (i = 0; i < VIR_DOMAIN_EVENT_ID_LAST; i++) { if (priv->domainEventCallbackID[i] != -1) { VIR_DEBUG("Deregistering to relay remote events %zu", i); @@ -678,6 +681,9 @@ void remoteClientFreeFunc(void *data) } virConnectClose(priv->conn); + + virIdentitySetCurrent(NULL); + virObjectUnref(sysident); } VIR_FREE(priv); -- 1.8.3.1

2 2

[libvirt] [PATCH] qemu: check actual netdev type rather than config netdev type during init
by Laine Stump 03 Oct '13

03 Oct '13

This resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1012834 Note that a similar problem was reported in: https://bugzilla.redhat.com/show_bug.cgi?id=827519 but the fix only worked for <interface type='hostdev'>, *not* for <interface type='network'> where the network itself was a pool of hostdevs. The symptom in both cases was this error message: internal error: Unable to determine device index for network device In both cases the cause was lack of proper handling for netdevs (<interface>) of type='hostdev' when scanning the netdev list looking for alias names in qemuAssignDeviceNetAlias() - those that aren't type='hostdev' have an alias of the form "net%d", while those that are hostdev use "hostdev%d". This special handling was completely lacking prior to the fix for Bug 827519 which was: When searching for the highest alias index, libvirt looks at the alias for each netdev and if it is type='hostdev' it ignores the entry. If the type is not hostdev, then it expects the "net%d" form; if it doesn't find that, it fails and logs the above error message. That fix works except in the case of <interface type='network'> where the network uses hostdev (i.e. the network is a pool of VFs to be assigned to the guests via PCI passthrough). In this case, the check for type='hostdev' would fail because it was done as: def->net[i]->type == VIR_DOMAIN_NET_TYPE_HOSTDEV (which compares what was written in the config) when it actually should have been: virDomainNetGetActualType(def->net[i]) == VIR_DOMAIN_NET_TYPE_HOSTDEV (which compares the type of netdev that was actually allocated from the network at runtime). Of course the latter wouldn't be of any use if the netdevs of type='network' hadn't already acquired their actual network connection yet, but manual examination of the code showed that this is never the case. While looking through qemu_command.c, two other places were found to directly compare the net[i]->type field rather than getting actualType: * qemuAssignDeviceAliases() - in this case, the incorrect comparison would cause us to create a "net%d" alias for a netdev with type='network' but actualType='hostdev'. This alias would be subsequently overwritten by the proper "hostdev%d" form, so everything would operate properly, but a string would be leaked. This patch also fixes this problem. * qemuAssignDevicePCISlots() - would defer assigning a PCI address to a netdev if it was type='hostdev', but not for type='network + actualType='hostdev'. In this case, the actual device usually hasn't been acquired yet anyway, and even in the case that it has, there is no practical difference between assigning a PCI address while traversing the netdev list or while traversing the hostdev list. Because changing it would be an effective NOP (but potentially cause some unexpected regression), this usage was left unchanged. --- src/qemu/qemu_command.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index ba102f4..e976466 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -817,7 +817,8 @@ qemuAssignDeviceNetAlias(virDomainDefPtr def, virDomainNetDefPtr net, int idx) for (i = 0; i < def->nnets; i++) { int thisidx; - if (def->nets[i]->type == VIR_DOMAIN_NET_TYPE_HOSTDEV) { + if (virDomainNetGetActualType(def->nets[i]) + == VIR_DOMAIN_NET_TYPE_HOSTDEV) { /* type='hostdev' interfaces have a hostdev%d alias */ continue; } @@ -987,8 +988,9 @@ qemuAssignDeviceAliases(virDomainDefPtr def, virQEMUCapsPtr qemuCaps) /* type='hostdev' interfaces are also on the hostdevs list, * and will have their alias assigned with other hostdevs. */ - if ((def->nets[i]->type != VIR_DOMAIN_NET_TYPE_HOSTDEV) && - (qemuAssignDeviceNetAlias(def, def->nets[i], i) < 0)) { + if (virDomainNetGetActualType(def->nets[i]) + != VIR_DOMAIN_NET_TYPE_HOSTDEV && + qemuAssignDeviceNetAlias(def, def->nets[i], i) < 0) { return -1; } } -- 1.7.11.7

2 1