- Devel - libvirt List Archives

[libvirt] [PATCH] qemu: Fix the memory leak
by Osier Yang 21 Feb '13

21 Feb '13

Found by John Ferlan (coverity script) --- Assuming an ACK from John, and it's trivial, so pushed --- src/qemu/qemu_conf.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 8299b79..33fd67d 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1042,6 +1042,7 @@ qemuAddSharedDisk(virQEMUDriverPtr driver, if ((VIR_ALLOC(entry) < 0) || (VIR_ALLOC_N(entry->domains, 1) < 0) || !(entry->domains[0] = strdup(name))) { + qemuSharedDiskEntryFree(entry, NULL); virReportOOMError(); goto cleanup; } -- 1.7.7.6

1 0

[libvirt] [PATCH] keepalive: Guard against integer overflow
by John Ferlan 21 Feb '13

21 Feb '13

Don't allow interval to be > MAX_INT/1000 in virKeepAliveStart() Guard against possible overflow in virKeepAliveTimeout() by setting the timeout to be MAX_INT/1000 since the math following will multiply it by 1000. This is a follow-up of sorts from a Coverity change made last month: https://www.redhat.com/archives/libvir-list/2013-January/msg02267.html where it was noted that the timeout value math needed overflow protection. --- src/rpc/virkeepalive.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/rpc/virkeepalive.c b/src/rpc/virkeepalive.c index d1fa642..6d69559 100644 --- a/src/rpc/virkeepalive.c +++ b/src/rpc/virkeepalive.c @@ -252,6 +252,12 @@ virKeepAliveStart(virKeepAlivePtr ka, _("keepalive interval already set")); goto cleanup; } + /* Guard against overflow */ + if (interval > INT_MAX / 1000) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("keepalive interval too large")); + goto cleanup; + } ka->interval = interval; ka->count = count; ka->countToDeath = count; @@ -323,6 +329,9 @@ virKeepAliveTimeout(virKeepAlivePtr ka) timeout = ka->interval - (time(NULL) - ka->intervalStart); if (timeout < 0) timeout = 0; + /* Guard against overflow */ + if (timeout > INT_MAX / 1000) + timeout = INT_MAX / 1000; } virObjectUnlock(ka); -- 1.7.11.7

2 3

[libvirt] [PATCH] libxl: Fix setting of disk backend
by Jim Fehlig 21 Feb '13

21 Feb '13

The libxl driver was setting the backend field of libxl_device_disk structure to LIBXL_DISK_BACKEND_TAP when the driver element of disk configuration was not specified. This needlessly forces the use of blktap driver, which may not be loaded in dom0 https://bugzilla.redhat.com/show_bug.cgi?id=912488 Ian Campbell suggested that LIBXL_DISK_BACKEND_UNKNOWN is a better default in this case https://www.redhat.com/archives/libvir-list/2013-February/msg01126.html --- src/libxl/libxl_conf.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 43fb8b1..4ce5dec 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -525,9 +525,13 @@ libxlMakeDisk(virDomainDiskDefPtr l_disk, libxl_device_disk *x_disk) return -1; } } else { - /* No driverName - default to raw/tap?? */ + /* + * If driverName is not specified, default to raw as per + * xl-disk-configuration.txt in the xen documentation and let + * libxl pick a suitable backend. + */ x_disk->format = LIBXL_DISK_FORMAT_RAW; - x_disk->backend = LIBXL_DISK_BACKEND_TAP; + x_disk->backend = LIBXL_DISK_BACKEND_UNKNOWN; } /* XXX is this right? */ -- 1.8.0.1

2 2

[libvirt] [PATCH] Remove a couple of misplaced VIR_FREE
by John Ferlan 21 Feb '13

21 Feb '13

