[libvirt] [PATCH] Properly check the return value of CCWAddressAsString
It returns NULL on failure. Checking if the negation of it is less than zero makes no sense. (Found by coverity after moving the code) In another case, the return value wasn't checked at all. --- src/conf/domain_addr.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c index a756f12..fb4a76f 100644 --- a/src/conf/domain_addr.c +++ b/src/conf/domain_addr.c @@ -614,7 +614,7 @@ virDomainCCWAddressAssign(virDomainDeviceInfoPtr dev, goto cleanup; } } else if (autoassign !dev-addr.ccw.assigned) { -if (!(addr = virDomainCCWAddressAsString(addrs-next)) 0) +if (!(addr = virDomainCCWAddressAsString(addrs-next))) goto cleanup; while (virHashLookup(addrs-defined, addr)) { @@ -624,7 +624,8 @@ virDomainCCWAddressAssign(virDomainDeviceInfoPtr dev, goto cleanup; } VIR_FREE(addr); -addr = virDomainCCWAddressAsString(addrs-next); +if (!(addr = virDomainCCWAddressAsString(addrs-next))) +goto cleanup; } dev-addr.ccw = addrs-next; dev-addr.ccw.assigned = true; -- 1.8.5.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Properly check the return value of CCWAddressAsString
On 06/23/14 08:36, Ján Tomko wrote: It returns NULL on failure. Checking if the negation of it is less than zero makes no sense. (Found by coverity after moving the code) In another case, the return value wasn't checked at all. --- src/conf/domain_addr.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) ACK, Peter signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] domain_addr: Resolve Coverity warning
On 06/21/14 13:54, John Ferlan wrote: Recent changes in the module : http://www.redhat.com/archives/libvir-list/2014-June/msg00978.html seem to have caused Coverity to take a look... The virDomainCCWAddressAsString() API returns either a NULL or a string and the comparison of the returned value to 0 caused following errors: [1] CONSTANT_EXPRESSION_RESULT (1) Event result_independent_of_operands: !(addr = virDomainCCWAddressAsString(addrs-next)) 0 is always false regardless of the values of its operands. This occurs as the logical operand of if. and 2] DEADCODE (1) Event between: At condition !(addr = virDomainCCWAddressAsString(addrs-next)) 0, the value of addr = virDomainCCWAddressAsString(addrs-next) must be between 0 and 1. Signed-off-by: John Ferlan jfer...@redhat.com --- src/conf/domain_addr.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Although your version was first I like Jan's better as it's fixing two issues of botched error checking of the function in question. Peter signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] lxc: don't update memUsage when failing to get memory usage
ping -Original Message- From: libvir-list-boun...@redhat.com [mailto:libvir-list-boun...@redhat.com] On Behalf Of Chen Hanxiao Sent: Friday, June 20, 2014 2:21 PM To: libvir-list@redhat.com Subject: [libvirt] [PATCH] lxc: don't update memUsage when failing to get memory usage Signed-off-by: Chen Hanxiao chenhanx...@cn.fujitsu.com --- src/lxc/lxc_cgroup.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/lxc/lxc_cgroup.c b/src/lxc/lxc_cgroup.c index 8dfdc60..af95941 100644 --- a/src/lxc/lxc_cgroup.c +++ b/src/lxc/lxc_cgroup.c @@ -194,7 +194,8 @@ static int virLXCCgroupGetMemUsage(virCgroupPtr cgroup, unsigned long memUsage; ret = virCgroupGetMemoryUsage(cgroup, memUsage); -meminfo-memusage = (unsigned long long) memUsage; +if (ret == 0) +meminfo-memusage = (unsigned long long) memUsage; return ret; } -- 1.9.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix closedir usage in virNumaGetPages
On 06/21/2014 05:29 PM, Roman Bogorodskiy wrote: virNumaGetPages calls closedir(dir) in cleanup and dir could be NULL if we jump there from the failed opendir() call. While it's not harmful on Linux, FreeBSD libc crashes [1], so make sure that dir is not NULL before calling closedir. 1: http://lists.freebsd.org/pipermail/freebsd-standards/2014-January/002704.html --- src/util/virnuma.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) ACK Jan signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Properly check the return value of CCWAddressAsString
On 06/23/2014 08:42 AM, Peter Krempa wrote: On 06/23/14 08:36, Ján Tomko wrote: It returns NULL on failure. Checking if the negation of it is less than zero makes no sense. (Found by coverity after moving the code) In another case, the return value wasn't checked at all. --- src/conf/domain_addr.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) ACK, Thanks, pushed. Jan signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Do not call closedir with NULL argument
Only three other callers possibly call closedir on a NULL argument. Even though these probably won't be used on FreeBSD where this crashes, let's be nice and only call closedir on an actual directory stream. --- src/parallels/parallels_storage.c | 2 +- src/util/virscsi.c| 6 -- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/parallels/parallels_storage.c b/src/parallels/parallels_storage.c index 4dbaed1..53bcfcb 100644 --- a/src/parallels/parallels_storage.c +++ b/src/parallels/parallels_storage.c @@ -340,7 +340,7 @@ static int parallelsFindVmVolumes(virStoragePoolObjPtr pool, virReportSystemError(errno, _(cannot open path '%s'), pdom-home); -goto cleanup; +return ret; } while ((direrr = virDirRead(dir, ent, pdom-home)) 0) { diff --git a/src/util/virscsi.c b/src/util/virscsi.c index 9a0205f..9f5cf0d 100644 --- a/src/util/virscsi.c +++ b/src/util/virscsi.c @@ -143,7 +143,8 @@ virSCSIDeviceGetSgName(const char *sysfs_prefix, } cleanup: -closedir(dir); +if (dir) +closedir(dir); VIR_FREE(path); return sg; } @@ -188,7 +189,8 @@ virSCSIDeviceGetDevName(const char *sysfs_prefix, } cleanup: -closedir(dir); +if (dir) +closedir(dir); VIR_FREE(path); return name; } -- 1.8.5.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] LXC: trivially support flag VIR_DRV_FEATURE_TYPED_PARAM_STRING
fix: virsh -c lxc:/// memtune DOMAIN error: Unable to get number of memory parameters error: unsupported flags (0x4) in function lxcDomainGetMemoryParameters Signed-off-by: Chen Hanxiao chenhanx...@cn.fujitsu.com --- src/lxc/lxc_driver.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 06f3e18..6b170fe 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -895,7 +895,11 @@ lxcDomainGetMemoryParameters(virDomainPtr dom, size_t i; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | - VIR_DOMAIN_AFFECT_CONFIG, -1); + VIR_DOMAIN_AFFECT_CONFIG, + VIR_TYPED_PARAM_STRING_OKAY, -1); + +/* We don't return strings, and thus trivially support this flag. */ +flags = ~VIR_TYPED_PARAM_STRING_OKAY; if (!(vm = lxcDomObjFromDomain(dom))) goto cleanup; -- 1.9.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/2] util: numa: Fix few issues with hugepage code
Peter Krempa (2): util: numa: Catch readdir errors in virNumaGetPages util: numa: Stub out hugepage code on non-Linux platforms src/util/virnuma.c | 52 +++- 1 file changed, 43 insertions(+), 9 deletions(-) -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/2] util: numa: Catch readdir errors in virNumaGetPages
Don't return possibly incomplete result if virDirRead fails. --- src/util/virnuma.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/util/virnuma.c b/src/util/virnuma.c index c8e7f40..9cf5a75 100644 --- a/src/util/virnuma.c +++ b/src/util/virnuma.c @@ -728,6 +728,7 @@ virNumaGetPages(int node, int ret = -1; char *path = NULL; DIR *dir = NULL; +int direrr; struct dirent *entry; unsigned int *tmp_size = NULL, *tmp_avail = NULL, *tmp_free = NULL; unsigned int ntmp = 0; @@ -768,7 +769,7 @@ virNumaGetPages(int node, goto cleanup; } -while (virDirRead(dir, entry, path) 0) { +while ((direrr = virDirRead(dir, entry, path)) 0) { const char *page_name = entry-d_name; unsigned int page_size, page_avail = 0, page_free = 0; char *end; @@ -805,6 +806,9 @@ virNumaGetPages(int node, ntmp++; } +if (direrr 0) +goto cleanup; + /* Just to produce nice output, sort the arrays by increasing page size */ do { exchange = false; -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/2] util: numa: Stub out hugepage code on non-Linux platforms
The hugepage sizing and counting code gathers the information from sysfs and thus isn't portable. Stub it out for non-Linux so that we can report a better error. This patch also avoids calling sysinfo() on Mingw where it isn't supported. --- src/util/virnuma.c | 46 ++ 1 file changed, 38 insertions(+), 8 deletions(-) diff --git a/src/util/virnuma.c b/src/util/virnuma.c index 9cf5a75..9cf9686 100644 --- a/src/util/virnuma.c +++ b/src/util/virnuma.c @@ -512,9 +512,12 @@ virNumaGetDistances(int node ATTRIBUTE_UNUSED, #endif -#define HUGEPAGES_NUMA_PREFIX /sys/devices/system/node/ -#define HUGEPAGES_SYSTEM_PREFIX /sys/kernel/mm/hugepages/ -#define HUGEPAGES_PREFIX hugepages- +/* currently all the hugepage stuff below is linux only */ +#if WITH_LINUX + +# define HUGEPAGES_NUMA_PREFIX /sys/devices/system/node/ +# define HUGEPAGES_SYSTEM_PREFIX /sys/kernel/mm/hugepages/ +# define HUGEPAGES_PREFIX hugepages- static int virNumaGetHugePageInfoPath(char **path, @@ -663,7 +666,7 @@ virNumaGetPageInfo(int node, /* sysconf() returns page size in bytes, * the @page_size is however in kibibytes */ if (page_size == system_page_size / 1024) { -#if 0 +# if 0 unsigned long long memsize, memfree; /* TODO: come up with better algorithm that takes huge pages into @@ -681,11 +684,11 @@ virNumaGetPageInfo(int node, if (page_free) *page_free = memfree / system_page_size; -#else +# else virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, %s, _(system page size are not supported yet)); goto cleanup; -#endif /* 0 */ +# endif /* 0 */ } else { if (virNumaGetHugePageInfo(node, page_size, page_avail, page_free) 0) goto cleanup; @@ -735,7 +738,7 @@ virNumaGetPages(int node, size_t i; bool exchange; -#if 0 +# if 0 /* This has to be disabled until the time the issue in * virNumaGetPageInfo is resolved. Sorry. */ long system_page_size; @@ -756,7 +759,7 @@ virNumaGetPages(int node, goto cleanup; tmp_size[ntmp] = system_page_size; ntmp++; -#endif /* 0 */ +# endif /* 0 */ /* Now that we got ordinary system pages, lets get info on huge pages */ if (virNumaGetHugePageInfoPath(path, node, 0, NULL) 0) @@ -844,3 +847,30 @@ virNumaGetPages(int node, VIR_FREE(path); return ret; } + + +#else /* #if WITH_LINUX */ +int +virNumaGetPageInfo(int node ATTRIBUTE_UNUSED, + unsigned int page_size ATTRIBUTE_UNUSED, + unsigned int *page_avail ATTRIBUTE_UNUSED, + unsigned int *page_free ATTRIBUTE_UNUSED) +{ +virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, %s, + _(system page size are not supported for this platform)); +return -1; +} + + +int +virNumaGetPages(int node ATTRIBUTE_UNUSED, +unsigned int **pages_size ATTRIBUTE_UNUSED, +unsigned int **pages_avail ATTRIBUTE_UNUSED, +unsigned int **pages_free ATTRIBUTE_UNUSED, +size_t *npages ATTRIBUTE_UNUSED) +{ +virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, %s, + _(system page size are not supported for this platform)); +return -1; +} +#endif /* #if WITH_LINUX */ -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 03/10] util: storagefile: Introduce helper to free storage source perms
On 06/20/14 12:09, John Ferlan wrote: On 06/20/2014 02:53 AM, Peter Krempa wrote: On 06/19/14 20:57, John Ferlan wrote: On 06/19/2014 09:46 AM, Peter Krempa wrote: It will also be reused later. --- src/util/virstoragefile.c | 16 1 file changed, 12 insertions(+), 4 deletions(-) Why is it that patches 1-3 do not update libvirt_private.syms to add new src/util/vir* APIs, but some of the future patches do update, such as 4, The functions added in 1-3 are used only in the utils module, especially in the storage source deep copy function and are not linked to from other modules and therefore the symbol export isn't really necessary. I might add them if you insist, but I don't expect the functions to be linked to other modules any soon. No insistence from me - just curious. It would only save you a step later on I suppose. Moot point now since they're pushed. I don't really plan on using those directly from other modules so it shouldn't be needed Peter signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv5 02/19] util: storagefile: Introduce universal function to canonicalize paths
On 06/20/14 16:35, Eric Blake wrote: On 06/19/2014 07:59 AM, Peter Krempa wrote: Introduce a common function that will take a callback to resolve links that will be used to canonicalize paths on various storage systems and add extensive tests. --- src/libvirt_private.syms | 1 + src/util/virstoragefile.c | 195 ++ src/util/virstoragefile.h | 7 ++ tests/virstoragetest.c| 108 + 4 files changed, 311 insertions(+) ACK if you can make those changes (you may want to post the interdiff) The required changes to pass your suggested changes are: diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index be33398..613ba3c 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -2040,6 +2040,7 @@ virStorageFileCanonicalizePath(const char *path, char *linkpath = NULL; char *currentpath = NULL; size_t i = 0; +size_t j = 0; int rc; char *ret = NULL; @@ -2056,10 +2057,21 @@ virStorageFileCanonicalizePath(const char *path, if (!(components = virStringSplitCount(path, /, 0, ncomponents))) goto cleanup; +j = 0; +while (j ncomponents) { +/* skip slashes */ +if (STREQ(components[j], )) { +VIR_FREE(components[j]); +VIR_DELETE_ELEMENT(components, j, ncomponents); +continue; +} +j++; +} + while (i ncomponents) { -/* skip slashes and '.'s */ -if (STREQ(components[i], ) || -STREQ(components[i], .)) { +/* skip '.'s unless it's the last one remaining */ +if (STREQ(components[i], .) +(beginSlash || ncomponents 1)) { VIR_FREE(components[i]); VIR_DELETE_ELEMENT(components, i, ncomponents); continue; @@ -2131,6 +2143,17 @@ virStorageFileCanonicalizePath(const char *path, ncomponents) 0) goto cleanup; +j = 0; +while (j ncomponents) { +/* skip slashes */ +if (STREQ(components[j], )) { +VIR_FREE(components[j]); +VIR_DELETE_ELEMENT(components, j, ncomponents); +continue; +} +j++; +} + VIR_FREE(linkpath); VIR_FREE(currentpath); diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c index 0bc4a42..f86d25c 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -1154,14 +1154,14 @@ mymain(void) TEST_PATH_CANONICALIZE(5, ///, /); TEST_PATH_CANONICALIZE(6, //, //); TEST_PATH_CANONICALIZE(7, , ); -TEST_PATH_CANONICALIZE(8, ., ); +TEST_PATH_CANONICALIZE(8, ., .); TEST_PATH_CANONICALIZE(9, ../, ..); TEST_PATH_CANONICALIZE(10, ../../, ../..); TEST_PATH_CANONICALIZE(11, ../../blah, ../../blah); TEST_PATH_CANONICALIZE(12, /./././blah, /blah); TEST_PATH_CANONICALIZE(13, .././../././../blah, ../../../blah); TEST_PATH_CANONICALIZE(14, /././, /); -TEST_PATH_CANONICALIZE(15, ./././, ); +TEST_PATH_CANONICALIZE(15, ./././, .); TEST_PATH_CANONICALIZE(16, blah/../foo, foo); TEST_PATH_CANONICALIZE(17, foo/bar/../blah, foo/blah); TEST_PATH_CANONICALIZE(18, foo/bar/.././blah, foo/blah); @@ -1179,6 +1179,7 @@ mymain(void) TEST_PATH_CANONICALIZE(28, /path/blah/yippee, /other/path/huzah/yippee); TEST_PATH_CANONICALIZE(29, /cycle, NULL); TEST_PATH_CANONICALIZE(30, /cycle2/link, NULL); +TEST_PATH_CANONICALIZE(31, ///, /); cleanup: /* Final cleanup */ Peter signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/3] Add pkg-config files to allow deps to build against source tree
On Fri, Jun 20, 2014 at 01:02:46PM -0600, Eric Blake wrote: On 06/20/2014 10:51 AM, Daniel P. Berrange wrote: When testing language bindings it is useful to be able to build them against an uninstalled libvirt source tree. Add a dummy set of pkg-config files to allow for this. This can be used by setting export PKG_CONFIG_PATH=/path/to/libvirt/git/src Yay - we need to document this trick in libvirt-python.git as well. Signed-off-by: Daniel P. Berrange berra...@redhat.com --- .gitignore | 1 + configure.ac | 3 +++ src/Makefile.am| 8 +++- src/libvirt-lxc.pc.in | 18 ++ src/libvirt-qemu.pc.in | 18 ++ src/libvirt.pc.in | 23 +++ 6 files changed, 70 insertions(+), 1 deletion(-) create mode 100644 src/libvirt-lxc.pc.in create mode 100644 src/libvirt-qemu.pc.in create mode 100644 src/libvirt.pc.in +++ b/src/Makefile.am @@ -45,7 +45,13 @@ AM_LDFLAGS = $(DRIVER_MODULE_LDFLAGS) \ $(NO_INDIRECT_LDFLAGS) \ $(NULL) -EXTRA_DIST = $(conf_DATA) util/keymaps.csv +EXTRA_DIST = \ + $(conf_DATA) \ + util/keymaps.csv \ + libvirt.pc \ + libvirt-qemu.pc \ + libvirt-lxc.pc \ + $(NULL) NACK to this hunk - the .pc files should NOT be part of the tarball, because they contain contents that depend on configure results, while the tarball must be independent. End users will get their own .pc file as soon as they do ./configure make. Obviously that was meant to be the .pc.in files Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/2] util: numa: Fix few issues with hugepage code
On 23.6.2014 09:29, Peter Krempa wrote: Peter Krempa (2): util: numa: Catch readdir errors in virNumaGetPages util: numa: Stub out hugepage code on non-Linux platforms src/util/virnuma.c | 52 +++- 1 file changed, 43 insertions(+), 9 deletions(-) ACK series, mingw build tested and it works Pavel -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Fix invalid write in virNumaGetDistances
Invalid write of size 4 at 0x52E678C: virNumaGetDistances (virnuma.c:479) by 0x5396890: nodeCapsInitNUMA (nodeinfo.c:1796) by 0x203C2B: virQEMUCapsInit (qemu_capabilities.c:960) Address 0xe10a1e0 is 0 bytes after a block of size 0 alloc'd at 0x4C2A6D0: calloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) by 0x52A10D6: virAllocN (viralloc.c:191) by 0x52E674D: virNumaGetDistances (virnuma.c:470) by 0x5396890: nodeCapsInitNUMA (nodeinfo.c:1796) by 0x203C2B: virQEMUCapsInit (qemu_capabilities.c:960) --- src/util/virnuma.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util/virnuma.c b/src/util/virnuma.c index c8e7f40..c494e17 100644 --- a/src/util/virnuma.c +++ b/src/util/virnuma.c @@ -467,7 +467,7 @@ virNumaGetDistances(int node, if ((max_node = virNumaGetMaxNode()) 0) goto cleanup; -if (VIR_ALLOC_N(*distances, max_node) 0) +if (VIR_ALLOC_N(*distances, max_node + 1) 0) goto cleanup; *ndistances = max_node + 1; -- 1.8.5.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] test: Skip storage test when filesystem storage backend isn't compiled in
'virstoragetest' accesses backing chains of files on local storage with the help of the storage driver. Disable the test on builds without the storage driver as the test is crashing otherwise. --- tests/virstoragetest.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c index e15578c..043c5e6 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -536,6 +536,12 @@ mymain(void) virStorageSourcePtr chain2; /* short for chain-backingStore */ virStorageSourcePtr chain3; /* short for chain2-backingStore */ +#if !WITH_STORAGE_FS +/* this test doesn't make sense without storage driver access + * to local files */ +return EXIT_AM_SKIP; +#endif + /* Prep some files with qemu-img; if that is not found on PATH, or * if it lacks support for qcow2 and qed, skip this test. */ if ((ret = testPrepImages()) != 0) -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt-glib] gobject: Fix GEnum generation through glib-mkenums
Ping? On Tue, Jun 03, 2014 at 11:16:14AM +0200, Christophe Fergeau wrote: We were only passing libvirt-gobject-domain.h and libvirt-gobject-connection.h through glib-mkenums, which causes it to only generate GEnum information for enums found in these headers. We want to do that for all enums defined in installed headers, so passing all headers listed in libvirt_gobject_1_0_la_HEADERS is more appropriate. --- libvirt-gobject/Makefile.am | 4 ++-- libvirt-gobject/libvirt-gobject.sym | 5 + 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/libvirt-gobject/Makefile.am b/libvirt-gobject/Makefile.am index 516c10f..7163c7d 100644 --- a/libvirt-gobject/Makefile.am +++ b/libvirt-gobject/Makefile.am @@ -93,7 +93,7 @@ libvirt_gobject_1_0_la_LDFLAGS = \ -Wl,--version-script=$(srcdir)/libvirt-gobject.sym \ -version-info $(LIBVIRT_GLIB_VERSION_INFO) -libvirt-gobject-enums.c: libvirt-gobject-domain.h libvirt-gobject-connection.h +libvirt-gobject-enums.c: $(libvirt_gobject_1_0_la_HEADERS) $(AM_V_GEN)glib-mkenums \ --fhead #include \libvirt-gobject/libvirt-gobject.h\\n\n \ --vhead static const G@Type@Value _@enum_name@_values[] = { \ @@ -106,7 +106,7 @@ libvirt-gobject-enums.c: libvirt-gobject-domain.h libvirt-gobject-connection.h --vtail return type;\n}\n\n \ $^ | sed -e 's/g_vir/gvir/g' $@ -libvirt-gobject-enums.h: libvirt-gobject-domain.h libvirt-gobject-connection.h +libvirt-gobject-enums.h: $(libvirt_gobject_1_0_la_HEADERS) $(AM_V_GEN)glib-mkenums--fhead #ifndef __LIBVIRT_GOBJECT_ENUMS_H__\n \ --fhead #define __LIBVIRT_GOBJECT_ENUMS_H__\n\n \ --fhead G_BEGIN_DECLS\n\n \ diff --git a/libvirt-gobject/libvirt-gobject.sym b/libvirt-gobject/libvirt-gobject.sym index b781cc6..4e856e7 100644 --- a/libvirt-gobject/libvirt-gobject.sym +++ b/libvirt-gobject/libvirt-gobject.sym @@ -237,6 +237,11 @@ LIBVIRT_GOBJECT_0.1.5 { LIBVIRT_GOBJECT_0.1.9 { global: gvir_domain_snapshot_delete; + gvir_domain_snapshot_delete_flags_get_type; + gvir_storage_pool_state_get_type; + gvir_storage_vol_resize_flags_get_type; + gvir_storage_vol_type_get_type; + gvir_stream_io_condition_get_type; } LIBVIRT_GOBJECT_0.1.5; # define new API here using predicted next version number -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list pgplTIGecyOpw.pgp Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] test: Skip storage test when filesystem storage backend isn't compiled in
On Mon, Jun 23, 2014 at 12:04:28 +0200, Peter Krempa wrote: 'virstoragetest' accesses backing chains of files on local storage with the help of the storage driver. Disable the test on builds without the storage driver as the test is crashing otherwise. --- tests/virstoragetest.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c index e15578c..043c5e6 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -536,6 +536,12 @@ mymain(void) virStorageSourcePtr chain2; /* short for chain-backingStore */ virStorageSourcePtr chain3; /* short for chain2-backingStore */ +#if !WITH_STORAGE_FS +/* this test doesn't make sense without storage driver access + * to local files */ +return EXIT_AM_SKIP; +#endif + /* Prep some files with qemu-img; if that is not found on PATH, or * if it lacks support for qcow2 and qed, skip this test. */ if ((ret = testPrepImages()) != 0) Static analysis tools will complain about dead code and unused variables. The way we usually implement this is to make the whole mymain() conditional. Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Do not call closedir with NULL argument
On Mon, Jun 23, 2014 at 12:36 PM, Ján Tomko jto...@redhat.com wrote: Only three other callers possibly call closedir on a NULL argument. Even though these probably won't be used on FreeBSD where this crashes, let's be nice and only call closedir on an actual directory stream. --- src/parallels/parallels_storage.c | 2 +- src/util/virscsi.c| 6 -- 2 files changed, 5 insertions(+), 3 deletions(-) ACK -- Nehal J Wani -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2] test: Disable storage test when FS backend isn't compiled in
'virstoragetest' accesses backing chains of files on local storage with the help of the storage driver. Disable the test on builds without the storage driver as the test is crashing otherwise. Reported by: Roman Bogorodskiy --- Notes: Version 2: - Don't run the test from the makefile instead of adding code that might not compile cleanly tests/Makefile.am | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tests/Makefile.am b/tests/Makefile.am index c999061..2f762a6 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -285,10 +285,13 @@ test_programs += nwfilterxml2firewalltest endif WITH_NWFILTER if WITH_STORAGE -test_programs += storagevolxml2argvtest \ - virstoragetest +test_programs += storagevolxml2argvtest endif WITH_STORAGE +if WITH_STORAGE_FS +test_programs += virstoragetest +endif + if WITH_LINUX test_programs += virscsitest endif WITH_LINUX -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2] test: Disable storage test when FS backend isn't compiled in
On Mon, Jun 23, 2014 at 13:21:49 +0200, Peter Krempa wrote: 'virstoragetest' accesses backing chains of files on local storage with the help of the storage driver. Disable the test on builds without the storage driver as the test is crashing otherwise. Reported by: Roman Bogorodskiy --- Notes: Version 2: - Don't run the test from the makefile instead of adding code that might not compile cleanly Looks better, ACK. Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2] test: Disable storage test when FS backend isn't compiled in
On 06/23/14 13:31, Jiri Denemark wrote: On Mon, Jun 23, 2014 at 13:21:49 +0200, Peter Krempa wrote: 'virstoragetest' accesses backing chains of files on local storage with the help of the storage driver. Disable the test on builds without the storage driver as the test is crashing otherwise. Reported by: Roman Bogorodskiy --- Notes: Version 2: - Don't run the test from the makefile instead of adding code that might not compile cleanly Looks better, ACK. Pushed; Thanks. Peter signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Do not call closedir with NULL argument
On 23.06.2014 09:06, Ján Tomko wrote: Only three other callers possibly call closedir on a NULL argument. Even though these probably won't be used on FreeBSD where this crashes, let's be nice and only call closedir on an actual directory stream. --- src/parallels/parallels_storage.c | 2 +- src/util/virscsi.c| 6 -- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/parallels/parallels_storage.c b/src/parallels/parallels_storage.c index 4dbaed1..53bcfcb 100644 --- a/src/parallels/parallels_storage.c +++ b/src/parallels/parallels_storage.c @@ -340,7 +340,7 @@ static int parallelsFindVmVolumes(virStoragePoolObjPtr pool, virReportSystemError(errno, _(cannot open path '%s'), pdom-home); -goto cleanup; +return ret; } while ((direrr = virDirRead(dir, ent, pdom-home)) 0) { diff --git a/src/util/virscsi.c b/src/util/virscsi.c index 9a0205f..9f5cf0d 100644 --- a/src/util/virscsi.c +++ b/src/util/virscsi.c @@ -143,7 +143,8 @@ virSCSIDeviceGetSgName(const char *sysfs_prefix, } cleanup: -closedir(dir); +if (dir) +closedir(dir); VIR_FREE(path); return sg; } @@ -188,7 +189,8 @@ virSCSIDeviceGetDevName(const char *sysfs_prefix, } cleanup: -closedir(dir); +if (dir) +closedir(dir); VIR_FREE(path); return name; } ACK Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix closedir usage in virNumaGetPages
On 21.06.2014 17:29, Roman Bogorodskiy wrote: virNumaGetPages calls closedir(dir) in cleanup and dir could be NULL if we jump there from the failed opendir() call. While it's not harmful on Linux, FreeBSD libc crashes [1], so make sure that dir is not NULL before calling closedir. 1: http://lists.freebsd.org/pipermail/freebsd-standards/2014-January/002704.html --- src/util/virnuma.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/util/virnuma.c b/src/util/virnuma.c index c8e7f40..1048033 100644 --- a/src/util/virnuma.c +++ b/src/util/virnuma.c @@ -836,7 +836,8 @@ virNumaGetPages(int node, VIR_FREE(tmp_free); VIR_FREE(tmp_avail); VIR_FREE(tmp_size); -closedir(dir); +if (dir) +closedir(dir); VIR_FREE(path); return ret; } So why is free(NULL) safe on FreeBSD then? I'd call this a libc bug not a libvirt one. But since even we already have such borken design (remember our publir vir*Free() APIs?) I can live with this patch. ACK -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] util: numa: Stub out hugepage code on non-Linux platforms
On 23.06.2014 09:29, Peter Krempa wrote: The hugepage sizing and counting code gathers the information from sysfs and thus isn't portable. Stub it out for non-Linux so that we can report a better error. This patch also avoids calling sysinfo() on Mingw where it isn't supported. --- src/util/virnuma.c | 46 ++ 1 file changed, 38 insertions(+), 8 deletions(-) diff --git a/src/util/virnuma.c b/src/util/virnuma.c index 9cf5a75..9cf9686 100644 --- a/src/util/virnuma.c +++ b/src/util/virnuma.c @@ -512,9 +512,12 @@ virNumaGetDistances(int node ATTRIBUTE_UNUSED, #endif -#define HUGEPAGES_NUMA_PREFIX /sys/devices/system/node/ -#define HUGEPAGES_SYSTEM_PREFIX /sys/kernel/mm/hugepages/ -#define HUGEPAGES_PREFIX hugepages- +/* currently all the hugepage stuff below is linux only */ +#if WITH_LINUX + +# define HUGEPAGES_NUMA_PREFIX /sys/devices/system/node/ +# define HUGEPAGES_SYSTEM_PREFIX /sys/kernel/mm/hugepages/ +# define HUGEPAGES_PREFIX hugepages- static int virNumaGetHugePageInfoPath(char **path, @@ -663,7 +666,7 @@ virNumaGetPageInfo(int node, /* sysconf() returns page size in bytes, * the @page_size is however in kibibytes */ if (page_size == system_page_size / 1024) { -#if 0 +# if 0 unsigned long long memsize, memfree; /* TODO: come up with better algorithm that takes huge pages into @@ -681,11 +684,11 @@ virNumaGetPageInfo(int node, if (page_free) *page_free = memfree / system_page_size; -#else +# else virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, %s, _(system page size are not supported yet)); goto cleanup; -#endif /* 0 */ +# endif /* 0 */ } else { if (virNumaGetHugePageInfo(node, page_size, page_avail, page_free) 0) goto cleanup; @@ -735,7 +738,7 @@ virNumaGetPages(int node, size_t i; bool exchange; -#if 0 +# if 0 /* This has to be disabled until the time the issue in * virNumaGetPageInfo is resolved. Sorry. */ long system_page_size; @@ -756,7 +759,7 @@ virNumaGetPages(int node, goto cleanup; tmp_size[ntmp] = system_page_size; ntmp++; -#endif /* 0 */ +# endif /* 0 */ /* Now that we got ordinary system pages, lets get info on huge pages */ if (virNumaGetHugePageInfoPath(path, node, 0, NULL) 0) @@ -844,3 +847,30 @@ virNumaGetPages(int node, VIR_FREE(path); return ret; Up till here everything's okay. But ... } + + +#else /* #if WITH_LINUX */ +int +virNumaGetPageInfo(int node ATTRIBUTE_UNUSED, + unsigned int page_size ATTRIBUTE_UNUSED, + unsigned int *page_avail ATTRIBUTE_UNUSED, + unsigned int *page_free ATTRIBUTE_UNUSED) +{ +virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, %s, + _(system page size are not supported for this platform)); +return -1; +} + + +int +virNumaGetPages(int node ATTRIBUTE_UNUSED, +unsigned int **pages_size ATTRIBUTE_UNUSED, +unsigned int **pages_avail ATTRIBUTE_UNUSED, +unsigned int **pages_free ATTRIBUTE_UNUSED, +size_t *npages ATTRIBUTE_UNUSED) +{ +virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, %s, + _(system page size are not supported for this platform)); +return -1; +} +#endif /* #if WITH_LINUX */ These two APIs are intended to get info for all page sizes, not only the ordinary system ones. ACK with the error message changed to reflect that. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix invalid write in virNumaGetDistances
On 23.06.2014 11:53, Ján Tomko wrote: Invalid write of size 4 at 0x52E678C: virNumaGetDistances (virnuma.c:479) by 0x5396890: nodeCapsInitNUMA (nodeinfo.c:1796) by 0x203C2B: virQEMUCapsInit (qemu_capabilities.c:960) Address 0xe10a1e0 is 0 bytes after a block of size 0 alloc'd at 0x4C2A6D0: calloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) by 0x52A10D6: virAllocN (viralloc.c:191) by 0x52E674D: virNumaGetDistances (virnuma.c:470) by 0x5396890: nodeCapsInitNUMA (nodeinfo.c:1796) by 0x203C2B: virQEMUCapsInit (qemu_capabilities.c:960) --- src/util/virnuma.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util/virnuma.c b/src/util/virnuma.c index c8e7f40..c494e17 100644 --- a/src/util/virnuma.c +++ b/src/util/virnuma.c @@ -467,7 +467,7 @@ virNumaGetDistances(int node, if ((max_node = virNumaGetMaxNode()) 0) goto cleanup; -if (VIR_ALLOC_N(*distances, max_node) 0) +if (VIR_ALLOC_N(*distances, max_node + 1) 0) goto cleanup; *ndistances = max_node + 1; Oops. ACK Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Build failed in Jenkins: libvirt-syntax-check #2398
See http://honk.sigxcpu.org:8001/job/libvirt-syntax-check/2398/ -- Started by upstream project libvirt-build build number 2706 Building on master in workspace http://honk.sigxcpu.org:8001/job/libvirt-syntax-check/ws/ [workspace] $ /bin/sh -xe /tmp/hudson5640577394465354092.sh + make syntax-check GENbracket-spacing-check GFDL_version 0.51 GFDL_version TAB_in_indentation 0.35 TAB_in_indentation Wundef_boolean 0.19 Wundef_boolean avoid_attribute_unused_in_header 0.25 avoid_attribute_unused_in_header avoid_ctype_macros 0.63 avoid_ctype_macros avoid_if_before_free 6.29 avoid_if_before_free avoid_strcase 0.70 avoid_strcase avoid_write 0.32 avoid_write bindtextdomain 0.26 bindtextdomain cast_of_argument_to_free 0.55 cast_of_argument_to_free cast_of_x_alloc_return_value 0.71 cast_of_x_alloc_return_value changelog 0.25 changelog const_long_option 0.48 const_long_option copyright_check 0.60 copyright_check copyright_format 1.39 copyright_format copyright_usage 1.52 copyright_usage correct_id_types 0.60 correct_id_types curly_braces_style 0.80 curly_braces_style error_message_period 0.43 error_message_period error_message_warn_fatal 0.43 error_message_warn_fatal flags_debug 0.98 flags_debug flags_usage 1.04 flags_usage forbid_const_pointer_typedef 1.06 forbid_const_pointer_typedef forbid_manual_xml_indent 0.52 forbid_manual_xml_indent libvirt_unmarked_diagnostics 1.58 libvirt_unmarked_diagnostics m4_quote_check 0.23 m4_quote_check makefile_TAB_only_indentation 0.23 makefile_TAB_only_indentation makefile_at_at_check 0.18 makefile_at_at_check makefile_conditionals tests/Makefile.am:293:endif maint.mk: match if FOO with endif FOO in Makefiles make: *** [sc_makefile_conditionals] Error 1 Build step 'Execute shell' marked build as failure -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Report correct error in virNetDevTapCreate
ioctl returns -1, not the errno value --- src/util/virnetdevtap.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util/virnetdevtap.c b/src/util/virnetdevtap.c index 0b444fa..d64e64f 100644 --- a/src/util/virnetdevtap.c +++ b/src/util/virnetdevtap.c @@ -294,7 +294,7 @@ int virNetDevTapCreate(char **ifname, } if ((flags VIR_NETDEV_TAP_CREATE_PERSIST) -(errno = ioctl(fd, TUNSETPERSIST, 1))) { +ioctl(fd, TUNSETPERSIST, 1) 0) { virReportSystemError(errno, _(Unable to set tap device %s to persistent), NULLSTR(*ifname)); -- 1.8.5.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Build failed in Jenkins: libvirt-syntax-check #2398
On 06/23/14 13:51, Jenkins CI wrote: See http://honk.sigxcpu.org:8001/job/libvirt-syntax-check/2398/ -- Started by upstream project libvirt-build build number 2706 Building on master in workspace http://honk.sigxcpu.org:8001/job/libvirt-syntax-check/ws/ [workspace] $ /bin/sh -xe /tmp/hudson5640577394465354092.sh + make syntax-check GENbracket-spacing-check ... 0.18 makefile_at_at_check makefile_conditionals tests/Makefile.am:293:endif maint.mk: match if FOO with endif FOO in Makefiles make: *** [sc_makefile_conditionals] Error 1 Build step 'Execute shell' marked build as failure I'd swear this didn't happen on my machine at the time I was checking the patch :/ Fix is on the way. Sorry for the mess :/ Peter signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] tests: Fix syntax-check after cdd11151791bc9e61538988438980f41c0185681
makefile_conditionals tests/Makefile.am:293:endif maint.mk: match if FOO with endif FOO in Makefiles make: *** [sc_makefile_conditionals] Error 1 --- Notes: Pushing under the build breaker rule and putting on the brown paper box of shame :/ tests/Makefile.am | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Makefile.am b/tests/Makefile.am index 2f762a6..025b847 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -290,7 +290,7 @@ endif WITH_STORAGE if WITH_STORAGE_FS test_programs += virstoragetest -endif +endif WITH_STORAGE_FS if WITH_LINUX test_programs += virscsitest -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Report correct error in virNetDevTapCreate
On 23.06.2014 13:53, Ján Tomko wrote: ioctl returns -1, not the errno value --- src/util/virnetdevtap.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util/virnetdevtap.c b/src/util/virnetdevtap.c index 0b444fa..d64e64f 100644 --- a/src/util/virnetdevtap.c +++ b/src/util/virnetdevtap.c @@ -294,7 +294,7 @@ int virNetDevTapCreate(char **ifname, } if ((flags VIR_NETDEV_TAP_CREATE_PERSIST) -(errno = ioctl(fd, TUNSETPERSIST, 1))) { +ioctl(fd, TUNSETPERSIST, 1) 0) { virReportSystemError(errno, _(Unable to set tap device %s to persistent), NULLSTR(*ifname)); ACK Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] util: numa: Stub out hugepage code on non-Linux platforms
On 06/23/14 13:49, Michal Privoznik wrote: On 23.06.2014 09:29, Peter Krempa wrote: The hugepage sizing and counting code gathers the information from sysfs and thus isn't portable. Stub it out for non-Linux so that we can report a better error. This patch also avoids calling sysinfo() on Mingw where it isn't supported. --- src/util/virnuma.c | 46 ++ 1 file changed, 38 insertions(+), 8 deletions(-) +int +virNumaGetPages(int node ATTRIBUTE_UNUSED, +unsigned int **pages_size ATTRIBUTE_UNUSED, +unsigned int **pages_avail ATTRIBUTE_UNUSED, +unsigned int **pages_free ATTRIBUTE_UNUSED, +size_t *npages ATTRIBUTE_UNUSED) +{ +virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, %s, + _(system page size are not supported for this platform)); +return -1; +} +#endif /* #if WITH_LINUX */ These two APIs are intended to get info for all page sizes, not only the ordinary system ones. ACK with the error message changed to reflect that. I'm going with page info is not supported on this platform and VIR_ERR_OPERATION_UNSUPPORTED code. Michal And pushing shortly. Peter signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Jenkins build is back to normal : libvirt-syntax-check #2399
See http://honk.sigxcpu.org:8001/job/libvirt-syntax-check/2399/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 0/5] Next round of active commit support
On Sat, Jun 21, 2014 at 04:04:45PM +0530, Kashyap Chamarthy wrote: I just tried to test this series built from Eric's libvirt git repo, and QEMU built from its git. On a live (or offline guest) I see the below: $ virsh blockcommit f20vm1 vda --shallow --wait \ --verbose --pivot --active error: unsupported flags (0x4) in function qemuDomainBlockCommit Okay, I had a chat with Peter Krempa on #virt (OFTC). Couple of things: - I didn't restart libvirtd after I updated RPMs built from git. - After a restart, I tried active commit again via libvirt, this time it failed saying it's not supported on the QEMU binary I built from commit 0360fbd (17JUN). - I guess for your testing you must have tested by applying Jef's patches block: make 'top' argument to bloc[1] manually. Eric just pinged on #virt (OFTC) to clarify the above as I was writing this email. Thanks. [1] https://lists.gnu.org/archive/html/qemu-devel/2014-06/msg04060.html I also tried: $ virsh blockcommit --domain f20vm1 vda \ --base /home/kashyap/vmimages/snap3-f20vm1.qcow2 \ --top /home/kashyap/vmimages/snap4-f20vm1.qcow2 \ --wait --verbose --pivot --active error: unsupported flags (0x4) in function qemuDomainBlockCommit Is that expected? Test notes below on QEMU and Libvirt commit details. Details === QEMU info - Build a local QEMU with this git commit. $ git log | head -6 commit 0360fbd076e8bdbb9498598b0c559464346babe4 Merge: af44da8 d3606f0 Author: Peter Maydell peter.mayd...@linaro.org Date: Tue Jun 17 16:08:06 2014 +0100 Merge remote-tracking branch 'remotes/riku/linux-user-for-upstream' into staging libvirt info How I built the libvirt: $ git clone git://repo.or.cz/libvirt/ericb.git cd ericb $ git log | head -5 commit c23db4dbc9b4ed91b37ba5d8ee1eff6bb8b32b0e Author: Eric Blake ebl...@redhat.com Date: Thu Jun 5 13:26:56 2014 -0600 blockcommit: turn on active commit $ ./autgen.sh make -j4 make rpm $ cd ~/rpmbuild/RPMS/x86_64 $ rpm -Uvh *.rpm $ virsh --version 1.2.6 Test 0. Edit the libvirt XML of the guest to point to the newly build QEMU from above. $ virsh dumpxml f20vm1 | grep qemu-system-x86_64 emulator/home/kashyap/build/qemu/x86_64-softmmu/qemu-system-x86_64/emulator 1. Start the guest and list it: $ virsh list IdName State 5 f20vm1 running (Note: All the images, including base are QCOW2 disk images.) 2. Create 4 live, disk-only external snapshots. $ virsh snapshot-create-as f20vm1 snap1 snap1-desc \ --diskspec vda,file=/home/kashyap/vmimages/snap1-f20vm1.qcow2 --disk-only (Repeat the above 4 times. Before taking each snapshot, add a file (/root/file1 or some such) to distinguish content from each snapshot image.) 3. List the snapshots $ virsh snapshot-list f20vm1 Name Creation Time State snap12014-06-21 02:28:26 +0530 disk-snapshot snap22014-06-21 02:30:27 +0530 disk-snapshot snap32014-06-21 02:31:38 +0530 disk-snapshot snap42014-06-21 02:33:52 +0530 disk-snapshot So, the current chain is: base -- snap1 -- snap2 -- snap3 -- snap4 Desired chain: base -- snap4 4. Invoke blockcommit test: $ virsh blockcommit f20vm1 vda --shallow --wait \ --verbose --pivot --active error: unsupported flags (0x4) in function qemuDomainBlockCommit What am I missing? -- /kashyap -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list -- /kashyap -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] LXC: trivially support flag VIR_DRV_FEATURE_TYPED_PARAM_STRING
On 06/23/2014 09:28 AM, Chen Hanxiao wrote: fix: virsh -c lxc:/// memtune DOMAIN error: Unable to get number of memory parameters error: unsupported flags (0x4) in function lxcDomainGetMemoryParameters Signed-off-by: Chen Hanxiao chenhanx...@cn.fujitsu.com --- src/lxc/lxc_driver.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) lxcDomainGetSchedulerParametersFlags will need a similar fix. It seems commit 399394 (released in v1.2.2) introduced this when it marked the LXC driver as supporting the option without marking it as supported in these APIs. diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 06f3e18..6b170fe 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -895,7 +895,11 @@ lxcDomainGetMemoryParameters(virDomainPtr dom, size_t i; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | - VIR_DOMAIN_AFFECT_CONFIG, -1); + VIR_DOMAIN_AFFECT_CONFIG, + VIR_TYPED_PARAM_STRING_OKAY, -1); + +/* We don't return strings, and thus trivially support this flag. */ +flags = ~VIR_TYPED_PARAM_STRING_OKAY; if (!(vm = lxcDomObjFromDomain(dom))) goto cleanup; signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/3] virNumaGetPageInfo: Take huge pages into account
On the Linux kernel, if huge pages are allocated the size they cut off from memory is accounted under the 'MemUsed' in the meminfo file. However, we want the sum to be subtracted from 'MemTotal'. This patch implements this feature. After this change, we can enable reporting of the ordinary system pages in the capability XML: capabilities host uuid01281cda-f352-cb11-a9db-e905fe22010c/uuid cpu archx86_64/arch modelHaswell/model vendorIntel/vendor topology sockets='1' cores='1' threads='1'/ feature/ pages unit='KiB' size='4'/ pages unit='KiB' size='2048'/ pages unit='KiB' size='1048576'/ /cpu power_management/ migration_features/ topology cells num='4' cell id='0' memory unit='KiB'4048248/memory pages unit='KiB' size='4'748382/pages pages unit='KiB' size='2048'3/pages pages unit='KiB' size='1048576'1/pages distances/ cpus num='1' cpu id='0' socket_id='0' core_id='0' siblings='0'/ /cpus /cell ... /cells /topology /host /capabilities You can see the beautiful thing about this: if you sum up all the pages/ you'll get memory/. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- src/nodeinfo.c | 2 +- src/util/virnuma.c | 65 +++--- src/util/virnuma.h | 1 + 3 files changed, 39 insertions(+), 29 deletions(-) diff --git a/src/nodeinfo.c b/src/nodeinfo.c index 8fe7adc..2c1c437 100644 --- a/src/nodeinfo.c +++ b/src/nodeinfo.c @@ -2038,7 +2038,7 @@ nodeGetFreePages(unsigned int npages, unsigned int page_size = pages[i]; unsigned int page_free; -if (virNumaGetPageInfo(cell, page_size, NULL, page_free) 0) +if (virNumaGetPageInfo(cell, page_size, 0, NULL, page_free) 0) goto cleanup; counts[ncounts++] = page_free; diff --git a/src/util/virnuma.c b/src/util/virnuma.c index 1745649..a176852 100644 --- a/src/util/virnuma.c +++ b/src/util/virnuma.c @@ -640,6 +640,8 @@ virNumaGetHugePageInfo(int node, * virNumaGetPageInfo: * @node: NUMA node id * @page_size: which huge page are we interested in (in KiB) + * @huge_page_sum: the sum of memory taken by huge pages (in + * bytes) * @page_avail: total number of huge pages in the pool * @page_free: the number of free huge pages in the pool * @@ -647,6 +649,13 @@ virNumaGetHugePageInfo(int node, * total number of pages in the pool (both free and taken) * and count for free pages in the pool. * + * The @huge_page_sum parameter exists there due to the Linux + * kernel limitation. The problem is, if there are some huge + * pages allocated, they are accounted under the 'MemUsed' field + * in the meminfo file instead of being subtracted from the + * 'MemTotal'. We must do the subtraction ourselves. + * If unsure, pass 0. + * * If you're interested in just one bit, pass NULL to the other one. * * As a special case, if @node == -1, overall info is fetched @@ -657,6 +666,7 @@ virNumaGetHugePageInfo(int node, int virNumaGetPageInfo(int node, unsigned int page_size, + unsigned long long huge_page_sum, unsigned int *page_avail, unsigned int *page_free) { @@ -666,7 +676,6 @@ virNumaGetPageInfo(int node, /* sysconf() returns page size in bytes, * the @page_size is however in kibibytes */ if (page_size == system_page_size / 1024) { -# if 0 unsigned long long memsize, memfree; /* TODO: come up with better algorithm that takes huge pages into @@ -679,16 +688,14 @@ virNumaGetPageInfo(int node, goto cleanup; } +/* see description above */ +memsize -= huge_page_sum; + if (page_avail) *page_avail = memsize / system_page_size; if (page_free) *page_free = memfree / system_page_size; -# else -virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, %s, - _(system page size are not supported yet)); -goto cleanup; -# endif /* 0 */ } else { if (virNumaGetHugePageInfo(node, page_size, page_avail, page_free) 0) goto cleanup; @@ -737,31 +744,18 @@ virNumaGetPages(int node, unsigned int ntmp = 0; size_t i; bool exchange; - -# if 0 -/* This has to be disabled until the time the issue in - * virNumaGetPageInfo is resolved. Sorry. */ long system_page_size; +unsigned long long huge_page_sum = 0; /* sysconf() returns page size in bytes, * but we are storing the page size in kibibytes. */ system_page_size = sysconf(_SC_PAGESIZE) / 1024; -/* We know that ordinary system pages are supported - * if nothing else is. */ -if (VIR_REALLOC_N(tmp_size, 1) 0 || -VIR_REALLOC_N(tmp_avail, 1) 0 || -VIR_REALLOC_N(tmp_free, 1) 0) -
[libvirt] [PATCH 0/3] Couple of huge pages improvements
*** BLURB HERE *** Michal Privoznik (3): virNumaGetPageInfo: Take huge pages into account virNumaGetPages: Don't fail on huge page-less systems virnuma: Actually build huge page code src/nodeinfo.c | 2 +- src/util/virnuma.c | 87 +++--- src/util/virnuma.h | 1 + 3 files changed, 52 insertions(+), 38 deletions(-) -- 1.8.5.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/3] virnuma: Actually build huge page code
One of previous commits (e6258a33) tried to build the huge page code only on Linux since it's Linux centric indeed. But it failed miserably as it used 'WITH_LINUX' which is an automake conditional not a gcc one. In the sources we need to use __linux__. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- src/util/virnuma.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/util/virnuma.c b/src/util/virnuma.c index 4901378..a0fa31c 100644 --- a/src/util/virnuma.c +++ b/src/util/virnuma.c @@ -513,7 +513,7 @@ virNumaGetDistances(int node ATTRIBUTE_UNUSED, /* currently all the hugepage stuff below is linux only */ -#if WITH_LINUX +#if __linux__ # define HUGEPAGES_NUMA_PREFIX /sys/devices/system/node/ # define HUGEPAGES_SYSTEM_PREFIX /sys/kernel/mm/hugepages/ @@ -861,7 +861,7 @@ virNumaGetPages(int node, } -#else /* #if WITH_LINUX */ +#else /* #ifdef __linux__ */ int virNumaGetPageInfo(int node ATTRIBUTE_UNUSED, unsigned int page_size ATTRIBUTE_UNUSED, @@ -886,4 +886,4 @@ virNumaGetPages(int node ATTRIBUTE_UNUSED, _(page info is not supported on this platform)); return -1; } -#endif /* #if WITH_LINUX */ +#endif /* #ifdef __linux__ */ -- 1.8.5.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/3] virNumaGetPages: Don't fail on huge page-less systems
If we are running on a system that is not capable of huge pages (e.g. because the kernel is not configured that way) we still try to open /sys/kernel/mm/hugepages/ which however does not exist. We should be tolerant to this specific use case. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- src/util/virnuma.c | 16 ++-- 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/src/util/virnuma.c b/src/util/virnuma.c index a176852..4901378 100644 --- a/src/util/virnuma.c +++ b/src/util/virnuma.c @@ -738,7 +738,7 @@ virNumaGetPages(int node, int ret = -1; char *path = NULL; DIR *dir = NULL; -int direrr; +int direrr = 0; struct dirent *entry; unsigned int *tmp_size = NULL, *tmp_avail = NULL, *tmp_free = NULL; unsigned int ntmp = 0; @@ -760,13 +760,17 @@ virNumaGetPages(int node, goto cleanup; if (!(dir = opendir(path))) { -virReportSystemError(errno, - _(unable to open path: %s), - path); -goto cleanup; +/* It's okay if the @path doesn't exist. Maybe we are running on + * system without huge pages support where the path may not exist. */ +if (errno != ENOENT) { +virReportSystemError(errno, + _(unable to open path: %s), + path); +goto cleanup; +} } -while ((direrr = virDirRead(dir, entry, path)) 0) { +while (dir (direrr = virDirRead(dir, entry, path)) 0) { const char *page_name = entry-d_name; unsigned int page_size, page_avail = 0, page_free = 0; char *end; -- 1.8.5.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/3] virnuma: Actually build huge page code
On 06/23/14 15:59, Michal Privoznik wrote: One of previous commits (e6258a33) tried to build the huge page code only on Linux since it's Linux centric indeed. But it failed miserably as it used 'WITH_LINUX' which is an automake conditional not a gcc one. In the sources we need to use __linux__. Sigh. I shouldn't touch any code today ... Signed-off-by: Michal Privoznik mpriv...@redhat.com --- src/util/virnuma.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/util/virnuma.c b/src/util/virnuma.c index 4901378..a0fa31c 100644 --- a/src/util/virnuma.c +++ b/src/util/virnuma.c @@ -513,7 +513,7 @@ virNumaGetDistances(int node ATTRIBUTE_UNUSED, /* currently all the hugepage stuff below is linux only */ -#if WITH_LINUX +#if __linux__ #ifdef # define HUGEPAGES_NUMA_PREFIX /sys/devices/system/node/ # define HUGEPAGES_SYSTEM_PREFIX /sys/kernel/mm/hugepages/ @@ -861,7 +861,7 @@ virNumaGetPages(int node, } -#else /* #if WITH_LINUX */ +#else /* #ifdef __linux__ */ int virNumaGetPageInfo(int node ATTRIBUTE_UNUSED, unsigned int page_size ATTRIBUTE_UNUSED, @@ -886,4 +886,4 @@ virNumaGetPages(int node ATTRIBUTE_UNUSED, _(page info is not supported on this platform)); return -1; } -#endif /* #if WITH_LINUX */ +#endif /* #ifdef __linux__ */ ACK with the macro fixed. Peter signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/5] virhostdev: Move IOMMU and VFIO funcs from qemu
On Fri, Jun 20, 2014 at 04:19:06PM +0200, Michal Privoznik wrote: The functions in question are qemuHostdevHostSupportsPassthroughLegacy and qemuHostdevHostSupportsPassthroughVFIO. At the same time the functions are renamed to match 'vir' prefix instead of 'qemu' one. I'm not sure I see the reason for moving these functions ? They're QEMU specific surely ? eg @@ -1635,3 +1640,71 @@ virHostdevUpdateDomainActiveDevices(virHostdevManagerPtr mgr, return 0; } + +#if HAVE_LINUX_KVM_H +bool +virHostdevHostSupportsPassthroughLegacy(void) +{ +int kvmfd = -1; +bool ret = false; + +if ((kvmfd = open(/dev/kvm, O_RDONLY)) 0) +goto cleanup; is going to be useless for anything except QEMU driver. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] build: link virstoragetest with libxml
Signed-off-by: Martin Kletzander mklet...@redhat.com --- Notes: To be honest, I have no idea why this fails for me in one situation, but it prevents the following error during compilation: /usr/lib/gcc/x86_64-pc-linux-gnu/4.9.0/../../../../x86_64-pc-linux-gnu/bin/ld: ../src/.libs/libvirt_driver_storage_impl.a(libvirt_driver_storage_impl_la-storage_backend.o): undefined reference to symbol 'xmlFreeDoc@@LIBXML2_2.4.30' /usr/lib/gcc/x86_64-pc-linux-gnu/4.9.0/../../../../lib64/libxml2.so: error adding symbols: DSO missing from command line collect2: error: ld returned 1 exit status Makefile:4228: recipe for target 'virstoragetest' failed Therefore I'm not pushing it as a build-breaker since this might not be the root cause or the best solution. The other fix (and probably more appropriate one) would be to add LIBXML_LIBS into libvirt_conf_la_LIBADD since the xmlFreeDoc() is called in storage_conf.c. Any other preferred way is accepted as well, feel free to comment. tests/Makefile.am | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/Makefile.am b/tests/Makefile.am index 025b847..457eb99 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -899,6 +899,7 @@ virstoragetest_LDADD = $(LDADDS) \ ../src/libvirt_util.la \ ../src/libvirt_driver_storage_impl.la \ ../gnulib/lib/libgnu.la \ + $(LIBXML_LIBS) \ $(NULL) viridentitytest_SOURCES = \ -- 2.0.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/5] virhostdev: Move IOMMU and VFIO funcs from qemu
On 23.06.2014 16:05, Daniel P. Berrange wrote: On Fri, Jun 20, 2014 at 04:19:06PM +0200, Michal Privoznik wrote: The functions in question are qemuHostdevHostSupportsPassthroughLegacy and qemuHostdevHostSupportsPassthroughVFIO. At the same time the functions are renamed to match 'vir' prefix instead of 'qemu' one. I'm not sure I see the reason for moving these functions ? They're QEMU specific surely ? eg @@ -1635,3 +1640,71 @@ virHostdevUpdateDomainActiveDevices(virHostdevManagerPtr mgr, return 0; } + +#if HAVE_LINUX_KVM_H +bool +virHostdevHostSupportsPassthroughLegacy(void) +{ +int kvmfd = -1; +bool ret = false; + +if ((kvmfd = open(/dev/kvm, O_RDONLY)) 0) +goto cleanup; is going to be useless for anything except QEMU driver. Regards, Daniel Well, I thought other drivers might use it in the future, but the patch set can live without this patch. I'm fine with dropping it. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/5] conf: Introduce viremulator_capabilities
On Fri, Jun 20, 2014 at 04:19:07PM +0200, Michal Privoznik wrote: --- docs/formatemulatorcaps.html.in| 52 docs/schemas/Makefile.am | 1 + docs/schemas/emulatorcapability.rng| 26 docs/sitemap.html.in | 4 + libvirt.spec.in| 1 + mingw-libvirt.spec.in | 2 + src/Makefile.am| 3 +- src/conf/viremulator_capabilities.c| 139 + src/conf/viremulator_capabilities.h| 47 +++ src/libvirt_private.syms | 6 + tests/Makefile.am | 10 +- .../viremulatorcaps-basic.xml | 5 + tests/viremulatorcapabilitiesschematest| 11 ++ tests/viremulatorcapabilitiestest.c| 117 + 14 files changed, 422 insertions(+), 2 deletions(-) create mode 100644 docs/formatemulatorcaps.html.in create mode 100644 docs/schemas/emulatorcapability.rng create mode 100644 src/conf/viremulator_capabilities.c create mode 100644 src/conf/viremulator_capabilities.h create mode 100644 tests/viremulatorcapabilitiesdata/viremulatorcaps-basic.xml create mode 100755 tests/viremulatorcapabilitiesschematest create mode 100644 tests/viremulatorcapabilitiestest.c As a nitpick on naming I think I'd call this 'domain capabilities' since it is about representing metadata about stuff you can put in the domain conf XML schema. so eg src/conf/domain_capabilties.{c.h} and virDomainCapsPtr for struct. diff --git a/src/conf/viremulator_capabilities.c b/src/conf/viremulator_capabilities.c new file mode 100644 index 000..8e7d4af --- /dev/null +++ b/src/conf/viremulator_capabilities.c @@ -0,0 +1,139 @@ +/* + * viremulator_capabilities.c: hypervisor capabilities + * + * Copyright (C) 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 + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * http://www.gnu.org/licenses/. + * + * Author: Michal Privoznik mpriv...@redhat.com + */ + +#include config.h + +#include viremulator_capabilities.h +#include virobject.h +#include viralloc.h +#include virbuffer.h +#include virstring.h + +#define VIR_FROM_THIS VIR_FROM_CAPABILITIES + +struct _virEmulatorCapabilities { +virObjectLockable parent; + +char *path; /* path to emulator binary */ +char *machine; /* machine type */ +virDomainVirtType virttype; /* virtualization type */ + +void *privateData; +virEmulatorCapabilitiesPrivateDataFormatFunc formatFunc; +}; +static int +virEmulatorCapabilitiesFormatInternal(virEmulatorCapabilitiesPtr caps, + virBufferPtr buf) +{ +const char *virttype_str = virDomainVirtTypeToString(caps-virttype); + +virBufferAddLit(buf, emulatorCapabilities\n); +virBufferAdjustIndent(buf, 2); +virBufferAsprintf(buf, path%s/path\n, caps-path); +virBufferAsprintf(buf, domain%s/domain\n, virttype_str); +virBufferAsprintf(buf, machine%s/machine\n, caps-machine); +if (caps-formatFunc) +caps-formatFunc(buf, caps-privateData, caps-machine); I'm really not a fan of this - it lets every virt driver in libvirt invent its own thing to put in these capabilities, which has bitten us in the past, since we end up with divergence between drivers I think we need to spec out some approach to representing the data in this struct which drivers populate. To start with we're looking for info on what enum values are supported in each device type. This should probably be preface with a way to express what device types are actually supported by the driver. eg so you can discover that channel is not supported by Xen or LXC. How about something like this: typedef const char * (*virDomainCapsEnumFormat)(int va;ue); struct _virDomainCapsEnum { int values; /* Bitmask of values supported in the corresponding enum */ }; struct _virDomainCapsDevice { bool supported; /* true if devtype is supported by hypervisor */ }; struct _virDomainCapsDeviceDisk { struct virDomainCapsDevice; virDomainCapsEnum device; /* Info about virDomainDiskDevice enum values */ virDomainCapsEnum
Re: [libvirt] [PATCH 3/5] Introduce virConnectGetEmulatorCapabilities
On Fri, Jun 20, 2014 at 04:19:08PM +0200, Michal Privoznik wrote: The API is supposed to fetch virEmulatorCapabilities once implemented in the hypervisor drivers. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- include/libvirt/libvirt.h.in | 6 + src/driver.h | 7 ++ src/libvirt.c| 52 src/libvirt_public.syms | 2 ++ src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 19 +++- src/remote_protocol-structs | 10 + 7 files changed, 96 insertions(+), 1 deletion(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index c83b20d..f71f732 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1585,6 +1585,12 @@ int virNodeGetInfo (virConnectPtr conn, virNodeInfoPtr info); char * virConnectGetCapabilities (virConnectPtr conn); +char * virConnectGetEmulatorCapabilities(virConnectPtr conn, + const char *emulatorbin, + const char *machine, + const char *virttype, + unsigned int flags); Following on from my prev comments lets call this virConnectGetDomainCapabilities since it is really about the domain XML schema and it is conceivable for a virt driver to not have any emulator at all. Hmm, this reminds me that we should have 'const char *arch' in there too. While for QEMU the architecture is implied by the emulator binary path provided, this doesn't hold true for say VMWare which doesn't use the emulator binary field. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 5/5] qemu: Implement virConnectGetEmulatorCapabilities
On Fri, Jun 20, 2014 at 04:19:10PM +0200, Michal Privoznik wrote: The commit produces this nice output: virsh # emulatorcaps /usr/bin/qemu-system-x86_64 kvm pc-1.3 emulatorCapabilities path/usr/bin/qemu-system-x86_64/path domainkvm/domain machinepc-1.3/machine archx86_64/arch vcpu255/vcpu devices hostdev driver enum name='driver' valuekvm/value valuevfio/value /enum /driver /hostdev /devices /emulatorCapabilities Following on from my comments in the earlier patch, this XML looks pretty much ok. Just s/emulatorCapabilities/domainCapabilities/ Also, within devices we would ultimately aim to have an entry for every device type we support, with a supported=yes|no attribute eg devices disk supported='no'/ hostdev supported='yes' driver enum name='driver' valuekvm/value valuevfio/value /enum /driver /hostdev /devices That way absence of information for a particular device type means no information available rather than unsupported. The unsupported case would always be explicit, so apps can deal with case where we have not fully converted all drivers to report this new info. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- docs/formatemulatorcaps.html.in| 63 +++ docs/schemas/emulatorcapability.rng| 49 src/libvirt_private.syms | 1 + src/qemu/qemu_capabilities.c | 78 ++ src/qemu/qemu_capabilities.h | 3 + src/qemu/qemu_capabilitiespriv.h | 55 + src/qemu/qemu_driver.c | 92 ++ tests/Makefile.am | 18 + .../viremulatorcaps-qemu-kvm-vfio.xml | 17 tests/viremulatorcapabilitiestest.c| 72 - tests/virhostdevmock.c | 40 ++ 11 files changed, 456 insertions(+), 32 deletions(-) create mode 100644 src/qemu/qemu_capabilitiespriv.h create mode 100644 tests/viremulatorcapabilitiesdata/viremulatorcaps-qemu-kvm-vfio.xml create mode 100644 tests/virhostdevmock.c diff --git a/docs/formatemulatorcaps.html.in b/docs/formatemulatorcaps.html.in index beea1a9..ba87ff1 100644 --- a/docs/formatemulatorcaps.html.in +++ b/docs/formatemulatorcaps.html.in @@ -48,5 +48,68 @@ ddThe domain's a href=formatdomain.html#elementsOSBIOSmachine type/a./dd /dl + +pBesides these three elements, depending on the hypervisor and the driver +used other elements may occur. For example:/p +pre +lt;archgt;x86_64lt;/archgt; +lt;vcpugt;255lt;/vcpugt; +lt;devices/gt; +/pre +pWhere:/p +dl + dtarch/dt + ddDenotes the a +href=formatdomain.html#elementsOSBIOSarchitecture/a of the + domain./dd + + dtvcpu/dt + ddTells the maximum virtual CPU count supported by the underlying + hypervisor./dd + + dtdevices/dt + ddContains info on supported a href=#elementsDevicesdevices/a features/dd +/dl + +h3a name=elementsDevicesDevices/a/h3 +pThe final set of XML elements is used to describe device capabilities +with respect to the hypervisor and host capabilities. For example:/p +pre +lt;devicesgt; + lt;hostdevgt; +lt;drivergt; + lt;enum name='driver'gt; +lt;valuegt;kvmlt;/valuegt; +lt;valuegt;vfiolt;/valuegt; + lt;/enumgt; +lt;/drivergt; + lt;/hostdevgt; +lt;/devicesgt; +/pre +pThe aim is to keep the element names as consistent with a + href=formatdomain.htmldomain element names/a as possible./p + +h4a name=elementsHostDevHost device assignment/a/h4 +pWhen deciding on the most suitable device passthrough mode, look into +this part of the XML document. For instance, in the following snippet both +VFIO and legacy KVM passthrough are supported:/p +pre +lt;devicesgt; + lt;hostdevgt; +lt;drivergt; + lt;enum name='driver'gt; +lt;valuegt;kvmlt;/valuegt; +lt;valuegt;vfiolt;/valuegt; + lt;/enumgt; +lt;/drivergt; + lt;/hostdevgt; +lt;/devicesgt; +/pre + +pThe codehostdev/code element can contain the following elements:/p +dl + dtcodedriver/code/dt + ddHere are the information on hostdev driver gathered./dd +/dl /body /html diff --git a/docs/schemas/emulatorcapability.rng b/docs/schemas/emulatorcapability.rng index 2548cef..0c4a0cb 100644 --- a/docs/schemas/emulatorcapability.rng +++ b/docs/schemas/emulatorcapability.rng @@ -20,7 +20,56 @@ element name='machine' text/ /element +optional + element name='arch' +text/ + /element + element name='vcpu' +
Re: [libvirt] [PATCHv5 02/19] util: storagefile: Introduce universal function to canonicalize paths
On 06/23/2014 02:28 AM, Peter Krempa wrote: On 06/20/14 16:35, Eric Blake wrote: On 06/19/2014 07:59 AM, Peter Krempa wrote: Introduce a common function that will take a callback to resolve links that will be used to canonicalize paths on various storage systems and add extensive tests. --- src/libvirt_private.syms | 1 + src/util/virstoragefile.c | 195 ++ src/util/virstoragefile.h | 7 ++ tests/virstoragetest.c| 108 + 4 files changed, 311 insertions(+) ACK if you can make those changes (you may want to post the interdiff) The required changes to pass your suggested changes are: +++ b/tests/virstoragetest.c @@ -1154,14 +1154,14 @@ mymain(void) TEST_PATH_CANONICALIZE(5, ///, /); TEST_PATH_CANONICALIZE(6, //, //); TEST_PATH_CANONICALIZE(7, , ); -TEST_PATH_CANONICALIZE(8, ., ); +TEST_PATH_CANONICALIZE(8, ., .); TEST_PATH_CANONICALIZE(9, ../, ..); TEST_PATH_CANONICALIZE(10, ../../, ../..); TEST_PATH_CANONICALIZE(11, ../../blah, ../../blah); TEST_PATH_CANONICALIZE(12, /./././blah, /blah); TEST_PATH_CANONICALIZE(13, .././../././../blah, ../../../blah); TEST_PATH_CANONICALIZE(14, /././, /); -TEST_PATH_CANONICALIZE(15, ./././, ); +TEST_PATH_CANONICALIZE(15, ./././, .); TEST_PATH_CANONICALIZE(16, blah/../foo, foo); TEST_PATH_CANONICALIZE(17, foo/bar/../blah, foo/blah); TEST_PATH_CANONICALIZE(18, foo/bar/.././blah, foo/blah); @@ -1179,6 +1179,7 @@ mymain(void) TEST_PATH_CANONICALIZE(28, /path/blah/yippee, /other/path/huzah/yippee); TEST_PATH_CANONICALIZE(29, /cycle, NULL); TEST_PATH_CANONICALIZE(30, /cycle2/link, NULL); +TEST_PATH_CANONICALIZE(31, ///, /); Looks good. ACK to the squash-in. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/3] Add pkg-config files to allow deps to build against source tree
On 06/23/2014 02:52 AM, Daniel P. Berrange wrote: On Fri, Jun 20, 2014 at 01:02:46PM -0600, Eric Blake wrote: On 06/20/2014 10:51 AM, Daniel P. Berrange wrote: When testing language bindings it is useful to be able to build them against an uninstalled libvirt source tree. Add a dummy set of pkg-config files to allow for this. This can be used by setting export PKG_CONFIG_PATH=/path/to/libvirt/git/src Yay - we need to document this trick in libvirt-python.git as well. -EXTRA_DIST = $(conf_DATA) util/keymaps.csv +EXTRA_DIST = \ + $(conf_DATA) \ + util/keymaps.csv \ + libvirt.pc \ + libvirt-qemu.pc \ + libvirt-lxc.pc \ + $(NULL) NACK to this hunk - the .pc files should NOT be part of the tarball, because they contain contents that depend on configure results, while the tarball must be independent. End users will get their own .pc file as soon as they do ./configure make. Obviously that was meant to be the .pc.in files Doesn't automake automatically ship any .in file that are required by their use in AC_CONFIG_FILES? [I'd have to actually test 'make dist' to prove one way or the other.] But if not, then yes, adding the .pc.in files to EXTRA_DIST is appropriate. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/3] Add pkg-config files to allow deps to build against source tree
On Mon, Jun 23, 2014 at 09:15:19AM -0600, Eric Blake wrote: On 06/23/2014 02:52 AM, Daniel P. Berrange wrote: On Fri, Jun 20, 2014 at 01:02:46PM -0600, Eric Blake wrote: On 06/20/2014 10:51 AM, Daniel P. Berrange wrote: When testing language bindings it is useful to be able to build them against an uninstalled libvirt source tree. Add a dummy set of pkg-config files to allow for this. This can be used by setting export PKG_CONFIG_PATH=/path/to/libvirt/git/src Yay - we need to document this trick in libvirt-python.git as well. -EXTRA_DIST = $(conf_DATA) util/keymaps.csv +EXTRA_DIST = \ + $(conf_DATA) \ + util/keymaps.csv \ + libvirt.pc \ + libvirt-qemu.pc \ + libvirt-lxc.pc \ + $(NULL) NACK to this hunk - the .pc files should NOT be part of the tarball, because they contain contents that depend on configure results, while the tarball must be independent. End users will get their own .pc file as soon as they do ./configure make. Obviously that was meant to be the .pc.in files Doesn't automake automatically ship any .in file that are required by their use in AC_CONFIG_FILES? [I'd have to actually test 'make dist' to prove one way or the other.] But if not, then yes, adding the .pc.in files to EXTRA_DIST is appropriate. You're right, it isn't needed at all. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [libvirt-glib] [PATCH 2/2] libvirt-gobject-domain: Add _get_snapshots
... which returns a GList of GVirDomainSnapshots, i.e. without any tree structure or other relationship between the snapshots. --- libvirt-gobject/libvirt-gobject-domain.c | 21 + libvirt-gobject/libvirt-gobject-domain.h | 4 libvirt-gobject/libvirt-gobject.sym | 1 + 3 files changed, 26 insertions(+) diff --git a/libvirt-gobject/libvirt-gobject-domain.c b/libvirt-gobject/libvirt-gobject-domain.c index f6b5837..2da99df 100644 --- a/libvirt-gobject/libvirt-gobject-domain.c +++ b/libvirt-gobject/libvirt-gobject-domain.c @@ -1573,3 +1573,24 @@ gboolean gvir_domain_fetch_snapshots(GVirDomain *dom, g_free(snapshots); return TRUE; } + +/** + * gvir_domain_get_snapshots: + * @dom: The domain + * Returns: (element-type LibvirtGObject.DomainSnapshot) (transfer full): A list of + * all the snapshots available for the given domain. The returned list + * should be freed with g_list_free(), after its elements have been unreffed + * with g_object_unref(). + */ +GList *gvir_domain_get_snapshots(GVirDomain *dom) +{ +GList *snapshots = NULL; +g_return_val_if_fail(GVIR_IS_DOMAIN(dom), NULL); + +if (dom-priv-snapshots != NULL) { +snapshots = g_hash_table_get_values(dom-priv-snapshots); +g_list_foreach(snapshots, (GFunc)g_object_ref, NULL); +} + +return snapshots; +} diff --git a/libvirt-gobject/libvirt-gobject-domain.h b/libvirt-gobject/libvirt-gobject-domain.h index fb33e2b..acea7c2 100644 --- a/libvirt-gobject/libvirt-gobject-domain.h +++ b/libvirt-gobject/libvirt-gobject-domain.h @@ -366,6 +366,10 @@ gvir_domain_create_snapshot(GVirDomain *dom, gboolean gvir_domain_fetch_snapshots(GVirDomain *dom, guint flags, GError **error); + +GList *gvir_domain_get_snapshots(GVirDomain *dom); + + G_END_DECLS #endif /* __LIBVIRT_GOBJECT_DOMAIN_H__ */ diff --git a/libvirt-gobject/libvirt-gobject.sym b/libvirt-gobject/libvirt-gobject.sym index 781310f..28e547a 100644 --- a/libvirt-gobject/libvirt-gobject.sym +++ b/libvirt-gobject/libvirt-gobject.sym @@ -237,6 +237,7 @@ LIBVIRT_GOBJECT_0.1.5 { LIBVIRT_GOBJECT_0.1.9 { global: gvir_domain_fetch_snapshots; + gvir_domain_get_snapshots; gvir_domain_snapshot_delete; gvir_domain_snapshot_list_flags_get_type; } LIBVIRT_GOBJECT_0.1.5; -- 2.0.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [libvirt-glib] [PATCH 1/2] libvirt-gobject-domain: Add _fetch_snapshots
This function can be used to fetch the snapshots of a domain (according to the given GVirDomainSnapshotListFlags) and save them in a domain-internal GHashTable. A function to access them from outside will be added in a later patch. --- libvirt-gobject/libvirt-gobject-domain.c | 59 libvirt-gobject/libvirt-gobject-domain.h | 36 +++ libvirt-gobject/libvirt-gobject.sym | 2 ++ 3 files changed, 97 insertions(+) diff --git a/libvirt-gobject/libvirt-gobject-domain.c b/libvirt-gobject/libvirt-gobject-domain.c index c6e30e5..f6b5837 100644 --- a/libvirt-gobject/libvirt-gobject-domain.c +++ b/libvirt-gobject/libvirt-gobject-domain.c @@ -38,6 +38,7 @@ struct _GVirDomainPrivate { virDomainPtr handle; gchar uuid[VIR_UUID_STRING_BUFLEN]; +GHashTable *snapshots; }; G_DEFINE_TYPE(GVirDomain, gvir_domain, G_TYPE_OBJECT); @@ -121,6 +122,8 @@ static void gvir_domain_finalize(GObject *object) g_debug(Finalize GVirDomain=%p, domain); +g_hash_table_unref (priv-snapshots); + virDomainFree(priv-handle); G_OBJECT_CLASS(gvir_domain_parent_class)-finalize(object); @@ -1514,3 +1517,59 @@ gvir_domain_create_snapshot(GVirDomain *dom, g_free(custom_xml); return dom_snapshot; } + + + +/** + * gvir_domain_fetch_snapshots: + * @dom: The domain + * @list_flags: bitwise-OR of #GVirDomainSnapshotListFlags + * @error: (allow-none): Place-holder for error or NULL + * + * Returns: TRUE on success, FALSE otherwise. + */ +gboolean gvir_domain_fetch_snapshots(GVirDomain *dom, + guint list_flags, + GError **error) +{ +GVirDomainPrivate *priv; +virDomainSnapshotPtr *snapshots = NULL; +GVirDomainSnapshot *snap; +int n_snaps = 0; +int i; + +g_return_val_if_fail(GVIR_IS_DOMAIN(dom), FALSE); +g_return_val_if_fail((error == NULL) || (*error == NULL), FALSE); + +priv = dom-priv; + +if (priv-snapshots != NULL) { +g_hash_table_destroy (priv-snapshots); +} + +priv-snapshots = g_hash_table_new_full(g_str_hash, +g_str_equal, +g_free, +g_object_unref); + + +n_snaps = virDomainListAllSnapshots(priv-handle, snapshots, list_flags); + +if (n_snaps 0) { +gvir_set_error(error, GVIR_DOMAIN_ERROR, 0, + Unable to fetch snapshots of %s, + gvir_domain_get_name (dom)); +return FALSE; +} + +for (i = 0; i n_snaps; i ++) { +snap = GVIR_DOMAIN_SNAPSHOT(g_object_new(GVIR_TYPE_DOMAIN_SNAPSHOT, + handle, snapshots[i], + NULL)); +g_hash_table_insert(priv-snapshots, +(gpointer)gvir_domain_snapshot_get_name(snap), +snap); +} +g_free(snapshots); +return TRUE; +} diff --git a/libvirt-gobject/libvirt-gobject-domain.h b/libvirt-gobject/libvirt-gobject-domain.h index 38d3458..fb33e2b 100644 --- a/libvirt-gobject/libvirt-gobject-domain.h +++ b/libvirt-gobject/libvirt-gobject-domain.h @@ -183,6 +183,39 @@ typedef enum { GVIR_DOMAIN_REBOOT_GUEST_AGENT= VIR_DOMAIN_REBOOT_GUEST_AGENT, } GVirDomainRebootFlags; +/** + * GVirDomainSnapshotListFlags: + * @GVIR_DOMAIN_SNAPSHOT_LIST_ALL: List all snapshots + * @GVIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS: List all descendants, not just + * children, when listing a snapshot. + * For historical reasons, groups do not use contiguous bits. + * @GVIR_DOMAIN_SNAPSHOT_LIST_ROOTS: Filter by snapshots with no parents, when listing a domain + * @GVIR_DOMAIN_SNAPSHOT_LIST_METADATA: Filter by snapshots which have metadata + * @GVIR_DOMAIN_SNAPSHOT_LIST_LEAVES: Filter by snapshots with no children + * @GVIR_DOMAIN_SNAPSHOT_LIST_NO_LEAVES: Filter by snapshots that have children + * @GVIR_DOMAIN_SNAPSHOT_LIST_NO_METADATA: Filter by snapshots with no metadata + * @GVIR_DOMAIN_SNAPSHOT_LIST_INACTIVE: Filter by snapshots taken while guest was shut off + * @GVIR_DOMAIN_SNAPSHOT_LIST_ACTIVE: Filter by snapshots taken while guest was active, and with memory state + * @GVIR_DOMAIN_SNAPSHOT_LIST_DISK_ONLY: Filter by snapshots taken while guest was active, but without memory state + * @GVIR_DOMAIN_SNAPSHOT_LIST_INTERNAL: Filter by snapshots stored internal to disk images + * @GVIR_DOMAIN_SNAPSHOT_LIST_EXTERNAL: Filter by snapshots that use files external to disk images + */ +typedef enum { +GVIR_DOMAIN_SNAPSHOT_LIST_ALL = 0, +GVIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS = VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS, +GVIR_DOMAIN_SNAPSHOT_LIST_ROOTS = VIR_DOMAIN_SNAPSHOT_LIST_ROOTS, +GVIR_DOMAIN_SNAPSHOT_LIST_METADATA=
[libvirt] [libvirt-glib] [PATCH 0/2] Add API to fetch the snapshots of a GVirDomain
The following two patches add gvir_domain_fetch_snapshots which uses virDomainListAllSnapshots to fetch snapshots from the virDomain (using the given GVirDomainSnapshotListFlags) and gvir_domain_get_snapshots which returns a GList of containing all of the snapshots last fetched. Timm Bäder (2): libvirt-gobject-domain: Add _fetch_snapshots libvirt-gobject-domain: Add _get_snapshots libvirt-gobject/libvirt-gobject-domain.c | 80 libvirt-gobject/libvirt-gobject-domain.h | 40 libvirt-gobject/libvirt-gobject.sym | 3 ++ 3 files changed, 123 insertions(+) -- 2.0.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix closedir usage in virNumaGetPages
On 06/23/2014 05:46 AM, Michal Privoznik wrote: -closedir(dir); +if (dir) +closedir(dir); VIR_FREE(path); return ret; } So why is free(NULL) safe on FreeBSD then? I'd call this a libc bug not a libvirt one. But since even we already have such borken design (remember our publir vir*Free() APIs?) I can live with this patch. free(NULL) is explicitly required by C (and therefore POSIX) to be safe. closedir(NULL) is intentionally unspecified by POSIX, and therefore unsafe. It would be nice if the two had similar requirements, but as POSIX was merely standardizing existing practice, we are stuck with them being different in behavior. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 0/5] Next round of active commit support
On 06/21/2014 04:34 AM, Kashyap Chamarthy wrote: I just tried to test this series built from Eric's libvirt git repo, and QEMU built from its git. On a live (or offline guest) I see the below: $ virsh blockcommit f20vm1 vda --shallow --wait \ --verbose --pivot --active error: unsupported flags (0x4) in function qemuDomainBlockCommit I also tried: $ virsh blockcommit --domain f20vm1 vda \ --base /home/kashyap/vmimages/snap3-f20vm1.qcow2 \ --top /home/kashyap/vmimages/snap4-f20vm1.qcow2 \ --wait --verbose --pivot --active error: unsupported flags (0x4) in function qemuDomainBlockCommit Is that expected? Unfortunately, yes, unless you have also applied Jeff's pending patches for qemu that actually enable optional 'top': https://lists.gnu.org/archive/html/qemu-devel/2014-06/msg04060.html What am I missing? You're very bleeding edge; I won't push my patches to libvirt.git until qemu.git also has matching patches, so in the meantime, you have to carry patches on both repos. Peter has been doing some rpm builds of the bits and pieces for integration testing, but I'm not sure if he is planning on publishing them for public consumption yet. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v6 0/4] Introduce APIs to extract DHCP leases info
On Mon, Jun 16, 2014 at 3:44 AM, Nehal J Wani nehaljw.k...@gmail.com wrote: This API returns the leases information stored in the DHCP leases file of dnsmasq for a given virtual network. It contacts the bridge network driver, which parses a custom leases file created by libvirt. It supports two methods: 1. Return info for all network interfaces connected to a given virtual network 2. Return information for a particular network interface in a given virtual network by providing its MAC Address v6 * Converted parsing mechanism to JSON. Leases helper program has been added as a separate patch Refer: https://www.redhat.com/archives/libvir-list/2014-May/msg00219.html v5 * Created a helper file to generate custom leases for libvirt. Since dnsmasq doesn't provide the MAC Address in case of DHCPv6, the need for this file was proposed. The design of virNetworkDHCPLeases struct has been updated and the previous use of union has been removed. OOM errors have been taken care of in the remote driver and remote daemon. Discussion on struct design: https://www.redhat.com/archives/libvir-list/2013-October/msg00326.html Discussion on helper file: https://www.redhat.com/archives/libvir-list/2013-October/msg00989.html Refer: http://www.redhat.com/archives/libvir-list/2013-November/msg01085.html v4 * Added support for DHCPv6, updated lease file parsing method Refer: https://www.redhat.com/archives/libvir-list/2013-September/msg01554.html v3 * Mostly small nits, change in MACRO names, use of virSocketAddrGetIpPrefix to retrieve IP prefix from @dom XML. Refer: https://www.redhat.com/archives/libvir-list/2013-September/msg00832.html v2 * Since DHCPv6 is supposed to be suported in future, virNetworkGetDHCPLeasesForMAC changed, prefix and virIPAddrType added in virNetworkDHCPLeases struct. Refer: https://www.redhat.com/archives/libvir-list/2013-September/msg00732.html v1 * Refer: https://www.redhat.com/archives/libvir-list/2013-September/msg00620.html * The need for these APIs were result of a RFC was proposed on the list. Refer: http://www.redhat.com/archives/libvir-list/2013-July/msg01603.html Nehal J Wani (4): net-dhcp-leases: Implement the public APIs net-dhcp-leases: Implement the remote protocol net-dhcp-leases: Private implementation inside network net-dhcp-leases: Add virsh support daemon/remote.c | 185 ++ include/libvirt/libvirt.h.in | 34 +++ src/driver.h | 13 +++ src/internal.h | 5 + src/libvirt.c| 179 + src/libvirt_public.syms | 6 ++ src/network/bridge_driver.c | 232 +++ src/remote/remote_driver.c | 186 ++ src/remote/remote_protocol.x | 51 +- src/remote_protocol-structs | 38 +++ src/rpc/gendispatch.pl | 1 + tools/virsh-network.c| 119 ++ tools/virsh.pod | 6 ++ 13 files changed, 1054 insertions(+), 1 deletion(-) -- 1.9.3 Ping! -- Nehal J Wani -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix closedir usage in virNumaGetPages
Eric Blake wrote: On 06/23/2014 05:46 AM, Michal Privoznik wrote: -closedir(dir); +if (dir) +closedir(dir); VIR_FREE(path); return ret; } So why is free(NULL) safe on FreeBSD then? I'd call this a libc bug not a libvirt one. But since even we already have such borken design (remember our publir vir*Free() APIs?) I can live with this patch. free(NULL) is explicitly required by C (and therefore POSIX) to be safe. closedir(NULL) is intentionally unspecified by POSIX, and therefore unsafe. It would be nice if the two had similar requirements, but as POSIX was merely standardizing existing practice, we are stuck with them being different in behavior. The patch is pushed now; thanks. Roman Bogorodskiy pgpJaW8oVBxxu.pgp Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/3] lxc: allow to keep or drop capabilities
On Thu, Jun 12, 2014 at 08:48:25AM +0200, Cédric Bosdonnat wrote: Added capabilities in the features section of LXC domains configuration. This section can contain elements named after the capabilities like: mknod state=on/, keep CAP_MKNOD capability sys_chroot state=off/ drop CAP_SYS_CHROOT capability Users can restrict or give more capabilities than the default using this mechanism. --- docs/schemas/domaincommon.rng | 196 src/conf/domain_conf.c | 93 ++- src/conf/domain_conf.h | 47 ++ src/libvirt_private.syms| 1 + src/lxc/lxc_cgroup.c| 5 + src/lxc/lxc_container.c | 90 +-- tests/domainschemadata/domain-caps-features.xml | 28 7 files changed, 442 insertions(+), 18 deletions(-) create mode 100644 tests/domainschemadata/domain-caps-features.xml diff --git a/src/lxc/lxc_cgroup.c b/src/lxc/lxc_cgroup.c index 8dfdc60..71a0d61 100644 --- a/src/lxc/lxc_cgroup.c +++ b/src/lxc/lxc_cgroup.c @@ -357,6 +357,11 @@ static int virLXCCgroupSetupDeviceACL(virDomainDefPtr def, {'c', LXC_DEV_MAJ_FUSE, LXC_DEV_MIN_FUSE}, {0, 0, 0}}; +/* No white list if CAP_MKNOD has to be kept */ +int capMknod = def-caps_features[VIR_DOMAIN_CAPS_FEATURE_MKNOD]; +if (capMknod == VIR_DOMAIN_FEATURE_STATE_ON) +return 0; + if (virCgroupDenyAllDevices(cgroup) 0) goto cleanup; So wrt device nodes we have two layers of defence - Blocking CAP_MKNOD - this means user can only have access to device nodes that are present in the /dev that we populate. - CGroups ACL blocking mkdir, read and write for everything except the device nodes that are implied by (or explicitly requested in) the XML I can see the value in allowing CAP_MKNOD because if we have granted the user '/dev/foo' access, there's no harm in letting them mknod /dev/foo too. If we have granted CAP_MKNOD though I'm not convinced that this implies we should allow access to any possible device ever. So at most I think we should allow 'mknod' in the cgroups, but still keep read+write blocked. We definitely shouldn't allow read+write just because they requested CAP_MKNOD. We already let users request access to arbitrary devices in the XML config to deal with the latter. -if ((ret = capng_updatev(CAPNG_DROP, - CAPNG_EFFECTIVE | CAPNG_PERMITTED | - CAPNG_INHERITABLE | CAPNG_BOUNDING_SET, - CAP_SYS_MODULE, /* No kernel module loading */ - CAP_SYS_TIME, /* No changing the clock */ - CAP_MKNOD, /* No creating device nodes */ - CAP_AUDIT_CONTROL, /* No messing with auditing status */ - CAP_MAC_ADMIN, /* No messing with LSM config */ - keepReboot ? -1 : CAP_SYS_BOOT, /* No use of reboot */ - -1)) 0) { -virReportError(VIR_ERR_INTERNAL_ERROR, - _(Failed to remove capabilities: %d), ret); -return -1; +for (i = 0; i VIR_DOMAIN_CAPS_FEATURE_LAST; i++) { +bool toDrop = false; +int state = def-caps_features[i]; + +switch ((virDomainCapsFeature) i) { +case VIR_DOMAIN_CAPS_FEATURE_SYS_BOOT: /* No use of reboot */ +toDrop = !keepReboot (state != VIR_DOMAIN_FEATURE_STATE_ON); +break; +case VIR_DOMAIN_CAPS_FEATURE_SYS_MODULE: /* No kernel module loading */ +case VIR_DOMAIN_CAPS_FEATURE_SYS_TIME: /* No changing the clock */ +case VIR_DOMAIN_CAPS_FEATURE_MKNOD: /* No creating device nodes */ +case VIR_DOMAIN_CAPS_FEATURE_AUDIT_CONTROL: /* No messing with auditing status */ +case VIR_DOMAIN_CAPS_FEATURE_MAC_ADMIN: /* No messing with LSM config */ +toDrop = (state != VIR_DOMAIN_FEATURE_STATE_ON); +break; +default: /* User specified capabilities to drop */ +toDrop = (state == VIR_DOMAIN_FEATURE_STATE_OFF); +} + +if (toDrop (ret = capng_update(CAPNG_DROP, + CAPNG_EFFECTIVE | CAPNG_PERMITTED | + CAPNG_INHERITABLE | CAPNG_BOUNDING_SET, + capsMapping[i])) 0) { +virReportError(VIR_ERR_INTERNAL_ERROR, + _(Failed to remove capability %s: %d), + virDomainCapsFeatureTypeToString(i), ret); +return -1; +} So what I don't like about the current code is that the set of 5 capabilities we drop is essentially arbitrary and has no inherant value. - If user namespaces are not active, then the container is
Re: [libvirt] [PATCH v6 0/4] Introduce APIs to extract DHCP leases info
Nehal J Wani (4): net-dhcp-leases: Implement the public APIs net-dhcp-leases: Implement the remote protocol net-dhcp-leases: Private implementation inside network net-dhcp-leases: Add virsh support daemon/remote.c | 185 ++ include/libvirt/libvirt.h.in | 34 +++ src/driver.h | 13 +++ src/internal.h | 5 + src/libvirt.c| 179 + src/libvirt_public.syms | 6 ++ src/network/bridge_driver.c | 232 +++ src/remote/remote_driver.c | 186 ++ src/remote/remote_protocol.x | 51 +- src/remote_protocol-structs | 38 +++ src/rpc/gendispatch.pl | 1 + tools/virsh-network.c| 119 ++ tools/virsh.pod | 6 ++ 13 files changed, 1054 insertions(+), 1 deletion(-) Ping! I tried to apply this locally to test it, but I'm afraid it has some nasty conflicts with the huge pages APIs that were added last week that were not going to be quick to resolve. Could you rebase repost it and I'll promise to review it then. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v6 1/4] net-dhcp-leases: Implement the public APIs
On Mon, Jun 16, 2014 at 03:44:53AM +0530, Nehal J Wani wrote: diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index cce6bdf..0ffbfd6 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -658,5 +658,11 @@ LIBVIRT_1.2.5 { virDomainSetTime; } LIBVIRT_1.2.3; +LIBVIRT_1.2.7 { +global: +virNetworkDHCPLeaseFree; +virNetworkGetDHCPLeases; +virNetworkGetDHCPLeasesForMAC; +} LIBVIRT_1.2.6; We've not released 1.2.6 yet, so you should be using a 1.2.5 {} 1.2.5 block not 1.2.7 Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] block/sheepdog: rename management program from collie to dog
On Mon, Jun 23, 2014 at 03:20:11PM +0900, Hitoshi Mitake wrote: The management program of latest sheepdog is named as dog, collie is obsolete. This patch updates the name in the configure script and the sheepdog driver. Signed-off-by: Vasiliy Tolstov v.tols...@selfip.ru Signed-off-by: Hitoshi Mitake mitake.hito...@lab.ntt.co.jp --- configure.ac | 10 +- src/storage/storage_backend_sheepdog.c | 12 ++-- 2 files changed, 11 insertions(+), 11 deletions(-) This will break libvirt when run against existing sheepdog releases which still have the name 'collie'. Libvirt needs to detect which command name is used and pick the right one to use, not just hardcode one of them. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] block/sheepdog: rename management program from collie to dog
The management program of latest sheepdog is named as dog, collie is obsolete. This patch updates the name in the configure script and the sheepdog driver. Signed-off-by: Vasiliy Tolstov v.tols...@selfip.ru Signed-off-by: Hitoshi Mitake mitake.hito...@lab.ntt.co.jp --- configure.ac | 10 +- src/storage/storage_backend_sheepdog.c | 12 ++-- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/configure.ac b/configure.ac index 710cb71..186d9e3 100644 --- a/configure.ac +++ b/configure.ac @@ -1926,14 +1926,14 @@ AC_SUBST([LIBRBD_LIBS]) if test $with_storage_sheepdog = yes || test $with_storage_sheepdog = check; then - AC_PATH_PROG([COLLIE], [collie], [], [$PATH:/sbin:/usr/sbin]) + AC_PATH_PROG([DOG], [dog], [], [$PATH:/sbin:/usr/sbin]) if test $with_storage_sheepdog = yes; then -if test -z $COLLIE; then - AC_MSG_ERROR([We need collie for Sheepdog storage driver]) +if test -z $DOG; then + AC_MSG_ERROR([We need dog for Sheepdog storage driver]) fi else -if test -z $COLLIE; then +if test -z $DOG; then with_storage_sheepdog=no fi @@ -1945,7 +1945,7 @@ if test $with_storage_sheepdog = yes || if test $with_storage_sheepdog = yes; then AC_DEFINE_UNQUOTED([WITH_STORAGE_SHEEPDOG], 1, [whether Sheepdog backend for storage driver is enabled]) -AC_DEFINE_UNQUOTED([COLLIE],[$COLLIE],[Location of collie program]) +AC_DEFINE_UNQUOTED([DOG],[$DOG],[Location of dog program]) fi fi AM_CONDITIONAL([WITH_STORAGE_SHEEPDOG], diff --git a/src/storage/storage_backend_sheepdog.c b/src/storage/storage_backend_sheepdog.c index 9419859..864ecd6 100644 --- a/src/storage/storage_backend_sheepdog.c +++ b/src/storage/storage_backend_sheepdog.c @@ -150,7 +150,7 @@ virStorageBackendSheepdogRefreshAllVol(virConnectPtr conn ATTRIBUTE_UNUSED, char **cells = NULL; size_t i; -virCommandPtr cmd = virCommandNewArgList(COLLIE, vdi, list, -r, NULL); +virCommandPtr cmd = virCommandNewArgList(DOG, vdi, list, -r, NULL); virStorageBackendSheepdogAddHostArg(cmd, pool); virCommandSetOutputBuffer(cmd, output); if (virCommandRun(cmd, NULL) 0) @@ -195,7 +195,7 @@ virStorageBackendSheepdogRefreshPool(virConnectPtr conn ATTRIBUTE_UNUSED, char *output = NULL; virCommandPtr cmd; -cmd = virCommandNewArgList(COLLIE, node, info, -r, NULL); +cmd = virCommandNewArgList(DOG, node, info, -r, NULL); virStorageBackendSheepdogAddHostArg(cmd, pool); virCommandSetOutputBuffer(cmd, output); if (virCommandRun(cmd, NULL) 0) @@ -221,7 +221,7 @@ virStorageBackendSheepdogDeleteVol(virConnectPtr conn ATTRIBUTE_UNUSED, virCheckFlags(0, -1); -virCommandPtr cmd = virCommandNewArgList(COLLIE, vdi, delete, vol-name, NULL); +virCommandPtr cmd = virCommandNewArgList(DOG, vdi, delete, vol-name, NULL); virStorageBackendSheepdogAddHostArg(cmd, pool); int ret = virCommandRun(cmd, NULL); @@ -266,7 +266,7 @@ virStorageBackendSheepdogBuildVol(virConnectPtr conn, virCheckFlags(0, -1); -virCommandPtr cmd = virCommandNewArgList(COLLIE, vdi, create, vol-name, NULL); +virCommandPtr cmd = virCommandNewArgList(DOG, vdi, create, vol-name, NULL); virCommandAddArgFormat(cmd, %llu, vol-target.capacity); virStorageBackendSheepdogAddHostArg(cmd, pool); if (virCommandRun(cmd, NULL) 0) @@ -351,7 +351,7 @@ virStorageBackendSheepdogRefreshVol(virConnectPtr conn ATTRIBUTE_UNUSED, int ret; char *output = NULL; -virCommandPtr cmd = virCommandNewArgList(COLLIE, vdi, list, vol-name, -r, NULL); +virCommandPtr cmd = virCommandNewArgList(DOG, vdi, list, vol-name, -r, NULL); virStorageBackendSheepdogAddHostArg(cmd, pool); virCommandSetOutputBuffer(cmd, output); ret = virCommandRun(cmd, NULL); @@ -387,7 +387,7 @@ virStorageBackendSheepdogResizeVol(virConnectPtr conn ATTRIBUTE_UNUSED, virCheckFlags(0, -1); -virCommandPtr cmd = virCommandNewArgList(COLLIE, vdi, resize, vol-name, NULL); +virCommandPtr cmd = virCommandNewArgList(DOG, vdi, resize, vol-name, NULL); virCommandAddArgFormat(cmd, %llu, capacity); virStorageBackendSheepdogAddHostArg(cmd, pool); int ret = virCommandRun(cmd, NULL); -- 1.7.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] block/sheepdog: rename management program from collie to dog
On 06/23/2014 12:20 AM, Hitoshi Mitake wrote: The management program of latest sheepdog is named as dog, collie is obsolete. This patch updates the name in the configure script and the sheepdog driver. Signed-off-by: Vasiliy Tolstov v.tols...@selfip.ru Signed-off-by: Hitoshi Mitake mitake.hito...@lab.ntt.co.jp --- configure.ac | 10 +- src/storage/storage_backend_sheepdog.c | 12 ++-- 2 files changed, 11 insertions(+), 11 deletions(-) Please re-read the comments given at the first attempt at this patch: https://www.redhat.com/archives/libvir-list/2014-June/msg00760.html diff --git a/configure.ac b/configure.ac index 710cb71..186d9e3 100644 --- a/configure.ac +++ b/configure.ac @@ -1926,14 +1926,14 @@ AC_SUBST([LIBRBD_LIBS]) if test $with_storage_sheepdog = yes || test $with_storage_sheepdog = check; then - AC_PATH_PROG([COLLIE], [collie], [], [$PATH:/sbin:/usr/sbin]) + AC_PATH_PROG([DOG], [dog], [], [$PATH:/sbin:/usr/sbin]) This is wrong; you need to test for both names, and use the correct one, in order to be back-compat safe. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v6 0/4] Introduce APIs to extract DHCP leases info
On Mon, Jun 23, 2014 at 9:36 PM, Daniel P. Berrange berra...@redhat.com wrote: Nehal J Wani (4): net-dhcp-leases: Implement the public APIs net-dhcp-leases: Implement the remote protocol net-dhcp-leases: Private implementation inside network net-dhcp-leases: Add virsh support daemon/remote.c | 185 ++ include/libvirt/libvirt.h.in | 34 +++ src/driver.h | 13 +++ src/internal.h | 5 + src/libvirt.c| 179 + src/libvirt_public.syms | 6 ++ src/network/bridge_driver.c | 232 +++ src/remote/remote_driver.c | 186 ++ src/remote/remote_protocol.x | 51 +- src/remote_protocol-structs | 38 +++ src/rpc/gendispatch.pl | 1 + tools/virsh-network.c| 119 ++ tools/virsh.pod | 6 ++ 13 files changed, 1054 insertions(+), 1 deletion(-) Ping! I tried to apply this locally to test it, but I'm afraid it has some nasty conflicts with the huge pages APIs that were added last week that were not going to be quick to resolve. Could you rebase repost it and I'll promise to review it then. Sure, I'll rebase and re-post. Thanks for advance-review :-) -- Nehal J Wani -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Errors from unprivileged libvirtd at startup
Current libvirt git really spews errors to the console at startup when run unprivileged $ ./daemon/libvirtd 2014-06-23 16:55:27.806+: 5207: info : libvirt version: 1.2.6 2014-06-23 16:55:27.806+: 5207: error : virPCIDeviceConfigOpen:300 : Failed to open config space file '/sys/bus/pci/devices/:00:00.0/config': Permission denied 2014-06-23 16:55:27.807+: 5207: error : virPCIDeviceConfigOpen:300 : Failed to open config space file '/sys/bus/pci/devices/:00:02.0/config': Permission denied 2014-06-23 16:55:27.808+: 5207: error : virPCIDeviceConfigOpen:300 : Failed to open config space file '/sys/bus/pci/devices/:00:14.0/config': Permission denied 2014-06-23 16:55:27.810+: 5207: error : virPCIDeviceConfigOpen:300 : Failed to open config space file '/sys/bus/pci/devices/:00:16.0/config': Permission denied 2014-06-23 16:55:27.810+: 5207: error : virPCIDeviceConfigOpen:300 : Failed to open config space file '/sys/bus/pci/devices/:00:19.0/config': Permission denied 2014-06-23 16:55:27.811+: 5207: error : virPCIDeviceConfigOpen:300 : Failed to open config space file '/sys/bus/pci/devices/:00:1a.0/config': Permission denied 2014-06-23 16:55:27.815+: 5207: error : virPCIDeviceConfigOpen:300 : Failed to open config space file '/sys/bus/pci/devices/:00:1b.0/config': Permission denied 2014-06-23 16:55:27.817+: 5207: error : virPCIDeviceConfigOpen:300 : Failed to open config space file '/sys/bus/pci/devices/:00:1c.0/config': Permission denied 2014-06-23 16:55:27.824+: 5207: error : virPCIDeviceConfigOpen:300 : Failed to open config space file '/sys/bus/pci/devices/:02:00.0/config': Permission denied 2014-06-23 16:55:27.825+: 5207: error : virPCIDeviceConfigOpen:300 : Failed to open config space file '/sys/bus/pci/devices/:00:1c.1/config': Permission denied 2014-06-23 16:55:27.826+: 5207: error : virPCIDeviceConfigOpen:300 : Failed to open config space file '/sys/bus/pci/devices/:03:00.0/config': Permission denied 2014-06-23 16:55:27.827+: 5207: error : virFileReadAll:1297 : Failed to read file '/sys/class/net/wlp3s0/speed': Invalid argument 2014-06-23 16:55:27.828+: 5207: error : virPCIDeviceConfigOpen:300 : Failed to open config space file '/sys/bus/pci/devices/:00:1c.2/config': Permission denied 2014-06-23 16:55:27.828+: 5207: error : virPCIDeviceConfigOpen:300 : Failed to open config space file '/sys/bus/pci/devices/:00:1d.0/config': Permission denied 2014-06-23 16:55:27.829+: 5207: error : virPCIDeviceConfigOpen:300 : Failed to open config space file '/sys/bus/pci/devices/:00:1f.0/config': Permission denied 2014-06-23 16:55:27.829+: 5207: error : virPCIDeviceConfigOpen:300 : Failed to open config space file '/sys/bus/pci/devices/:00:1f.2/config': Permission denied 2014-06-23 16:55:27.834+: 5207: error : virPCIDeviceConfigOpen:300 : Failed to open config space file '/sys/bus/pci/devices/:00:1f.3/config': Permission denied Whatever is doing this needs to skip itfrom unprivileged daemons. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 0/5] Next round of active commit support
On Mon, Jun 23, 2014 at 09:39:08AM -0600, Eric Blake wrote: On 06/21/2014 04:34 AM, Kashyap Chamarthy wrote: I just tried to test this series built from Eric's libvirt git repo, and QEMU built from its git. On a live (or offline guest) I see the below: $ virsh blockcommit f20vm1 vda --shallow --wait \ --verbose --pivot --active error: unsupported flags (0x4) in function qemuDomainBlockCommit I also tried: $ virsh blockcommit --domain f20vm1 vda \ --base /home/kashyap/vmimages/snap3-f20vm1.qcow2 \ --top /home/kashyap/vmimages/snap4-f20vm1.qcow2 \ --wait --verbose --pivot --active error: unsupported flags (0x4) in function qemuDomainBlockCommit Is that expected? Unfortunately, yes, unless you have also applied Jeff's pending patches for qemu that actually enable optional 'top': https://lists.gnu.org/archive/html/qemu-devel/2014-06/msg04060.html Yep, I just applied the above patch to today's QEMU git and tested with same libvirt (from your repos.or.cz instance) commit ID that I pointed to in my earlier email in this thread, and active blockcommit w/ 'virsh' works for me. Following are details of a simple test I did. Before active blockcommit - - I had a chain like that: [base] -- [snap1] -- [snap2] -- [snap3] -- [snap4] (current-active) - Current active block device in use: $ virsh domblklist f20vm1 Target Source vda/home/kashyap/vmimages/snap4.qcow2 Perform active blockcommit -- The below commmand will commit contents of [snap4] into [snap3] and makes [snap3] as the current active image invalidating [snap4]: $ virsh blockcommit f20vm1 vda --shallow --wait \ --verbose --pivot --active Block Commit: [100 %] Successfully pivoted After active blockcommit The chain has reduced to below (shortened by one image): [base] -- [snap1] -- [snap2] -- [snap3] (current-active) Again, check what's the current active block device via 'virsh' (it should reflect 'snap3'): $ virsh domblklist f20vm1 Target Source vda/home/kashyap/vmimages/snap3.qcow2 Also, it updated the 'virsh dumpxml' output just fine. Lastly, I ran active blockcommit again (w/ --shallow), so the chain is now reduced to [base] -- [snap1] -- [snap2] (current-active) And, here's its associated 'virsh dumpxml' output showing the uppdated backing store (chain reduced by two): $ virsh dumpxml f20vm1 | grep snap2.qcow2 -A10 -B2 disk type='file' device='disk' driver name='qemu' type='qcow2'/ source file='/home/kashyap/vmimages/snap2.qcow2'/ backingStore type='file' index='1' format type='qcow2'/ source file='/home/kashyap/vmimages/snap1.qcow2'/ backingStore type='file' index='2' format type='qcow2'/ source file='/home/kashyap/vmimages/f20vm1.qcow2'/ backingStore/ /backingStore /backingStore target dev='vda' bus='virtio'/ So, works as described in the patch titled blockcommit: turn on active commit (on libvir-list). Will test more once patches are committed to git. You're very bleeding edge; I won't push my patches to libvirt.git until qemu.git also has matching patches, so in the meantime, you have to carry patches on both repos. Peter has been doing some rpm builds of the bits and pieces for integration testing, but I'm not sure if he is planning on publishing them for public consumption yet. No worries, I can test manually apply patches ('git am -3') from the list to test. -- /kashyap -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/2] virtportallocator: new function virPortAllocatorSetUsed
virPortAllocatorSetUsed permits to set a port as already used and prevent the port allocator to use it without any attempt to bind it. Signed-off-by: Giuseppe Scrivano gscri...@redhat.com --- src/libvirt_private.syms| 1 + src/util/virportallocator.c | 40 +++- src/util/virportallocator.h | 4 3 files changed, 44 insertions(+), 1 deletion(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 2a2b9c0..95a45aa 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1770,6 +1770,7 @@ virPidFileWritePath; virPortAllocatorAcquire; virPortAllocatorNew; virPortAllocatorRelease; +virPortAllocatorSetUsed; # util/virprocess.h diff --git a/src/util/virportallocator.c b/src/util/virportallocator.c index b68133a..e98fc8a 100644 --- a/src/util/virportallocator.c +++ b/src/util/virportallocator.c @@ -1,7 +1,7 @@ /* * virportallocator.c: Allocate track TCP port allocations * - * 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 @@ -250,3 +250,41 @@ int virPortAllocatorRelease(virPortAllocatorPtr pa, virObjectUnlock(pa); return ret; } + +int virPortAllocatorSetUsed(virPortAllocatorPtr pa, +unsigned short port, +bool used) +{ +int ret = -1; + +virObjectLock(pa); + + +if (port pa-start || +port pa-end) { +ret = 0; +goto cleanup; +} + +if (used) { +if (virBitmapSetBit(pa-bitmap, port - pa-start) 0) { +virReportError(VIR_ERR_INTERNAL_ERROR, + _(Failed to reserve port %d), port); +goto cleanup; +} +} +else { +if (virBitmapClearBit(pa-bitmap, + port - pa-start) 0) { +virReportError(VIR_ERR_INTERNAL_ERROR, + _(Failed to release port %d), + port); +goto cleanup; +} +} + +ret = 0; + cleanup: +virObjectUnlock(pa); +return ret; +} diff --git a/src/util/virportallocator.h b/src/util/virportallocator.h index c8aa6de..e5ee56d 100644 --- a/src/util/virportallocator.h +++ b/src/util/virportallocator.h @@ -38,4 +38,8 @@ int virPortAllocatorAcquire(virPortAllocatorPtr pa, int virPortAllocatorRelease(virPortAllocatorPtr pa, unsigned short port); +int virPortAllocatorSetUsed(virPortAllocatorPtr pa, +unsigned short port, +bool value); + #endif /* __VIR_PORT_ALLOCATOR_H__ */ -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/2] virtportallocator: remember not auto allocated graphics ports
Fix a conflict when both autoport graphics and statically configured ports are used, like: graphics type='spice' autoport='yes'/ graphics type='vnc' port='5900' autoport='no'/ More details in 2/2 Closes: https://bugzilla.redhat.com/show_bug.cgi?id=1081881 Giuseppe Scrivano (2): virtportallocator: new function virPortAllocatorSetUsed graphics: remember graphics not auto allocated ports src/conf/domain_conf.h | 3 ++ src/libvirt_private.syms| 1 + src/qemu/qemu_process.c | 79 +++-- src/util/virportallocator.c | 40 ++- src/util/virportallocator.h | 4 +++ 5 files changed, 117 insertions(+), 10 deletions(-) -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/2] graphics: remember graphics not auto allocated ports
When looking for a port to allocate, the port allocator didn't take in consideration ports that are statically set by the user. Defining these two graphics elements in the XML would cause an error, as the port allocator would try to use the same port for the spice graphics element: graphics type='spice' autoport='yes'/ graphics type='vnc' port='5900' autoport='no'/ The new *[pP]ortAllocated variables keep track of the ports that were successfully registered (either bound or simply tracked as used) by the port allocator to allow a clean rollback on errors. Closes: https://bugzilla.redhat.com/show_bug.cgi?id=1081881 Signed-off-by: Giuseppe Scrivano gscri...@redhat.com --- src/conf/domain_conf.h | 3 ++ src/qemu/qemu_process.c | 79 +++-- 2 files changed, 73 insertions(+), 9 deletions(-) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 6779a41..7617574 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1385,6 +1385,7 @@ struct _virDomainGraphicsDef { union { struct { int port; +bool portAllocated; int websocket; bool autoport; char *keymap; @@ -1410,6 +1411,8 @@ struct _virDomainGraphicsDef { struct { int port; int tlsPort; +bool portAllocated; +bool tlsPortAllocated; int mousemode; char *keymap; virDomainGraphicsAuthDef auth; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 0b8155b..06f1e54 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3487,6 +3487,7 @@ qemuProcessVNCAllocatePorts(virQEMUDriverPtr driver, if (virPortAllocatorAcquire(driver-remotePorts, port) 0) return -1; graphics-data.vnc.port = port; +graphics-data.vnc.portAllocated = true; } if (graphics-data.vnc.websocket == -1) { @@ -3548,6 +3549,7 @@ qemuProcessSPICEAllocatePorts(virQEMUDriverPtr driver, goto error; graphics-data.spice.port = port; +graphics-data.spice.portAllocated = true; } if (needTLSPort || graphics-data.spice.tlsPort == -1) { @@ -3573,6 +3575,7 @@ qemuProcessSPICEAllocatePorts(virQEMUDriverPtr driver, goto error; graphics-data.spice.tlsPort = tlsPort; +graphics-data.spice.tlsPortAllocated = true; } } @@ -3803,6 +3806,36 @@ int qemuProcessStart(virConnectPtr conn, for (i = 0; i vm-def-ngraphics; ++i) { virDomainGraphicsDefPtr graphics = vm-def-graphics[i]; +if (graphics-type == VIR_DOMAIN_GRAPHICS_TYPE_VNC +!graphics-data.vnc.autoport) { +if (virPortAllocatorSetUsed(driver-remotePorts, +graphics-data.vnc.port, +true) 0) { +goto cleanup; +} + +graphics-data.vnc.portAllocated = true; + +} else if (graphics-type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE + !graphics-data.spice.autoport) { +if (virPortAllocatorSetUsed(driver-remotePorts, +graphics-data.spice.port, +true) 0) +goto cleanup; + +graphics-data.spice.portAllocated = true; + +if (virPortAllocatorSetUsed(driver-remotePorts, +graphics-data.spice.tlsPort, +true) 0) +goto cleanup; + +graphics-data.spice.tlsPortAllocated = true; +} +} + +for (i = 0; i vm-def-ngraphics; ++i) { +virDomainGraphicsDefPtr graphics = vm-def-graphics[i]; if (graphics-type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) { if (qemuProcessVNCAllocatePorts(driver, graphics) 0) goto cleanup; @@ -4509,19 +4542,47 @@ void qemuProcessStop(virQEMUDriverPtr driver, for (i = 0; i vm-def-ngraphics; ++i) { virDomainGraphicsDefPtr graphics = vm-def-graphics[i]; if (graphics-type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) { -if (graphics-data.vnc.autoport) { -virPortAllocatorRelease(driver-remotePorts, -graphics-data.vnc.port); +if (graphics-data.vnc.portAllocated) { +if (graphics-data.vnc.autoport) { +virPortAllocatorRelease(driver-remotePorts, +graphics-data.vnc.port); +} else { +virPortAllocatorSetUsed(driver-remotePorts, +graphics-data.vnc.port, +false); +} +graphics-data.vnc.portAllocated = false; } +
[libvirt] [PATCH] cmdFreepages: initialize @tmp
In the 404bac14 the @tmp variable was introduced. It's purpose is to avoid typecasting when parsing --pagesize argument. However, if the argument is not presented, tmp may be used uninitialized resulting in bogus virNodeGetFreePages() API call: virsh freepages --cellno 2 error: Failed to open file '/sys/devices/system/node/node2/hugepages/hugepages-4294967295kB/free_hugepages': No such file or directory Signed-off-by: Michal Privoznik mpriv...@redhat.com --- tools/virsh-host.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/virsh-host.c b/tools/virsh-host.c index 13d4c5c..734f1a8 100644 --- a/tools/virsh-host.c +++ b/tools/virsh-host.c @@ -229,7 +229,7 @@ cmdFreepages(vshControl *ctl, const vshCmd *cmd) bool ret = false; unsigned int npages; unsigned int *pagesize = NULL; -unsigned long long tmp; +unsigned long long tmp = 0; int cell; unsigned long long *counts = NULL; size_t i, j; -- 1.8.5.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] cmdFreepages: initialize @tmp
On 06/23/2014 12:34 PM, Michal Privoznik wrote: In the 404bac14 the @tmp variable was introduced. It's purpose is to avoid typecasting when parsing --pagesize argument. However, if the argument is not presented, tmp may be used uninitialized resulting in bogus virNodeGetFreePages() API call: virsh freepages --cellno 2 error: Failed to open file '/sys/devices/system/node/node2/hugepages/hugepages-4294967295kB/free_hugepages': No such file or directory Signed-off-by: Michal Privoznik mpriv...@redhat.com --- tools/virsh-host.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) ACK diff --git a/tools/virsh-host.c b/tools/virsh-host.c index 13d4c5c..734f1a8 100644 --- a/tools/virsh-host.c +++ b/tools/virsh-host.c @@ -229,7 +229,7 @@ cmdFreepages(vshControl *ctl, const vshCmd *cmd) bool ret = false; unsigned int npages; unsigned int *pagesize = NULL; -unsigned long long tmp; +unsigned long long tmp = 0; int cell; unsigned long long *counts = NULL; size_t i, j; -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v7 3/4] net-dhcp-leases: Private implementation inside network
Query the network driver for the path of the custom leases file for the given virtual network and parse it to retrieve info. src/network/bridge_driver.c: * Implement networkGetDHCPLeases * Implement networkGetDHCPLeasesForMAC * Implement networkGetDHCPLeasesHelper --- src/network/bridge_driver.c | 232 1 file changed, 232 insertions(+) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 8de7a59..d5577e0 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -73,9 +73,17 @@ #include viraccessapicheck.h #include network_event.h #include virhook.h +#include virjson.h #define VIR_FROM_THIS VIR_FROM_NETWORK +/** + * VIR_NETWORK_DHCP_LEASE_FILE_SIZE_MAX: + * + * Macro providing the upper limit on the size of leases file + */ +#define VIR_NETWORK_DHCP_LEASE_FILE_SIZE_MAX (32 * 1024 * 1024) + VIR_LOG_INIT(network.bridge_driver); static void networkDriverLock(virNetworkDriverStatePtr driver) @@ -3360,6 +3368,228 @@ static int networkSetAutostart(virNetworkPtr net, return ret; } +static int +networkGetDHCPLeasesHelper(virNetworkObjPtr obj, + const char *mac, + virNetworkDHCPLeasePtr **leases) +{ +size_t i, j; +size_t nleases = 0; +int rv = -1; +int size = 0; +int custom_lease_file_len = 0; +bool need_results = !!leases; +long long currtime = 0; +long long expirytime_tmp = -1; +bool ipv6 = false; +char *lease_entries = NULL; +char *custom_lease_file = NULL; +const char *ip_tmp = NULL; +const char *mac_tmp = NULL; +virJSONValuePtr lease_tmp = NULL; +virJSONValuePtr leases_array = NULL; +virNetworkIpDefPtr ipdef_tmp = NULL; +virNetworkDHCPLeasePtr lease = NULL; +virNetworkDHCPLeasePtr *leases_ret = NULL; + +/* Retrieve custom leases file location */ +custom_lease_file = networkDnsmasqLeaseFileNameCustom(obj-def-bridge); + +/* Read entire contents */ +if ((custom_lease_file_len = virFileReadAll(custom_lease_file, + VIR_NETWORK_DHCP_LEASE_FILE_SIZE_MAX, +lease_entries)) 0) { +/* Even though src/network/leaseshelper.c guarantees the existence of + * leases file (even if no leases are present), and the control reaches + * here, instead of reporting error, return 0 leases */ +rv = 0; +goto error; +} + +if (custom_lease_file_len) { +if (!(leases_array = virJSONValueFromString(lease_entries))) { +virReportError(VIR_ERR_INTERNAL_ERROR, + _(invalid json in file: %s), custom_lease_file); +goto error; +} + +if ((size = virJSONValueArraySize(leases_array)) 0) { +virReportError(VIR_ERR_INTERNAL_ERROR, %s, + _(couldn't fetch array of leases)); +goto error; +} +} + +currtime = (long long) time(NULL); + +for (i = 0; i size; i++) { +if (!(lease_tmp = virJSONValueArrayGet(leases_array, i))) { +virReportError(VIR_ERR_INTERNAL_ERROR, %s, + _(failed to parse json)); +goto error; +} + +if (!(mac_tmp = virJSONValueObjectGetString(lease_tmp, mac-address))) { +/* leaseshelper program guarantees that lease will be stored only if + * mac-address is known otherwise not */ +virReportError(VIR_ERR_INTERNAL_ERROR, %s, + _(found lease without mac-address)); +goto error; +} + +if (mac virMacAddrCompare(mac, mac_tmp)) { +virJSONValueFree(lease_tmp); +continue; +} + +if (virJSONValueObjectGetNumberLong(lease_tmp, expiry-time, expirytime_tmp) 0) { +/* A lease cannot be present without expiry-time */ +virReportError(VIR_ERR_INTERNAL_ERROR, %s, + _(found lease without expiry-time)); +goto error; +} + +/* Do not report expired lease */ +if (expirytime_tmp currtime) +continue; + +if (need_results) { +if (VIR_ALLOC(lease) 0) +goto error; + +lease-expirytime = expirytime_tmp; + +if (!(ip_tmp = virJSONValueObjectGetString(lease_tmp, ip-address))) { +/* A lease without ip-address makes no sense */ +virReportError(VIR_ERR_INTERNAL_ERROR, %s, + _(found lease without ip-address)); +goto error; +} + +/* Unlike IPv4, IPv6 uses ':' instead of '.' as separator */ +ipv6 = strchr(ip_tmp, ':') ? true : false; +lease-type = ipv6 ? VIR_IP_ADDR_TYPE_IPV6 : VIR_IP_ADDR_TYPE_IPV4; + +/* Obtain prefix */ +for (j = 0; j
[libvirt] [PATCH v7 0/4] Introduce APIs to extract DHCP leases info
This API returns the leases information stored in the DHCP leases file of dnsmasq for a given virtual network. It contacts the bridge network driver, which parses a custom leases file created by libvirt. It supports two methods: 1. Return info for all network interfaces connected to a given virtual network 2. Return information for a particular network interface in a given virtual network by providing its MAC Address v7 * Just rebased and resent v6 * Converted parsing mechanism to JSON. Leases helper program has been added as a separate patch Refer: https://www.redhat.com/archives/libvir-list/2014-May/msg00219.html Refer: https://www.redhat.com/archives/libvir-list/2014-June/msg00678.html v5 * Created a helper file to generate custom leases for libvirt. Since dnsmasq doesn't provide the MAC Address in case of DHCPv6, the need for this file was proposed. The design of virNetworkDHCPLeases struct has been updated and the previous use of union has been removed. OOM errors have been taken care of in the remote driver and remote daemon. Discussion on struct design: https://www.redhat.com/archives/libvir-list/2013-October/msg00326.html Discussion on helper file: https://www.redhat.com/archives/libvir-list/2013-October/msg00989.html Refer: http://www.redhat.com/archives/libvir-list/2013-November/msg01085.html v4 * Added support for DHCPv6, updated lease file parsing method Refer: https://www.redhat.com/archives/libvir-list/2013-September/msg01554.html v3 * Mostly small nits, change in MACRO names, use of virSocketAddrGetIpPrefix to retrieve IP prefix from @dom XML. Refer: https://www.redhat.com/archives/libvir-list/2013-September/msg00832.html v2 * Since DHCPv6 is supposed to be suported in future, virNetworkGetDHCPLeasesForMAC changed, prefix and virIPAddrType added in virNetworkDHCPLeases struct. Refer: https://www.redhat.com/archives/libvir-list/2013-September/msg00732.html v1 * Refer: https://www.redhat.com/archives/libvir-list/2013-September/msg00620.html * The need for these APIs were result of a RFC was proposed on the list. Refer: http://www.redhat.com/archives/libvir-list/2013-July/msg01603.html Nehal J Wani (4): net-dhcp-leases: Implement the public APIs net-dhcp-leases: Implement the remote protocol net-dhcp-leases: Private implementation inside network net-dhcp-leases: Add virsh support daemon/remote.c | 183 ++ include/libvirt/libvirt.h.in | 34 +++ src/driver.h | 13 +++ src/internal.h | 5 + src/libvirt.c| 179 + src/libvirt_public.syms | 3 + src/network/bridge_driver.c | 232 +++ src/remote/remote_driver.c | 186 ++ src/remote/remote_protocol.x | 51 +- src/remote_protocol-structs | 38 +++ src/rpc/gendispatch.pl | 1 + tools/virsh-network.c| 119 ++ tools/virsh.pod | 6 ++ 13 files changed, 1049 insertions(+), 1 deletion(-) -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v7 1/4] net-dhcp-leases: Implement the public APIs
Introduce 3 new APIs, virNetworkGetDHCPLeases, virNetworkGetDHCPLeasesForMAC and virNetworkDHCPLeaseFree. * virNetworkGetDHCPLeases: returns the dhcp leases information for a given virtual network. For DHCPv4, the information returned: - Network Interface Name - Expiry Time - MAC address - IAID (NULL) - IPv4 address (with type and prefix) - Hostname (can be NULL) - Client ID (can be NULL) For DHCPv6, the information returned: - Network Interface Name - Expiry Time - MAC address - IAID (can be NULL, only in rare cases) - IPv6 address (with type and prefix) - Hostname (can be NULL) - Client DUID Note: @mac, @iaid, @ipaddr, @clientid are in ASCII form, not raw bytes. Note: @expirytime can 0, in case the lease is for infinite time. * virNetworkGetDHCPLeasesForMAC: returns the dhcp leases information for a given virtual network and specified MAC Address. * virNetworkDHCPLeaseFree: allows the upper layer application to free the network interface object conveniently. There is no support for flags, so user is expected to pass 0 for both the APIs. include/libvirt/libvirt.h.in: * Define virNetworkGetDHCPLeases * Define virNetworkGetDHCPLeasesForMAC * Define virNetworkDHCPLeaseFree src/driver.h: * Define networkGetDHCPLeases * Define networkGetDHCPLeasesForMAC src/libvirt.c: * Implement virNetworkGetDHCPLeases * Implement virNetworkGetDHCPLeasesForMAC * Implement virNetworkDHCPLeaseFree src/libvirt_public.syms: * Export the new symbols --- include/libvirt/libvirt.h.in | 34 src/driver.h | 13 src/libvirt.c| 179 +++ src/libvirt_public.syms | 3 + 4 files changed, 229 insertions(+) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index c83b20d..b3120e2 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -5162,6 +5162,40 @@ typedef enum { #endif } virNetworkEventID; +typedef enum { +VIR_IP_ADDR_TYPE_IPV4, +VIR_IP_ADDR_TYPE_IPV6, + +#ifdef VIR_ENUM_SENTINELS +VIR_IP_ADDR_TYPE_LAST +#endif +} virIPAddrType; + +typedef struct _virNetworkDHCPLease virNetworkDHCPLease; +typedef virNetworkDHCPLease *virNetworkDHCPLeasePtr; +struct _virNetworkDHCPLease { +char *interface;/* Network interface name */ +long long expirytime; /* Seconds since epoch */ +int type; /* virIPAddrType */ +char *mac; /* MAC address */ +char *iaid; /* IAID */ +char *ipaddr; /* IP address */ +unsigned int prefix;/* IP address prefix */ +char *hostname; /* Hostname */ +char *clientid; /* Client ID or DUID */ +}; + +void virNetworkDHCPLeaseFree(virNetworkDHCPLeasePtr lease); + +int virNetworkGetDHCPLeases(virNetworkPtr network, +virNetworkDHCPLeasePtr **leases, +unsigned int flags); + +int virNetworkGetDHCPLeasesForMAC(virNetworkPtr network, + const char *mac, + virNetworkDHCPLeasePtr **leases, + unsigned int flags); + /** * virConnectNetworkEventGenericCallback: * @conn: the connection pointer diff --git a/src/driver.h b/src/driver.h index a0b258a..6e72e92 100644 --- a/src/driver.h +++ b/src/driver.h @@ -1182,6 +1182,17 @@ typedef int unsigned long long *counts, unsigned int flags); +typedef int +(*virDrvNetworkGetDHCPLeases)(virNetworkPtr network, + virNetworkDHCPLeasePtr **leases, + unsigned int flags); + +typedef int +(*virDrvNetworkGetDHCPLeasesForMAC)(virNetworkPtr network, +const char *mac, +virNetworkDHCPLeasePtr **leases, +unsigned int flags); + typedef struct _virDriver virDriver; typedef virDriver *virDriverPtr; @@ -1534,6 +1545,8 @@ struct _virNetworkDriver { virDrvNetworkSetAutostart networkSetAutostart; virDrvNetworkIsActive networkIsActive; virDrvNetworkIsPersistent networkIsPersistent; +virDrvNetworkGetDHCPLeases networkGetDHCPLeases; +virDrvNetworkGetDHCPLeasesForMAC networkGetDHCPLeasesForMAC; }; diff --git a/src/libvirt.c b/src/libvirt.c index 83b7437..f4d03bc 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -21005,3 +21005,182 @@ virNodeGetFreePages(virConnectPtr conn, virDispatchError(conn); return -1; } + +/** + * virNetworkGetDHCPLeases: + * @network: Pointer to network object + * @leases: Pointer to a variable to store the array containing details on + * obtained leases, or NULL if the list is not required (just returns + * number of leases). + * @flags: Extra flags, not used yet, so callers should always
[libvirt] [PATCH v7 2/4] net-dhcp-leases: Implement the remote protocol
Implement RPC calls for virNetworkGetDHCPLeases, virNetworkGetDHCPLeasesForMAC daemon/remote.c * Define remoteSerializeNetworkDHCPLeases, remoteDispatchNetworkGetDHCPLeases * Define remoteDispatchNetworkGetDHCPLeasesForMAC * Define helper function remoteSerializeDHCPLease src/remote/remote_driver.c * Define remoteNetworkGetDHCPLeases * Define remoteNetworkGetDHCPLeasesForMAC * Define helper function remoteSerializeDHCPLease src/remote/remote_protocol.x * New RPC procedure: REMOTE_PROC_NETWORK_GET_DHCP_LEASES * Define structs remote_network_dhcp_leases, remote_network_get_dhcp_leases_args, remote_network_get_dhcp_leases_ret * New RPC procedure: REMOTE_PROC_NETWORK_GET_DHCP_LEASES_FOR_MAC * Define structs remote_network_dhcp_leases_for_mac, remote_network_get_dhcp_leases_for_mac_args, remote_network_get_dhcp_leases_for_mac_ret src/remote_protocol-structs * New structs added src/rpc/gendispatch.pl * Add exception (s/Dhcp/DHCP) for auto-generating names of the remote functions in daemon/remote_dispatch.h --- daemon/remote.c | 183 ++ src/remote/remote_driver.c | 186 +++ src/remote/remote_protocol.x | 51 +++- src/remote_protocol-structs | 38 + src/rpc/gendispatch.pl | 1 + 5 files changed, 458 insertions(+), 1 deletion(-) diff --git a/daemon/remote.c b/daemon/remote.c index d09ebad..f4ec8ea 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -6202,7 +6202,190 @@ remoteDispatchNodeGetFreePages(virNetServerPtr server ATTRIBUTE_UNUSED, VIR_FREE(ret-counts.counts_val); } return rv; +} + +/* Copy contents of virNetworkDHCPLeasePtr to remote_network_dhcp_lease */ +static int +remoteSerializeDHCPLease(remote_network_dhcp_lease *lease_dst, virNetworkDHCPLeasePtr lease_src) +{ +char **mac_tmp = NULL; +char **iaid_tmp = NULL; +char **hostname_tmp = NULL; +char **clientid_tmp = NULL; + +if (VIR_ALLOC(mac_tmp) 0 || +VIR_ALLOC(iaid_tmp) 0 || +VIR_ALLOC(hostname_tmp) 0 || +VIR_ALLOC(clientid_tmp) 0) +goto error; + +lease_dst-expirytime = lease_src-expirytime; +lease_dst-type = lease_src-type; +lease_dst-prefix = lease_src-prefix; + +if (VIR_STRDUP(lease_dst-interface, lease_src-interface) 0 || +VIR_STRDUP(lease_dst-ipaddr, lease_src-ipaddr) 0 || +VIR_STRDUP(*mac_tmp, lease_src-mac) 0 || +VIR_STRDUP(*iaid_tmp, lease_src-iaid) 0 || +VIR_STRDUP(*hostname_tmp, lease_src-hostname) 0 || +VIR_STRDUP(*clientid_tmp, lease_src-clientid) 0) +goto error; + +lease_dst-mac = *mac_tmp ? mac_tmp : NULL; +lease_dst-iaid = *iaid_tmp ? iaid_tmp : NULL; +lease_dst-hostname = *hostname_tmp ? hostname_tmp : NULL; +lease_dst-clientid = *clientid_tmp ? clientid_tmp : NULL; + +return 0; + + error: +VIR_FREE(*mac_tmp); +VIR_FREE(*iaid_tmp); +VIR_FREE(*hostname_tmp); +VIR_FREE(*clientid_tmp); +VIR_FREE(mac_tmp); +VIR_FREE(iaid_tmp); +VIR_FREE(hostname_tmp); +VIR_FREE(clientid_tmp); +VIR_FREE(lease_dst-ipaddr); +VIR_FREE(lease_dst-interface); +return -1; +} + + +static int +remoteDispatchNetworkGetDHCPLeases(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client, + virNetMessagePtr msg ATTRIBUTE_UNUSED, + virNetMessageErrorPtr rerr, + remote_network_get_dhcp_leases_args *args, + remote_network_get_dhcp_leases_ret *ret) +{ +int rv = -1; +size_t i; +struct daemonClientPrivate *priv = virNetServerClientGetPrivateData(client); +virNetworkDHCPLeasePtr *leases = NULL; +virNetworkPtr net = NULL; +int nleases = 0; +if (!priv-conn) { +virReportError(VIR_ERR_INTERNAL_ERROR, %s, _(connection not open)); +goto cleanup; +} + +if (!(net = get_nonnull_network(priv-conn, args-net))) +goto cleanup; + +if ((nleases = virNetworkGetDHCPLeases(net, + args-need_results ? leases : NULL, + args-flags)) 0) +goto cleanup; + +if (nleases REMOTE_NETWORK_DHCP_LEASES_MAX) { +virReportError(VIR_ERR_INTERNAL_ERROR, + _(Number of leases is %d, which exceeds max limit: %d), + nleases, REMOTE_NETWORK_DHCP_LEASES_MAX); +goto cleanup; +} + +if (leases nleases) { +if (VIR_ALLOC_N(ret-leases.leases_val, nleases) 0) +goto cleanup; + +ret-leases.leases_len = nleases; + +for (i = 0; i nleases; i++) { +if (remoteSerializeDHCPLease(ret-leases.leases_val + i, leases[i]) 0) +goto
[libvirt] [PATCH v7 4/4] net-dhcp-leases: Add virsh support
Use virNetworkGetDHCPLeases and virNetworkGetDHCPLeasesForMAC in virsh. The new feature supports the follwing methods: 1. Retrieve leases info for a given virtual network 2. Retrieve leases info for given network interface tools/virsh-domain-monitor.c * Introduce new command : net-dhcp-leases Example Usage: net-dhcp-leases network [mac] virsh # net-dhcp-leases --network default6 Expiry Time MAC addressProtocol IP address HostnameClient ID or DUID --- 2014-06-16 03:40:14 52:54:00:85:90:e2 ipv4 192.168.150.231/24 fedora20-test 01:52:54:00:85:90:e2 2014-06-16 03:40:17 52:54:00:85:90:e2 ipv6 2001:db8:ca2:2:1::c0/64 fedora20-test 00:04:b1:d8:86:42:e1:6a:aa:cf:d5:86:94:23:6f:94:04:cd 2014-06-16 03:34:42 52:54:00:e8:73:eb ipv4 192.168.150.181/24 ubuntu14-vm - 2014-06-16 03:34:46 52:54:00:e8:73:eb ipv6 2001:db8:ca2:2:1::5b/64 - 00:01:00:01:1b:30:c6:aa:52:54:00:e8:73:eb tools/virsh.pod * Document new command src/internal.h * Introduce new macro: EMPTYSTR --- src/internal.h| 5 +++ tools/virsh-network.c | 119 ++ tools/virsh.pod | 6 +++ 3 files changed, 130 insertions(+) diff --git a/src/internal.h b/src/internal.h index a9e2065..d355344 100644 --- a/src/internal.h +++ b/src/internal.h @@ -246,6 +246,11 @@ */ # define NULLSTR(s) ((s) ? (s) : null) +/* + * Similar to NULLSTR, but print '-' to make it more user friendly. + */ +# define EMPTYSTR(s) ((s) ? (s) : -) + /** * TODO: * diff --git a/tools/virsh-network.c b/tools/virsh-network.c index 4b0df62..2d5b9be 100644 --- a/tools/virsh-network.c +++ b/tools/virsh-network.c @@ -1285,6 +1285,119 @@ cmdNetworkEvent(vshControl *ctl, const vshCmd *cmd) } +/* + * net-dhcp-leases command + */ +static const vshCmdInfo info_network_dhcp_leases[] = { +{.name = help, + .data = N_(print lease info for a given network) +}, +{.name = desc, + .data = N_(Print lease info for a given network) +}, +{.name = NULL} +}; + +static const vshCmdOptDef opts_network_dhcp_leases[] = { +{.name = network, + .type = VSH_OT_DATA, + .flags = VSH_OFLAG_REQ, + .help = N_(network name or uuid) +}, +{.name = mac, + .type = VSH_OT_DATA, + .flags = VSH_OFLAG_NONE, + .help = N_(MAC address) +}, +{.name = NULL} +}; + +static int +vshNetworkDHCPLeaseSorter(const void *a, const void *b) +{ +int rv = -1; + +virNetworkDHCPLeasePtr *lease1 = (virNetworkDHCPLeasePtr *) a; +virNetworkDHCPLeasePtr *lease2 = (virNetworkDHCPLeasePtr *) b; + +if (*lease1 !*lease2) +return -1; + +if (!*lease1) +return *lease2 != NULL; + +rv = vshStrcasecmp((*lease1)-mac, (*lease2)-mac); +return rv; +} + +static bool +cmdNetworkDHCPLeases(vshControl *ctl, const vshCmd *cmd) +{ +const char *name = NULL; +const char *mac = NULL; +virNetworkDHCPLeasePtr *leases = NULL; +int nleases = 0; +bool ret = false; +size_t i; +unsigned int flags = 0; +virNetworkPtr network = NULL; + +if (vshCommandOptString(cmd, mac, mac) 0) +return false; + +if (!(network = vshCommandOptNetwork(ctl, cmd, name))) +return false; + +nleases = mac ? virNetworkGetDHCPLeasesForMAC(network, mac, leases, flags) +: virNetworkGetDHCPLeases(network, leases, flags); + +if (nleases 0) { +vshError(ctl, _(Failed to get leases info for %s), name); +goto cleanup; +} + +/* Sort the list according to MAC Address/IAID */ +qsort(leases, nleases, sizeof(*leases), vshNetworkDHCPLeaseSorter); + +vshPrintExtra(ctl, %-20s %-18s %-9s %-25s %-15s %s\n%s%s\n, + _(Expiry Time), _(MAC address), _(Protocol), + _(IP address), _(Hostname), _(Client ID or DUID), + --, + -); + +for (i = 0; i nleases; i++) { +const char *type = NULL; +char *cidr_format = NULL; +virNetworkDHCPLeasePtr lease = leases[i]; +time_t expirytime_tmp = lease-expirytime; +struct tm ts; +char expirytime[32]; +ts = *localtime_r(expirytime_tmp, ts); +strftime(expirytime, sizeof(expirytime), %Y-%m-%d %H:%M:%S, ts); + +type = (lease-type == VIR_IP_ADDR_TYPE_IPV4) ? ipv4 : +(lease-type == VIR_IP_ADDR_TYPE_IPV6) ? ipv6 : ; + +ignore_value(virAsprintf(cidr_format, %s/%d, + lease-ipaddr, lease-prefix)); + +vshPrintExtra(ctl, %-20s %-18s %-9s %-25s %-15s %s\n, + expirytime, EMPTYSTR(lease-mac), EMPTYSTR(type), cidr_format, +
Re: [libvirt] [Xen-devel] [PATCH] libxl: prefer qdisk for driver name='file'
Stefano Stabellini wrote: On Fri, 20 Jun 2014, Jim Fehlig wrote: The libxl driver currently sets the disk backend to LIBXL_DISK_BACKEND_TAP when driver name='file' is specified in the disk config. qdisk should be prefered with this configuration, otherwise existing configuration such as the following, which worked with the old Xen driver, will not work with the libxl driver disk type='file' device='cdrom' driver name='file'/ source file='/path/to/some/iso'/ target dev='hdc' bus='ide'/ readonly/ /disk In addition, tap performs poorly compared to qdisk. Signed-off-by: Jim Fehlig jfeh...@suse.com Acked-by: Stefano Stabellini stefano.stabell...@eu.citrix.com Thanks Stefano. This is a change that needed blessing from one of the Xen tools devs IMO. I've pushed the patch. Regards, Jim -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v4 4/5] blockcommit: track job type in xml
A future patch is going to wire up qemu active block commit jobs; but as they have similar events and are canceled/pivoted in the same way as block copy jobs, it is easiest to track all bookkeeping for the commit job by reusing the mirror element. This patch adds domain XML to track which job was responsible for creating a mirroring situation, and adds a job='copy' attribute to all existing uses of mirror. Along the way, it also massages the qemu monitor backend to read the new field in order to generate the correct type of libvirt job (even though it requires a future patch to actually cause a qemu event that can be reported as an active commit). * docs/schemas/domaincommon.rng: Enhance schema. * docs/formatdomain.html.in: Document it. * src/conf/domain_conf.h (_virDomainDiskDef): Add a field. * src/conf/domain_conf.c (virDomainBlockJobType): String conversion. (virDomainDiskDefParseXML): Parse job type. (virDomainDiskDefFormat): Output job type. * src/qemu/qemu_process.c (qemuProcessHandleBlockJob): Distinguish active from regular commit. * src/qemu/qemu_driver.c (qemuDomainBlockCopy): Set job type. (qemuDomainBlockPivot, qemuDomainBlockJobImpl): Clean up job type on completion. * tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-mirror-old.xml: Update tests. * tests/qemuxml2argvdata/qemuxml2argv-disk-mirror.xml: Likewise. * tests/qemuxml2argvdata/qemuxml2argv-disk-active-commit.xml: New file. * tests/qemuxml2xmltest.c (mymain): Drive new test. Signed-off-by: Eric Blake ebl...@redhat.com --- docs/formatdomain.html.in | 20 ++-- docs/schemas/domaincommon.rng | 13 src/conf/domain_conf.c | 33 ++- src/conf/domain_conf.h | 1 + src/qemu/qemu_driver.c | 3 ++ src/qemu/qemu_process.c| 18 +++ .../qemuxml2argv-disk-active-commit.xml| 37 ++ .../qemuxml2argvdata/qemuxml2argv-disk-mirror.xml | 4 +-- .../qemuxml2xmlout-disk-mirror-old.xml | 4 +-- tests/qemuxml2xmltest.c| 1 + 10 files changed, 113 insertions(+), 21 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-active-commit.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 3075e16..16061b8 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1878,16 +1878,18 @@ /dd dtcodemirror/code/dt dd -This element is present if the hypervisor has started a block -copy operation (via the codevirDomainBlockRebase/code -API), where the mirror location in the codesource/code -sub-element will eventually have the same contents as the -source, and with the file format in the +This element is present if the hypervisor has started a +long-running block job operation, where the mirror location in +the codesource/code sub-element will eventually have the +same contents as the source, and with the file format in the sub-element codeformat/code (which might differ from the -format of the source). The details of the codesource/code -sub-element are determined by the codetype/code attribute -of the mirror, similar to what is done for the -overall codedisk/code device element. If +format of the source). The codejob/code attribute +mentions which API started the operation (copy for +the codevirDomainBlockRebase/code API, or active-commit +for the codevirDomainBlockCommit/code API). The details +of the codesource/code sub-element are determined by +the codetype/code attribute of the mirror, similar to what +is done for the overall codedisk/code device element. If attribute codeready/code is present, then it is known the disk is ready to pivot; otherwise, the disk is probably still copying. For now, this element only valid in output; it is diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 33d0308..fe943f9 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -4186,6 +4186,13 @@ /attribute /optional optional +attribute name='job' + choice +valuecopy/value + /choice +/attribute + /optional + optional interleave ref name='diskSourceFile'/ optional @@ -4195,6 +4202,12 @@ /optional /group group !-- preferred format -- + attribute name='job' +choice + valuecopy/value + valueactive-commit/value +/choice + /attribute interleave ref name=diskSource/ optional diff --git
[libvirt] [PATCH v4 3/5] blockjob: expose active commit capability
Add an element to QEMU's capability XML, to show if the underlying qemu binary supports active commit. This allows the client to know ahead of time if they can rely on this method, or must fall back to older techniques such as blockpull. Without this information, the only way to check for active commit is to attempt one and check for errors. This attribute can be a simple binary (active commit is supported only if activecommit/ is in the capabilities) rather than a full-blown toggle definition. (In contrast, the disksnapshot capability had to be a toggle because we forgot to add it at the same time as turning on the feature of external snapshots; and therefore, the absence of the attribute is not sufficient to conclude whether disk snapshots are supported.) Our documentation for features was rather sparse; this fleshes out more of the details for other existing capabilities (and cost me some time trawling git history). * docs/schemas/capability.rng (features): Add activecommit. * docs/formatcaps.html.in: Document it, and other features. * src/qemu/qemu_capabilities.c (virQEMUCapsInitGuestFromBinary): Set it. * src/conf/capabilities.c (virCapabilitiesFormatXML): Expose as simpler binary. Signed-off-by: Eric Blake ebl...@redhat.com --- docs/formatcaps.html.in | 46 +++- docs/schemas/capability.rng | 5 + src/conf/capabilities.c | 3 ++- src/qemu/qemu_capabilities.c | 6 ++ 4 files changed, 58 insertions(+), 2 deletions(-) diff --git a/docs/formatcaps.html.in b/docs/formatcaps.html.in index 137af25..6ac0f4a 100644 --- a/docs/formatcaps.html.in +++ b/docs/formatcaps.html.in @@ -95,7 +95,51 @@ dtcodefeatures/code/dt ddThis optional element encases possible features that can be used -with a guest of described type./dd + with a guest of described type. Possible subelements are: + dl +dtpae/dtddIf present, 32-bit guests can use PAE + address space extensions, span class=sincesince + 0.4.1/span/dd +dtnonpae/dtddIf present, 32-bit guests can be run + without requiring PAE, span class=sincesince + 0.4.1/span/dd +dtia64_be/dtddIf present, IA64 guests can be run in + big-endian mode, span class=sincesince 0.4.1/span/dd +dtacpi/dtddIf this element is present, + the codedefault/code attribute describes whether the + hypervisor exposes ACPI to the guest by default, and + the codetoggle/code attribute describes whether the + user can override this + default. span class=sinceSince 0.4.1/span/dd +dtapic/dtddIf this element is present, + the codedefault/code attribute describes whether the + hypervisor exposes APIC to the guest by default, and + the codetoggle/code attribute describes whether the + user can override this + default. span class=sinceSince 0.4.1/span/dd +dtcpuselection/dtddIf this element is present, the + hypervisor supports the codelt;cpugt;/code element + within a domain definition for fine-grained control over + the CPU presented to the + guest. span class=sinceSince 0.7.5/span/dd +dtdeviceboot/dtddIf this element is present, + the codelt;boot order='...'/gt;/code element can + be used inside devices, rather than the older boot + specification by category. span class=sinceSince + 0.8.8/span/dd +dtdisksnapshot/dtddIf this element is present, + the codedefault/code attribute describes whether + external disk snapshots are supported. If absent, + external snapshots may still be supported, but it + requires attempting the API and checking for an error to + find out for sure. span class=sinceSince + 1.2.3/span/dd +dtactivecommit/dtddActive commit (via the + virDomainBlockCommit API) is supported only if this + element is present. span class=sinceSince + 1.2.6/span/dd + /dl +/dd /dl h3a name=elementExamplesExamples/a/h3 diff --git a/docs/schemas/capability.rng b/docs/schemas/capability.rng index f954599..de32ee9 100644 --- a/docs/schemas/capability.rng +++ b/docs/schemas/capability.rng @@ -399,6 +399,11 @@ empty/ /element /optional +optional + element name='activecommit' +empty/ + /element +/optional /interleave /element /define diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c index 19359a5..ebf6121 100644 --- a/src/conf/capabilities.c +++ b/src/conf/capabilities.c @@ -1005,7 +1005,8 @@ virCapabilitiesFormatXML(virCapsPtr caps)
[libvirt] [PATCH v4 0/5] Another version of active commit
Rebased to latest libvirt.git, and still waiting on Jeff's qemu patches to go into qemu.git before I can push any of these, https://lists.gnu.org/archive/html/qemu-devel/2014-06/msg04060.html but reposting because of a feature Adam had been requesting: Patch 3/5 is new, and adds a new activecommit/ marker to the 'virsh capabilities' output if qemu is detected as supporting active commit. I've resolved some of Peter's review comments in patches 1 and 2 (patch 1 underwent a bit of a rewrite to differentiate the qemu capability probe from normal use of the interface, and 2 dropped mention of RHEL 6.3 except in the commit message). At this point, I haven't altered 4 or 5, although I still need to do something about getting a persistent domain definition updated after a pivot changes the file in use by a running domain. I've forced an update to my git repo: http://repo.or.cz/w/libvirt/ericb.git/shortlog/refs/heads/gluster or git checkout git://repo.or.cz/libvirt/ericb.git gluster Eric Blake (5): blockjob: allow omitted arguments to QMP block-commit blockjob: turn on qemu capability bit for active commit blockjob: expose active commit capability blockcommit: track job type in xml blockcommit: turn on active commit docs/formatcaps.html.in| 46 +++- docs/formatdomain.html.in | 20 +++ docs/schemas/capability.rng| 5 ++ docs/schemas/domaincommon.rng | 13 + src/conf/capabilities.c| 3 +- src/conf/domain_conf.c | 33 +++- src/conf/domain_conf.h | 1 + src/qemu/qemu_capabilities.c | 13 + src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_driver.c | 53 ++ src/qemu/qemu_monitor.c| 12 + src/qemu/qemu_monitor.h| 1 + src/qemu/qemu_monitor_json.c | 29 -- src/qemu/qemu_monitor_json.h | 3 +- src/qemu/qemu_process.c| 18 --- tests/qemucapabilitiesdata/caps_1.3.1-1.replies| 62 -- tests/qemucapabilitiesdata/caps_1.4.2-1.replies| 62 -- tests/qemucapabilitiesdata/caps_1.5.3-1.replies| 62 -- tests/qemucapabilitiesdata/caps_1.6.0-1.replies| 62 -- tests/qemucapabilitiesdata/caps_1.6.50-1.replies | 62 -- tests/qemumonitorjsontest.c| 47 .../qemuxml2argv-disk-active-commit.xml| 37 + .../qemuxml2argvdata/qemuxml2argv-disk-mirror.xml | 4 +- .../qemuxml2xmlout-disk-mirror-old.xml | 4 +- tests/qemuxml2xmltest.c| 1 + 25 files changed, 481 insertions(+), 173 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-active-commit.xml -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v4 5/5] blockcommit: turn on active commit
With this in place, I can (finally!) now do: virsh blockcommit $dom vda --shallow --verbose --pivot and watch qemu shorten the backing chain by one, followed by libvirt automatically updating the dumpxml output, effectively undoing the work of virsh snapshot-commit --no-metadata --disk-only. Commit is S much faster than blockpull, when I'm still fairly close in time to when the temporary qcow2 wrapper file was created via a snapshot operation! Still not done: the persistent XML is not updated; which means stopping and restarting a persistent guest will use the wrong file and likely fail to boot. * src/qemu/qemu_driver.c (qemuDomainBlockCommit): Implement live commit to regular files. Signed-off-by: Eric Blake ebl...@redhat.com --- src/qemu/qemu_driver.c | 40 +++- 1 file changed, 35 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 79554e9..ae81712 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -15493,9 +15493,12 @@ qemuDomainBlockCommit(virDomainPtr dom, unsigned int baseIndex = 0; const char *top_parent = NULL; bool clean_access = false; +virStorageSourcePtr mirror = NULL; + -/* XXX Add support for COMMIT_ACTIVE, COMMIT_DELETE */ -virCheckFlags(VIR_DOMAIN_BLOCK_COMMIT_SHALLOW, -1); +/* XXX Add support for COMMIT_DELETE */ +virCheckFlags(VIR_DOMAIN_BLOCK_COMMIT_SHALLOW | + VIR_DOMAIN_BLOCK_COMMIT_ACTIVE, -1); if (!(vm = qemuDomObjFromDomain(dom))) goto cleanup; @@ -15543,9 +15546,6 @@ qemuDomainBlockCommit(virDomainPtr dom, top_parent))) goto endjob; -/* FIXME: qemu 2.0 supports active commit, but as a two-stage - * process; qemu 2.1 is further improving active commit. We need - * to start supporting it in libvirt. */ if (topSource == disk-src) { if (!virQEMUCapsGet(priv-qemuCaps, QEMU_CAPS_ACTIVE_COMMIT)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s, @@ -15559,6 +15559,14 @@ qemuDomainBlockCommit(virDomainPtr dom, disk-dst); goto endjob; } +if (disk-mirror) { +virReportError(VIR_ERR_BLOCK_COPY_ACTIVE, + _(disk '%s' already in active block job), + disk-dst); +goto endjob; +} +if (VIR_ALLOC(mirror) 0) +goto endjob; } else if (flags VIR_DOMAIN_BLOCK_COMMIT_ACTIVE) { virReportError(VIR_ERR_INVALID_ARG, _(active commit requested but '%s' is not active), @@ -15605,6 +15613,21 @@ qemuDomainBlockCommit(virDomainPtr dom, VIR_DISK_CHAIN_READ_WRITE) 0)) goto endjob; +/* For an active commit, clone enough of the base to act as the mirror */ +if (mirror) { +/* XXX Support network commits */ +if (baseSource-type != VIR_STORAGE_TYPE_FILE +baseSource-type != VIR_STORAGE_TYPE_BLOCK) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s, + _(active commit to non-file not supported yet)); +goto endjob; +} +mirror-type = baseSource-type; +if (VIR_STRDUP(mirror-path, baseSource-path) 0) +goto endjob; +mirror-format = baseSource-format; +} + /* Start the commit operation. Pass the user's original spelling, * if any, through to qemu, since qemu may behave differently * depending on whether the input was specified as relative or @@ -15617,6 +15640,12 @@ qemuDomainBlockCommit(virDomainPtr dom, bandwidth); qemuDomainObjExitMonitor(driver, vm); +if (ret == 0 mirror) { +disk-mirror = mirror; +mirror = NULL; +disk-mirrorJob = VIR_DOMAIN_BLOCK_JOB_TYPE_ACTIVE_COMMIT; +} + endjob: if (ret 0 clean_access) { /* Revert access to read-only, if possible. */ @@ -15627,6 +15656,7 @@ qemuDomainBlockCommit(virDomainPtr dom, top_parent, VIR_DISK_CHAIN_READ_ONLY); } +virStorageSourceFree(mirror); if (!qemuDomainObjEndJob(driver, vm)) vm = NULL; -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v4 1/5] blockjob: allow omitted arguments to QMP block-commit
We are about to turn on support for active block commit. Although qemu 2.0 was the first version to mostly support it, that version mis-handles 0-length files, and doesn't have anything available for easy probing. But qemu 2.1 fixed bugs, and made life simpler by letting the 'top' argument be optional. Unless someone begs for active commit with qemu 2.0, for now we are just going to enable it only by probing for qemu 2.1 behavior (anyone backporting active commit can also backport the optional argument behavior). Although all our actual uses of block-commit supply arguments for both base and top, we can omit both arguments and use a bogus device string to trigger an interesting behavior in qemu. All QMP commands first do argument validation, failing with GenericError if a mandatory argument is missing. Once that passes, the code in the specific command gets to do further checking, and the qemu developers made sure that if device is the only supplied argument, then the block-commit code will look up the device first, with a failure of DeviceNotFound, before attempting any further argument validation (most other validations fail with GenericError). Thus, the category of error class can reliably be used to decipher whether the top argument was optional, which in turn implies a working active commit. Since we expect our bogus device string to trigger an error either way, the code is written to return a distinct return value without spamming the logs. * src/qemu/qemu_monitor.h (qemuMonitorSupportsActiveCommit): New prototype. * src/qemu/qemu_monitor.c (qemuMonitorSupportsActiveCommit): Implement it. * src/qemu/qemu_monitor_json.h (qemuMonitorJSONBlockCommit): Allow NULL for top and base, for probing purposes. * src/qemu/qemu_monitor_json.c (qemuMonitorJSONBlockCommit): Likewise, implementing the probe. * tests/qemumonitorjsontest.c (mymain): Enable... (testQemuMonitorJSONqemuMonitorSupportsActiveCommit): ...a new test. Signed-off-by: Eric Blake ebl...@redhat.com --- src/qemu/qemu_monitor.c | 12 +++ src/qemu/qemu_monitor.h | 1 + src/qemu/qemu_monitor_json.c | 29 --- src/qemu/qemu_monitor_json.h | 3 +-- tests/qemumonitorjsontest.c | 47 5 files changed, 87 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 80d7b9d..2d584fc 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -3261,6 +3261,18 @@ qemuMonitorBlockCommit(qemuMonitorPtr mon, const char *device, return ret; } + +/* Probe whether active commits are supported by a given qemu binary. */ +bool +qemuMonitorSupportsActiveCommit(qemuMonitorPtr mon) +{ +if (!mon-json) +return false; + +return qemuMonitorJSONBlockCommit(mon, bogus, NULL, NULL, 0) == -2; +} + + /* Use the block-job-complete monitor command to pivot a block copy * job. */ int diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 31b3204..63e78d8 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -665,6 +665,7 @@ int qemuMonitorBlockCommit(qemuMonitorPtr mon, unsigned long bandwidth) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4); +bool qemuMonitorSupportsActiveCommit(qemuMonitorPtr mon); int qemuMonitorArbitraryCommand(qemuMonitorPtr mon, const char *cmd, diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index bedd959..75b33e8 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -3454,7 +3454,14 @@ qemuMonitorJSONTransaction(qemuMonitorPtr mon, virJSONValuePtr actions) return ret; } -/* speed is in bytes/sec */ +/* speed is in bytes/sec. Returns 0 on success, -1 with error message + * emitted on failure. + * + * Additionally, can be used to probe if active commit is supported: + * pass in a bogus device and NULL top and base. The probe return is + * -2 if active commit is detected, -3 if absent; with no error + * message issued. + */ int qemuMonitorJSONBlockCommit(qemuMonitorPtr mon, const char *device, const char *top, const char *base, @@ -3467,14 +3474,30 @@ qemuMonitorJSONBlockCommit(qemuMonitorPtr mon, const char *device, cmd = qemuMonitorJSONMakeCommand(block-commit, s:device, device, U:speed, speed, - s:top, top, - s:base, base, + S:top, top, + S:base, base, NULL); if (!cmd) return -1; if ((ret = qemuMonitorJSONCommand(mon, cmd, reply)) 0) goto cleanup; +if (!top !base) { +/* Normally we always specify top and base; but omitting them + * allows for
[libvirt] [PATCH v4 2/5] blockjob: turn on qemu capability bit for active commit
Use the probing functionality added in the last patch to turn on a capability bit when active commit is present, and gate active commit on that capability. For my own reference: the difference between BLOCKJOB_SYNC and BLOCKJOB_ASYNC is whether qemu generated an event at the conclusion of blockpull; basically, RHEL 6.2 was the only release of qemu that has the sync semantics and lacks the event. RHEL 6.3 added blockcopy, but also picked up on the upstream style of qemu generating events. As no one is likely to backport active commit to RHEL 6.2, it's safe for blockcommit to always require async blockjob support. Modifying qemucapabilitiestest is painful; the .replies files would be so much easier if they had comments correlating which command generated the given reply. Maybe I'll fix that up later... * src/qemu/qemu_capabilities.h (QEMU_CAPS_ACTIVE_COMMIT): New capability. * src/qemu/qemu_driver.c (qemuDomainBlockCommit): Use the new bit * src/qemu/qemu_capabilities.c (virQEMUCaps): Name the new bit. (virQEMUCapsProbeQMPCommands): Set it. * tests/qemucapabilitiesdata/caps_1.3.1-1.replies: Update. * tests/qemucapabilitiesdata/caps_1.4.2-1.replies: Likewise. * tests/qemucapabilitiesdata/caps_1.5.3-1.replies: Likewise. * tests/qemucapabilitiesdata/caps_1.6.0-1.replies: Likewise. * tests/qemucapabilitiesdata/caps_1.6.50-1.replies: Likewise. Signed-off-by: Eric Blake ebl...@redhat.com --- src/qemu/qemu_capabilities.c | 7 +++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_driver.c | 10 ++-- tests/qemucapabilitiesdata/caps_1.3.1-1.replies | 62 +--- tests/qemucapabilitiesdata/caps_1.4.2-1.replies | 62 +--- tests/qemucapabilitiesdata/caps_1.5.3-1.replies | 62 +--- tests/qemucapabilitiesdata/caps_1.6.0-1.replies | 62 +--- tests/qemucapabilitiesdata/caps_1.6.50-1.replies | 62 +--- 8 files changed, 188 insertions(+), 140 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 245d6b5..d698db9 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -256,6 +256,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, usb-kbd, /* 165 */ host-pci-multidomain, msg-timestamp, + active-commit, ); @@ -2176,6 +2177,12 @@ virQEMUCapsProbeQMPCommands(virQEMUCapsPtr qemuCaps, VIR_FORCE_CLOSE(fd); } +/* Probe for active commit of qemu 2.1 (for now, we are choosing + * to ignore the fact that qemu 2.0 can also do active commit) */ +if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_BLOCK_COMMIT) +qemuMonitorSupportsActiveCommit(mon)) +virQEMUCapsSet(qemuCaps, QEMU_CAPS_ACTIVE_COMMIT); + return 0; } diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index d755caa..a18d298 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -206,6 +206,7 @@ enum virQEMUCapsFlags { QEMU_CAPS_DEVICE_USB_KBD = 165, /* -device usb-kbd */ QEMU_CAPS_HOST_PCI_MULTIDOMAIN = 166, /* support domain 0 in host pci address */ QEMU_CAPS_MSG_TIMESTAMP = 167, /* -msg timestamp */ +QEMU_CAPS_ACTIVE_COMMIT = 168, /* block-commit works without 'top' */ QEMU_CAPS_LAST, /* this must always be the last item */ }; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 22a8ca5..b57f77b 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -15509,7 +15509,10 @@ qemuDomainBlockCommit(virDomainPtr dom, %s, _(domain is not running)); goto endjob; } -if (!virQEMUCapsGet(priv-qemuCaps, QEMU_CAPS_BLOCK_COMMIT)) { +/* Ensure that no one backports commit to RHEL 6.2, where cancel + * behaved differently */ +if (!(virQEMUCapsGet(priv-qemuCaps, QEMU_CAPS_BLOCK_COMMIT) + virQEMUCapsGet(priv-qemuCaps, QEMU_CAPS_BLOCKJOB_ASYNC))) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s, _(online commit not supported with this QEMU binary)); goto endjob; @@ -15541,10 +15544,7 @@ qemuDomainBlockCommit(virDomainPtr dom, * process; qemu 2.1 is further improving active commit. We need * to start supporting it in libvirt. */ if (topSource == disk-src) { -/* We assume that no one will backport qemu 2.0 active commit - * to an earlier qemu without also backporting the block job - * ready event; but this makes sure of that fact */ -if (!virQEMUCapsGet(priv-qemuCaps, QEMU_CAPS_BLOCKJOB_ASYNC)) { +if (!virQEMUCapsGet(priv-qemuCaps, QEMU_CAPS_ACTIVE_COMMIT)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s, _(active commit not supported with this QEMU binary)); goto endjob; diff --git
Re: [libvirt] [PATCHv5 03/19] storage: gluster: Add backend to return unique storage file path
On 06/19/2014 07:59 AM, Peter Krempa wrote: Use virStorageFileSimplifyPathInternal to canonicalize gluster paths via a callback and use it for the unique volume path retrieval API. --- src/storage/storage_backend_gluster.c | 80 +++ 1 file changed, 80 insertions(+) + + realloc: +if (VIR_EXPAND_N(buf, bufsiz, 256) 0) Expanding by the same length each iteration is quadratic in behavior, compared to expanding by a geometrically larger value (256 on iteration 1, 512 on iteration 2, ...). BUT, that is true only if you return to the label more than once. However, gluster has some (current) hard-baked limits of 256 as the maximum length, so you will only be repeating the label at most once, so it really doesn't matter in this patch :) ACK -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list