- Devel - libvirt List Archives

[libvirt] [PATCH] Fix memory leak when lookup pool list with invalid type option
by shyu 24 Feb '14

24 Feb '14

There will be memory leak when lookup pool list with invalid type option https://bugzilla.redhat.com/show_bug.cgi?id=1069068 ==23060== Memcheck, a memory error detector ==23060== Copyright (C) 2002-2012, and GNU GPL'd, by Julian Seward et al. ==23060== Using Valgrind-3.8.1 and LibVEX; rerun with -h for copyright info ==23060== Command: virsh pool-list --type invalid ==23060== error: Invalid pool type ==23060== ==23060== HEAP SUMMARY: ==23060== in use at exit: 138,383 bytes in 1,559 blocks ==23060== total heap usage: 4,199 allocs, 2,640 frees, 21,372,544 bytes allocated ==23060== ==23060== LEAK SUMMARY: ==23060== definitely lost: 8 bytes in 1 blocks ==23060== indirectly lost: 0 bytes in 0 blocks ==23060== possibly lost: 0 bytes in 0 blocks ==23060== still reachable: 138,375 bytes in 1,558 blocks ==23060== suppressed: 0 bytes in 0 blocks ==23060== Rerun with --leak-check=full to see details of leaked memory ==23060== ==23060== For counts of detected and suppressed errors, rerun with: -v ==23060== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 8 from 6) Signed-off-by: shyu <shyu(a)redhat.com> --- tools/virsh-pool.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/tools/virsh-pool.c b/tools/virsh-pool.c index 642b078..db4d3ae 100644 --- a/tools/virsh-pool.c +++ b/tools/virsh-pool.c @@ -859,6 +859,7 @@ cmdPoolList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) for (i = 0; i < npoolTypes; i++) { if ((poolType = virStoragePoolTypeFromString(poolTypes[i])) < 0) { vshError(ctl, "%s", _("Invalid pool type")); + VIR_FREE(*poolTypes); VIR_FREE(poolTypes); return false; } -- 1.7.1

3 3

[libvirt] block pull/commit for non-local storage
by Peter Krempa 24 Feb '14

24 Feb '14

Hi, with my recent work into snapshots with native libgfapi support I've run into an issue with libvirt APIs used to delete snapshots by management apps. The management apps use the APIs to do the job: int virDomainBlockCommit(virDomainPtr dom, const char *disk, const char *base, const char *top, unsigned long bandwidth, unsigned int flags) int virDomainBlockPull(virDomainPtr dom, const char *disk, unsigned long bandwidth, unsigned int flags) int virDomainBlockRebase(virDomainPtr dom, const char *disk, const char *base, unsigned long bandwidth, unsigned int flags) As you can see in the prototypes of these functions (and from the docs for them which I'm not going to copy here) the user can provide the disk specification in two possible options: 1) a full path if it's unambiguous - this is file centric and requires the file to be in the local filesystem 2) a disk name "vda" - this is selected automatically by libvirt but allows to specify only the top image. For systems that want to use remote storage without local representation such as gluster+libgfapi, this doesn't allow to use the APIs to start block jobs. To solve this issue we need a way to specify paths on remote storage in some way. Below are two options we've discussed on IRC. 1) Use URIs along with the file path to specify disk images. This option would add a new, possibly well documented URIs to specify paths for disk images. These would be libvirt defined URIs (but surprisingly "similar" to qemu URIS) so that hypervisors with different storage specification would need a conversion. This would allow to specify the targets as: vda - disk name /path/to/file - legacy way, path file:///path/to/file - new way of file paths block:///dev/blah - new way, block devs gluster://server/vol/img - new way, remote images ... Possible caveats: RBD for example allows to use multiple hosts and we'd need to introduce a possibility to specify it if we'd add support for this on rbd. 2) Export the image chain in the XML and allow to use indexed disk names This option would require to export the backing chain in the XML in some way, either the existing disk source specification in multiple elements (which I don't like as it is a bit convoluted), or possibly again via URIs. Then the user would be allowed to specify vda[2] for the second backing image of the vda disk. With this the internal representations of the backing chain would be used without the need for the user to specify path. A possible caveat here is that if backing chains for some reason will be converted to backing trees, this approach will be invalid. 3) ? anyone suggesting something better? :) Thanks in advance for suggestions and/or new ideas. Peter

4 7