While working on the hellolibvirt example code, I stumbled across a couple extraneous VIR_FREE()'s in qemuStop(). I was looking at all callers of virConnectListAllDomains()... --- src/qemu/qemu_driver.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index be01ec6..45bd341 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -920,9 +920,6 @@ qemuStop(void) { if (virDomainManagedSave(domains[i], flags[i]) < 0) ret = -1; - VIR_FREE(domains); - VIR_FREE(flags); - cleanup: for (i = 0 ; i < numDomains ; i++) virDomainFree(domains[i]); -- 1.7.11.7

2 2

[libvirt] libvirt-libxl driver defaulting to tap disk and not working (on Fedora 18 and rawhide)
by Dario Faggioli 20 Feb '13

20 Feb '13

Hi Jim, Everyone, I'm having some issues when trying out libvirt-libxl driver on my Fedora, bot on Fedora 18 and Fedorra rawhide (the former seems to be running 0.10.2.3, the later 1.0.2). Here's what happen: [root@localhost ~]# systemctl status xend.service xend.service - Xend - interface between hypervisor and some applications Loaded: loaded (/usr/lib/systemd/system/xend.service; disabled) Active: inactive (dead) [root@localhost ~]# virt-install -l http://fedora.mirror.constant.com/linux/releases/18/Fedora/x86_64/os/ --ram 1024 --disk /dev/vms/F18_x64 --name F18_x64 Starting install... Retrieving file .treeinfo... | 2.2 kB 00:00 !!! Retrieving file vmlinuz... | 9.3 MB 00:06 !!! Retrieving file initrd.img... | 53 MB 00:33 !!! ERROR internal error libxenlight failed to create new domain 'F18_x64' Domain installation does not appear to have been successful. If it was, you can restart your domain by running: virsh --connect xen:/// start F18_x64 otherwise, please restart your installation. [root@localhost ~]# cat /var/log/libvirt/libxl/libxl.log ... libxl: verbose: libxl_create.c:158:libxl__domain_build_info_setdefault: qemu-xen is unavailable, use qemu-xen-traditional instead: No such file or directory libxl: debug: libxl_create.c:1174:do_domain_create: ao 0x7f566c008d90: create: how=(nil) callback=(nil) poller=0x7f566c009030 libxl: debug: libxl_device.c:229:libxl__device_disk_set_backend: Disk vdev=xvda spec.backend=tap libxl: debug: libxl_device.c:184:disk_try_backend: Disk vdev=xvda, backend tap unsuitable because blktap not available libxl: error: libxl_device.c:269:libxl__device_disk_set_backend: no suitable backend for disk xvda libxl: debug: libxl_event.c:1499:libxl__ao_complete: ao 0x7f566c008d90: complete, rc=-3 libxl: debug: libxl_create.c:1187:do_domain_create: ao 0x7f566c008d90: inprogress: poller=0x7f566c009030, flags=ic libxl: debug: libxl_event.c:1471:libxl__ao__destroy: ao 0x7f566c008d90: destroy So, it looks like it tries to use blktap, even if I'm using an LVM volume, and fails. If I go for this: [root@localhost ~]# virt-install -l http://fedora.mirror.constant.com/linux/releases/18/Fedora/x86_64/os/ --ram 1024 --disk /dev/vms/F18_x64,driver_name=phy --name F18_x64 --bridge virbr0 it works, but only because (or at least so it looks to me) I'm manually providing ",driver_name=phy". Also (as it could have been expected, I guess) there is no way I can get it to work with a file backed VHD. Is all this supposed to happen? I haven't had the chance to try out the code from git yet, do you think it would make any difference? FYI, I've also opened this Fedora bugreport, about this very same issue: https://bugzilla.redhat.com/show_bug.cgi?id=912488 Thanks and Regards, Dario -- <<This happens because I choose it to happen!>> (Raistlin Majere) ----------------------------------------------------------------- Dario Faggioli, Ph.D, http://about.me/dario.faggioli Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)

3 5

[libvirt] libvirt-client leaks memory, Ubuntu and Debian-specific
by Igor Lukyanov 20 Feb '13

20 Feb '13

