[libvirt] [PATCH] Properly check the return value of CCWAddressAsString

2014-06-23 Thread Ján Tomko
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

2014-06-23 Thread Peter Krempa
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

2014-06-23 Thread Peter Krempa
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

2014-06-23 Thread chenhanx...@cn.fujitsu.com
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

2014-06-23 Thread Ján Tomko
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

2014-06-23 Thread Ján Tomko
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

2014-06-23 Thread Ján Tomko
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

2014-06-23 Thread Chen Hanxiao
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

2014-06-23 Thread Peter Krempa
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

2014-06-23 Thread Peter Krempa
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

2014-06-23 Thread Peter Krempa
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

2014-06-23 Thread Peter Krempa
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

2014-06-23 Thread Peter Krempa
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

2014-06-23 Thread Daniel P. Berrange
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

2014-06-23 Thread Pavel Hrdina
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

2014-06-23 Thread Ján Tomko
 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

2014-06-23 Thread Peter Krempa
'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

2014-06-23 Thread Christophe Fergeau
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

2014-06-23 Thread Jiri Denemark
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

2014-06-23 Thread Nehal J Wani
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

2014-06-23 Thread Peter Krempa
'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

2014-06-23 Thread Jiri Denemark
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

2014-06-23 Thread Peter Krempa
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

2014-06-23 Thread Michal Privoznik

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

2014-06-23 Thread Michal Privoznik

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

2014-06-23 Thread Michal Privoznik

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

2014-06-23 Thread Michal Privoznik

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

2014-06-23 Thread Jenkins CI
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

2014-06-23 Thread Ján Tomko
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

2014-06-23 Thread Peter Krempa
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

2014-06-23 Thread Peter Krempa
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

2014-06-23 Thread Michal Privoznik

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

2014-06-23 Thread Peter Krempa
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

2014-06-23 Thread Jenkins CI
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

2014-06-23 Thread Kashyap Chamarthy
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

2014-06-23 Thread Ján Tomko
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

2014-06-23 Thread Michal Privoznik
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

2014-06-23 Thread Michal Privoznik
*** 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

2014-06-23 Thread Michal Privoznik
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

2014-06-23 Thread Michal Privoznik
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

2014-06-23 Thread Peter Krempa
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

2014-06-23 Thread Daniel P. Berrange
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

2014-06-23 Thread Martin Kletzander
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

2014-06-23 Thread Michal Privoznik

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

2014-06-23 Thread Daniel P. Berrange
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

2014-06-23 Thread Daniel P. Berrange
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

2014-06-23 Thread Daniel P. Berrange
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

2014-06-23 Thread Eric Blake
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

2014-06-23 Thread Eric Blake
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

2014-06-23 Thread Daniel P. Berrange
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

2014-06-23 Thread Timm Bäder
... 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

2014-06-23 Thread Timm Bäder
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

2014-06-23 Thread Timm Bäder
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

2014-06-23 Thread Eric Blake
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

2014-06-23 Thread Eric Blake
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

2014-06-23 Thread Nehal J Wani
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

2014-06-23 Thread Roman Bogorodskiy
  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

2014-06-23 Thread Daniel P. Berrange
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

2014-06-23 Thread Daniel P. Berrange
  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

2014-06-23 Thread Daniel P. Berrange
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

2014-06-23 Thread Daniel P. Berrange
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

2014-06-23 Thread Hitoshi Mitake
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

2014-06-23 Thread Eric Blake
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

2014-06-23 Thread Nehal J Wani
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

2014-06-23 Thread Daniel P. Berrange
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

2014-06-23 Thread Kashyap Chamarthy
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

2014-06-23 Thread Giuseppe Scrivano
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

2014-06-23 Thread Giuseppe Scrivano
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

2014-06-23 Thread Giuseppe Scrivano
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

2014-06-23 Thread Michal Privoznik
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

2014-06-23 Thread Eric Blake
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

2014-06-23 Thread Nehal J Wani
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

2014-06-23 Thread Nehal J Wani
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

2014-06-23 Thread Nehal J Wani
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

2014-06-23 Thread Nehal J Wani
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

2014-06-23 Thread Nehal J Wani
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'

2014-06-23 Thread Jim Fehlig
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

2014-06-23 Thread Eric Blake
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

2014-06-23 Thread Eric Blake
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

2014-06-23 Thread Eric Blake
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

2014-06-23 Thread Eric Blake
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

2014-06-23 Thread Eric Blake
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

2014-06-23 Thread Eric Blake
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

2014-06-23 Thread Eric Blake
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