[libvirt] [PATCH] cpu: break out when a right cpuCandidate found
by Wangyufei (James) 24 Feb '14

24 Feb '14

>From 8123c5d64f940fa0fb0de32fc5e68035980b6b01 Mon Sep 17 00:00:00 2001 From: WangYufei <james.wangyufei(a)huawei.com> Date: Thu, 13 Feb 2014 07:17:11 +0000 Subject: [PATCH] cpu: break out when a right cpuCandidate found In function x86Decode there's a code segment in while cycle like this: if (cpuModel == NULL || cpuModel->nfeatures > cpuCandidate->nfeatures) { virCPUDefFree(cpuModel); cpuModel = cpuCandidate; cpuData = candidate->data; } else { virCPUDefFree(cpuCandidate); } when it finds the right cpuCandidate, it doesn't break out the cycle, but continues run in it, and cpuModel will never get a new value, it's meaningless. It should break out when a right cpuCndidate found. Signed-off-by: WangYufei <james.wangyufei(a)huawei.com> --- src/cpu/cpu_x86.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index 56080ef..546b757 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c @@ -1558,6 +1558,7 @@ x86Decode(virCPUDefPtr cpu, virCPUDefFree(cpuModel); cpuModel = cpuCandidate; cpuData = candidate->data; + break; } else { virCPUDefFree(cpuCandidate); } -- 1.7.12.4 Best Regards, -WangYufei

4 4

[libvirt] [PATCH] bhyve: implement node information reporting
by Roman Bogorodskiy 24 Feb '14

24 Feb '14

- Implement nodeGetCPUStats using nodeGetCPUStats() - Implement nodeGetMemoryStats using nodeGetMemoryStats() --- src/bhyve/bhyve_driver.c | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c index 7c6500f..35171d2 100644 --- a/src/bhyve/bhyve_driver.c +++ b/src/bhyve/bhyve_driver.c @@ -48,6 +48,7 @@ #include "virstring.h" #include "cpu/cpu.h" #include "viraccessapicheck.h" +#include "nodeinfo.h" #include "bhyve_driver.h" #include "bhyve_process.h" @@ -497,6 +498,32 @@ cleanup: } static int +bhyveNodeGetCPUStats(virConnectPtr conn, + int cpuNum, + virNodeCPUStatsPtr params, + int *nparams, + unsigned int flags) +{ + if (virNodeGetCPUStatsEnsureACL(conn) < 0) + return -1; + + return nodeGetCPUStats(cpuNum, params, nparams, flags); +} + +static int +bhyveNodeGetMemoryStats(virConnectPtr conn, + int cellNum, + virNodeMemoryStatsPtr params, + int *nparams, + unsigned int flags) +{ + if (virNodeGetMemoryStatsEnsureACL(conn) < 0) + return -1; + + return nodeGetMemoryStats(cellNum, params, nparams, flags); +} + +static int bhyveStateCleanup(void) { VIR_DEBUG("bhyve state cleanup"); @@ -594,6 +621,8 @@ static virDriver bhyveDriver = { .domainLookupByName = bhyveDomainLookupByName, /* 1.2.2 */ .domainDefineXML = bhyveDomainDefineXML, /* 1.2.2 */ .domainGetXMLDesc = bhyveDomainGetXMLDesc, /* 1.2.2 */ + .nodeGetCPUStats = bhyveNodeGetCPUStats, /* 1.2.2 */ + .nodeGetMemoryStats = bhyveNodeGetMemoryStats, /* 1.2.2 */ }; -- 1.8.4.3

2 2

[libvirt] [PATCH v3] Ensure systemd cgroup ownership is delegated to container with userns
by Daniel P. Berrange 24 Feb '14

24 Feb '14