Hello. We faced a very strange leak while using libvirt library in long-running server application for cluster orchestration. Leak does not directly related to libvirt code and exposed only on specific build options (?) and/or system environment (?). Here are the key points: 1. Libvirt client leaks memory while making (RPC) calls to a server. I mean that RSS memory usage shown by ps, top, etc. indefinitely grows (plz check an attachment ps.log). Test app attached. 2. Leak detected on Debian and Ubuntu and absent on Mac OS and Gentoo, so it's exactly an environment or build problem. 3. Valgrind does not see the leak. From valgrind's point of view from start to finish application constantly consumes 110kb of memory (while ps shows multiple megabytes) and does not contain any leaks. 4. Logging activity of virMalloc, virRealloc, etc. functions does not show anything: as expected, all allocated memory is correctly freed, so it's definitely not a bug of code (we tested that before recognized that problem is distrib/platform specific). Some useful logs and test code attached. I think digging build options and legoing system libraries will help us to beat the problem but it would be nice if someone already had a working solution. Thank you for help.

3 5

[libvirt] [PATCH 0/6 v3] Shared disk table related fixes/improvements
by Osier Yang 20 Feb '13

20 Feb '13

Inspires by the crash when using CD-ROM or Floppy with empty source, except the fixes for the crashes, this also enrichs the hash table with a list of names of domain which use the shared disk. And to keep the hash table to be in consistent state, this also fixes the problems on adding/removing the hash entry when updateing/ejecting media of CD-ROM or Floppy. And updates the hash table when reconnecting domain process. v2 - v3: * Rebase on the top * Small changes after more testing. Osier Yang (6): qemu: Add checking in helpers for sgio setting qemu: Merge qemuCheckSharedDisk into qemuAddSharedDisk qemu: Record names of domain which uses the shared disk in hash table qemu: Update shared disk table when reconnecting qemu process qemu: Move the shared disk adding and sgio setting prior to attaching qemu: Remove the shared disk entry if the operation is ejecting or updating src/conf/domain_conf.c | 20 ++++ src/conf/domain_conf.h | 3 + src/libvirt_private.syms | 1 + src/qemu/qemu_conf.c | 243 ++++++++++++++++++++++++++++++++++++++++++--- src/qemu/qemu_conf.h | 26 ++++- src/qemu/qemu_driver.c | 108 ++++++++++++++++----- src/qemu/qemu_hotplug.c | 19 +---- src/qemu/qemu_hotplug.h | 1 + src/qemu/qemu_process.c | 81 ++++------------ src/qemu/qemu_process.h | 3 - 10 files changed, 377 insertions(+), 128 deletions(-) -- 1.7.7.6

3 19

[libvirt] [PATCH] maint: enforce private symbol section sorting
by Eric Blake 20 Feb '13

20 Feb '13