From: Richard Weinberger <richard(a)nod.at> This function is needed for user namespaces, where we need to chmod() the cgroup to the initial uid/gid such that systemd is allowed to use the cgroup. Signed-off-by: Richard Weinberger <richard(a)nod.at> Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com> Changed in v3: - Centralized all error reporting - Use virReportSystemError not VIR_WARN/VIR_ERROR --- src/libvirt_private.syms | 1 + src/lxc/lxc_cgroup.c | 12 +++++++++ src/util/vircgroup.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++++ src/util/vircgroup.h | 5 ++++ 4 files changed, 88 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 0b28bac..cfa9f75 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1056,6 +1056,7 @@ virCgroupSetMemory; virCgroupSetMemoryHardLimit; virCgroupSetMemorySoftLimit; virCgroupSetMemSwapHardLimit; +virCgroupSetOwner; virCgroupSupportsCpuBW; diff --git a/src/lxc/lxc_cgroup.c b/src/lxc/lxc_cgroup.c index cc0d5e8..39d955c 100644 --- a/src/lxc/lxc_cgroup.c +++ b/src/lxc/lxc_cgroup.c @@ -484,6 +484,18 @@ virCgroupPtr virLXCCgroupCreate(virDomainDefPtr def) &cgroup) < 0) goto cleanup; + /* setup control group permissions for user namespace */ + if (def->idmap.uidmap) { + if (virCgroupSetOwner(cgroup, + def->idmap.uidmap[0].target, + def->idmap.gidmap[0].target, + (1 << VIR_CGROUP_CONTROLLER_SYSTEMD)) < 0) { + virCgroupFree(&cgroup); + cgroup = NULL; + goto cleanup; + } + } + cleanup: return cgroup; } diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index a6d60c5..18c891c 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -3253,6 +3253,76 @@ cleanup: } +int virCgroupSetOwner(virCgroupPtr cgroup, + uid_t uid, + gid_t gid, + int controllers) +{ + int ret = -1; + size_t i; + char *base = NULL, *entry = NULL; + DIR *dh = NULL; + + for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) { + struct dirent *de; + + if (!((1 << i) & controllers)) + continue; + + if (!cgroup->controllers[i].mountPoint) + continue; + + if (virAsprintf(&base, "%s%s", cgroup->controllers[i].mountPoint, + cgroup->controllers[i].placement) < 0) + goto cleanup; + + if (!(dh = opendir(base))) { + virReportSystemError(errno, + _("Unable to open dir '%s'"), base); + goto cleanup; + } + + while ((de = readdir(dh)) != NULL) { + if (STREQ(de->d_name, ".") || + STREQ(de->d_name, "..")) + continue; + + if (virAsprintf(&entry, "%s/%s", base, de->d_name) < 0) + goto cleanup; + + if (chown(entry, uid, gid) < 0) { + virReportSystemError(errno, + _("cannot chown '%s' to (%u, %u)"), + entry, uid, gid); + goto cleanup; + } + + VIR_FREE(entry); + } + + if (chown(base, uid, gid) < 0) { + virReportSystemError(errno, + _("cannot chown '%s' to (%u, %u)"), + base, uid, gid); + goto cleanup; + } + + VIR_FREE(base); + closedir(dh); + dh = NULL; + } + + ret = 0; + + cleanup: + if (dh) + closedir(dh); + VIR_FREE(entry); + VIR_FREE(base); + return ret; +} + + /** * virCgroupSupportsCpuBW(): * Check whether the host supports CFS bandwidth. diff --git a/src/util/vircgroup.h b/src/util/vircgroup.h index a70eb18..38d94f3 100644 --- a/src/util/vircgroup.h +++ b/src/util/vircgroup.h @@ -225,4 +225,9 @@ int virCgroupIsolateMount(virCgroupPtr group, bool virCgroupSupportsCpuBW(virCgroupPtr cgroup); +int virCgroupSetOwner(virCgroupPtr cgroup, + uid_t uid, + gid_t gid, + int controllers); + #endif /* __VIR_CGROUP_H__ */ -- 1.8.5.3

3 2

[libvirt] [PATCH v2] lxc: Add virCgroupSetOwner()
by Richard Weinberger 24 Feb '14

24 Feb '14

Add a new helper function to change the permissions of a control group. This function is needed for user namespaces, we need to chmod() the cgroup to the initial uid/gid such that systemd is allowed to use the cgroup. Signed-off-by: Richard Weinberger <richard(a)nod.at> --- Changes between v1 and v2: - Addressed Martin Kletzander's comments - Fixed opendir() error handling --- src/libvirt_private.syms | 1 + src/lxc/lxc_cgroup.c | 12 ++++++++++ src/util/vircgroup.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++++ src/util/vircgroup.h | 5 ++++ 4 files changed, 78 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 2c9536a..40e72f2 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1056,6 +1056,7 @@ virCgroupSetMemory; virCgroupSetMemoryHardLimit; virCgroupSetMemorySoftLimit; virCgroupSetMemSwapHardLimit; +virCgroupSetOwner; virCgroupSupportsCpuBW; diff --git a/src/lxc/lxc_cgroup.c b/src/lxc/lxc_cgroup.c index cc0d5e8..39d955c 100644 --- a/src/lxc/lxc_cgroup.c +++ b/src/lxc/lxc_cgroup.c @@ -484,6 +484,18 @@ virCgroupPtr virLXCCgroupCreate(virDomainDefPtr def) &cgroup) < 0) goto cleanup; + /* setup control group permissions for user namespace */ + if (def->idmap.uidmap) { + if (virCgroupSetOwner(cgroup, + def->idmap.uidmap[0].target, + def->idmap.gidmap[0].target, + (1 << VIR_CGROUP_CONTROLLER_SYSTEMD)) < 0) { + virCgroupFree(&cgroup); + cgroup = NULL; + goto cleanup; + } + } + cleanup: return cgroup; } diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index a6d60c5..4bef0db 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -3253,6 +3253,66 @@ cleanup: } +int virCgroupSetOwner(virCgroupPtr cgroup, + uid_t uid, + gid_t gid, + int controllers) +{ + size_t i; + + for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) { + char *base, *entry; + DIR *dh; + struct dirent *de; + + if (!((1 << i) & controllers)) + continue; + + if (!cgroup->controllers[i].mountPoint) + continue; + + if (virAsprintf(&base, "%s%s", cgroup->controllers[i].mountPoint, + cgroup->controllers[i].placement) < 0) { + return -1; + } + + dh = opendir(base); + if (!dh) { + VIR_ERROR(_("Unable to open %s: %s"), base, strerror(errno)); + VIR_FREE(base); + return -1; + } + + while ((de = readdir(dh)) != NULL) { + if (STREQ(de->d_name, ".") || + STREQ(de->d_name, "..")) + continue; + + if (virAsprintf(&entry, "%s/%s", base, de->d_name) < 0) { + VIR_FREE(base); + closedir(dh); + return -1; + } + + if (chown(entry, uid, gid) < 0) + VIR_WARN(_("cannot chown '%s' to (%u, %u): %s"), entry, uid, gid, + strerror(errno)); + + VIR_FREE(entry); + } + closedir(dh); + + if (chown(base, uid, gid) < 0) + VIR_WARN(_("cannot chown '%s' to (%u, %u): %s"), entry, uid, gid, + strerror(errno)); + + VIR_FREE(base); + } + + return 0; +} + + /** * virCgroupSupportsCpuBW(): * Check whether the host supports CFS bandwidth. diff --git a/src/util/vircgroup.h b/src/util/vircgroup.h index a70eb18..38d94f3 100644 --- a/src/util/vircgroup.h +++ b/src/util/vircgroup.h @@ -225,4 +225,9 @@ int virCgroupIsolateMount(virCgroupPtr group, bool virCgroupSupportsCpuBW(virCgroupPtr cgroup); +int virCgroupSetOwner(virCgroupPtr cgroup, + uid_t uid, + gid_t gid, + int controllers); + #endif /* __VIR_CGROUP_H__ */ -- 1.8.4.5

2 3

[libvirt] Error while running event-test.c in Libvirt package
by Avanti Ajay 24 Feb '14

24 Feb '14

Hello, I am trying to run event-test.c code available in libvirt-0.10.2 package under the folder /examples/domain-events/events-c. I am getting the following error /usr/bin/ld: cannot find -lvirt collect2: ld returned 1 exit status Please help me to solve this issue.

2 1

[libvirt] [libvirt-glib 0/2] Fix make distcheck
by Christophe Fergeau 24 Feb '14

24 Feb '14

Hey, After the introduction of the GTest unit tests, make distcheck still has a few issues, with this series, make distcheck passes on my box (using glib 2.38). Christophe

2 4

[libvirt] [PATCH glib] Disable test suite unless glib >= 2.38
by Daniel P. Berrange 24 Feb '14

24 Feb '14