Automating a sorting check is the only way to ensure we don't regress. Suggested by Dan Berrange. * src/check-symsorting.pl (check_sorting): Add a parameter, validate that groups are in order, and that files exist. * src/Makefile.am (check-symsorting): Adjust caller. * src/libvirt_private.syms: Fix typo. * src/libvirt_linux.syms: Fix file name. * src/libvirt_vmx.syms: Likewise. * src/libvirt_xenxs.syms: Likewise. * src/libvirt_sasl.syms: Likewise. * src/libvirt_libssh2.syms: Likewise. * src/libvirt_esx.syms: Mention file name. * src/libvirt_openvz.syms: Likewise. --- While I can manage in perl, I probably don't have the best usage patterns; suggestions for improving this patch are quite welcome. src/Makefile.am | 3 ++- src/check-symsorting.pl | 35 +++++++++++++++++++++++++++++------ src/libvirt_esx.syms | 1 + src/libvirt_libssh2.syms | 3 +-- src/libvirt_linux.syms | 2 +- src/libvirt_openvz.syms | 2 ++ src/libvirt_private.syms | 2 +- src/libvirt_sasl.syms | 8 ++++---- src/libvirt_vmx.syms | 2 +- src/libvirt_xenxs.syms | 4 ++-- 10 files changed, 44 insertions(+), 18 deletions(-) diff --git a/src/Makefile.am b/src/Makefile.am index 780cd52..3ef9a9c 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -388,7 +388,8 @@ else check-symfile: endif check-symsorting: - $(AM_V_GEN)$(PERL) $(srcdir)/check-symsorting.pl $(USED_SYM_FILES) + $(AM_V_GEN)$(PERL) $(srcdir)/check-symsorting.pl \ + $(srcdir) $(USED_SYM_FILES) EXTRA_DIST += check-symfile.pl check-symsorting.pl PROTOCOL_STRUCTS = \ diff --git a/src/check-symsorting.pl b/src/check-symsorting.pl index 9c62246..cbcb737 100755 --- a/src/check-symsorting.pl +++ b/src/check-symsorting.pl @@ -3,22 +3,27 @@ use strict; use warnings; -die "syntax: $0 SYMFILE..." unless int(@ARGV) >= 1; +die "syntax: $0 SRCDIR SYMFILE..." unless int(@ARGV) >= 2; my $ret = 0; +my $srcdir = shift; +my $lastgroup = undef; foreach my $symfile (@ARGV) { open SYMFILE, $symfile or die "cannot read $symfile: $!"; my $line; + my $groupfile; my @group; while (<SYMFILE>) { chomp; - next if /^#/; - if (/^\s*$/) { + if (/^#/) { + $_ =~ s/^# //; + $groupfile = $_; + } elsif (/^\s*$/) { if (@group) { - &check_sorting(\@group, $symfile, $line); + &check_sorting(\@group, $symfile, $line, $groupfile); } @group = (); $line = $.; @@ -30,20 +35,38 @@ foreach my $symfile (@ARGV) { close SYMFILE; if (@group) { - &check_sorting(\@group, $symfile, $line); + &check_sorting(\@group, $symfile, $line, $groupfile); } + $lastgroup = undef; } sub check_sorting { my $group = shift; my $symfile = shift; my $line = shift; + my $groupfile = shift; my @group = @{$group}; my @sorted = sort { lc $a cmp lc $b } @group; my $sorted = 1; my $first; my $last; + + # Check that groups are in order and groupfile exists + if (defined $lastgroup && lc $lastgroup ge lc $groupfile) { + print "Symbol block at $symfile:$line: block not sorted\n"; + print "Move $groupfile block before $lastgroup block\n"; + print "\n"; + $ret = 1; + } + if (! -e "$srcdir/$groupfile") { + print "Symbol block at $symfile:$line: $groupfile not found\n"; + print "\n"; + $ret = 1; + } + $lastgroup = $groupfile; + + # Check that symbols within a group are in order for (my $i = 0 ; $i <= $#sorted ; $i++) { if ($sorted[$i] ne $group[$i]) { $first = $i unless defined $first; @@ -54,7 +77,7 @@ sub check_sorting { if (!$sorted) { @group = splice @group, $first, ($last-$first+1); @sorted = splice @sorted, $first, ($last-$first+1); - print "Symbol block at $symfile:$line symbols not sorted\n"; + print "Symbol block at $symfile:$line: symbols not sorted\n"; print map { " " . $_ . "\n" } @group; print "Correct ordering\n"; print map { " " . $_ . "\n" } @sorted; diff --git a/src/libvirt_esx.syms b/src/libvirt_esx.syms index ad3d60a..3c14b94 100644 --- a/src/libvirt_esx.syms +++ b/src/libvirt_esx.syms @@ -2,6 +2,7 @@ # These symbols are dependent upon --with-esx via WITH_ESX # +# esx/esx_util.h esxUtil_EscapeDatastoreItem; esxUtil_ParseDatastorePath; esxVI_DateTime_ConvertToCalendarTime; diff --git a/src/libvirt_libssh2.syms b/src/libvirt_libssh2.syms index 815195b..55022b5 100644 --- a/src/libvirt_libssh2.syms +++ b/src/libvirt_libssh2.syms @@ -2,8 +2,7 @@ # ssh session - specific symbols # -# virnetsshsession.h -# +# rpc/virnetsshsession.h virNetSSHChannelRead; virNetSSHChannelWrite; virNetSSHSessionAuthAddAgentAuth; diff --git a/src/libvirt_linux.syms b/src/libvirt_linux.syms index b5f7ee2..3500898 100644 --- a/src/libvirt_linux.syms +++ b/src/libvirt_linux.syms @@ -5,7 +5,7 @@ # nodeinfo.h linuxNodeInfoCPUPopulate; -# stats_linux.h +# util/virstatslinux.h linuxDomainInterfaceStats; # Let emacs know we want case-insensitive sorting diff --git a/src/libvirt_openvz.syms b/src/libvirt_openvz.syms index 58b93ae..ac0ed0d 100644 --- a/src/libvirt_openvz.syms +++ b/src/libvirt_openvz.syms @@ -1,6 +1,8 @@ # # These symbols are dependent upon --with-openvz via WITH_OPENVZ # + +# openvz/openvz_conf.h openvzLocateConfFile; openvzReadConfigParam; openvzReadNetworkConf; diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 1686bb8..3c44cc3 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -420,7 +420,7 @@ virNetDevVlanFormat; virNetDevVlanParse; -# conf/netdev_vportprofile_conf.h +# conf/netdev_vport_profile_conf.h virNetDevVPortProfileFormat; virNetDevVPortProfileParse; virNetDevVPortTypeFromString; diff --git a/src/libvirt_sasl.syms b/src/libvirt_sasl.syms index 099f48a..beb8825 100644 --- a/src/libvirt_sasl.syms +++ b/src/libvirt_sasl.syms @@ -2,10 +2,10 @@ # SASL-specific symbols # -# virnetclient.h +# rpc/virnetclient.h virNetClientSetSASLSession; -# virnetsaslcontext.h +# rpc/virnetsaslcontext.h virNetSASLContextCheckIdentity; virNetSASLContextNewClient; virNetSASLContextNewServer; @@ -25,11 +25,11 @@ virNetSASLSessionServerStart; virNetSASLSessionServerStep; -# virnetserverclient.h +# rpc/virnetserverclient.h virNetServerClientSetSASLSession; -# virnetsocket.h +# rpc/virnetsocket.h virNetSocketSetSASLSession; # Let emacs know we want case-insensitive sorting diff --git a/src/libvirt_vmx.syms b/src/libvirt_vmx.syms index 99fe590..0b15f49 100644 --- a/src/libvirt_vmx.syms +++ b/src/libvirt_vmx.syms @@ -2,7 +2,7 @@ # These symbols are dependent upon --with-esx via WITH_ESX or --with-vmware via WITH_VMWARE. # -# vmx.h +# vmx/vmx.h virVMXConvertToUTF8; virVMXEscapeHex; virVMXFormatCDROM; diff --git a/src/libvirt_xenxs.syms b/src/libvirt_xenxs.syms index 75d5322..04b35c4 100644 --- a/src/libvirt_xenxs.syms +++ b/src/libvirt_xenxs.syms @@ -2,7 +2,7 @@ # These symbols are dependent upon --with-xen via WITH_XEN or --with-libxl via WITH_LIBXL. # -# xen_sxpr.h +# xenxs/xen_sxpr.h xenFormatSxpr; xenFormatSxprChr; xenFormatSxprDisk; @@ -16,7 +16,7 @@ xenParseSxprChar; xenParseSxprSound; xenParseSxprString; -# xen_xm.h +# xenxs/xen_xm.h xenFormatXM; xenParseXM; -- 1.8.1.2

2 2

[libvirt] [PATCH] qemu: Do not ignore mandatory features in migration cookie
by Jiri Denemark 20 Feb '13

20 Feb '13

Due to "feature"/"features" nasty typo, any features marked as mandatory by one side of a migration are silently considered optional by the other side. The following is the code that formats mandatory features in migration cookie: for (i = 0 ; i < QEMU_MIGRATION_COOKIE_FLAG_LAST ; i++) { if (mig->flagsMandatory & (1 << i)) virBufferAsprintf(buf, " <feature name='%s'/>\n", qemuMigrationCookieFlagTypeToString(i)); } --- src/qemu/qemu_migration.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 82c3f97..17c1714 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -863,7 +863,7 @@ qemuMigrationCookieXMLParse(qemuMigrationCookiePtr mig, /* Check to ensure all mandatory features from XML are also * present in 'flags' */ - if ((n = virXPathNodeSet("./features", ctxt, &nodes)) < 0) + if ((n = virXPathNodeSet("./feature", ctxt, &nodes)) < 0) goto error; for (i = 0 ; i < n ; i++) { -- 1.8.1.2

2 2

[libvirt] [PATCH v2] qemu: Run lzop with '--ignore-warn'
by Michal Privoznik 20 Feb '13

20 Feb '13

Currently, if lzop decompression binary produces a warning, it doesn't exit with zero status but 2 instead. Terrifying, but true. However, warnings may be ignored using '--ignore-warn' command line argument. Moreover, in which case, the exit status will be zero. --- src/qemu/qemu_driver.c | 62 ++++++++++++++++++++++++++++++++------------------ 1 file changed, 40 insertions(+), 22 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index dc35b91..a0a1f04 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -2507,6 +2507,34 @@ qemuCompressProgramName(int compress) qemuSaveCompressionTypeToString(compress)); } +static virCommandPtr +qemuCompressGetCommand(virQEMUSaveFormat compression) +{ + virCommandPtr ret = NULL; + const char *prog = qemuSaveCompressionTypeToString(compression); + + if (!prog) { + virReportError(VIR_ERR_OPERATION_FAILED, + _("Invalid compressed save format %d"), + compression); + return NULL; + } + + ret = virCommandNew(prog); + virCommandAddArg(ret, "-dc"); + + switch (compression) { + case QEMU_SAVE_FORMAT_LZOP: + virCommandAddArg(ret, "--ignore-warn"); + break; + default: + /* Ain't no valid compressed save format */ + break; + } + + return ret; +} + /* Internal function to properly create or open existing files, with * ownership affected by qemu driver setup. */ static int @@ -4775,32 +4803,22 @@ qemuDomainSaveImageStartVM(virConnectPtr conn, if (!(caps = virQEMUDriverGetCapabilities(driver, false))) goto cleanup; - if (header->version == 2) { - const char *prog = qemuSaveCompressionTypeToString(header->compressed); - if (prog == NULL) { - virReportError(VIR_ERR_OPERATION_FAILED, - _("Invalid compressed save format %d"), - header->compressed); + if ((header->version == 2) && + (header->compressed != QEMU_SAVE_FORMAT_RAW)) { + if (!(cmd = qemuCompressGetCommand(header->compressed))) goto cleanup; - } - if (header->compressed != QEMU_SAVE_FORMAT_RAW) { - cmd = virCommandNewArgList(prog, "-dc", NULL); - intermediatefd = *fd; - *fd = -1; + intermediatefd = *fd; + *fd = -1; - virCommandSetInputFD(cmd, intermediatefd); - virCommandSetOutputFD(cmd, fd); - virCommandSetErrorBuffer(cmd, &errbuf); - virCommandDoAsyncIO(cmd); + virCommandSetInputFD(cmd, intermediatefd); + virCommandSetOutputFD(cmd, fd); + virCommandSetErrorBuffer(cmd, &errbuf); + virCommandDoAsyncIO(cmd); - if (virCommandRunAsync(cmd, NULL) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Failed to start decompression binary %s"), - prog); - *fd = intermediatefd; - goto cleanup; - } + if (virCommandRunAsync(cmd, NULL) < 0) { + *fd = intermediatefd; + goto cleanup; } } -- 1.8.1.2

2 1