The TAP harness for glib only works with version 2.38 or later, so must be disabled for earlier versions Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com> --- configure.ac | 8 +++++++- tests/Makefile.am | 18 ++++++++++++++---- 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/configure.ac b/configure.ac index 4ad636a..d5b2971 100644 --- a/configure.ac +++ b/configure.ac @@ -12,6 +12,7 @@ AM_SILENT_RULES([yes]) LIBVIRT_REQUIRED=0.10.2 AC_SUBST([LIBVIRT_REQUIRED]) dnl used in the .spec file GLIB2_REQUIRED=2.22.0 +GLIB2_TEST_REQUIRED=2.38.0 GOBJECT2_REQUIRED=2.10.0 GIO_REQUIRED=2.10.0 GOBJECT_INTROSPECTION_REQUIRED=0.10.8 @@ -91,12 +92,17 @@ m4_if(m4_version_compare([2.61a.100], LIBVIRT_GLIB_COMPILE_WARNINGS PKG_CHECK_MODULES(LIBVIRT, libvirt >= $LIBVIRT_REQUIRED) -PKG_CHECK_MODULES(GLIB2, glib-2.0 >= $GLIB2_REQUIRED) +enable_tests=no +PKG_CHECK_MODULES(GLIB2, glib-2.0 >= $GLIB2_TEST_REQUIRED, + [enable_tests=yes], + [PKG_CHECK_MODULES(GLIB2, glib-2.0 >= $GLIB2_REQUIRED)]) PKG_CHECK_MODULES(GTHREAD2, gthread-2.0 >= $GLIB2_REQUIRED) PKG_CHECK_MODULES(GOBJECT2, gobject-2.0 >= $GLIB2_REQUIRED) PKG_CHECK_MODULES(GIO2, gio-2.0 >= $GLIB2_REQUIRED) PKG_CHECK_MODULES(LIBXML2, libxml-2.0 >= $LIBXML2_REQUIRED) +AM_CONDITIONAL([ENABLE_TESTS], [test "$enable_tests" = "yes"]) + LIBVIRT_GLIB_GETTEXT dnl Should be in m4/virt-gettext.m4 but intltoolize is too dnl dumb to find it there diff --git a/tests/Makefile.am b/tests/Makefile.am index 56887ce..8cb98e5 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -1,3 +1,11 @@ +EXTRA_DIST = \ + xml/gconfig-domain.xml \ + xml/gconfig-domain-clock.xml \ + xml/gconfig-domain-os.xml \ + $(NULL) + +if ENABLE_TESTS + include $(top_srcdir)/build-aux/glib-tap.mk AM_CFLAGS = \ @@ -14,7 +22,9 @@ LDADD = \ test_programs = test-gconfig test-events -dist_test_data = \ - xml/gconfig-domain.xml \ - xml/gconfig-domain-clock.xml \ - xml/gconfig-domain-os.xml +else +EXTRA_DIST += \ + test-events.c \ + test-gconfig.c \ + $(NULL) +endif -- 1.8.5.3

3 2

[libvirt] [PATCH] build: fix build on 32-bit hosts
by Eric Blake 22 Feb '14

22 Feb '14

vircgrouptest.c: In function 'testCgroupGetPercpuStats': vircgrouptest.c:543: warning: integer constatnt is too large for 'long' type Signed-off-by: Eric Blake <eblake(a)redhat.com> --- Pushing under the build-breaker rule. tests/vircgrouptest.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/tests/vircgrouptest.c b/tests/vircgrouptest.c index 5c14efb..5d5eccc 100644 --- a/tests/vircgrouptest.c +++ b/tests/vircgrouptest.c @@ -1,5 +1,5 @@ /* - * Copyright (C) 2013 Red Hat, Inc. + * Copyright (C) 2013-2014 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -540,7 +540,7 @@ static int testCgroupGetPercpuStats(const void *args ATTRIBUTE_UNUSED) // TODO: mock nodeGetCPUCount() as well & check 2nd cpu, too unsigned long long expected[] = { - 1413142688153030 + 1413142688153030ULL }; if ((rv = virCgroupNewPartition("/virtualmachines", true, @@ -632,8 +632,8 @@ static int testCgroupGetBlkioIoServiced(const void *args ATTRIBUTE_UNUSED) int rv, ret = -1; const long long expected_values[] = { - 119084214273, - 822880960513, + 119084214273ULL, + 822880960513ULL, 9665167, 73283807 }; @@ -681,14 +681,14 @@ static int testCgroupGetBlkioIoDeviceServiced(const void *args ATTRIBUTE_UNUSED) size_t i; int rv, ret = -1; const long long expected_values0[] = { - 59542107136, - 411440480256, + 59542107136ULL, + 411440480256ULL, 4832583, 36641903 }; const long long expected_values1[] = { - 59542107137, - 411440480257, + 59542107137ULL, + 411440480257ULL, 4832584, 36641904 }; -- 1.8.5.3

1 0