[libvirt] [PATCH 3/3] Fix build in qemu_command

2014-09-18 Thread Roman Bogorodskiy
Currently, build with clang fails with:

  CC   qemu/libvirt_driver_qemu_impl_la-qemu_command.lo
qemu/qemu_command.c:6580:58: error: implicit conversion from enumeration type
'virMemAccess' to different enumeration type 'virTristateSwitch'
[-Werror,-Wenum-conversion]
virTristateSwitch memAccess = def-cpu-cells[i].memAccess;
  ~   ~~~^
1 error generated.

Fix that by using virMemAccess instead of virTristateSwitch.
---
 src/qemu/qemu_command.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index f2e6e5a..ce320de 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -6577,7 +6577,7 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg,
 for (i = 0; i  def-cpu-ncells; i++) {
 int cellmem = VIR_DIV_UP(def-cpu-cells[i].mem, 1024);
 def-cpu-cells[i].mem = cellmem * 1024;
-virTristateSwitch memAccess = def-cpu-cells[i].memAccess;
+virMemAccess memAccess = def-cpu-cells[i].memAccess;
 
 VIR_FREE(cpumask);
 VIR_FREE(nodemask);
-- 
2.1.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH 1/3] bhyve: tests: fix build

2014-09-18 Thread Roman Bogorodskiy
Commit b20d39a introduced a new argument for the
virNetDevTapCreateInBridgePort function, however, its mock
in bhyve tests wasn't updated, so the build failed.

Fix build by adding this new argument to the mock version.
---
 tests/bhyvexml2argvmock.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/bhyvexml2argvmock.c b/tests/bhyvexml2argvmock.c
index fa2f14b..0cbea29 100644
--- a/tests/bhyvexml2argvmock.c
+++ b/tests/bhyvexml2argvmock.c
@@ -22,6 +22,7 @@ int virNetDevTapCreateInBridgePort(const char *brname 
ATTRIBUTE_UNUSED,
char **ifname,
const virMacAddr *macaddr ATTRIBUTE_UNUSED,
const unsigned char *vmuuid 
ATTRIBUTE_UNUSED,
+   const char *tunpath ATTRIBUTE_UNUSED,
int *tapfd ATTRIBUTE_UNUSED,
int tapfdSize ATTRIBUTE_UNUSED,
virNetDevVPortProfilePtr virtPortProfile 
ATTRIBUTE_UNUSED,
-- 
2.1.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH 2/3] Fix build in qemu_capabilities

2014-09-18 Thread Roman Bogorodskiy
Commit f05b6a91 added virQEMUDriverConfigPtr argument to the
virQEMUCapsFillDomainCaps function and it uses forward declaration
of virQEMUDriverConfig and virQEMUDriverConfigPtr that casues clang
build to fail:

gmake[3]: Entering directory `/usr/home/novel/code/libvirt/src'
  CC   qemu/libvirt_driver_qemu_impl_la-qemu_capabilities.lo
In file included from qemu/qemu_capabilities.c:43:
In file included from qemu/qemu_hostdev.h:27:
qemu/qemu_conf.h:63:37: error: redefinition of typedef 'virQEMUDriverConfig'
is a C11 feature [-Werror,-Wtypedef-redefinition]
typedef struct _virQEMUDriverConfig virQEMUDriverConfig;
^
qemu/qemu_capabilities.h:328:37: note: previous definition is here
typedef struct _virQEMUDriverConfig virQEMUDriverConfig;
^

Fix that by passing loader and nloader config attributes directly
instead of passing complete config.
---
 src/qemu/qemu_capabilities.c | 37 +
 src/qemu/qemu_capabilities.h |  7 ++-
 src/qemu/qemu_driver.c   |  3 ++-
 tests/domaincapstest.c   |  3 ++-
 4 files changed, 27 insertions(+), 23 deletions(-)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 9246813..d0b19c8 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -3610,43 +3610,44 @@ virQEMUCapsGetDefaultMachine(virQEMUCapsPtr qemuCaps)
 
 static int
 virQEMUCapsFillDomainLoaderCaps(virQEMUCapsPtr qemuCaps,
-virDomainCapsLoaderPtr loader,
+virDomainCapsLoaderPtr capsLoader,
 virArch arch,
-virQEMUDriverConfigPtr cfg)
+char **loader,
+size_t nloader)
 {
 size_t i;
 
-loader-device.supported = true;
+capsLoader-device.supported = true;
 
-if (VIR_ALLOC_N(loader-values.values, cfg-nloader)  0)
+if (VIR_ALLOC_N(capsLoader-values.values, nloader)  0)
 return -1;
 
-for (i = 0; i  cfg-nloader; i++) {
-const char *filename = cfg-loader[i];
+for (i = 0; i  nloader; i++) {
+const char *filename = loader[i];
 
 if (!virFileExists(filename)) {
 VIR_DEBUG(loader filename=%s does not exist, filename);
 continue;
 }
 
-if (VIR_STRDUP(loader-values.values[loader-values.nvalues],
+if (VIR_STRDUP(capsLoader-values.values[capsLoader-values.nvalues],
filename)  0)
 return -1;
-loader-values.nvalues++;
+capsLoader-values.nvalues++;
 }
 
-VIR_DOMAIN_CAPS_ENUM_SET(loader-type,
+VIR_DOMAIN_CAPS_ENUM_SET(capsLoader-type,
  VIR_DOMAIN_LOADER_TYPE_ROM);
 
 if (arch == VIR_ARCH_X86_64 
 virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE) 
 virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE_FORMAT))
-VIR_DOMAIN_CAPS_ENUM_SET(loader-type,
+VIR_DOMAIN_CAPS_ENUM_SET(capsLoader-type,
  VIR_DOMAIN_LOADER_TYPE_PFLASH);
 
 
 if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE_READONLY))
-VIR_DOMAIN_CAPS_ENUM_SET(loader-readonly,
+VIR_DOMAIN_CAPS_ENUM_SET(capsLoader-readonly,
  VIR_TRISTATE_BOOL_YES,
  VIR_TRISTATE_BOOL_NO);
 return 0;
@@ -3657,12 +3658,14 @@ static int
 virQEMUCapsFillDomainOSCaps(virQEMUCapsPtr qemuCaps,
 virDomainCapsOSPtr os,
 virArch arch,
-virQEMUDriverConfigPtr cfg)
+char **loader,
+size_t nloader)
 {
-virDomainCapsLoaderPtr loader = os-loader;
+virDomainCapsLoaderPtr capsLoader = os-loader;
 
 os-device.supported = true;
-if (virQEMUCapsFillDomainLoaderCaps(qemuCaps, loader, arch, cfg)  0)
+if (virQEMUCapsFillDomainLoaderCaps(qemuCaps, capsLoader, arch,
+loader, nloader)  0)
 return -1;
 return 0;
 }
@@ -3747,7 +3750,8 @@ virQEMUCapsFillDomainDeviceHostdevCaps(virQEMUCapsPtr 
qemuCaps,
 int
 virQEMUCapsFillDomainCaps(virDomainCapsPtr domCaps,
   virQEMUCapsPtr qemuCaps,
-  virQEMUDriverConfigPtr cfg)
+  char **loader,
+  size_t nloader)
 {
 virDomainCapsOSPtr os = domCaps-os;
 virDomainCapsDeviceDiskPtr disk = domCaps-disk;
@@ -3756,7 +3760,8 @@ virQEMUCapsFillDomainCaps(virDomainCapsPtr domCaps,
 
 domCaps-maxvcpus = maxvcpus;
 
-if (virQEMUCapsFillDomainOSCaps(qemuCaps, os, domCaps-arch, cfg)  0 ||
+if (virQEMUCapsFillDomainOSCaps(qemuCaps, os, domCaps-arch,
+loader, nloader)  0 ||
 virQEMUCapsFillDomainDeviceDiskCaps(qemuCaps, disk)  0 ||
 

Re: [libvirt] [PATCH] qemu: fix crash with shared disks

2014-09-18 Thread Ján Tomko
On 09/17/2014 11:05 PM, John Ferlan wrote:
 On 09/17/2014 06:45 AM, Ján Tomko wrote:
 Commit f36a94f introduced a double free on all success paths
 in qemuSharedDeviceEntryInsert.

 Only call qemuSharedDeviceEntryFree on the error path and
 set entry to NULL before jumping there if the entry already
 is in the hash table.

 https://bugzilla.redhat.com/show_bug.cgi?id=1142722
 ---
  src/qemu/qemu_conf.c | 26 --
  1 file changed, 12 insertions(+), 14 deletions(-)


...

 +entry = NULL;
 
 [1] Assigning to NULL causes an issue
 
 +goto error;
 +}
  }

...

 +return 0;
  
 - cleanup:
 + error:
  qemuSharedDeviceEntryFree(entry, NULL);
 [1]
 Because this is prototyped as:
 
 void qemuSharedDeviceEntryFree(void *payload, const void *name)
 ATTRIBUTE_NONNULL(1);
 
 Coverity gives us a warning when entry = NULL...
 
 It's solveable by either allowing NULL for the function or only calling
 if (entry)
 
 ACK as long as we handle in some manner.

I removed the ATTRIBUTE_NONNULL as the function already handles NULL and
pushed the patch.

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 2/4] virnuma: Introduce virNumaSetPagePoolSize

2014-09-18 Thread Michal Privoznik
This internal API can be used to allocate or free some pages in
the huge pages pool.

Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
 src/libvirt_private.syms |   1 +
 src/util/virnuma.c   | 108 +++
 src/util/virnuma.h   |   4 ++
 3 files changed, 113 insertions(+)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 51a692b..aabc11f 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1699,6 +1699,7 @@ virNumaGetPageInfo;
 virNumaGetPages;
 virNumaIsAvailable;
 virNumaNodeIsAvailable;
+virNumaSetPagePoolSize;
 virNumaSetupMemoryPolicy;
 
 
diff --git a/src/util/virnuma.c b/src/util/virnuma.c
index 1a34398..690615f 100644
--- a/src/util/virnuma.c
+++ b/src/util/virnuma.c
@@ -841,6 +841,102 @@ virNumaGetPages(int node,
 }
 
 
+int
+virNumaSetPagePoolSize(int node,
+   unsigned int page_size,
+   unsigned long long page_count,
+   bool add)
+{
+int ret = -1;
+char *nr_path = NULL, *nr_buf =  NULL;
+char *end;
+unsigned long long nr_count;
+
+if (page_size == sysconf(_SC_PAGESIZE) / 1024) {
+/* Special case as kernel handles system pages
+ * differently to huge pages. */
+virReportError(VIR_ERR_OPERATION_UNSUPPORTED, %s,
+   _(system pages pool can't be modified));
+goto cleanup;
+}
+
+if (virNumaGetHugePageInfoPath(nr_path, node, page_size, nr_hugepages) 
 0)
+goto cleanup;
+
+/* Firstly check, if there's anything for us to do */
+if (virFileReadAll(nr_path, 1024, nr_buf)  0)
+goto cleanup;
+
+if (virStrToLong_ull(nr_buf, end, 10, nr_count)  0 ||
+*end != '\n') {
+virReportError(VIR_ERR_OPERATION_FAILED,
+   _(invalid number '%s' in '%s'),
+   nr_buf, nr_path);
+goto cleanup;
+}
+
+if (add) {
+if (!page_count) {
+VIR_DEBUG(Nothing left to do: add = true page_count = 0);
+ret = 0;
+goto cleanup;
+}
+page_count += nr_count;
+} else {
+if (nr_count == page_count) {
+VIR_DEBUG(Nothing left to do: nr_count = page_count = %llu,
+  page_count);
+ret = 0;
+goto cleanup;
+}
+}
+
+/* Okay, page pool adjustment must be done in two steps. In
+ * first we write the desired number into nr_hugepages file.
+ * Kernel then starts to allocate the pages (return from
+ * write should be postponed until the kernel is finished).
+ * However, kernel may have not been successful and reserved
+ * all the pages we wanted. So do the second read to check.
+ */
+VIR_FREE(nr_buf);
+if (virAsprintf(nr_buf, %llu, page_count)  0)
+goto cleanup;
+
+if (virFileWriteStr(nr_path, nr_buf, 0)  0) {
+virReportSystemError(errno,
+ _(Unable to write to: %s), nr_path);
+goto cleanup;
+}
+
+/* And now do the check. */
+
+VIR_FREE(nr_buf);
+if (virFileReadAll(nr_path, 1024, nr_buf)  0)
+goto cleanup;
+
+if (virStrToLong_ull(nr_buf, end, 10, nr_count)  0 ||
+*end != '\n') {
+virReportError(VIR_ERR_OPERATION_FAILED,
+   _(invalid number '%s' in '%s'),
+   nr_buf, nr_path);
+goto cleanup;
+}
+
+if (nr_count != page_count) {
+virReportError(VIR_ERR_OPERATION_FAILED,
+   _(Unable to allocate %llu pages. Allocated only %llu),
+   page_count, nr_count);
+goto cleanup;
+}
+
+ret = 0;
+ cleanup:
+VIR_FREE(nr_buf);
+VIR_FREE(nr_path);
+return ret;
+}
+
+
 #else /* #ifdef __linux__ */
 int
 virNumaGetPageInfo(int node ATTRIBUTE_UNUSED,
@@ -866,4 +962,16 @@ virNumaGetPages(int node ATTRIBUTE_UNUSED,
_(page info is not supported on this platform));
 return -1;
 }
+
+
+int
+virNumaSetPagePoolSize(int node ATTRIBUTE_UNUSED,
+   unsigned int page_size ATTRIBUTE_UNUSED,
+   unsigned long long page_count ATTRIBUTE_UNUSED,
+   bool add ATTRIBUTE_UNUSED)
+{
+virReportError(VIR_ERR_OPERATION_UNSUPPORTED, %s,
+   _(page pool allocation is not supported on this 
platform));
+return -1;
+}
 #endif /* #ifdef __linux__ */
diff --git a/src/util/virnuma.h b/src/util/virnuma.h
index 13ebec6..04b6b8c 100644
--- a/src/util/virnuma.h
+++ b/src/util/virnuma.h
@@ -59,4 +59,8 @@ int virNumaGetPages(int node,
 unsigned int **pages_free,
 size_t *npages)
 ATTRIBUTE_NONNULL(5);
+int virNumaSetPagePoolSize(int node,
+   unsigned int page_size,
+   unsigned long long page_count,
+   bool add);
 #endif /* __VIR_NUMA_H__ */
-- 
1.8.5.5

--

[libvirt] [PATCH 3/4] nodeinfo: Implement nodeAllocPages

2014-09-18 Thread Michal Privoznik
And add stubs to other drivers like: lxc, qemu, uml and vbox.

Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
 src/libvirt_private.syms |  1 +
 src/lxc/lxc_driver.c | 22 ++
 src/nodeinfo.c   | 29 +
 src/nodeinfo.h   |  7 +++
 src/qemu/qemu_driver.c   | 22 ++
 src/uml/uml_driver.c | 22 ++
 src/vbox/vbox_common.c   | 19 +++
 7 files changed, 122 insertions(+)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index aabc11f..c75da80 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -891,6 +891,7 @@ virLockManagerRelease;
 
 
 # nodeinfo.h
+nodeAllocPages;
 nodeCapsInitNUMA;
 nodeGetCellsFreeMemory;
 nodeGetCPUBitmap;
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index c3cd62c..38763de 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -5685,6 +5685,27 @@ lxcNodeGetFreePages(virConnectPtr conn,
 }
 
 
+static int
+lxcNodeAllocPages(virConnectPtr conn,
+  unsigned int npages,
+  unsigned int *pageSizes,
+  unsigned long long *pageCounts,
+  int startCell,
+  unsigned int cellCount,
+  unsigned int flags)
+{
+bool add = !(flags  VIR_NODE_ALLOC_PAGES_SET);
+
+virCheckFlags(VIR_NODE_ALLOC_PAGES_SET, -1);
+
+if (virNodeAllocPagesEnsureACL(conn)  0)
+return -1;
+
+return nodeAllocPages(npages, pageSizes, pageCounts,
+  startCell, cellCount, add);
+}
+
+
 /* Function Tables */
 static virDriver lxcDriver = {
 .no = VIR_DRV_LXC,
@@ -5776,6 +5797,7 @@ static virDriver lxcDriver = {
 .domainReboot = lxcDomainReboot, /* 1.0.1 */
 .domainLxcOpenNamespace = lxcDomainLxcOpenNamespace, /* 1.0.2 */
 .nodeGetFreePages = lxcNodeGetFreePages, /* 1.2.6 */
+.nodeAllocPages = lxcNodeAllocPages, /* 1.2.8 */
 };
 
 static virStateDriver lxcStateDriver = {
diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index af23b8b..4c92626 100644
--- a/src/nodeinfo.c
+++ b/src/nodeinfo.c
@@ -2053,3 +2053,32 @@ nodeGetFreePages(unsigned int npages,
  cleanup:
 return ret;
 }
+
+int
+nodeAllocPages(unsigned int npages,
+   unsigned int *pageSizes,
+   unsigned long long *pageCounts,
+   int startCell,
+   unsigned int cellCount,
+   bool add)
+{
+int ret = -1;
+int cell;
+size_t i, ncounts = 0;
+
+for (cell = startCell; cell  (int) (startCell + cellCount); cell++) {
+for (i = 0; i  npages; i++) {
+unsigned int page_size = pageSizes[i];
+unsigned long long page_count = pageCounts[i];
+
+if (virNumaSetPagePoolSize(cell, page_size, page_count, add)  0)
+goto cleanup;
+
+ncounts++;
+}
+}
+
+ret = ncounts;
+ cleanup:
+return ret;
+}
diff --git a/src/nodeinfo.h b/src/nodeinfo.h
index 0896c6c..a993c63 100644
--- a/src/nodeinfo.h
+++ b/src/nodeinfo.h
@@ -63,4 +63,11 @@ int nodeGetFreePages(unsigned int npages,
  int startCell,
  unsigned int cellCount,
  unsigned long long *counts);
+
+int nodeAllocPages(unsigned int npages,
+   unsigned int *pageSizes,
+   unsigned long long *pageCounts,
+   int startCell,
+   unsigned int cellCount,
+   bool add);
 #endif /* __VIR_NODEINFO_H__*/
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 5fd89a3..a8f3dca 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -17953,6 +17953,27 @@ qemuConnectGetAllDomainStats(virConnectPtr conn,
 }
 
 
+static int
+qemuNodeAllocPages(virConnectPtr conn,
+   unsigned int npages,
+   unsigned int *pageSizes,
+   unsigned long long *pageCounts,
+   int startCell,
+   unsigned int cellCount,
+   unsigned int flags)
+{
+bool add = !(flags  VIR_NODE_ALLOC_PAGES_SET);
+
+virCheckFlags(VIR_NODE_ALLOC_PAGES_SET, -1);
+
+if (virNodeAllocPagesEnsureACL(conn)  0)
+return -1;
+
+return nodeAllocPages(npages, pageSizes, pageCounts,
+  startCell, cellCount, add);
+}
+
+
 static virDriver qemuDriver = {
 .no = VIR_DRV_QEMU,
 .name = QEMU_DRIVER_NAME,
@@ -18152,6 +18173,7 @@ static virDriver qemuDriver = {
 .nodeGetFreePages = qemuNodeGetFreePages, /* 1.2.6 */
 .connectGetDomainCapabilities = qemuConnectGetDomainCapabilities, /* 1.2.7 
*/
 .connectGetAllDomainStats = qemuConnectGetAllDomainStats, /* 1.2.8 */
+.nodeAllocPages = qemuNodeAllocPages, /* 1.2.8 */
 };
 
 
diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c
index 12b0ba7..c255c07 100644
--- a/src/uml/uml_driver.c
+++ b/src/uml/uml_driver.c
@@ -2897,6 +2897,27 @@ 

[libvirt] [PATCH 0/4] Add API to manipulate huge pages pool

2014-09-18 Thread Michal Privoznik
This may seem like a cherry on top of the cake, but once we allow
guests to use huge pages we must allow admins to allocate ones.

Michal Privoznik (4):
  Introduce virNodeAllocPages
  virnuma: Introduce virNumaSetPagePoolSize
  nodeinfo: Implement nodeAllocPages
  virsh: Expose virNodeAllocPages

 daemon/remote.c  |  37 
 include/libvirt/libvirt.h.in |  16 ++
 src/driver.h |  10 
 src/libvirt.c|  67 ++
 src/libvirt_private.syms |   2 +
 src/libvirt_public.syms  |   1 +
 src/lxc/lxc_driver.c |  22 +++
 src/nodeinfo.c   |  29 ++
 src/nodeinfo.h   |   7 +++
 src/qemu/qemu_driver.c   |  22 +++
 src/remote/remote_driver.c   |  47 +++
 src/remote/remote_protocol.x |  20 ++-
 src/remote_protocol-structs  |  17 ++
 src/uml/uml_driver.c |  22 +++
 src/util/virnuma.c   | 108 ++
 src/util/virnuma.h   |   4 ++
 src/vbox/vbox_common.c   |  19 ++
 tools/virsh-host.c   | 134 +++
 tools/virsh.pod  |  12 
 19 files changed, 595 insertions(+), 1 deletion(-)

-- 
1.8.5.5

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH 4/4] virsh: Expose virNodeAllocPages

2014-09-18 Thread Michal Privoznik
The new virsh command is named 'allocpages'.

Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
 tools/virsh-host.c | 134 +
 tools/virsh.pod|  12 +
 2 files changed, 146 insertions(+)

diff --git a/tools/virsh-host.c b/tools/virsh-host.c
index 7fc2120..15bfb7a 100644
--- a/tools/virsh-host.c
+++ b/tools/virsh-host.c
@@ -418,6 +418,134 @@ cmdFreepages(vshControl *ctl, const vshCmd *cmd)
 
 
 /*
+ * allocpages command
+ */
+static const vshCmdInfo info_allocpages[] = {
+{.name = help,
+ .data = N_(Manipulate pages pool size)
+},
+{.name = desc,
+ .data = N_(Allocate or free some pages in the pool for NUMA cell.)
+},
+{.name = NULL}
+};
+static const vshCmdOptDef opts_allocpages[] = {
+{.name = pagesize,
+ .type = VSH_OT_INT,
+ .flags = VSH_OFLAG_REQ,
+ .help = N_(page size (in kibibytes))
+},
+{.name = pagecount,
+ .type = VSH_OT_INT,
+ .flags = VSH_OFLAG_REQ,
+ .help = N_(page count)
+},
+{.name = cellno,
+ .type = VSH_OT_INT,
+ .help = N_(NUMA cell number)
+},
+{.name = add,
+ .type = VSH_OT_BOOL,
+ .help = N_(instead of setting new pool size add pages to it)
+},
+{.name = all,
+ .type = VSH_OT_BOOL,
+ .help = N_(set on all NUMA cells)
+},
+{.name = NULL}
+};
+
+static bool
+cmdAllocpages(vshControl *ctl, const vshCmd *cmd)
+{
+bool ret = false;
+bool add = vshCommandOptBool(cmd, add);
+bool all = vshCommandOptBool(cmd, all);
+bool cellno = vshCommandOptBool(cmd, cellno);
+int startCell = -1;
+int cellCount = 1;
+unsigned int pageSizes[1];
+unsigned long long pageCounts[1];
+unsigned int flags = 0;
+char *cap_xml = NULL;
+xmlDocPtr xml = NULL;
+xmlXPathContextPtr ctxt = NULL;
+xmlNodePtr *nodes = NULL;
+
+VSH_EXCLUSIVE_OPTIONS_VAR(all, cellno);
+
+if (cellno  vshCommandOptInt(cmd, cellno, startCell)  0) {
+vshError(ctl, %s, _(cell number has to be a number));
+return false;
+}
+
+if (vshCommandOptUInt(cmd, pagesize, pageSizes[0])  0) {
+vshError(ctl, %s, _(pagesize has to be a number));
+return false;
+}
+
+if (vshCommandOptULongLong(cmd, pagecount, pageCounts[0])  0) {
+vshError(ctl, %s, _(pagecount hat to be a number));
+return false;
+}
+
+flags |= add ? VIR_NODE_ALLOC_PAGES_ADD : VIR_NODE_ALLOC_PAGES_SET;
+
+if (all) {
+unsigned long nodes_cnt;
+size_t i;
+
+if (!(cap_xml = virConnectGetCapabilities(ctl-conn))) {
+vshError(ctl, %s, _(unable to get node capabilities));
+goto cleanup;
+}
+
+xml = virXMLParseStringCtxt(cap_xml, _((capabilities)), ctxt);
+if (!xml) {
+vshError(ctl, %s, _(unable to get node capabilities));
+goto cleanup;
+}
+
+nodes_cnt = virXPathNodeSet(/capabilities/host/topology/cells/cell,
+ctxt, nodes);
+
+if (nodes_cnt == -1) {
+vshError(ctl, %s, _(could not get information about 
+  NUMA topology));
+goto cleanup;
+}
+
+for (i = 0; i  nodes_cnt; i++) {
+unsigned long id;
+char *val = virXMLPropString(nodes[i], id);
+if (virStrToLong_ul(val, NULL, 10, id)) {
+vshError(ctl, %s, _(conversion from string failed));
+VIR_FREE(val);
+goto cleanup;
+}
+VIR_FREE(val);
+
+if (virNodeAllocPages(ctl-conn, 1, pageSizes,
+  pageCounts, id, 1, flags)  0)
+goto cleanup;
+}
+} else {
+if (virNodeAllocPages(ctl-conn, 1, pageSizes, pageCounts,
+  startCell, cellCount, flags)  0)
+goto cleanup;
+}
+
+ret = true;
+ cleanup:
+xmlXPathFreeContext(ctxt);
+xmlFreeDoc(xml);
+VIR_FREE(nodes);
+VIR_FREE(cap_xml);
+return ret;
+}
+
+
+/*
  * maxvcpus command
  */
 static const vshCmdInfo info_maxvcpus[] = {
@@ -1183,6 +1311,12 @@ cmdNodeMemoryTune(vshControl *ctl, const vshCmd *cmd)
 }
 
 const vshCmdDef hostAndHypervisorCmds[] = {
+{.name = allocpages,
+ .handler = cmdAllocpages,
+ .opts = opts_allocpages,
+ .info = info_capabilities,
+ .flags = 0
+},
 {.name = capabilities,
  .handler = cmdCapabilities,
  .opts = NULL,
diff --git a/tools/virsh.pod b/tools/virsh.pod
index 9919f92..eae9195 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -561,6 +561,18 @@ to the NUMA cell you're interested in. Ipagesize is a 
scaled integer (see
 BNOTES above).  Alternatively, if I--all is used, info on each possible
 combination of NUMA cell and page size is printed out.
 
+=item Ballocpages [I--pagesize] Ipagesize [I--pagecount] Ipagecount
+[[I--cellno] Icellno] [I--add] [I--all]
+
+Change the 

[libvirt] [PATCH 1/4] Introduce virNodeAllocPages

2014-09-18 Thread Michal Privoznik
A long time ago in a galaxy far, far away it has been decided
that libvirt will manage not only domains but host as well. And
with my latest work on qemu driver supporting huge pages, we miss
the cherry on top: an API to allocate huge pages on the run.
Currently users are forced to log into the host and adjust the
huge pages pool themselves.  However, with this API the problem
is gone - they can both size up and size down the pool.

Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
 daemon/remote.c  | 37 
 include/libvirt/libvirt.h.in | 16 +++
 src/driver.h | 10 +++
 src/libvirt.c| 67 
 src/libvirt_public.syms  |  1 +
 src/remote/remote_driver.c   | 47 +++
 src/remote/remote_protocol.x | 20 -
 src/remote_protocol-structs  | 17 +++
 8 files changed, 214 insertions(+), 1 deletion(-)

diff --git a/daemon/remote.c b/daemon/remote.c
index daa4b60..020772d 100644
--- a/daemon/remote.c
+++ b/daemon/remote.c
@@ -6471,6 +6471,43 @@ remoteDispatchConnectGetAllDomainStats(virNetServerPtr 
server ATTRIBUTE_UNUSED,
 }
 
 
+static int
+remoteDispatchNodeAllocPages(virNetServerPtr server ATTRIBUTE_UNUSED,
+ virNetServerClientPtr client,
+ virNetMessagePtr msg ATTRIBUTE_UNUSED,
+ virNetMessageErrorPtr rerr,
+ remote_node_alloc_pages_args *args,
+ remote_node_alloc_pages_ret *ret)
+{
+int rv = -1;
+int len;
+struct daemonClientPrivate *priv =
+virNetServerClientGetPrivateData(client);
+
+if (!priv-conn) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s, _(connection not open));
+goto cleanup;
+}
+
+if ((len = virNodeAllocPages(priv-conn,
+ args-pageSizes.pageSizes_len,
+ args-pageSizes.pageSizes_val,
+ (unsigned long long *) 
args-pageCounts.pageCounts_val,
+ args-startCell,
+ args-cellCount,
+ args-flags))  0)
+goto cleanup;
+
+ret-ret = len;
+rv = 0;
+
+ cleanup:
+if (rv  0)
+virNetMessageSaveError(rerr);
+return rv;
+}
+
+
 /*- Helpers. -*/
 
 /* get_nonnull_domain and get_nonnull_network turn an on-wire
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index 724314e..1a47bae 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -5505,6 +5505,22 @@ int virNodeGetFreePages(virConnectPtr conn,
 unsigned int cellcount,
 unsigned long long *counts,
 unsigned int flags);
+
+typedef enum {
+VIR_NODE_ALLOC_PAGES_ADD = 0, /* Add @pageCounts to the pages pool. This
+ can be used only to size up the pool. */
+VIR_NODE_ALLOC_PAGES_SET = (1  0), /* Don't add @pageCounts, instead set
+passed number of pages. This can be
+used to free allocated pages. */
+} virNodeAllocPagesFlags;
+
+int virNodeAllocPages(virConnectPtr conn,
+  unsigned int npages,
+  unsigned int *pageSizes,
+  unsigned long long *pageCounts,
+  int startCell,
+  unsigned int cellCount,
+  unsigned int flags);
 /**
  * virSchedParameterType:
  *
diff --git a/src/driver.h b/src/driver.h
index bb748c4..dc62a8e 100644
--- a/src/driver.h
+++ b/src/driver.h
@@ -1212,6 +1212,15 @@ typedef int
   virDomainStatsRecordPtr **retStats,
   unsigned int flags);
 
+typedef int
+(*virDrvNodeAllocPages)(virConnectPtr conn,
+unsigned int npages,
+unsigned int *pageSizes,
+unsigned long long *pageCounts,
+int startCell,
+unsigned int cellCount,
+unsigned int flags);
+
 typedef struct _virDriver virDriver;
 typedef virDriver *virDriverPtr;
 
@@ -1435,6 +1444,7 @@ struct _virDriver {
 virDrvNodeGetFreePages nodeGetFreePages;
 virDrvConnectGetDomainCapabilities connectGetDomainCapabilities;
 virDrvConnectGetAllDomainStats connectGetAllDomainStats;
+virDrvNodeAllocPages nodeAllocPages;
 };
 
 
diff --git a/src/libvirt.c b/src/libvirt.c
index 7c63825..27445e5 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -21841,3 +21841,70 @@ virDomainStatsRecordListFree(virDomainStatsRecordPtr 
*stats)
 
 VIR_FREE(stats);
 }
+
+
+/**
+ * virNodeAllocPages:
+ * @conn: pointer to the hypervisor connection
+ * @npages: number 

Re: [libvirt] retiring v0.9.6-maint

2014-09-18 Thread Daniel P. Berrange
On Wed, Sep 17, 2014 at 04:24:07PM -0600, Eric Blake wrote:
 Any objections to retiring the v0.9.6-maint branch?  After all, we have
 already retired the v0.9.11-maint branch
 (http://libvirt.org/git/?p=libvirt.git;a=commit;h=cd0d348ed), and the
 only activity on v0.9.6-maint since 0.9.6.4 was released in January 2013
 was the backport of a single CVE fix.  The branch no longer builds
 cleanly on Fedora 20, and while I could identify patches to backport to
 fix the build situation, it's not worth my time if we can just retire
 the branch.

FWIW, I'm not really a fan of deleting the branches. Is there any harm
to just leaving it there idle ?

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 v4 1/4] domain_conf: separate structures from virDomainDef

2014-09-18 Thread Pavel Hrdina
Cleanup virDomanDef structure from other nested structure and create
separate type definition for them.

Fix a typo in virDomainHugePage.

Signed-off-by: Pavel Hrdina phrd...@redhat.com
---
 src/conf/domain_conf.h | 102 +
 1 file changed, 61 insertions(+), 41 deletions(-)

diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 640a4c5..62faa7d 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1890,14 +1890,69 @@ struct _virDomainResourceDef {
 char *partition;
 };
 
-typedef struct _virDomaiHugePage virDomainHugePage;
+typedef struct _virDomainHugePage virDomainHugePage;
 typedef virDomainHugePage *virDomainHugePagePtr;
 
-struct _virDomaiHugePage {
+struct _virDomainHugePage {
 virBitmapPtr nodemask;  /* guest's NUMA node mask */
 unsigned long long size;/* hugepage size in KiB */
 };
 
+typedef struct _virDomainCputune virDomainCputune;
+typedef virDomainCputune *virDomainCputunePtr;
+
+struct _virDomainCputune {
+unsigned long shares;
+bool sharesSpecified;
+unsigned long long period;
+long long quota;
+unsigned long long emulator_period;
+long long emulator_quota;
+size_t nvcpupin;
+virDomainVcpuPinDefPtr *vcpupin;
+virDomainVcpuPinDefPtr emulatorpin;
+size_t niothreadspin;
+virDomainVcpuPinDefPtr *iothreadspin;
+};
+
+typedef struct _virDomainBlkiotune virDomainBlkiotune;
+typedef virDomainBlkiotune *virDomainBlkiotunePtr;
+
+struct _virDomainBlkiotune {
+unsigned int weight;
+
+size_t ndevices;
+virBlkioDevicePtr devices;
+};
+
+typedef struct _virDomainMemtune virDomainMemtune;
+typedef virDomainMemtune *virDomainMemtunePtr;
+
+struct _virDomainMemtune {
+unsigned long long max_balloon; /* in kibibytes */
+unsigned long long cur_balloon; /* in kibibytes */
+
+virDomainHugePagePtr hugepages;
+size_t nhugepages;
+
+bool nosharepages;
+bool locked;
+int dump_core; /* enum virTristateSwitch */
+unsigned long long hard_limit; /* in kibibytes */
+unsigned long long soft_limit; /* in kibibytes */
+unsigned long long min_guarantee; /* in kibibytes */
+unsigned long long swap_hard_limit; /* in kibibytes */
+};
+
+typedef struct _virDomainPowerManagement virDomainPowerManagement;
+typedef virDomainPowerManagement *virDomainPowerManagementPtr;
+
+struct _virDomainPowerManagement {
+/* These options are of type enum virTristateBool */
+int s3;
+int s4;
+};
+
 /*
  * Guest VM main configuration
  *
@@ -1914,28 +1969,9 @@ struct _virDomainDef {
 char *title;
 char *description;
 
-struct {
-unsigned int weight;
-
-size_t ndevices;
-virBlkioDevicePtr devices;
-} blkio;
+virDomainBlkiotune blkio;
+virDomainMemtune mem;
 
-struct {
-unsigned long long max_balloon; /* in kibibytes */
-unsigned long long cur_balloon; /* in kibibytes */
-
-virDomainHugePagePtr hugepages;
-size_t nhugepages;
-
-bool nosharepages;
-bool locked;
-int dump_core; /* enum virTristateSwitch */
-unsigned long long hard_limit; /* in kibibytes */
-unsigned long long soft_limit; /* in kibibytes */
-unsigned long long min_guarantee; /* in kibibytes */
-unsigned long long swap_hard_limit; /* in kibibytes */
-} mem;
 unsigned short vcpus;
 unsigned short maxvcpus;
 int placement_mode;
@@ -1943,19 +1979,7 @@ struct _virDomainDef {
 
 unsigned int iothreads;
 
-struct {
-unsigned long shares;
-bool sharesSpecified;
-unsigned long long period;
-long long quota;
-unsigned long long emulator_period;
-long long emulator_quota;
-size_t nvcpupin;
-virDomainVcpuPinDefPtr *vcpupin;
-virDomainVcpuPinDefPtr emulatorpin;
-size_t niothreadspin;
-virDomainVcpuPinDefPtr *iothreadspin;
-} cputune;
+virDomainCputune cputune;
 
 virDomainNumatunePtr numatune;
 virDomainResourceDefPtr resource;
@@ -1968,11 +1992,7 @@ struct _virDomainDef {
 
 int onLockFailure; /* enum virDomainLockFailureAction */
 
-struct {
-/* These options are of type enum virTristateBool */
-int s3;
-int s4;
-} pm;
+virDomainPowerManagement pm;
 
 virDomainOSDef os;
 char *emulator;
-- 
1.8.5.5

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH v4 0/4] Introduce new universal tunable event

2014-09-18 Thread Pavel Hrdina
This patch series introduce new tunable event to inform management
applications about changes of tunable values. With this universal
event we will be able to report updates for all different tunable
values like cpu tuning, block tinning, memory tinning, etc...

There is missing documentation for all events so the documentation
for this event will be part of the patches to document all events.

The format of returned typedParams will contain params with name
composed from prefix and the exact value. For example for cputune
the typedParam's field would be cputune.shares. List of actually
returned values will be part of the documentation for the events.

Changes from v3:
- the cputune event is gone, now we will have one universal event
  for all tunable values

Pavel Hrdina (4):
  domain_conf: separate structures from virDomainDef
  event: introduce new event for tunable values
  add an example how to use tunable event
  cputune_event: queue the event for cputune updates

 daemon/remote.c |  45 
 examples/object-events/event-test.c |  52 +-
 include/libvirt/libvirt.h.in|  22 
 src/conf/domain_conf.h  | 102 +---
 src/conf/domain_event.c |  93 
 src/conf/domain_event.h |   9 
 src/libvirt_private.syms|   2 +
 src/qemu/qemu_cgroup.c  |  18 ++-
 src/qemu/qemu_driver.c  |  76 +++
 src/remote/remote_driver.c  |  42 +++
 src/remote/remote_protocol.x|  14 -
 src/remote_protocol-structs |   9 
 tools/virsh-domain.c|  33 
 13 files changed, 473 insertions(+), 44 deletions(-)

-- 
1.8.5.5

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH v4 2/4] event: introduce new event for tunable values

2014-09-18 Thread Pavel Hrdina
This new event will use typedParameters to expose what has been actually
updated and the reason is that we can in the future extend any tunable
values or add new tunable values. With typedParameters we don't have to
worry about creating some other events, we will just use this universal
event to inform user about updates.

Signed-off-by: Pavel Hrdina phrd...@redhat.com
---
 daemon/remote.c  | 45 +
 include/libvirt/libvirt.h.in | 22 +++
 src/conf/domain_event.c  | 93 
 src/conf/domain_event.h  |  9 +
 src/libvirt_private.syms |  2 +
 src/remote/remote_driver.c   | 42 
 src/remote/remote_protocol.x | 14 ++-
 src/remote_protocol-structs  |  9 +
 tools/virsh-domain.c | 33 
 9 files changed, 268 insertions(+), 1 deletion(-)

diff --git a/daemon/remote.c b/daemon/remote.c
index daa4b60..ddd510c 100644
--- a/daemon/remote.c
+++ b/daemon/remote.c
@@ -111,6 +111,13 @@ remoteDeserializeTypedParameters(remote_typed_param 
*args_params_val,
  int *nparams);
 
 static int
+remoteSerializeTypedParameters(virTypedParameterPtr params,
+   int nparams,
+   remote_typed_param **ret_params_val,
+   u_int *ret_params_len,
+   unsigned int flags);
+
+static int
 remoteSerializeDomainDiskErrors(virDomainDiskErrorPtr errors,
 int nerrors,
 remote_domain_disk_error **ret_errors_val,
@@ -969,6 +976,43 @@ remoteRelayDomainEventBlockJob2(virConnectPtr conn,
 }
 
 
+static int
+remoteRelayDomainEventTunable(virConnectPtr conn,
+  virDomainPtr dom,
+  virTypedParameterPtr params,
+  int nparams,
+  void *opaque)
+{
+daemonClientEventCallbackPtr callback = opaque;
+remote_domain_event_callback_tunable_msg data;
+
+if (callback-callbackID  0 ||
+!remoteRelayDomainEventCheckACL(callback-client, conn, dom))
+return -1;
+
+VIR_DEBUG(Relaying domain tunable event %s %d, callback %d,
+  dom-name, dom-id, callback-callbackID);
+
+/* build return data */
+memset(data, 0, sizeof(data));
+data.callbackID = callback-callbackID;
+make_nonnull_domain(data.dom, dom);
+
+if (remoteSerializeTypedParameters(params, nparams,
+   data.params.params_val,
+   data.params.params_len,
+   VIR_TYPED_PARAM_STRING_OKAY)  0)
+return -1;
+
+remoteDispatchObjectEventSend(callback-client, remoteProgram,
+  REMOTE_PROC_DOMAIN_EVENT_CALLBACK_TUNABLE,
+  
(xdrproc_t)xdr_remote_domain_event_callback_tunable_msg,
+  data);
+
+return 0;
+}
+
+
 static virConnectDomainEventGenericCallback domainEventCallbacks[] = {
 VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventLifecycle),
 VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventReboot),
@@ -987,6 +1031,7 @@ static virConnectDomainEventGenericCallback 
domainEventCallbacks[] = {
 VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventPMSuspendDisk),
 VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventDeviceRemoved),
 VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventBlockJob2),
+VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventTunable),
 };
 
 verify(ARRAY_CARDINALITY(domainEventCallbacks) == VIR_DOMAIN_EVENT_ID_LAST);
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index 724314e..4149596 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -5177,6 +5177,27 @@ typedef void 
(*virConnectDomainEventDeviceRemovedCallback)(virConnectPtr conn,
const char 
*devAlias,
void *opaque);
 
+/**
+ * virConnectDomainEventTunableCallback:
+ * @conn: connection object
+ * @dom: domain on which the event occurred
+ * @params: changed tunable values stored as array of virTypedParameter
+ * @nparams: size of the array
+ * @opaque: application specified data
+ *
+ * This callback occurs when tunable values are updated. The params must not
+ * be freed in the callback handler as it's done internally after the callback
+ * handler is executed.
+ *
+ * The callback signature to use when registering for an event of type
+ * VIR_DOMAIN_EVENT_ID_TUNABLE with virConnectDomainEventRegisterAny()
+ */
+typedef void (*virConnectDomainEventTunableCallback)(virConnectPtr conn,
+ virDomainPtr dom,
+ virTypedParameterPtr 
params,
+

[libvirt] [PATCH v4 3/4] add an example how to use tunable event

2014-09-18 Thread Pavel Hrdina
Signed-off-by: Pavel Hrdina phrd...@redhat.com
---
 examples/object-events/event-test.c | 52 -
 1 file changed, 51 insertions(+), 1 deletion(-)

diff --git a/examples/object-events/event-test.c 
b/examples/object-events/event-test.c
index d6cfe46..9e09736 100644
--- a/examples/object-events/event-test.c
+++ b/examples/object-events/event-test.c
@@ -464,6 +464,48 @@ static int myNetworkEventCallback(virConnectPtr conn 
ATTRIBUTE_UNUSED,
 return 0;
 }
 
+static int
+myDomainEventTunableCallback(virConnectPtr conn ATTRIBUTE_UNUSED,
+ virDomainPtr dom,
+ virTypedParameterPtr params,
+ int nparams,
+ void *opaque ATTRIBUTE_UNUSED)
+{
+size_t i;
+
+printf(%s EVENT: Domain %s(%d) tunable updated:\n,
+   __func__, virDomainGetName(dom), virDomainGetID(dom));
+
+for (i = 0; i  nparams; i++) {
+switch (params[i].type) {
+case VIR_TYPED_PARAM_INT:
+printf(\t%s: %d\n, params[i].field, params[i].value.i);
+break;
+case VIR_TYPED_PARAM_UINT:
+printf(\t%s: %u\n, params[i].field, params[i].value.ui);
+break;
+case VIR_TYPED_PARAM_LLONG:
+printf(\t%s: %lld\n, params[i].field, params[i].value.l);
+break;
+case VIR_TYPED_PARAM_ULLONG:
+printf(\t%s: %llu\n, params[i].field, params[i].value.ul);
+break;
+case VIR_TYPED_PARAM_DOUBLE:
+printf(\t%s: %g\n, params[i].field, params[i].value.d);
+break;
+case VIR_TYPED_PARAM_BOOLEAN:
+printf(\t%s: %d\n, params[i].field, params[i].value.b);
+break;
+case VIR_TYPED_PARAM_STRING:
+printf(\t%s: %s\n, params[i].field, params[i].value.s);
+break;
+default:
+printf(\t%s: unknown type\n, params[i].field);
+}
+}
+
+return 0;
+}
 
 static void myFreeFunc(void *opaque)
 {
@@ -506,6 +548,7 @@ int main(int argc, char **argv)
 int callback14ret = -1;
 int callback15ret = -1;
 int callback16ret = -1;
+int callback17ret = -1;
 struct sigaction action_stop;
 
 memset(action_stop, 0, sizeof(action_stop));
@@ -624,6 +667,11 @@ int main(int argc, char **argv)
   
VIR_NETWORK_EVENT_ID_LIFECYCLE,
   
VIR_NETWORK_EVENT_CALLBACK(myNetworkEventCallback),
   strdup(net callback), 
myFreeFunc);
+callback17ret = virConnectDomainEventRegisterAny(dconn,
+ NULL,
+ 
VIR_DOMAIN_EVENT_ID_TUNABLE,
+ 
VIR_DOMAIN_EVENT_CALLBACK(myDomainEventTunableCallback),
+ strdup(tunable), 
myFreeFunc);
 
 if ((callback1ret != -1) 
 (callback2ret != -1) 
@@ -639,7 +687,8 @@ int main(int argc, char **argv)
 (callback13ret != -1) 
 (callback14ret != -1) 
 (callback15ret != -1) 
-(callback16ret != -1)) {
+(callback16ret != -1) 
+(callback17ret != -1)) {
 if (virConnectSetKeepAlive(dconn, 5, 3)  0) {
 virErrorPtr err = virGetLastError();
 fprintf(stderr, Failed to start keepalive protocol: %s\n,
@@ -671,6 +720,7 @@ int main(int argc, char **argv)
 virConnectDomainEventDeregisterAny(dconn, callback14ret);
 virConnectDomainEventDeregisterAny(dconn, callback15ret);
 virConnectNetworkEventDeregisterAny(dconn, callback16ret);
+virConnectDomainEventDeregisterAny(dconn, callback17ret);
 if (callback8ret != -1)
 virConnectDomainEventDeregisterAny(dconn, callback8ret);
 }
-- 
1.8.5.5

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH] qemu: Don't fail qemuProcessAttach for IOThreads if no JSON

2014-09-18 Thread Daniel P. Berrange
On Wed, Sep 17, 2014 at 03:07:47PM -0400, John Ferlan wrote:
 While doing some investigation for another bug I found that I could
 not qemu-attach to the process and got the following:
 
error: Operation not supported: JSON monitor is required
 
 while running thru qemuProcessAttach. Since we can only get the data
 using the JSON parser and if the guest to be attached to doesn't have
 it we shouldn't just fail. See example in virsh qemu-attach for sample
 command that failed.

It isn't simply qemu-attach that's affected. If you merely try to
start a guest normally with a QEMU that predates JSON support this
would fail too.

 Signed-off-by: John Ferlan jfer...@redhat.com
 ---
 
 I also considered removing the call from qemuProcessAttach rather than
 this approach.
 
  src/qemu/qemu_monitor.c | 8 +++-
  1 file changed, 3 insertions(+), 5 deletions(-)
 
 diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
 index 8927dbb..4342088 100644
 --- a/src/qemu/qemu_monitor.c
 +++ b/src/qemu/qemu_monitor.c
 @@ -4112,11 +4112,9 @@ qemuMonitorGetIOThreads(qemuMonitorPtr mon,
  return -1;
  }
  
 -if (!mon-json) {
 -virReportError(VIR_ERR_OPERATION_UNSUPPORTED, %s,
 -   _(JSON monitor is required));
 -return -1;
 -}
 +/* Requires JSON to make the query */
 +if (!mon-json)
 +return 0;

I think you need should do '*iothreads = NULL' for safety too.

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 v4 4/4] cputune_event: queue the event for cputune updates

2014-09-18 Thread Pavel Hrdina
Now we have universal tunable event so we can use it for reporting
changes to user. The cputune values will be prefixed with cputune to
distinguish it from other tunable events.

Signed-off-by: Pavel Hrdina phrd...@redhat.com
---
 src/qemu/qemu_cgroup.c | 18 +++-
 src/qemu/qemu_driver.c | 76 ++
 2 files changed, 93 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index 9d39370..27dcba3 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -34,6 +34,7 @@
 #include virscsi.h
 #include virstring.h
 #include virfile.h
+#include virtypedparam.h
 
 #define VIR_FROM_THIS VIR_FROM_QEMU
 
@@ -676,6 +677,10 @@ static int
 qemuSetupCpuCgroup(virDomainObjPtr vm)
 {
 qemuDomainObjPrivatePtr priv = vm-privateData;
+virObjectEventPtr event = NULL;
+virTypedParameterPtr eventParams;
+int eventNparams = 0;
+int eventMaxparams = 0;
 
 if (!virCgroupHasController(priv-cgroup, VIR_CGROUP_CONTROLLER_CPU)) {
if (vm-def-cputune.sharesSpecified) {
@@ -694,7 +699,18 @@ qemuSetupCpuCgroup(virDomainObjPtr vm)
 
 if (virCgroupGetCpuShares(priv-cgroup, val)  0)
 return -1;
-vm-def-cputune.shares = val;
+if (vm-def-cputune.shares != val) {
+vm-def-cputune.shares = val;
+if (virTypedParamsAddULLong(eventParams, eventNparams,
+eventMaxparams, cputune.shares,
+val)  0)
+return -1;
+
+event = virDomainEventTunableNewFromObj(vm, eventParams, 
eventNparams);
+}
+
+if (event)
+qemuDomainEventQueue(vm-privateData, event);
 }
 
 return 0;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 5fd89a3..22c6e55 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4538,6 +4538,12 @@ qemuDomainPinVcpuFlags(virDomainPtr dom,
 virBitmapPtr pcpumap = NULL;
 virQEMUDriverConfigPtr cfg = NULL;
 virCapsPtr caps = NULL;
+virObjectEventPtr event = NULL;
+char paramField[VIR_TYPED_PARAM_FIELD_LENGTH] = ;
+char *str = NULL;
+virTypedParameterPtr eventParams = NULL;
+int eventNparams = 0;
+int eventMaxparams = 0;
 
 virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
   VIR_DOMAIN_AFFECT_CONFIG, -1);
@@ -4645,6 +4651,18 @@ qemuDomainPinVcpuFlags(virDomainPtr dom,
 
 if (virDomainSaveStatus(driver-xmlopt, cfg-stateDir, vm)  0)
 goto cleanup;
+
+if (snprintf(paramField, VIR_TYPED_PARAM_FIELD_LENGTH,
+ cputune.vcpupin%d, vcpu)  0) {
+goto cleanup;
+}
+
+str = virBitmapFormat(pcpumap);
+if (virTypedParamsAddString(eventParams, eventNparams,
+eventMaxparams, paramField, str)  0)
+goto cleanup;
+
+event = virDomainEventTunableNewFromDom(dom, eventParams, 
eventNparams);
 }
 
 if (flags  VIR_DOMAIN_AFFECT_CONFIG) {
@@ -4680,6 +4698,9 @@ qemuDomainPinVcpuFlags(virDomainPtr dom,
 virCgroupFree(cgroup_vcpu);
 if (vm)
 virObjectUnlock(vm);
+if (event)
+qemuDomainEventQueue(driver, event);
+VIR_FREE(str);
 virBitmapFree(pcpumap);
 virObjectUnref(caps);
 virObjectUnref(cfg);
@@ -4804,6 +4825,12 @@ qemuDomainPinEmulator(virDomainPtr dom,
 virBitmapPtr pcpumap = NULL;
 virQEMUDriverConfigPtr cfg = NULL;
 virCapsPtr caps = NULL;
+virObjectEventPtr event = NULL;
+char * str = NULL;
+virTypedParameterPtr eventParams = NULL;
+int eventNparams = 0;
+int eventMaxparams = 0;
+
 
 virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
   VIR_DOMAIN_AFFECT_CONFIG, -1);
@@ -4909,6 +4936,13 @@ qemuDomainPinEmulator(virDomainPtr dom,
 
 if (virDomainSaveStatus(driver-xmlopt, cfg-stateDir, vm)  0)
 goto cleanup;
+
+str = virBitmapFormat(pcpumap);
+if (virTypedParamsAddString(eventParams, eventNparams,
+eventMaxparams, cputune.emulatorpin, 
str)  0)
+goto cleanup;
+
+event = virDomainEventTunableNewFromDom(dom, eventParams, 
eventNparams);
 }
 
 if (flags  VIR_DOMAIN_AFFECT_CONFIG) {
@@ -4938,6 +4972,9 @@ qemuDomainPinEmulator(virDomainPtr dom,
  cleanup:
 if (cgroup_emulator)
 virCgroupFree(cgroup_emulator);
+if (event)
+qemuDomainEventQueue(driver, event);
+VIR_FREE(str);
 virBitmapFree(pcpumap);
 virObjectUnref(caps);
 if (vm)
@@ -9096,6 +9133,10 @@ qemuDomainSetSchedulerParametersFlags(virDomainPtr dom,
 virQEMUDriverConfigPtr cfg = NULL;
 virCapsPtr caps = NULL;
 qemuDomainObjPrivatePtr priv;
+virObjectEventPtr event = NULL;
+virTypedParameterPtr eventParams = NULL;
+int eventNparams = 0;
+int eventMaxNparams = 0;
 
 virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
   

Re: [libvirt] [PATCH 1/3] bhyve: tests: fix build

2014-09-18 Thread Michal Privoznik

On 18.09.2014 08:47, Roman Bogorodskiy wrote:

Commit b20d39a introduced a new argument for the
virNetDevTapCreateInBridgePort function, however, its mock
in bhyve tests wasn't updated, so the build failed.

Fix build by adding this new argument to the mock version.
---
  tests/bhyvexml2argvmock.c | 1 +
  1 file changed, 1 insertion(+)

diff --git a/tests/bhyvexml2argvmock.c b/tests/bhyvexml2argvmock.c
index fa2f14b..0cbea29 100644
--- a/tests/bhyvexml2argvmock.c
+++ b/tests/bhyvexml2argvmock.c
@@ -22,6 +22,7 @@ int virNetDevTapCreateInBridgePort(const char *brname 
ATTRIBUTE_UNUSED,
 char **ifname,
 const virMacAddr *macaddr ATTRIBUTE_UNUSED,
 const unsigned char *vmuuid 
ATTRIBUTE_UNUSED,
+   const char *tunpath ATTRIBUTE_UNUSED,
 int *tapfd ATTRIBUTE_UNUSED,
 int tapfdSize ATTRIBUTE_UNUSED,
 virNetDevVPortProfilePtr virtPortProfile 
ATTRIBUTE_UNUSED,



ACK series

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH 1/3] bhyve: tests: fix build

2014-09-18 Thread Roman Bogorodskiy
  Michal Privoznik wrote:

 On 18.09.2014 08:47, Roman Bogorodskiy wrote:
  Commit b20d39a introduced a new argument for the
  virNetDevTapCreateInBridgePort function, however, its mock
  in bhyve tests wasn't updated, so the build failed.
 
  Fix build by adding this new argument to the mock version.
  ---
tests/bhyvexml2argvmock.c | 1 +
1 file changed, 1 insertion(+)
 
  diff --git a/tests/bhyvexml2argvmock.c b/tests/bhyvexml2argvmock.c
  index fa2f14b..0cbea29 100644
  --- a/tests/bhyvexml2argvmock.c
  +++ b/tests/bhyvexml2argvmock.c
  @@ -22,6 +22,7 @@ int virNetDevTapCreateInBridgePort(const char *brname 
  ATTRIBUTE_UNUSED,
   char **ifname,
   const virMacAddr *macaddr 
  ATTRIBUTE_UNUSED,
   const unsigned char *vmuuid 
  ATTRIBUTE_UNUSED,
  +   const char *tunpath ATTRIBUTE_UNUSED,
   int *tapfd ATTRIBUTE_UNUSED,
   int tapfdSize ATTRIBUTE_UNUSED,
   virNetDevVPortProfilePtr 
  virtPortProfile ATTRIBUTE_UNUSED,
 
 
 ACK series

Pushed, thanks!

Roman Bogorodskiy

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCHv2 1/4] util: storage: Allow metadata crawler to report useful errors

2014-09-18 Thread Peter Krempa
Add a new parameter to virStorageFileGetMetadata that will break the
backing chain detection process and report useful error message rather
than having to use virStorageFileChainGetBroken.

This patch just introduces the option, usage will be provided
separately.
---
 src/qemu/qemu_domain.c|  3 ++-
 src/security/virt-aa-helper.c |  2 +-
 src/storage/storage_driver.c  | 24 +---
 src/storage/storage_driver.h  |  3 ++-
 tests/virstoragetest.c|  2 +-
 5 files changed, 23 insertions(+), 11 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 5859ba7..19b935d 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -2693,7 +2693,8 @@ qemuDomainDetermineDiskChain(virQEMUDriverPtr driver,

 if (virStorageFileGetMetadata(disk-src,
   uid, gid,
-  cfg-allowDiskFormatProbing)  0)
+  cfg-allowDiskFormatProbing,
+  false)  0)
 ret = -1;

  cleanup:
diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
index a06ba44..9afc8db 100644
--- a/src/security/virt-aa-helper.c
+++ b/src/security/virt-aa-helper.c
@@ -932,7 +932,7 @@ get_files(vahControl * ctl)
  */
 if (!disk-src-backingStore) {
 bool probe = ctl-allowDiskFormatProbing;
-virStorageFileGetMetadata(disk-src, -1, -1, probe);
+virStorageFileGetMetadata(disk-src, -1, -1, probe, false);
 }

 /* XXX passing ignoreOpenFailure = true to get back to the behavior
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 433d7b7..c3b29c4 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -2783,6 +2783,7 @@ static int
 virStorageFileGetMetadataRecurse(virStorageSourcePtr src,
  uid_t uid, gid_t gid,
  bool allow_probe,
+ bool report_broken,
  virHashTablePtr cycle)
 {
 int ret = -1;
@@ -2847,9 +2848,13 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr src,
 else
 backingStore-format = backingFormat;

-if (virStorageFileGetMetadataRecurse(backingStore,
- uid, gid, allow_probe,
- cycle)  0) {
+if ((ret = virStorageFileGetMetadataRecurse(backingStore,
+uid, gid,
+allow_probe, report_broken,
+cycle))  0) {
+if (report_broken)
+goto cleanup;
+
 /* if we fail somewhere midway, just accept and return a
  * broken chain */
 ret = 0;
@@ -2883,15 +2888,20 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr 
src,
  * format, since a malicious guest can turn a raw file into any
  * other non-raw format at will.
  *
+ * If @report_broken is true, the whole function fails with a possibly sane
+ * error instead of just returning a broken chain.
+ *
  * Caller MUST free result after use via virStorageSourceFree.
  */
 int
 virStorageFileGetMetadata(virStorageSourcePtr src,
   uid_t uid, gid_t gid,
-  bool allow_probe)
+  bool allow_probe,
+  bool report_broken)
 {
-VIR_DEBUG(path=%s format=%d uid=%d gid=%d probe=%d,
-  src-path, src-format, (int)uid, (int)gid, allow_probe);
+VIR_DEBUG(path=%s format=%d uid=%d gid=%d probe=%d, report_broken=%d,
+  src-path, src-format, (int)uid, (int)gid,
+  allow_probe, report_broken);

 virHashTablePtr cycle = NULL;
 int ret = -1;
@@ -2903,7 +2913,7 @@ virStorageFileGetMetadata(virStorageSourcePtr src,
 src-format = allow_probe ? VIR_STORAGE_FILE_AUTO : 
VIR_STORAGE_FILE_RAW;

 ret = virStorageFileGetMetadataRecurse(src, uid, gid,
-   allow_probe, cycle);
+   allow_probe, report_broken, cycle);

 virHashFree(cycle);
 return ret;
diff --git a/src/storage/storage_driver.h b/src/storage/storage_driver.h
index e773928..54a17a3 100644
--- a/src/storage/storage_driver.h
+++ b/src/storage/storage_driver.h
@@ -50,7 +50,8 @@ bool virStorageFileSupportsSecurityDriver(virStorageSourcePtr 
src);

 int virStorageFileGetMetadata(virStorageSourcePtr src,
   uid_t uid, gid_t gid,
-  bool allow_probe)
+  bool allow_probe,
+  bool fail)
 ATTRIBUTE_NONNULL(1);

 int virStorageTranslateDiskSourcePool(virConnectPtr conn,
diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c
index e2ee3ff..29f5c7a 100644
--- a/tests/virstoragetest.c
+++ 

[libvirt] [PATCHv2 0/4] Improve backing store error reporting

2014-09-18 Thread Peter Krempa
New version incorporates some review feedback from John. The changes were 
borderline-trivial so I've reposted the series.

Patch 2/4 is new.

Peter Krempa (4):
  util: storage: Allow metadata crawler to report useful errors
  qemu: Sanitize argument names and empty disk check in
qemuDomainDetermineDiskChain
  qemu: Report better errors from broken backing chains
  storage: Improve error message when traversing backing chains

 src/qemu/qemu_domain.c| 36 +++-
 src/qemu/qemu_domain.h|  3 ++-
 src/qemu/qemu_driver.c| 10 +-
 src/qemu/qemu_hotplug.c   |  2 +-
 src/qemu/qemu_process.c   | 11 +++
 src/security/virt-aa-helper.c |  2 +-
 src/storage/storage_driver.c  | 42 +++---
 src/storage/storage_driver.h  |  3 ++-
 tests/virstoragetest.c|  2 +-
 9 files changed, 57 insertions(+), 54 deletions(-)

-- 
2.1.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCHv2 3/4] qemu: Report better errors from broken backing chains

2014-09-18 Thread Peter Krempa
Request erroring out from the backing chain traveller and drop qemu's
internal backing chain integrity tester.

The backin chain traveller reports errors by itself with possibly more
detail than qemuDiskChainCheckBroken ever could.

We also need to make sure that we reconnect to existing qemu instances
even at the cost of losing the backing chain info (this really should be
stored in the XML rather than reloaded from disk, but that needs some
work).
---
 src/qemu/qemu_domain.c  | 29 -
 src/qemu/qemu_domain.h  |  3 ++-
 src/qemu/qemu_driver.c  | 10 +-
 src/qemu/qemu_hotplug.c |  2 +-
 src/qemu/qemu_process.c | 11 +++
 5 files changed, 19 insertions(+), 36 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 515bcac..b93e0d5 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -2476,27 +2476,6 @@ qemuDomainCheckDiskStartupPolicy(virQEMUDriverPtr driver,
 return -1;
 }

-static int
-qemuDiskChainCheckBroken(virDomainDiskDefPtr disk)
-{
-char *brokenFile = NULL;
-
-if (!virDomainDiskGetSource(disk))
-return 0;
-
-if (virStorageFileChainGetBroken(disk-src, brokenFile)  0)
-return -1;
-
-if (brokenFile) {
-virReportError(VIR_ERR_INVALID_ARG,
-   _(Backing file '%s' of image '%s' is missing.),
-   brokenFile, virDomainDiskGetSource(disk));
-VIR_FREE(brokenFile);
-return -1;
-}
-
-return 0;
-}

 int
 qemuDomainCheckDiskPresence(virQEMUDriverPtr driver,
@@ -2524,8 +2503,7 @@ qemuDomainCheckDiskPresence(virQEMUDriverPtr driver,
 virFileExists(virDomainDiskGetSource(disk)))
 continue;

-if (qemuDomainDetermineDiskChain(driver, vm, disk, false) = 0 
-qemuDiskChainCheckBroken(disk) = 0)
+if (qemuDomainDetermineDiskChain(driver, vm, disk, false, true) = 0)
 continue;

 if (disk-startupPolicy 
@@ -2670,7 +2648,8 @@ int
 qemuDomainDetermineDiskChain(virQEMUDriverPtr driver,
  virDomainObjPtr vm,
  virDomainDiskDefPtr disk,
- bool force_probe)
+ bool force_probe,
+ bool report_broken)
 {
 virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
 int ret = 0;
@@ -2692,7 +2671,7 @@ qemuDomainDetermineDiskChain(virQEMUDriverPtr driver,
 if (virStorageFileGetMetadata(disk-src,
   uid, gid,
   cfg-allowDiskFormatProbing,
-  false)  0)
+  report_broken)  0)
 ret = -1;

  cleanup:
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 494e9f8..cb0115a 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -366,7 +366,8 @@ int qemuDomainCheckDiskPresence(virQEMUDriverPtr driver,
 int qemuDomainDetermineDiskChain(virQEMUDriverPtr driver,
  virDomainObjPtr vm,
  virDomainDiskDefPtr disk,
- bool force_probe);
+ bool force_probe,
+ bool report_broken);

 int qemuDomainStorageFileInit(virQEMUDriverPtr driver,
   virDomainObjPtr vm,
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 5fd89a3..d0aff1a 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -6688,7 +6688,7 @@ qemuDomainChangeDiskMediaLive(virConnectPtr conn,
 if (virStorageTranslateDiskSourcePool(conn, disk)  0)
 goto end;

-if (qemuDomainDetermineDiskChain(driver, vm, disk, false)  0)
+if (qemuDomainDetermineDiskChain(driver, vm, disk, false, true)  0)
 goto end;

 switch (disk-device) {
@@ -13067,7 +13067,7 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr 
driver,
 for (i = 0; i  snap-def-ndisks; i++) {
 if (snap-def-disks[i].snapshot != 
VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL)
 continue;
-qemuDomainDetermineDiskChain(driver, vm, vm-def-disks[i], true);
+qemuDomainDetermineDiskChain(driver, vm, vm-def-disks[i], true, 
true);
 }
 if (orig_err) {
 virSetError(orig_err);
@@ -14875,7 +14875,7 @@ qemuDomainBlockPivot(virConnectPtr conn,
 oldsrc = disk-src;
 disk-src = disk-mirror;

-if (qemuDomainDetermineDiskChain(driver, vm, disk, false)  0)
+if (qemuDomainDetermineDiskChain(driver, vm, disk, false, true)  0)
 goto cleanup;

 if (disk-mirror-format 
@@ -15388,7 +15388,7 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm,
 goto endjob;
 }

-if (qemuDomainDetermineDiskChain(driver, vm, disk, false)  0)
+if (qemuDomainDetermineDiskChain(driver, vm, disk, false, true)  0)
 goto endjob;

 if ((flags  

[libvirt] [PATCHv2 2/4] qemu: Sanitize argument names and empty disk check in qemuDomainDetermineDiskChain

2014-09-18 Thread Peter Krempa
Reuse virStorageSourceIsEmpty and rename force argument to
force_probe.
---
 src/qemu/qemu_domain.c | 8 +++-
 src/qemu/qemu_domain.h | 2 +-
 2 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 19b935d..515bcac 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -2670,20 +2670,18 @@ int
 qemuDomainDetermineDiskChain(virQEMUDriverPtr driver,
  virDomainObjPtr vm,
  virDomainDiskDefPtr disk,
- bool force)
+ bool force_probe)
 {
 virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
 int ret = 0;
 uid_t uid;
 gid_t gid;
-int type = virStorageSourceGetActualType(disk-src);

-if (type != VIR_STORAGE_TYPE_NETWORK 
-!disk-src-path)
+if (virStorageSourceIsEmpty(disk-src))
 goto cleanup;

 if (disk-src-backingStore) {
-if (force)
+if (force_probe)
 virStorageSourceBackingStoreClear(disk-src);
 else
 goto cleanup;
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 4ae2c57..494e9f8 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -366,7 +366,7 @@ int qemuDomainCheckDiskPresence(virQEMUDriverPtr driver,
 int qemuDomainDetermineDiskChain(virQEMUDriverPtr driver,
  virDomainObjPtr vm,
  virDomainDiskDefPtr disk,
- bool force);
+ bool force_probe);

 int qemuDomainStorageFileInit(virQEMUDriverPtr driver,
   virDomainObjPtr vm,
-- 
2.1.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCHv2 4/4] storage: Improve error message when traversing backing chains

2014-09-18 Thread Peter Krempa
Report also the name of the parent file and uid/gid used to access it to
help debugging broken storage configurations.
---
 src/storage/storage_driver.c | 20 +++-
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index c3b29c4..7c518bf 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -2781,6 +2781,7 @@ virStorageFileChown(virStorageSourcePtr src,
 /* Recursive workhorse for virStorageFileGetMetadata.  */
 static int
 virStorageFileGetMetadataRecurse(virStorageSourcePtr src,
+ virStorageSourcePtr parent,
  uid_t uid, gid_t gid,
  bool allow_probe,
  bool report_broken,
@@ -2805,9 +2806,18 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr src,
 return -1;

 if (virStorageFileAccess(src, F_OK)  0) {
-virReportSystemError(errno,
- _(Cannot access backing file %s),
- src-path);
+if (src == parent) {
+virReportSystemError(errno,
+ _(Cannot access storage file '%s' 
+   (as uid:%d, gid:%d)),
+ src-path, (int)uid, (int)gid);
+} else {
+virReportSystemError(errno,
+ _(Cannot access backing file '%s' 
+   of storage file '%s' (as uid:%d, gid:%d)),
+ src-path, parent-path, (int)uid, (int)gid);
+}
+
 goto cleanup;
 }

@@ -2848,7 +2858,7 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr src,
 else
 backingStore-format = backingFormat;

-if ((ret = virStorageFileGetMetadataRecurse(backingStore,
+if ((ret = virStorageFileGetMetadataRecurse(backingStore, parent,
 uid, gid,
 allow_probe, report_broken,
 cycle))  0) {
@@ -2912,7 +2922,7 @@ virStorageFileGetMetadata(virStorageSourcePtr src,
 if (src-format = VIR_STORAGE_FILE_NONE)
 src-format = allow_probe ? VIR_STORAGE_FILE_AUTO : 
VIR_STORAGE_FILE_RAW;

-ret = virStorageFileGetMetadataRecurse(src, uid, gid,
+ret = virStorageFileGetMetadataRecurse(src, src, uid, gid,
allow_probe, report_broken, cycle);

 virHashFree(cycle);
-- 
2.1.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH 0/4] Memory leak fixes

2014-09-18 Thread Ján Tomko
Ján Tomko (4):
  Fix leak in x86UpdateHostModel
  Fixes for domains with no iothreads
  audit: remove redundant NULL assignment
  audit: fix memory leak without WITH_AUDIT

 src/cpu/cpu_x86.c   |  4 +++-
 src/qemu/qemu_cgroup.c  |  2 +-
 src/qemu/qemu_process.c |  6 --
 src/util/viraudit.c | 13 +++--
 4 files changed, 11 insertions(+), 14 deletions(-)

-- 
1.8.5.5

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 1/4] Fix leak in x86UpdateHostModel

2014-09-18 Thread Ján Tomko
Commit de0aeaf introduced a memory leak.
---
 src/cpu/cpu_x86.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c
index 7571f16..a98a847 100644
--- a/src/cpu/cpu_x86.c
+++ b/src/cpu/cpu_x86.c
@@ -2094,8 +2094,10 @@ x86UpdateHostModel(virCPUDefPtr guest,
  * features directly */
 for (i = 0; i  guest-nfeatures; i++) {
 for (feat = map-migrate_blockers; feat; feat = feat-next) {
-if (STREQ(feat-name, guest-features[i].name))
+if (STREQ(feat-name, guest-features[i].name)) {
+VIR_FREE(guest-features[i].name);
 VIR_DELETE_ELEMENT_INPLACE(guest-features, i, 
guest-nfeatures);
+}
 }
 }
 
-- 
1.8.5.5

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH 2/4] Fixes for domains with no iothreads

2014-09-18 Thread Ján Tomko
Plug a memory leak and silence a warning.
---
 src/qemu/qemu_cgroup.c  | 2 +-
 src/qemu/qemu_process.c | 6 --
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index 9d39370..7c6b2c1 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -1131,7 +1131,7 @@ qemuSetupCgroupForIOThreads(virDomainObjPtr vm)
 if (priv-cgroup == NULL)
 return 0;
 
-if (priv-niothreadpids == 0) {
+if (def-iothreads  priv-niothreadpids == 0) {
 VIR_WARN(Unable to get iothreads' pids.);
 return 0;
 }
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 8853d27..c5b6263 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -2117,8 +2117,10 @@ qemuProcessDetectIOThreadPIDs(virQEMUDriverPtr driver,
 goto cleanup;
 
 /* Nothing to do */
-if (niothreads == 0)
-return 0;
+if (niothreads == 0) {
+ret = 0;
+goto cleanup;
+}
 
 if (niothreads != vm-def-iothreads) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
-- 
1.8.5.5

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH 4/4] audit: fix memory leak without WITH_AUDIT

2014-09-18 Thread Ján Tomko
Free str unconditionally since we allocate it without WITH_AUDIT
too.
---
 src/util/viraudit.c | 9 ++---
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/src/util/viraudit.c b/src/util/viraudit.c
index d0ad9b9..23928fd 100644
--- a/src/util/viraudit.c
+++ b/src/util/viraudit.c
@@ -115,12 +115,7 @@ void virAuditSend(virLogSourcePtr source,
 }
 
 #if WITH_AUDIT
-if (auditfd  0) {
-VIR_FREE(str);
-return;
-}
-
-if (str) {
+if (str  auditfd = 0) {
 static const int record_types[] = {
 [VIR_AUDIT_RECORD_MACHINE_CONTROL] = AUDIT_VIRT_CONTROL,
 [VIR_AUDIT_RECORD_MACHINE_ID] = AUDIT_VIRT_MACHINE_ID,
@@ -135,9 +130,9 @@ void virAuditSend(virLogSourcePtr source,
 VIR_WARN(Failed to send audit message %s: %s,
  NULLSTR(str), virStrerror(errno, ebuf, sizeof(ebuf)));
 }
-VIR_FREE(str);
 }
 #endif
+VIR_FREE(str);
 }
 
 void virAuditClose(void)
-- 
1.8.5.5

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH 3/4] audit: remove redundant NULL assignment

2014-09-18 Thread Ján Tomko
virVasprintf sets the output to NULL on failure.
---
 src/util/viraudit.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/src/util/viraudit.c b/src/util/viraudit.c
index 8023c60..d0ad9b9 100644
--- a/src/util/viraudit.c
+++ b/src/util/viraudit.c
@@ -99,10 +99,8 @@ void virAuditSend(virLogSourcePtr source,
 #endif
 
 va_start(args, fmt);
-if (virVasprintf(str, fmt, args)  0) {
+if (virVasprintf(str, fmt, args)  0)
 VIR_WARN(Out of memory while formatting audit message);
-str = NULL;
-}
 va_end(args);
 
 if (auditlog  str) {
-- 
1.8.5.5

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH] qemu: Don't fail qemuProcessAttach for IOThreads if no JSON

2014-09-18 Thread John Ferlan


On 09/18/2014 04:39 AM, Daniel P. Berrange wrote:
 On Wed, Sep 17, 2014 at 03:07:47PM -0400, John Ferlan wrote:
 While doing some investigation for another bug I found that I could
 not qemu-attach to the process and got the following:

error: Operation not supported: JSON monitor is required

 while running thru qemuProcessAttach. Since we can only get the data
 using the JSON parser and if the guest to be attached to doesn't have
 it we shouldn't just fail. See example in virsh qemu-attach for sample
 command that failed.
 
 It isn't simply qemu-attach that's affected. If you merely try to
 start a guest normally with a QEMU that predates JSON support this
 would fail too.
 
 Signed-off-by: John Ferlan jfer...@redhat.com
 ---

 I also considered removing the call from qemuProcessAttach rather than
 this approach.

  src/qemu/qemu_monitor.c | 8 +++-
  1 file changed, 3 insertions(+), 5 deletions(-)

 diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
 index 8927dbb..4342088 100644
 --- a/src/qemu/qemu_monitor.c
 +++ b/src/qemu/qemu_monitor.c
 @@ -4112,11 +4112,9 @@ qemuMonitorGetIOThreads(qemuMonitorPtr mon,
  return -1;
  }
  
 -if (!mon-json) {
 -virReportError(VIR_ERR_OPERATION_UNSUPPORTED, %s,
 -   _(JSON monitor is required));
 -return -1;
 -}
 +/* Requires JSON to make the query */
 +if (!mon-json)
 +return 0;
 
 I think you need should do '*iothreads = NULL' for safety too.
 

OK with the following squashed in?

diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 4342088..10f51c5 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -4113,8 +4113,10 @@ qemuMonitorGetIOThreads(qemuMonitorPtr mon,
 }

 /* Requires JSON to make the query */
-if (!mon-json)
+if (!mon-json) {
+*iothreads = NULL;
 return 0;
+}

 return qemuMonitorJSONGetIOThreads(mon, iothreads);
 }


John

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH] qemu: Don't fail qemuProcessAttach for IOThreads if no JSON

2014-09-18 Thread Daniel P. Berrange
On Thu, Sep 18, 2014 at 06:18:22AM -0400, John Ferlan wrote:
 
 
 On 09/18/2014 04:39 AM, Daniel P. Berrange wrote:
  On Wed, Sep 17, 2014 at 03:07:47PM -0400, John Ferlan wrote:
  While doing some investigation for another bug I found that I could
  not qemu-attach to the process and got the following:
 
 error: Operation not supported: JSON monitor is required
 
  while running thru qemuProcessAttach. Since we can only get the data
  using the JSON parser and if the guest to be attached to doesn't have
  it we shouldn't just fail. See example in virsh qemu-attach for sample
  command that failed.
  
  It isn't simply qemu-attach that's affected. If you merely try to
  start a guest normally with a QEMU that predates JSON support this
  would fail too.
  
  Signed-off-by: John Ferlan jfer...@redhat.com
  ---
 
  I also considered removing the call from qemuProcessAttach rather than
  this approach.
 
   src/qemu/qemu_monitor.c | 8 +++-
   1 file changed, 3 insertions(+), 5 deletions(-)
 
  diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
  index 8927dbb..4342088 100644
  --- a/src/qemu/qemu_monitor.c
  +++ b/src/qemu/qemu_monitor.c
  @@ -4112,11 +4112,9 @@ qemuMonitorGetIOThreads(qemuMonitorPtr mon,
   return -1;
   }
   
  -if (!mon-json) {
  -virReportError(VIR_ERR_OPERATION_UNSUPPORTED, %s,
  -   _(JSON monitor is required));
  -return -1;
  -}
  +/* Requires JSON to make the query */
  +if (!mon-json)
  +return 0;
  
  I think you need should do '*iothreads = NULL' for safety too.
  
 
 OK with the following squashed in?
 
 diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
 index 4342088..10f51c5 100644
 --- a/src/qemu/qemu_monitor.c
 +++ b/src/qemu/qemu_monitor.c
 @@ -4113,8 +4113,10 @@ qemuMonitorGetIOThreads(qemuMonitorPtr mon,
  }
 
  /* Requires JSON to make the query */
 -if (!mon-json)
 +if (!mon-json) {
 +*iothreads = NULL;
  return 0;
 +}
 
  return qemuMonitorJSONGetIOThreads(mon, iothreads);
  }

ACK

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 3/4] audit: remove redundant NULL assignment

2014-09-18 Thread Peter Krempa
On 09/18/14 12:14, Ján Tomko wrote:
 virVasprintf sets the output to NULL on failure.
 ---
  src/util/viraudit.c | 4 +---
  1 file changed, 1 insertion(+), 3 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 1/4] Fix leak in x86UpdateHostModel

2014-09-18 Thread Peter Krempa
On 09/18/14 12:14, Ján Tomko wrote:
 Commit de0aeaf introduced a memory leak.
 ---
  src/cpu/cpu_x86.c | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)
 

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 2/4] Fixes for domains with no iothreads

2014-09-18 Thread Peter Krempa
On 09/18/14 12:14, Ján Tomko wrote:
 Plug a memory leak and silence a warning.
 ---
  src/qemu/qemu_cgroup.c  | 2 +-
  src/qemu/qemu_process.c | 6 --
  2 files changed, 5 insertions(+), 3 deletions(-)
 
ACK




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 4/4] audit: fix memory leak without WITH_AUDIT

2014-09-18 Thread Peter Krempa
On 09/18/14 12:14, Ján Tomko wrote:
 Free str unconditionally since we allocate it without WITH_AUDIT
 too.
 ---
  src/util/viraudit.c | 9 ++---
  1 file changed, 2 insertions(+), 7 deletions(-)
 

ACK




signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] Fix libvirtd crash when removing metadata

2014-09-18 Thread Erik Skultety
When trying to remove nonexistent metadata from XML, libvirt daemon
crashes due to dereferencing NULL pointer.

Resolves https://bugzilla.redhat.com/show_bug.cgi?id=1143955
---
 src/util/virxml.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/util/virxml.c b/src/util/virxml.c
index a91da05..f386956 100644
--- a/src/util/virxml.c
+++ b/src/util/virxml.c
@@ -972,7 +972,9 @@ xmlNodePtr
 virXMLFindChildNodeByNs(xmlNodePtr root,
 const char *uri)
 {
-xmlNodePtr next;
+xmlNodePtr next = NULL;
+if (!root)
+goto cleanup;
 
 for (next = root-children; next; next = next-next) {
 if (next-ns 
@@ -980,6 +982,7 @@ virXMLFindChildNodeByNs(xmlNodePtr root,
 return next;
 }
 
+ cleanup:
 return NULL;
 }
 
-- 
1.9.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH] Fix libvirtd crash when removing metadata

2014-09-18 Thread Peter Krempa
On 09/18/14 14:25, Erik Skultety wrote:
 When trying to remove nonexistent metadata from XML, libvirt daemon
 crashes due to dereferencing NULL pointer.
 
 Resolves https://bugzilla.redhat.com/show_bug.cgi?id=1143955
 ---
  src/util/virxml.c | 5 -
  1 file changed, 4 insertions(+), 1 deletion(-)
 
 diff --git a/src/util/virxml.c b/src/util/virxml.c
 index a91da05..f386956 100644
 --- a/src/util/virxml.c
 +++ b/src/util/virxml.c
 @@ -972,7 +972,9 @@ xmlNodePtr
  virXMLFindChildNodeByNs(xmlNodePtr root,
  const char *uri)
  {
 -xmlNodePtr next;
 +xmlNodePtr next = NULL;
 +if (!root)
 +goto cleanup;

You can return NULL right away

  
  for (next = root-children; next; next = next-next) {
  if (next-ns 
 @@ -980,6 +982,7 @@ virXMLFindChildNodeByNs(xmlNodePtr root,
  return next;
  }
  
 + cleanup:

and don't bother with adding a label.

  return NULL;
  }
  
 

I'll push this shortly without the unnecessary label.

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] maint: clean up _virDomainMemoryStat

2014-09-18 Thread Wang Yufei
Yes, all of these patches below are from me.

On 2014/9/18 1:21, Eric Blake wrote:

 On 09/16/2014 07:19 AM, James wrote:
 I clean up all _virDomainMemoryStat.

 Signed-off-by: James james.wangyu...@huawei.com
 Signed-off-by: Wang Rui moon.wang...@huawei.com
 ---
  daemon/remote.c  | 2 +-
  src/driver.h | 2 +-
  src/lxc/lxc_driver.c | 2 +-
  src/qemu/qemu_driver.c   | 2 +-
  src/qemu/qemu_monitor_text.c | 2 +-
  src/remote/remote_driver.c   | 2 +-
  tools/virsh-domain-monitor.c | 2 +-
  7 files changed, 7 insertions(+), 7 deletions(-)
 
 ACK; same story about the commit cleanup.  I'd also like to push a
 .mailmap entry to consolidate your prior entries; am I correct that all
 of these are from you?
 
 $ git shortlog --author=james.wangyu...@huawei.com
 James (1):
   util: virTimeFieldsThenRaw never returns negative
 
 Wang Yufei (4):
   qemu: Avoid assigning unavailable migration ports
   docs: fix virDomainRestoreFlags description bug
   docs: fix double articles bug
   cgroup: Fix start VMs coincidently failed
 
 Wangyufei (A) (1):
   docs: delete extra character
 
 Wangyufei (James) (1):
   qemuAgentDispose: Reset lastError
 
 
 

 diff --git a/daemon/remote.c b/daemon/remote.c
 index 0ea2815..daa4b60 100644
 --- a/daemon/remote.c
 +++ b/daemon/remote.c
 @@ -1604,7 +1604,7 @@ remoteDispatchDomainMemoryStats(virNetServerPtr server 
 ATTRIBUTE_UNUSED,
  remote_domain_memory_stats_ret *ret)
  {
  virDomainPtr dom = NULL;
 -struct _virDomainMemoryStat *stats = NULL;
 +virDomainMemoryStatPtr stats = NULL;
  int nr_stats;
  size_t i;
  int rv = -1;
 diff --git a/src/driver.h b/src/driver.h
 index 76142bd..bb748c4 100644
 --- a/src/driver.h
 +++ b/src/driver.h
 @@ -535,7 +535,7 @@ typedef int
  
  typedef int
  (*virDrvDomainMemoryStats)(virDomainPtr domain,
 -   struct _virDomainMemoryStat *stats,
 +   virDomainMemoryStatPtr stats,
 unsigned int nr_stats,
 unsigned int flags);
  
 diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
 index 8ab4cf2..c3cd62c 100644
 --- a/src/lxc/lxc_driver.c
 +++ b/src/lxc/lxc_driver.c
 @@ -5387,7 +5387,7 @@ lxcNodeGetInfo(virConnectPtr conn,
  
  static int
  lxcDomainMemoryStats(virDomainPtr dom,
 - struct _virDomainMemoryStat *stats,
 + virDomainMemoryStatPtr stats,
   unsigned int nr_stats,
   unsigned int flags)
  {
 diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
 index 9f5c977..59a4b3c 100644
 --- a/src/qemu/qemu_driver.c
 +++ b/src/qemu/qemu_driver.c
 @@ -10178,7 +10178,7 @@ qemuDomainGetInterfaceParameters(virDomainPtr dom,
  
  static int
  qemuDomainMemoryStats(virDomainPtr dom,
 -  struct _virDomainMemoryStat *stats,
 +  virDomainMemoryStatPtr stats,
unsigned int nr_stats,
unsigned int flags)
  {
 diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c
 index 2bc8261..46d2782 100644
 --- a/src/qemu/qemu_monitor_text.c
 +++ b/src/qemu/qemu_monitor_text.c
 @@ -684,7 +684,7 @@ int qemuMonitorTextGetBalloonInfo(qemuMonitorPtr mon,
  
  if ((offset = strstr(reply, BALLOON_PREFIX)) != NULL) {
  offset += strlen(BALLOON_PREFIX);
 -struct _virDomainMemoryStat stats[1];
 +virDomainMemoryStatStruct stats[1];
  
  if (qemuMonitorParseBalloonInfo(offset, stats, 1) == 0) {
  virReportError(VIR_ERR_INTERNAL_ERROR,
 diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
 index 4a1b04b..75a3a7b 100644
 --- a/src/remote/remote_driver.c
 +++ b/src/remote/remote_driver.c
 @@ -2690,7 +2690,7 @@ remoteDomainGetSchedulerType(virDomainPtr domain, int 
 *nparams)
  
  static int
  remoteDomainMemoryStats(virDomainPtr domain,
 -struct _virDomainMemoryStat *stats,
 +virDomainMemoryStatPtr stats,
  unsigned int nr_stats,
  unsigned int flags)
  {
 diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
 index b2e1fc8..4f6aaa3 100644
 --- a/tools/virsh-domain-monitor.c
 +++ b/tools/virsh-domain-monitor.c
 @@ -295,7 +295,7 @@ cmdDomMemStat(vshControl *ctl, const vshCmd *cmd)
  {
  virDomainPtr dom;
  const char *name;
 -struct _virDomainMemoryStat stats[VIR_DOMAIN_MEMORY_STAT_NR];
 +virDomainMemoryStatStruct stats[VIR_DOMAIN_MEMORY_STAT_NR];
  unsigned int nr_stats;
  size_t i;
  int ret = false;

 



-- 
Best Regards

Wang Yufei

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH] Fix bug with loading bridge name for active domain during libvirtd start

2014-09-18 Thread Pavel Hrdina
If you have a bridge network in running domain and libvirtd is restarted
the information about host bridge interface is lost from live xml.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1140085

Signed-off-by: Pavel Hrdina phrd...@redhat.com
---
 src/conf/domain_conf.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 3ccec1c..fa4166c 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -6837,6 +6837,10 @@ virDomainActualNetDefParseXML(xmlNodePtr node,
 goto error;
 }
 VIR_FREE(class_id);
+} else if (actual-type == VIR_DOMAIN_NET_TYPE_BRIDGE) {
+char *brname = virXPathString(string(./source/@bridge), ctxt);
+if (brname)
+actual-data.bridge.brname = brname;
 }
 
 bandwidth_node = virXPathNode(./bandwidth, ctxt);
-- 
1.8.5.5

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH] libxl: Implement basic video device selection

2014-09-18 Thread Stefan Bader
Re-pushing this as the old thread got rather stale. Some of the
VFB setup went in a bug fix. Not sure I missed a detail in rebasing
bug the keyboard setting may be the only thing missing...

-Stefan

[v2: Check return code of VIR_STRDUP and fix indentation]
[v3: Split out VRAM fixup and return error for unsupported video type]
[v4: Re-arrange code and move VFB setup into libxlMakeVfbList]
[v5: Rebased against head which already had some VFB setup code]

From b3ff8f4c658d29f15e673af88b9ae2fdfa3c1317 Mon Sep 17 00:00:00 2001
From: Stefan Bader stefan.ba...@canonical.com
Date: Thu, 27 Mar 2014 16:01:18 +0100
Subject: [PATCH] libxl: Implement basic video device selection

This started as an investigation into an issue where libvirt (using the
libxl driver) and the Xen host, like an old couple, could not agree on
who is responsible for selecting the VNC port to use.

Things usually (and a bit surprisingly) did work because, just like that
old couple, they had the same idea on what to do by default. However it
was possible that this ended up in a big argument.

The problem is that display information exists in two different places:
in the vfbs list and in the build info. And for launching the device model,
only the latter is used. But that never gets initialized from libvirt. So
Xen allows the device model to select a default port while libvirt thinks
it has told Xen that this is done by libvirt (though the vfbs config).

While fixing that, I made a stab at actually evaluating the configuration
of the video device. So that it is now possible to at least decide between
a Cirrus or standard VGA emulation and to modify the VRAM within certain
limits using libvirt.

Signed-off-by: Stefan Bader stefan.ba...@canonical.com
---
 src/libxl/libxl_conf.c | 50 ++
 1 file changed, 50 insertions(+)

diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index acba69c..2727230 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -1200,6 +1200,8 @@ libxlMakeVfbList(virPortAllocatorPtr graphicsports,
 if (VIR_STRDUP(b_info-u.hvm.sdl.xauthority, vfb.sdl.xauthority)  
0)
 goto error;
 }
+if (VIR_STRDUP(b_info-u.hvm.keymap, vfb.keymap)  0)
+goto error;
 }
 
 return 0;
@@ -1462,6 +1464,45 @@ libxlMakeCapabilities(libxl_ctx *ctx)
 return NULL;
 }
 
+static int
+libxlMakeVideo(virDomainDefPtr def, libxl_domain_config *d_config)
+{
+libxl_domain_build_info *b_info = d_config-b_info;
+
+if (d_config-c_info.type != LIBXL_DOMAIN_TYPE_HVM)
+return 0;
+
+/*
+ * Take the first defined video device (graphics card) to display
+ * on the first graphics device (display).
+ * Right now only type and vram info is used and anything beside
+ * type xen and vga is mapped to cirrus.
+ */
+if (def-nvideos) {
+switch (def-videos[0]-type) {
+case VIR_DOMAIN_VIDEO_TYPE_VGA:
+case VIR_DOMAIN_VIDEO_TYPE_XEN:
+b_info-u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_STD;
+break;
+case VIR_DOMAIN_VIDEO_TYPE_CIRRUS:
+b_info-u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_CIRRUS;
+break;
+default:
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   %s,
+   _(video type not supported by libxl));
+return -1;
+}
+b_info-video_memkb = def-videos[0]-vram ?
+  def-videos[0]-vram :
+  LIBXL_MEMKB_DEFAULT;
+} else {
+libxl_defbool_set(b_info-u.hvm.nographic, 1);
+}
+
+return 0;
+}
+
 int
 libxlBuildDomainConfig(virPortAllocatorPtr graphicsports,
virDomainDefPtr def,
@@ -1488,6 +1529,15 @@ libxlBuildDomainConfig(virPortAllocatorPtr graphicsports,
 if (libxlMakePCIList(def, d_config)  0)
 return -1;
 
+/*
+ * Now that any potential VFBs are defined, it is time to update the
+ * build info with the data of the primary display. Some day libxl
+ * might implicitely do so but as it does not right now, better be
+ * explicit.
+ */
+if (libxlMakeVideo(def, d_config)  0)
+return -1;
+
 d_config-on_reboot = libxlActionFromVirLifecycle(def-onReboot);
 d_config-on_poweroff = libxlActionFromVirLifecycle(def-onPoweroff);
 d_config-on_crash = libxlActionFromVirLifecycleCrash(def-onCrash);
-- 
1.9.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCHv2 1/2] conf: add options for disabling segment offloading

2014-09-18 Thread Ján Tomko
Add options for tuning segment offloading:
driver
  host csum='off' gso='off' tso4='off' tso6='off'
ecn='off' ufo='off'/
  guest csum='off' tso4='off' tso6='off' ecn='off' ufo='off'/
/driver
which control the respective host_ and guest_ properties
of the virtio-net device.
---
 docs/formatdomain.html.in  |  24 ++-
 docs/schemas/domaincommon.rng  |  44 -
 src/conf/domain_conf.c | 218 -
 src/conf/domain_conf.h |  15 ++
 .../qemuxml2argv-net-virtio-disable-offloads.xml   |  35 
 tests/qemuxml2xmltest.c|   1 +
 6 files changed, 329 insertions(+), 8 deletions(-)
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-net-virtio-disable-offloads.xml

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 8c03ebb..5dd70a4 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -3868,7 +3868,11 @@ qemu-kvm -net nic,model=? /dev/null
   lt;source network='default'/gt;
   lt;target dev='vnet1'/gt;
   lt;model type='virtio'/gt;
-  blt;driver name='vhost' txmode='iothread' ioeventfd='on' 
event_idx='off' queues='5'/gt;/b
+  blt;driver name='vhost' txmode='iothread' ioeventfd='on' 
event_idx='off' queues='5'gt;
+lt;host csum='off' gso='off' tso4='off' tso6='off' ecn='off' 
ufo='off'/gt;
+lt;guest csum='off' tso4='off' tso6='off' ecn='off' ufo='off'/gt;
+  lt;/drivergt;
+  /b
 lt;/interfacegt;
   lt;/devicesgt;
   .../pre
@@ -3972,6 +3976,24 @@ qemu-kvm -net nic,model=? /dev/null
 processor, resulting in much higher throughput.
 span class=sinceSince 1.0.6 (QEMU and KVM only)/span
   /dd
+  dtcodehost offloading options/code/dt
+  dd
+The codecsum/code, codegso/code, codetso4/code,
+codetso6/code, codeecn/code, codeufo/code
+attributes with possible values codeon/code
+and codeoff/code can be used to turn host offloads off.
+By default, the supported offloads are enabled by QEMU.
+span class=sinceSince 1.2.9 (QEMU only)/span
+  /dd
+  dtguest offloading options/dt
+  dd
+The codecsum/code, codetso4/code,
+codetso6/code, codeecn/code, codeufo/code
+attributes with possible values codeon/code
+and codeoff/code can be used to turn guest offloads off.
+By default, the supported offloads are enabled by QEMU.
+span class=sinceSince 1.2.9 (QEMU only)/span
+  /dd
 /dl
 
 h5a name=elementsBackendOptionsSetting network backend-specific 
options/a/h5
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 19dc82f..1e00afd 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -2364,7 +2364,49 @@
   /optional
 /group
   /choice
-  empty/
+  interleave
+optional
+  element name='host'
+attribute name='csum'
+  ref name=virOnOff/
+/attribute
+attribute name='gso'
+  ref name=virOnOff/
+/attribute
+attribute name='tso4'
+  ref name=virOnOff/
+/attribute
+attribute name='tso6'
+  ref name=virOnOff/
+/attribute
+attribute name='ecn'
+  ref name=virOnOff/
+/attribute
+attribute name='ufo'
+  ref name=virOnOff/
+/attribute
+  /element
+/optional
+optional
+  element name='guest'
+attribute name='csum'
+  ref name=virOnOff/
+/attribute
+attribute name='tso4'
+  ref name=virOnOff/
+/attribute
+attribute name='tso6'
+  ref name=virOnOff/
+/attribute
+attribute name='ecn'
+  ref name=virOnOff/
+/attribute
+attribute name='ufo'
+  ref name=virOnOff/
+/attribute
+  /element
+/optional
+  /interleave
 /element
   /optional
   optional
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 3ccec1c..cdaafa6 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -6897,6 +6897,7 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
 char *ioeventfd = NULL;
 char *event_idx = NULL;
 char *queues = NULL;
+char *str = NULL;
 char *filter = NULL;
 char *internal = NULL;
 char *devaddr = NULL;
@@ -7385,6 +7386,115 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
 }
 def-driver.virtio.queues = q;
 }
+if ((str = 

[libvirt] [PATCHv2 0/2] Add support for turning off offloading for virtio-net

2014-09-18 Thread Ján Tomko
v2: rework XML to avoid underscores

Ján Tomko (2):
  conf: add options for disabling segment offloading
  qemu: wire up virtio-net segment offloading options

 docs/formatdomain.html.in  |  24 ++-
 docs/schemas/domaincommon.rng  |  44 -
 src/conf/domain_conf.c | 218 -
 src/conf/domain_conf.h |  15 ++
 src/qemu/qemu_command.c|  40 
 .../qemuxml2argv-net-virtio-disable-offloads.args  |  10 +
 .../qemuxml2argv-net-virtio-disable-offloads.xml   |  35 
 tests/qemuxml2argvtest.c   |   2 +
 tests/qemuxml2xmltest.c|   1 +
 9 files changed, 381 insertions(+), 8 deletions(-)
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-net-virtio-disable-offloads.args
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-net-virtio-disable-offloads.xml

-- 
1.8.5.5

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCHv2 2/2] qemu: wire up virtio-net segment offloading options

2014-09-18 Thread Ján Tomko
Format the segment offloading options specified by
driver
  host .../
  guest .../
/driver
on virtio-net command line.
---
 src/qemu/qemu_command.c| 40 ++
 .../qemuxml2argv-net-virtio-disable-offloads.args  | 10 ++
 tests/qemuxml2argvtest.c   |  2 ++
 3 files changed, 52 insertions(+)
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-net-virtio-disable-offloads.args

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index ce320de..ee49db5 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -4438,6 +4438,46 @@ qemuBuildNicDevStr(virDomainDefPtr def,
 virBufferAsprintf(buf, ,event_idx=%s,
   
virTristateSwitchTypeToString(net-driver.virtio.event_idx));
 }
+if (net-driver.virtio.guest.csum) {
+virBufferAsprintf(buf, ,csum=%s,
+  
virTristateSwitchTypeToString(net-driver.virtio.guest.csum));
+}
+if (net-driver.virtio.host.gso) {
+virBufferAsprintf(buf, ,gso=%s,
+  
virTristateSwitchTypeToString(net-driver.virtio.host.gso));
+}
+if (net-driver.virtio.host.tso4) {
+virBufferAsprintf(buf, ,host_tso4=%s,
+  
virTristateSwitchTypeToString(net-driver.virtio.host.tso4));
+}
+if (net-driver.virtio.host.tso6) {
+virBufferAsprintf(buf, ,host_tso6=%s,
+  
virTristateSwitchTypeToString(net-driver.virtio.host.tso6));
+}
+if (net-driver.virtio.host.ecn) {
+virBufferAsprintf(buf, ,host_ecn=%s,
+  
virTristateSwitchTypeToString(net-driver.virtio.host.ecn));
+}
+if (net-driver.virtio.host.ufo) {
+virBufferAsprintf(buf, ,host_ufo=%s,
+  
virTristateSwitchTypeToString(net-driver.virtio.host.ufo));
+}
+if (net-driver.virtio.guest.tso4) {
+virBufferAsprintf(buf, ,guest_tso4=%s,
+  
virTristateSwitchTypeToString(net-driver.virtio.guest.tso4));
+}
+if (net-driver.virtio.guest.tso6) {
+virBufferAsprintf(buf, ,guest_tso6=%s,
+  
virTristateSwitchTypeToString(net-driver.virtio.guest.tso6));
+}
+if (net-driver.virtio.guest.ecn) {
+virBufferAsprintf(buf, ,guest_ecn=%s,
+  
virTristateSwitchTypeToString(net-driver.virtio.guest.ecn));
+}
+if (net-driver.virtio.guest.ufo) {
+virBufferAsprintf(buf, ,guest_ufo=%s,
+  
virTristateSwitchTypeToString(net-driver.virtio.guest.ufo));
+}
 }
 if (usingVirtio  vhostfdSize  1) {
 /* As advised at http://www.linux-kvm.org/page/Multiqueue
diff --git 
a/tests/qemuxml2argvdata/qemuxml2argv-net-virtio-disable-offloads.args 
b/tests/qemuxml2argvdata/qemuxml2argv-net-virtio-disable-offloads.args
new file mode 100644
index 000..3328988
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-net-virtio-disable-offloads.args
@@ -0,0 +1,10 @@
+LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \
+/usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults \
+-monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb \
+-hda /dev/HostVG/QEMUGuest7 \
+-device virtio-net-pci,csum=off,gso=off,\
+host_tso4=off,host_tso6=off,host_ecn=off,host_ufo=off,\
+guest_tso4=off,guest_tso6=off,guest_ecn=off,guest_ufo=off,\
+vlan=0,id=net0,mac=00:22:44:66:88:aa,bus=pci.0,addr=0x3 \
+-net user,vlan=0,name=hostnet0 \
+-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x4
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 275e699..d3da1e9 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -965,6 +965,8 @@ mymain(void)
 DO_TEST(net-virtio, NONE);
 DO_TEST(net-virtio-device,
 QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_VIRTIO_TX_ALG);
+DO_TEST(net-virtio-disable-offloads,
+QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG);
 DO_TEST(net-virtio-netdev,
 QEMU_CAPS_DEVICE, QEMU_CAPS_NETDEV, QEMU_CAPS_NODEFCONFIG);
 DO_TEST(net-virtio-s390,
-- 
1.8.5.5

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH v2] storage: zfs: implement pool build and delete

2014-09-18 Thread Roman Bogorodskiy
  Ján Tomko wrote:

 On 09/14/2014 05:46 AM, Roman Bogorodskiy wrote:
   - Provide an implementation for buildPool and deletePool operations
 for the ZFS storage backend.
   - Add VIR_STORAGE_POOL_SOURCE_DEVICE flag to ZFS pool poolOptions
 as now we can specify devices to build pool from
   - storagepool.rng: add an optional 'sourceinfodev' to 'sourcezfs' and
 add an optional 'target' to 'poolzfs' entity
   - Add a couple of tests to storagepoolxml2xmltest
  ---
   docs/schemas/storagepool.rng   |  6 +++
   src/conf/storage_conf.c|  3 +-
   src/storage/storage_backend_zfs.c  | 57 
  ++
   tests/storagepoolxml2xmlin/pool-zfs-sourcedev.xml  |  8 +++
   tests/storagepoolxml2xmlin/pool-zfs.xml|  7 +++
   tests/storagepoolxml2xmlout/pool-zfs-sourcedev.xml | 19 
   tests/storagepoolxml2xmlout/pool-zfs.xml   | 18 +++
   tests/storagepoolxml2xmltest.c |  2 +
   8 files changed, 119 insertions(+), 1 deletion(-)
   create mode 100644 tests/storagepoolxml2xmlin/pool-zfs-sourcedev.xml
   create mode 100644 tests/storagepoolxml2xmlin/pool-zfs.xml
   create mode 100644 tests/storagepoolxml2xmlout/pool-zfs-sourcedev.xml
   create mode 100644 tests/storagepoolxml2xmlout/pool-zfs.xml
  
 
 ACK

Thanks; pushed.

Roman Bogorodskiy


pgpNylPKMYhRD.pgp
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] docs: update zfs documentation

2014-09-18 Thread Roman Bogorodskiy
  Ján Tomko wrote:

 On 09/14/2014 07:24 AM, Roman Bogorodskiy wrote:
   - docs/formatstorage.html.in: document 'zfs' pool type, add it
 to a list of pool types that could use source physical devices
   - docs/storage.html.in: update a ZFS pool example XML with
 source physical devices, mention that starting from 1.2.9 a
 pool could be created from this devices by libvirt and in earlier
 versions user still has to create a pool manually
   - docs/drvbhyve.html.in: add an example with ZFS pools
  ---
   docs/drvbhyve.html.in  | 16 
   docs/formatstorage.html.in |  7 ---
   docs/storage.html.in   | 18 --
   3 files changed, 32 insertions(+), 9 deletions(-)
 
 ACK

Thanks; pushed.

Roman Bogorodskiy


pgpWkmV2QnpPV.pgp
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] virSecuritySELinuxSetTapFDLabel: Temporarily revert to old behavior

2014-09-18 Thread Michal Privoznik
https://bugzilla.redhat.com/show_bug.cgi?id=1141879

A long time ago I've implemented support for so called multiqueue
net.  The idea was to let guest network traffic be processed by
multiple host CPUs and thus increasing performance. However, this
behavior is enabled by QEMU via special ioctl() iterated over the
all tap FDs passed in by libvirt. Unfortunately, SELinux comes in
and disallows the ioctl() call because the /dev/net/tun has label
system_u:object_r:tun_tap_device_t:s0 and 'attach_queue' ioctl()
is not allowed on tun_tap_device_t type. So after discussion with
a SELinux developer we've decided that the FDs passed to the QEMU
should be labelled with svirt_t type and SELinux policy will
allow the ioctl(). Therefore I've made a patch
(cf976d9dcf4e592261b14f03572) that does exactly this. However,
things are not that easy - even though the API to label FD is
called (fsetfilecon_raw) the underlying file is labelled too! So
effectively we are mangling /dev/net/tun label. Yes, that broke
dozen of other application from openvpn, or boxes, to qemu
running other domains.

The best solution would be if SELinux provides a way to label an
FD only, which could be then labeled when passed to the qemu.
However that's a long path to go and we should fix this
regression AQAP. So I went to talk to the SELinux developer again
and we agreed on temporary solution that:

1) my patch is reverted
2) SELinux temporarily allows 'attach_queue' on the
tun_tap_device_t

Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
 src/security/security_selinux.c | 36 +---
 1 file changed, 33 insertions(+), 3 deletions(-)

diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index 3db2b27..c69eeb9 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -2343,17 +2343,47 @@ virSecuritySELinuxSetImageFDLabel(virSecurityManagerPtr 
mgr ATTRIBUTE_UNUSED,
 }
 
 static int
-virSecuritySELinuxSetTapFDLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
+virSecuritySELinuxSetTapFDLabel(virSecurityManagerPtr mgr,
 virDomainDefPtr def,
 int fd)
 {
+struct stat buf;
+security_context_t fcon = NULL;
 virSecurityLabelDefPtr secdef;
+char *str = NULL;
+int rc = -1;
 
 secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME);
-if (!secdef || !secdef-imagelabel)
+if (!secdef || !secdef-label)
 return 0;
 
-return virSecuritySELinuxFSetFilecon(fd, secdef-imagelabel);
+if (fstat(fd, buf)  0) {
+virReportSystemError(errno, _(cannot stat tap fd %d), fd);
+goto cleanup;
+}
+
+if ((buf.st_mode  S_IFMT) != S_IFCHR) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _(tap fd %d is not character device), fd);
+goto cleanup;
+}
+
+if (getContext(mgr, /dev/tap.*, buf.st_mode, fcon)  0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _(cannot lookup default selinux label for tap fd %d), 
fd);
+goto cleanup;
+}
+
+if (!(str = virSecuritySELinuxContextAddRange(secdef-label, fcon))) {
+goto cleanup;
+} else {
+rc = virSecuritySELinuxFSetFilecon(fd, str);
+}
+
+ cleanup:
+freecon(fcon);
+VIR_FREE(str);
+return rc;
 }
 
 static char *
-- 
1.8.5.5

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] LSN-2014-0004: Querying blkiotune after disk hotplug can lead to libvirtd crash

2014-09-18 Thread Daniel P. Berrange
Libvirt Security Notice: LSN-2014-0004
==

   Summary: Querying blkiotune after disk hotplug can lead to
libvirtd crash
   Reported on: 20140911
  Published on: 20140917
  Fixed on: 20140917
   Reported by: Luyao Huang lhu...@redhat.com
Patched by: Peter Krempa pkre...@redhat.com
  See also: CVE-2014-3633

Description
---

The qemu implementation of virDomainGetBlockIoTune computed an index
into the array of disks for the live definition, then used it as the
index into the array of disks for the persistent definition. If
management had hot-plugged disks to the live definition, the two
arrays are not necessarily the same length, and this could result in
the persistent definition dereferencing an out-of-bounds pointer.

Impact
--

A read-only client can cause a denial of service attack against a
privileged client if the out-of-bounds dereference causes libvirtd
to crash, or possibly gain read access to sensitive information
residing in the heap.

Workaround
--

The out-of-bounds access is only possible on domains that have had
disks hot-plugged or removed from the live image without also
updating the persistent definition to match; keeping the two
definitions matched or using only transient domains will avoid the
problem. Denying access to the readonly libvirt socket will avoid
the potential for a denial of service attack, but will not prevent
the out-of-bounds access from causing a crash for a privileged
client, although such a crash is no longer a security problem.

Affected product


Name: libvirt
  Repository: git://libvirt.org/git/libvirt.git
  http://libvirt.org/git/?p=libvirt.git

  Branch: master
   Broken in: v0.9.8
   Broken in: v0.9.9
   Broken in: v0.9.10
   Broken in: v0.9.11
   Broken in: v0.9.12
   Broken in: v0.9.13
   Broken in: v1.0.0
   Broken in: v1.0.1
   Broken in: v1.0.2
   Broken in: v1.0.3
   Broken in: v1.0.4
   Broken in: v1.0.5
   Broken in: v1.0.6
   Broken in: v1.1.0
   Broken in: v1.1.1
   Broken in: v1.1.2
   Broken in: v1.1.3
   Broken in: v1.1.4
   Broken in: v1.2.0
   Broken in: v1.2.1
   Broken in: v1.2.2
   Broken in: v1.2.3
   Broken in: v1.2.4
   Broken in: v1.2.5
   Broken in: v1.2.6
   Broken in: v1.2.7
   Broken in: v1.2.8
   Broken by: eca96694a7f992be633d48d5ca03cedc9bbc3c9aa
Fixed by: 3e745e8f775dfe6f64f18b5c2fe4791b35d3546b

  Branch: v0.9.11-maint
   Broken in: v0.9.11.1
   Broken in: v0.9.11.2
   Broken in: v0.9.11.3
   Broken in: v0.9.11.4
   Broken in: v0.9.11.5
   Broken in: v0.9.11.6
   Broken in: v0.9.11.7
   Broken in: v0.9.11.8
   Broken in: v0.9.11.9
   Broken in: v0.9.11.10
   Broken by: eca96694a7f992be633d48d5ca03cedc9bbc3c9aa

  Branch: v0.9.12-maint
   Broken in: v0.9.12.1
   Broken in: v0.9.12.2
   Broken in: v0.9.12.3
   Broken by: eca96694a7f992be633d48d5ca03cedc9bbc3c9aa
Fixed by: 750280023cc0896b05f86e292857ceef5eee3a72

  Branch: v0.10.2-maint
   Broken in: v0.10.2.1
   Broken in: v0.10.2.2
   Broken in: v0.10.2.3
   Broken in: v0.10.2.4
   Broken in: v0.10.2.5
   Broken in: v0.10.2.6
   Broken in: v0.10.2.7
   Broken in: v0.10.2.8
   Broken by: eca96694a7f992be633d48d5ca03cedc9bbc3c9aa
Fixed by: 0fa54204f264e3d39387f5762f810d31cce770b2

  Branch: v1.0.2-maint
   Broken by: eca96694a7f992be633d48d5ca03cedc9bbc3c9aa
Fixed by: d30fea03a545a2d9f5f228cd3292484ce7850256

  Branch: v1.0.3-maint
   Broken by: eca96694a7f992be633d48d5ca03cedc9bbc3c9aa
Fixed by: 35a802639d713054503f7243e39be0503fe19ec3

  Branch: v1.0.4-maint
   Broken by: eca96694a7f992be633d48d5ca03cedc9bbc3c9aa
Fixed by: a45c8466fa3531d35728575a1facc0406f97079a

  Branch: v1.0.5-maint
   Broken in: v1.0.5.1
   Broken in: v1.0.5.2
   Broken in: v1.0.5.3
   Broken in: v1.0.5.4
   Broken in: v1.0.5.5
   Broken in: v1.0.5.6
   Broken in: v1.0.5.7
   Broken in: v1.0.5.8
   Broken in: v1.0.5.9
   Broken by: eca96694a7f992be633d48d5ca03cedc9bbc3c9aa
Fixed by: cc05c6d5d2f7a577a1a365fbc5451fb6b5f57445

  Branch: v1.0.6-maint
   Broken by: eca96694a7f992be633d48d5ca03cedc9bbc3c9aa
Fixed by: cc19d1c08f49acdcfd5eb0e26561ea88e800f177

  Branch: v1.1.0-maint
   Broken by: eca96694a7f992be633d48d5ca03cedc9bbc3c9aa
Fixed by: dd8a348e4747a59c60991f3b41567ab0a1dcca0e

  Branch: v1.1.1-maint
   Broken by: eca96694a7f992be633d48d5ca03cedc9bbc3c9aa
Fixed by: ed071fee073bc5a439ec64f0e501d5f90c41dec5

  Branch: v1.1.2-maint
   Broken by: eca96694a7f992be633d48d5ca03cedc9bbc3c9aa
Fixed by: d4360edd1ca88cb1f144bf77f7df23ebf1f90632

  Branch: v1.1.3-maint
   Broken in: v1.1.3.1
   Broken in: v1.1.3.2
   Broken in: v1.1.3.3
   Broken in: v1.1.3.4
   Broken in: v1.1.3.5
   Broken in: v1.1.3.6
   Broken by: eca96694a7f992be633d48d5ca03cedc9bbc3c9aa
Fixed by: eefe2e013820a76dfe5132431db72aade911eeab

  Branch: v1.1.4-maint
   Broken by: 

Re: [libvirt] retiring v0.9.6-maint

2014-09-18 Thread Eric Blake
On 09/18/2014 02:36 AM, Daniel P. Berrange wrote:
 On Wed, Sep 17, 2014 at 04:24:07PM -0600, Eric Blake wrote:
 Any objections to retiring the v0.9.6-maint branch?  After all, we have
 already retired the v0.9.11-maint branch
 (http://libvirt.org/git/?p=libvirt.git;a=commit;h=cd0d348ed), and the
 only activity on v0.9.6-maint since 0.9.6.4 was released in January 2013
 was the backport of a single CVE fix.  The branch no longer builds
 cleanly on Fedora 20, and while I could identify patches to backport to
 fix the build situation, it's not worth my time if we can just retire
 the branch.
 
 FWIW, I'm not really a fan of deleting the branches. Is there any harm
 to just leaving it there idle ?

The branches aren't deleted, per se, just a new commit added on top of
the branch that declares the intent.  For example, all you see if you
check out v0.9.11-maint is this README file:

http://libvirt.org/git/?p=libvirt.git;a=blob;f=README;h=68aeed1ae7d131661f2ba07eff1b4ae16ac4f3b8;hb=cd0d348ed

The branch would still usable by checking out v0.9.11-maint^ as a
detached head, so the history is still there.  All I'm proposing is
documenting that we aren't going to try and port security fixes to the
branch any longer, because no one appears to be actively using it.

-- 
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] retiring v0.9.6-maint

2014-09-18 Thread Daniel P. Berrange
On Thu, Sep 18, 2014 at 09:15:10AM -0600, Eric Blake wrote:
 On 09/18/2014 02:36 AM, Daniel P. Berrange wrote:
  On Wed, Sep 17, 2014 at 04:24:07PM -0600, Eric Blake wrote:
  Any objections to retiring the v0.9.6-maint branch?  After all, we have
  already retired the v0.9.11-maint branch
  (http://libvirt.org/git/?p=libvirt.git;a=commit;h=cd0d348ed), and the
  only activity on v0.9.6-maint since 0.9.6.4 was released in January 2013
  was the backport of a single CVE fix.  The branch no longer builds
  cleanly on Fedora 20, and while I could identify patches to backport to
  fix the build situation, it's not worth my time if we can just retire
  the branch.
  
  FWIW, I'm not really a fan of deleting the branches. Is there any harm
  to just leaving it there idle ?
 
 The branches aren't deleted, per se, just a new commit added on top of
 the branch that declares the intent.  For example, all you see if you
 check out v0.9.11-maint is this README file:
 
 http://libvirt.org/git/?p=libvirt.git;a=blob;f=README;h=68aeed1ae7d131661f2ba07eff1b4ae16ac4f3b8;hb=cd0d348ed
 
 The branch would still usable by checking out v0.9.11-maint^ as a
 detached head, so the history is still there.  All I'm proposing is
 documenting that we aren't going to try and port security fixes to the
 branch any longer, because no one appears to be actively using it.

Ah, Ok, that seems fine.


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] qemu_capabilities: fix issue with discarding old capabilities

2014-09-18 Thread Pavel Hrdina

On 09/15/2014 11:49 AM, Daniel P. Berrange wrote:

On Mon, Sep 15, 2014 at 11:43:10AM +0200, Pavel Hrdina wrote:

On 09/15/2014 11:24 AM, Daniel P. Berrange wrote:

On Fri, Sep 12, 2014 at 06:42:08PM +0200, Pavel Hrdina wrote:

On 09/12/2014 06:25 PM, Daniel P. Berrange wrote:

On Fri, Sep 12, 2014 at 06:10:44PM +0200, Pavel Hrdina wrote:

There was a bug that if libvirtd binary has been updated than the
capability file wasn't reloaded therefore new capabilities introduced
in libvirt cannot be used because the cached version was loaded.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1135431


That bug is all about FIPS support.


Yes it's about FIPS support but it's already in libvirt. I've tested it
and actually by removing cached file to force detect new capabilities and
after that it worked.

Now I realized that even checking the selfctime during start of libvirtd
isn't sufficient because you can enable the FIPS support for kenrel without
updating the libvirtd binary.


Ah, so the actual bug is that the capabilities we detect have a dependancy
on (libvirtd binary, qemu binary, sysfs/procfs settings). It is pretty
difficult to deal with sysfs/procfs chances  caching here, since there's
no way I know to detect when sysfs/procfs settings change.


Yes, that's the real bug and I also didn't realize that at first.

There is however one more think I'm not sure about. I didn't find any place
where we are discarding old capabilities if the libvirtd binary has been
changed. The only check for that update is in function
virQEMUCapsInitCached and its called only from virQEMUCapsNewForBinary
and this function is called only if there is no cached caps or the qemu
binary has changed. See the virQEMUCapsCacheLookup.

So it seems that there is also a bug that we don't check on libvirtd start
if there was an update of that binary.


When libvirtd starts up the cache will be empty. So virQEMUCapsCacheLookup
will always call virQEMUCapsNewForBinary which will call virQEMUCapsInitCached.
So it will always check timestamps on startup.


Well, that's not true. The cache file survive libvirtd stop/start and if 
there is existing cache file, it will be dropped only if qemu binary has 
been changed.





I wouldn't want to check the sysfs/procfs settings every time. Perhaps it
would suffice to just do a check on sysfs/procfs when libvirtd starts up,
so we can say that if you change FIPS sysfs settings you must restart
libvirtd ?


I think that would be good enough.


The difficultly though is figuring out whehther the files have changed. For
this, we'd need to record what the original sysfs/procfs values were in the
caps cache, so we then have something to compare.

A completely different way of looking at this problem would be to say that
the virQEMUCapsPtr should *only* reflect settings that are related to the
QEMU binary capabilities. ie sysfs/procs should not be allowed to influence
the capabilities flags. This is kind of what was originally assumed for this
data.

I wonder how many of our capability flags are set based on data that is
not from the QEMU binary ?  If we can elminate that, then the caching problem
will go away.


It seems that the FIPS is the only case that shouldn't be in qemuCaps. 
I'll create a new patch where I'll move that code directly to qemu_command.


Pavel



Regards,
Daniel



--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] retiring v0.9.6-maint

2014-09-18 Thread Eric Blake
On 09/18/2014 09:22 AM, Daniel P. Berrange wrote:
 On Thu, Sep 18, 2014 at 09:15:10AM -0600, Eric Blake wrote:
 On 09/18/2014 02:36 AM, Daniel P. Berrange wrote:
 On Wed, Sep 17, 2014 at 04:24:07PM -0600, Eric Blake wrote:
 Any objections to retiring the v0.9.6-maint branch?  After all, we have
 already retired the v0.9.11-maint branch

 The branch would still usable by checking out v0.9.11-maint^ as a
 detached head, so the history is still there.  All I'm proposing is
 documenting that we aren't going to try and port security fixes to the
 branch any longer, because no one appears to be actively using it.
 
 Ah, Ok, that seems fine.

Done; v0.9.6-maint still exists, but with a README file documenting that
we aren't going to backport any further fixes here.

-- 
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] Move the FIPS detection from capabilities

2014-09-18 Thread Pavel Hrdina
We are not detecting the presence of FIPS from QEMU, but from procfs and
that means it's not QEMU capability. It was decided that we will pass
this flag to QEMU even if it's not supported by old QEMU binaries.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1135431

Signed-off-by: Pavel Hrdina phrd...@redhat.com
---

Note: The original bug is that we are not detecting whether libvirtd
binary has been updated, we detect that only for QEMU binary. So you
could update libvirt without updating QEMU and new capabilities that could
already exists in QEMU, but was recently implemented in libvirt wasn't
enabled. I'll post a patch to fix this bug.

 src/qemu/qemu_capabilities.c | 24 
 src/qemu/qemu_command.c  | 25 +++--
 2 files changed, 23 insertions(+), 26 deletions(-)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 9246813..5c3778d 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -3215,30 +3215,6 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps,
 config.data.nix.path = monpath;
 config.data.nix.listen = false;
 
-/* Qemu 1.2 and later have a binary flag -enable-fips that must be
- * used for VNC auth to obey FIPS settings; but the flag only
- * exists on Linux, and with no way to probe for it via QMP.  Our
- * solution: if FIPS mode is required, then unconditionally use
- * the flag, regardless of qemu version, for the following matrix:
- *
- *  old QEMUnew QEMU
- * FIPS enabled doesn't start   VNC auth disabled
- * FIPS disabled/missingVNC auth enabledVNC auth enabled
- *
- * Setting the flag here instead of in virQEMUCapsInitQMPMonitor
- * or virQEMUCapsInitHelp also allows the testsuite to be
- * independent of FIPS setting.
- */
-if (virFileExists(/proc/sys/crypto/fips_enabled)) {
-char *buf = NULL;
-
-if (virFileReadAll(/proc/sys/crypto/fips_enabled, 10, buf)  0)
-goto cleanup;
-if (STREQ(buf, 1\n))
-virQEMUCapsSet(qemuCaps, QEMU_CAPS_ENABLE_FIPS);
-VIR_FREE(buf);
-}
-
 VIR_DEBUG(Try to get caps via QMP qemuCaps=%p, qemuCaps);
 
 /*
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index f2e6e5a..3532518 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -7656,8 +7656,29 @@ qemuBuildCommandLine(virConnectPtr conn,
 if (!standalone)
 virCommandAddArg(cmd, -S); /* freeze CPU */
 
-if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_ENABLE_FIPS))
-virCommandAddArg(cmd, -enable-fips);
+/* Qemu 1.2 and later have a binary flag -enable-fips that must be
+ * used for VNC auth to obey FIPS settings; but the flag only
+ * exists on Linux, and with no way to probe for it via QMP.  Our
+ * solution: if FIPS mode is required, then unconditionally use
+ * the flag, regardless of qemu version, for the following matrix:
+ *
+ *  old QEMUnew QEMU
+ * FIPS enabled doesn't start   VNC auth disabled
+ * FIPS disabled/missingVNC auth enabledVNC auth enabled
+ *
+ * Setting the flag here instead of in virQEMUCapsInitQMPMonitor
+ * or virQEMUCapsInitHelp also allows the testsuite to be
+ * independent of FIPS setting.
+ */
+if (virFileExists(/proc/sys/crypto/fips_enabled)) {
+char *buf = NULL;
+
+if (virFileReadAll(/proc/sys/crypto/fips_enabled, 10, buf)  0)
+goto error;
+if (STREQ(buf, 1\n))
+virCommandAddArg(cmd, -enable-fips);
+VIR_FREE(buf);
+}
 
 if (qemuBuildMachineArgStr(cmd, def, qemuCaps)  0)
 goto error;
-- 
1.8.5.5

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [virt-tools-list] [PATCH virt-manager] details: Add UI for enabling UEFI via 'customize before install'

2014-09-18 Thread Eric Blake
[adding libvirt]

On 09/18/2014 06:28 AM, Cole Robinson wrote:

 - Say you are connecting from a new libvirt UNDEFINE_NVRAM support (say Fedora
 21 GA), to an old libvirt without it, like F20 or RHEL7.0. If we specify the
 flag unconditionally, the undefineFlags call will fail, which also means that
 we lose the benefit of the other UNDEFINE flags. So if the VM had managedsave
 data, the undefine will fail saying we need to remove it first.
 
 - Not all drivers support the UNDEFINE_NVRAM flag, like the 'test' driver
 which we use heavily for virt-manager development. If we pass the flag
 unconditionally, then the current code loses the benefit of all UNDEFINE
 flags, and trying to delete a VM with managedsave data will fail.
 
 The existing code has similar problems as well though, since we lump all the
 UNDEFINE flags together, so it certainly isn't perfect. However my hack of
 checking for nvram in the XML avoids some of the common issues, so I'll extend
 it with the logic you pointed out.
 
 Trying to figure out if an API or flag is supported with an arbitrary libvirt
 connection is a pain: it's a factor of host libvirt version, host
 libvirt-python version, remote libvirt version, libvirt hv driver, hv version.
 Some readonly APIs you can get away with just testing first, but undefine
 isn't one of them :) Maybe we can get a libvirt API exposing the driver
 function table at least?

Hmm, I've been thinking about that too.  It would indeed be nice to
introspect what APIs are available, and for those APIs with a flags
parameter, what flag(s) bits are supported.  What signature would we
want?  Thinking aloud here...

Maybe:
struct virAPI {
const char *name;/* Name of the C API call */
unsigned int flags;  /* The flag bits that are understood, if the
C API includes a flags argument */
};
/* Allocate a list of supported API information into *list, and return
the length of the list.  User must call free() on the array when done.
flags is 0 for now */
int virConnectListAPI(virConnectPtr conn, virAPIPtr *list,
  unsigned int flags);

Or even having an enum mapping of C API names, instead of passing around
char*.  But the enum would not quite match the RPC call numbers.

Implementation-wise, we've got a lot of driver calls that manually call
virCheckFlags() at the front; maybe what we would do is modify the
driver.h callback list to have a struct that contains both a callback
pointer AND a flags value, rather than the current use of just a
callback pointer; then we can make the rejection of unsupported flags in
common code instead of manual virCheckFlags() everywhere, and we could
also use the population of that registration as our point of
documentation for when support for given bits in the flags argument is
added.

There's still the question of how dynamic this can be - for example,
some qemu driver APIs accept a flag in name, but then fail if the
underlying qemu binary is too old.  Is it sufficient to document that
the API knows about the flag, or do we really want the more complex
solution of getting the exact subset of flags that have a chance of
succeeding based on the underlying qemu binary?  I'm leaning towards
having the introspection API just report a flag if the driver plans on
handling it; and in the cases where the underlying binary can cause the
flag to succeed or fail, we can then use virConnectGetDomainCapabilities
to advertise that sort of runtime difference (in other words, the API
I'm thinking about adding is static and won't change answers based on a
qemu upgrade; anything that depends on the actual running qemu affects
is reflected through existing API).

What about APIs that accept more than just a flags argument, such as the
recent virConnectGetAllDomainStats, which as both a stats and flags
parameter?  Should the virAPI return struct contain a virTypedParameters
or similar thing that can let us describe multiple details about a given
API?

-- 
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] Move the FIPS detection from capabilities

2014-09-18 Thread Eric Blake
On 09/18/2014 09:52 AM, Pavel Hrdina wrote:
 We are not detecting the presence of FIPS from QEMU, but from procfs and
 that means it's not QEMU capability. It was decided that we will pass
 this flag to QEMU even if it's not supported by old QEMU binaries.
 
 Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1135431
 
 Signed-off-by: Pavel Hrdina phrd...@redhat.com
 ---
 
 Note: The original bug is that we are not detecting whether libvirtd
 binary has been updated, we detect that only for QEMU binary. So you
 could update libvirt without updating QEMU and new capabilities that could
 already exists in QEMU, but was recently implemented in libvirt wasn't
 enabled. I'll post a patch to fix this bug.
 
  src/qemu/qemu_capabilities.c | 24 
  src/qemu/qemu_command.c  | 25 +++--
  2 files changed, 23 insertions(+), 26 deletions(-)
 

I agree with moving the detection of the bit out of cached capability
bit and delaying it instead to qemu startup time (although it means a
stat() call for every qemu spawned, instead of the former behavior of
checking only once).  I also agree with the way you removed setting the
bit in qemu_capabilities.c but not the bit itself (the bit is still
defined and named from qemu_capabilities.h, so that if we upgrade
libvirtd and parse the XML for a running qemu domain that was started
from earlier libvirt, we don't mis-behave when seeing that capability
bit, even if we no longer use it for anything).

However, I still think you need a v2:

 +++ b/src/qemu/qemu_command.c
 @@ -7656,8 +7656,29 @@ qemuBuildCommandLine(virConnectPtr conn,
  if (!standalone)
  virCommandAddArg(cmd, -S); /* freeze CPU */
  
 -if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_ENABLE_FIPS))
 -virCommandAddArg(cmd, -enable-fips);
 +/* Qemu 1.2 and later have a binary flag -enable-fips that must be
 + * used for VNC auth to obey FIPS settings; but the flag only
 + * exists on Linux, and with no way to probe for it via QMP.  Our
 + * solution: if FIPS mode is required, then unconditionally use
 + * the flag, regardless of qemu version, for the following matrix:
 + *
 + *  old QEMUnew QEMU
 + * FIPS enabled doesn't start   VNC auth disabled
 + * FIPS disabled/missingVNC auth enabledVNC auth enabled
 + *
 + * Setting the flag here instead of in virQEMUCapsInitQMPMonitor
 + * or virQEMUCapsInitHelp also allows the testsuite to be
 + * independent of FIPS setting.
 + */
 +if (virFileExists(/proc/sys/crypto/fips_enabled)) {

Ouch.  This will make our testsuite differ based on whether it is run on
Linux in FIPS mode (where FIPS might exist) or on any other setup.  I
think you need to hoist the check for virFileExists() to the caller, and
pass in the result as a new bool parameter to this function, so that the
testsuite has full control over the boolean without regards to the
current system's level of FIPS support.

-- 
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] virSecuritySELinuxSetTapFDLabel: Temporarily revert to old behavior

2014-09-18 Thread Cole Robinson
On 09/18/2014 10:20 AM, Michal Privoznik wrote:
 https://bugzilla.redhat.com/show_bug.cgi?id=1141879
 
 A long time ago I've implemented support for so called multiqueue
 net.  The idea was to let guest network traffic be processed by
 multiple host CPUs and thus increasing performance. However, this
 behavior is enabled by QEMU via special ioctl() iterated over the
 all tap FDs passed in by libvirt. Unfortunately, SELinux comes in
 and disallows the ioctl() call because the /dev/net/tun has label
 system_u:object_r:tun_tap_device_t:s0 and 'attach_queue' ioctl()
 is not allowed on tun_tap_device_t type. So after discussion with
 a SELinux developer we've decided that the FDs passed to the QEMU
 should be labelled with svirt_t type and SELinux policy will
 allow the ioctl(). Therefore I've made a patch
 (cf976d9dcf4e592261b14f03572) that does exactly this. However,
 things are not that easy - even though the API to label FD is
 called (fsetfilecon_raw) the underlying file is labelled too! So
 effectively we are mangling /dev/net/tun label. Yes, that broke
 dozen of other application from openvpn, or boxes, to qemu
 running other domains.
 
 The best solution would be if SELinux provides a way to label an
 FD only, which could be then labeled when passed to the qemu.
 However that's a long path to go and we should fix this
 regression AQAP. So I went to talk to the SELinux developer again
 and we agreed on temporary solution that:
 
 1) my patch is reverted
 2) SELinux temporarily allows 'attach_queue' on the
 tun_tap_device_t
 
 Signed-off-by: Michal Privoznik mpriv...@redhat.com

ACK

- Cole

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH] virSecuritySELinuxSetTapFDLabel: Temporarily revert to old behavior

2014-09-18 Thread John Ferlan


On 09/18/2014 10:20 AM, Michal Privoznik wrote:
 https://bugzilla.redhat.com/show_bug.cgi?id=1141879
 
 A long time ago I've implemented support for so called multiqueue
 net.  The idea was to let guest network traffic be processed by
 multiple host CPUs and thus increasing performance. However, this
 behavior is enabled by QEMU via special ioctl() iterated over the
 all tap FDs passed in by libvirt. Unfortunately, SELinux comes in
 and disallows the ioctl() call because the /dev/net/tun has label
 system_u:object_r:tun_tap_device_t:s0 and 'attach_queue' ioctl()
 is not allowed on tun_tap_device_t type. So after discussion with
 a SELinux developer we've decided that the FDs passed to the QEMU
 should be labelled with svirt_t type and SELinux policy will
 allow the ioctl(). Therefore I've made a patch
 (cf976d9dcf4e592261b14f03572) that does exactly this. However,
 things are not that easy - even though the API to label FD is
 called (fsetfilecon_raw) the underlying file is labelled too! So
 effectively we are mangling /dev/net/tun label. Yes, that broke
 dozen of other application from openvpn, or boxes, to qemu
 running other domains.
 
 The best solution would be if SELinux provides a way to label an
 FD only, which could be then labeled when passed to the qemu.
 However that's a long path to go and we should fix this
 regression AQAP. So I went to talk to the SELinux developer again
 and we agreed on temporary solution that:
 
 1) my patch is reverted
 2) SELinux temporarily allows 'attach_queue' on the
 tun_tap_device_t
 
 Signed-off-by: Michal Privoznik mpriv...@redhat.com
 ---
  src/security/security_selinux.c | 36 +---
  1 file changed, 33 insertions(+), 3 deletions(-)
 

Probably should note that this also reverts

a4431931393aeb1ac5893f121151fa3df4fde612   (in 1.2.8)

and

b635b7a1af0e64754016d758376f382470bc11e7   (post 1.2.8)

Although neither really matters with this change - it just points out
the trail for the secdef-imagelabel - secdef-label change that
isn't present in cf976d...

ACK

John

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH] Move the FIPS detection from capabilities

2014-09-18 Thread Pavel Hrdina

On 09/18/2014 06:29 PM, Eric Blake wrote:

On 09/18/2014 09:52 AM, Pavel Hrdina wrote:

We are not detecting the presence of FIPS from QEMU, but from procfs and
that means it's not QEMU capability. It was decided that we will pass
this flag to QEMU even if it's not supported by old QEMU binaries.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1135431

Signed-off-by: Pavel Hrdina phrd...@redhat.com
---

Note: The original bug is that we are not detecting whether libvirtd
binary has been updated, we detect that only for QEMU binary. So you
could update libvirt without updating QEMU and new capabilities that could
already exists in QEMU, but was recently implemented in libvirt wasn't
enabled. I'll post a patch to fix this bug.

  src/qemu/qemu_capabilities.c | 24 
  src/qemu/qemu_command.c  | 25 +++--
  2 files changed, 23 insertions(+), 26 deletions(-)



I agree with moving the detection of the bit out of cached capability
bit and delaying it instead to qemu startup time (although it means a
stat() call for every qemu spawned, instead of the former behavior of
checking only once).  I also agree with the way you removed setting the
bit in qemu_capabilities.c but not the bit itself (the bit is still
defined and named from qemu_capabilities.h, so that if we upgrade
libvirtd and parse the XML for a running qemu domain that was started
from earlier libvirt, we don't mis-behave when seeing that capability
bit, even if we no longer use it for anything).

However, I still think you need a v2:


+++ b/src/qemu/qemu_command.c
@@ -7656,8 +7656,29 @@ qemuBuildCommandLine(virConnectPtr conn,
  if (!standalone)
  virCommandAddArg(cmd, -S); /* freeze CPU */

-if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_ENABLE_FIPS))
-virCommandAddArg(cmd, -enable-fips);
+/* Qemu 1.2 and later have a binary flag -enable-fips that must be
+ * used for VNC auth to obey FIPS settings; but the flag only
+ * exists on Linux, and with no way to probe for it via QMP.  Our
+ * solution: if FIPS mode is required, then unconditionally use
+ * the flag, regardless of qemu version, for the following matrix:
+ *
+ *  old QEMUnew QEMU
+ * FIPS enabled doesn't start   VNC auth disabled
+ * FIPS disabled/missingVNC auth enabledVNC auth enabled
+ *
+ * Setting the flag here instead of in virQEMUCapsInitQMPMonitor
+ * or virQEMUCapsInitHelp also allows the testsuite to be
+ * independent of FIPS setting.
+ */
+if (virFileExists(/proc/sys/crypto/fips_enabled)) {


Ouch.  This will make our testsuite differ based on whether it is run on
Linux in FIPS mode (where FIPS might exist) or on any other setup.  I
think you need to hoist the check for virFileExists() to the caller, and
pass in the result as a new bool parameter to this function, so that the
testsuite has full control over the boolean without regards to the
current system's level of FIPS support.



Sigh, that's right. I've completely forget about the tests. And that 
reminds me whether we should also revert the test for enable-fips as it 
seems to be pointless?


Pavel

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH 3/3] domain_conf: Process the rawio for a hostdev device

2014-09-18 Thread Michal Privoznik

On 10.09.2014 01:40, John Ferlan wrote:

Add a rawio to the hostdev XML and process it mimicing the
disk XML for a lun which supports/requires rawio

Signed-off-by: John Ferlan jfer...@redhat.com
---
  docs/formatdomain.html.in  | 12 ++--
  docs/schemas/domaincommon.rng  | 18 +++
  src/conf/domain_conf.c | 31 +++
  .../qemuxml2argv-hostdev-scsi-rawio.xml| 35 ++
  tests/qemuxml2xmltest.c|  1 +
  5 files changed, 88 insertions(+), 9 deletions(-)
  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-rawio.xml

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 94236dd..07640af 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -1774,7 +1774,7 @@
dtcoderawio/code attribute
span class=sincesince 0.9.10/span/dt
  dd
-Indicates whether the disk is needs rawio capability; valid
+Indicates whether the disk needs rawio capability. Valid
  settings are yes or no (default is no). If any one disk
  in a domain has rawio='yes', rawio capability will be enabled
  for all disks in the domain (because, in the case of QEMU, this
@@ -2884,7 +2884,7 @@
  pre
...
lt;devicesgt;
-lt;hostdev mode='subsystem' type='scsi'gt;
+lt;hostdev mode='subsystem' type='scsi' sgio='filtered' rawio='yes'gt;
lt;sourcegt;
  lt;adapter name='scsi_host0'/gt;
  lt;address type='scsi' bus='0' target='0' unit='0'/gt;
@@ -2943,7 +2943,13 @@
  (span class=sincesince 1.0.6/span) attribute indicates
  whether the kernel will filter unprivileged SG_IO commands for
  the disk, valid settings are filtered or unfiltered.
-The default is filtered.
+The default is filtered. The optional coderawio/code
+(span class=sincesince 1.2.9/span) attribute indicates
+whether the lun needs the rawio capability. Valid settings are
+yes or no. See the rawio description within the
+a href=#elementsDisksdisk/a section.
+If a disk lun in the domain already has the rawio capability,
+then this setting not required.
/dd
  /dl
/dd
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index cedceae..84f0b28 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -1224,12 +1224,7 @@
  /choice
/attribute
optional
-attribute name=rawio
-  choice
-valueyes/value
-valueno/value
-  /choice
-/attribute
+ref name=rawIO/
/optional
optional
  attribute name=sgio
@@ -3608,6 +3603,9 @@
  /choice
/attribute
  /optional
+optional
+  ref name=rawIO/
+/optional
  element name=source
choice
  group  !-- scsi_host adapter --
@@ -4972,4 +4970,12 @@
/optional
  /element
/define
+  define name=rawIO
+attribute name=rawio
+  choice
+valueyes/value
+valueno/value
+  /choice


or instead of enumerating the possible values just:
ref name=virYesNo/


+/attribute
+  /define
  /grammar
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index aac78a6..c1f23bc 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -4482,6 +4482,7 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr node,
  xmlNodePtr sourcenode;
  char *managed = NULL;
  char *sgio = NULL;
+char *rawio = NULL;
  char *backendStr = NULL;
  int backend;
  int ret = -1;
@@ -4499,6 +4500,7 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr node,
  }

  sgio = virXMLPropString(node, sgio);
+rawio = virXMLPropString(node, rawio);

  /* @type is passed in from the caller rather than read from the
   * xml document, because it is specified in different places for
@@ -4550,6 +4552,26 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr node,
  }
  }

+if (rawio) {
+if (def-source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI) {
+virReportError(VIR_ERR_XML_ERROR, %s,
+   _(rawio is only supported for scsi host device));
+goto error;
+}
+
+scsisrc-rawio_specified = true;
+if (STREQ(rawio, yes)) {
+scsisrc-rawio = 1;
+} else if (STREQ(rawio, no)) {
+scsisrc-rawio = 0;
+} else {
+virReportError(VIR_ERR_XML_ERROR,
+   _(unknown hostdev rawio setting '%s'),
+   rawio);
+goto error;
+}


You can save a few lines if you use the virTristateBoolTypeFromString().


+}
+
 

Re: [libvirt] [PATCH 2/3] qemu: Process the hostdev rawio setting

2014-09-18 Thread Michal Privoznik

On 10.09.2014 01:40, John Ferlan wrote:

Mimic the Disk processing for 'rawio', but for a scsi_host hostdev
lun device.

Signed-off-by: John Ferlan jfer...@redhat.com
---
  src/qemu/qemu_domain.c  | 21 +
  src/qemu/qemu_domain.h  |  4 
  src/qemu/qemu_driver.c  |  1 +
  src/qemu/qemu_process.c | 20 +++-
  4 files changed, 45 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 306ff10..166fadb 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1715,6 +1715,10 @@ void qemuDomainObjCheckTaint(virQEMUDriverPtr driver,
  for (i = 0; i  obj-def-ndisks; i++)
  qemuDomainObjCheckDiskTaint(driver, obj, obj-def-disks[i], logFD);

+for (i = 0; i  obj-def-nhostdevs; i++)
+qemuDomainObjCheckHostdevTaint(driver, obj, obj-def-hostdevs[i],
+   logFD);
+
  for (i = 0; i  obj-def-nnets; i++)
  qemuDomainObjCheckNetTaint(driver, obj, obj-def-nets[i], logFD);

@@ -1741,6 +1745,23 @@ void qemuDomainObjCheckDiskTaint(virQEMUDriverPtr driver,
  }


+void qemuDomainObjCheckHostdevTaint(virQEMUDriverPtr driver,
+virDomainObjPtr obj,
+virDomainHostdevDefPtr hostdev,
+int logFD)
+{
+virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);


This @cfg is not used anywhere in the function. Well, technically is - 
it's unref'd but besides that, it isn't. Is it a leftover from previous 
code of yours that you were testing?



+virDomainHostdevSubsysSCSIPtr scsisrc = hostdev-source.subsys.u.scsi;
+
+if (hostdev-source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI 
+scsisrc-rawio == 1)


If you follow my advice from 1/3 then this would be 
s/1/VIR_TRISTATE_BOOL_YES/



+qemuDomainObjTaint(driver, obj, VIR_DOMAIN_TAINT_HIGH_PRIVILEGES,
+   logFD);
+
+virObjectUnref(cfg);
+}
+
+
  void qemuDomainObjCheckNetTaint(virQEMUDriverPtr driver,
  virDomainObjPtr obj,
  virDomainNetDefPtr net,
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index f353d90..7aebb0f 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -286,6 +286,10 @@ void qemuDomainObjCheckDiskTaint(virQEMUDriverPtr driver,
   virDomainObjPtr obj,
   virDomainDiskDefPtr disk,
   int logFD);
+void qemuDomainObjCheckHostdevTaint(virQEMUDriverPtr driver,
+virDomainObjPtr obj,
+virDomainHostdevDefPtr disk,
+int logFD);
  void qemuDomainObjCheckNetTaint(virQEMUDriverPtr driver,
  virDomainObjPtr obj,
  virDomainNetDefPtr net,
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index d724eeb..78ecb3e 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -6548,6 +6548,7 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm,
  break;

  case VIR_DOMAIN_DEVICE_HOSTDEV:
+qemuDomainObjCheckHostdevTaint(driver, vm, dev-data.hostdev, -1);
  ret = qemuDomainAttachHostDevice(dom-conn, driver, vm,
   dev-data.hostdev);
  if (!ret)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index b1d8a32..3544716 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3756,6 +3756,7 @@ int qemuProcessStart(virConnectPtr conn,
  struct qemuProcessHookData hookData;
  unsigned long cur_balloon;
  size_t i;
+bool rawio_set = false;
  char *nodeset = NULL;
  virBitmapPtr nodemask = NULL;
  unsigned int stop_flags;
@@ -4122,13 +4123,15 @@ int qemuProcessStart(virConnectPtr conn,
  virDomainDeviceDef dev;
  virDomainDiskDefPtr disk = vm-def-disks[i];

-if (vm-def-disks[i]-rawio == 1)
+if (vm-def-disks[i]-rawio == 1) {
  #ifdef CAP_SYS_RAWIO
  virCommandAllowCap(cmd, CAP_SYS_RAWIO);
  #else
  virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
 _(Raw I/O is not supported on this platform));
  #endif
+rawio_set = true;
+}


Interesting. So if rawio is requested we shout an error but don't fail 
actually. I think we are missing 'goto cleanup' here.




  dev.type = VIR_DOMAIN_DEVICE_DISK;
  dev.data.disk = disk;
@@ -4139,6 +4142,21 @@ int qemuProcessStart(virConnectPtr conn,
  goto cleanup;
  }

+/* If rawio not already set, check hostdevs as well */
+if (!rawio_set) {
+for (i = 0; i  vm-def-nhostdevs; i++) {
+virDomainHostdevSubsysSCSIPtr scsisrc =
+vm-def-hostdevs[i]-source.subsys.u.scsi;
+ 

Re: [libvirt] [PATCH 1/3] hostdev: Add rawio attribute to _virDomainHostdevSubsysSCSI

2014-09-18 Thread Michal Privoznik

On 10.09.2014 01:40, John Ferlan wrote:

Add the 'rawio' attribute to match _virDomainDiskDef

Signed-off-by: John Ferlan jfer...@redhat.com
---
  src/conf/domain_conf.h | 2 ++
  1 file changed, 2 insertions(+)

diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 1107fa8..b1d13ef 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -439,6 +439,8 @@ typedef virDomainHostdevSubsysSCSI 
*virDomainHostdevSubsysSCSIPtr;
  struct _virDomainHostdevSubsysSCSI {
  int protocol; /* enum virDomainHostdevSCSIProtocolType */
  int sgio; /* enum virDomainDeviceSGIO */
+bool rawio_specified;
+int rawio; /* no = 0, yes = 1 */


Instead of introducing these two values, would it be possible just to 
use virTristateBool which already contains the tristate values? Oh, and 
virDomainDiskDef should be fixed too.



  union {
  virDomainHostdevSubsysSCSIHost host;
  virDomainHostdevSubsysSCSIiSCSI iscsi;



Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH] Move the FIPS detection from capabilities

2014-09-18 Thread Pavel Hrdina

On 09/18/2014 06:48 PM, Eric Blake wrote:

On 09/18/2014 10:44 AM, Pavel Hrdina wrote:


Ouch.  This will make our testsuite differ based on whether it is run on
Linux in FIPS mode (where FIPS might exist) or on any other setup.  I
think you need to hoist the check for virFileExists() to the caller, and
pass in the result as a new bool parameter to this function, so that the
testsuite has full control over the boolean without regards to the
current system's level of FIPS support.



Sigh, that's right. I've completely forget about the tests. And that
reminds me whether we should also revert the test for enable-fips as it
seems to be pointless?


It's still worth testing that we can add or avoid '-enable-fips' in the
generated command line according to the boolean value we pass in.



Yes I agree and that's why I would like to remove the test from 
qemucapabilitiestest and create a new one in qemuxml2argvtest.


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH] Move the FIPS detection from capabilities

2014-09-18 Thread Eric Blake
On 09/18/2014 10:44 AM, Pavel Hrdina wrote:

 Ouch.  This will make our testsuite differ based on whether it is run on
 Linux in FIPS mode (where FIPS might exist) or on any other setup.  I
 think you need to hoist the check for virFileExists() to the caller, and
 pass in the result as a new bool parameter to this function, so that the
 testsuite has full control over the boolean without regards to the
 current system's level of FIPS support.

 
 Sigh, that's right. I've completely forget about the tests. And that
 reminds me whether we should also revert the test for enable-fips as it
 seems to be pointless?

It's still worth testing that we can add or avoid '-enable-fips' in the
generated command line according to the boolean value we pass 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] virSecuritySELinuxSetTapFDLabel: Temporarily revert to old behavior

2014-09-18 Thread Michal Privoznik

On 18.09.2014 18:36, John Ferlan wrote:



On 09/18/2014 10:20 AM, Michal Privoznik wrote:

https://bugzilla.redhat.com/show_bug.cgi?id=1141879

A long time ago I've implemented support for so called multiqueue
net.  The idea was to let guest network traffic be processed by
multiple host CPUs and thus increasing performance. However, this
behavior is enabled by QEMU via special ioctl() iterated over the
all tap FDs passed in by libvirt. Unfortunately, SELinux comes in
and disallows the ioctl() call because the /dev/net/tun has label
system_u:object_r:tun_tap_device_t:s0 and 'attach_queue' ioctl()
is not allowed on tun_tap_device_t type. So after discussion with
a SELinux developer we've decided that the FDs passed to the QEMU
should be labelled with svirt_t type and SELinux policy will
allow the ioctl(). Therefore I've made a patch
(cf976d9dcf4e592261b14f03572) that does exactly this. However,
things are not that easy - even though the API to label FD is
called (fsetfilecon_raw) the underlying file is labelled too! So
effectively we are mangling /dev/net/tun label. Yes, that broke
dozen of other application from openvpn, or boxes, to qemu
running other domains.

The best solution would be if SELinux provides a way to label an
FD only, which could be then labeled when passed to the qemu.
However that's a long path to go and we should fix this
regression AQAP. So I went to talk to the SELinux developer again
and we agreed on temporary solution that:

1) my patch is reverted
2) SELinux temporarily allows 'attach_queue' on the
tun_tap_device_t

Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
  src/security/security_selinux.c | 36 +---
  1 file changed, 33 insertions(+), 3 deletions(-)



Probably should note that this also reverts

a4431931393aeb1ac5893f121151fa3df4fde612   (in 1.2.8)

and

b635b7a1af0e64754016d758376f382470bc11e7   (post 1.2.8)

Although neither really matters with this change - it just points out
the trail for the secdef-imagelabel - secdef-label change that
isn't present in cf976d...

ACK

John



Okay, I've fixed the commit message and pushed. Thanks!

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH] Move the FIPS detection from capabilities

2014-09-18 Thread Eric Blake
On 09/18/2014 10:55 AM, Pavel Hrdina wrote:
 On 09/18/2014 06:48 PM, Eric Blake wrote:
 On 09/18/2014 10:44 AM, Pavel Hrdina wrote:

 Ouch.  This will make our testsuite differ based on whether it is
 run on
 Linux in FIPS mode (where FIPS might exist) or on any other setup.  I
 think you need to hoist the check for virFileExists() to the caller,
 and
 pass in the result as a new bool parameter to this function, so that
 the
 testsuite has full control over the boolean without regards to the
 current system's level of FIPS support.


 Sigh, that's right. I've completely forget about the tests. And that
 reminds me whether we should also revert the test for enable-fips as it
 seems to be pointless?

 It's still worth testing that we can add or avoid '-enable-fips' in the
 generated command line according to the boolean value we pass in.

 
 Yes I agree and that's why I would like to remove the test from
 qemucapabilitiestest and create a new one in qemuxml2argvtest.

Yes, that makes sense.

-- 
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 v2] Move the FIPS detection from capabilities

2014-09-18 Thread Pavel Hrdina
We are not detecting the presence of FIPS from QEMU, but from procfs and
that means it's not QEMU capability. It was decided that we will pass
this flag to QEMU even if it's not supported by old QEMU binaries.

This patch also reverts changes done by commit a21cfb0f to
qemucapabilitestest and implements a new test case in qemuxml2argvtest.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1135431

Signed-off-by: Pavel Hrdina phrd...@redhat.com
---
 src/qemu/qemu_capabilities.c   | 24 ---
 src/qemu/qemu_command.c| 34 --
 src/qemu/qemu_command.h|  6 +++-
 src/qemu/qemu_driver.c |  3 +-
 src/qemu/qemu_process.c|  3 +-
 tests/qemucapabilitiesdata/caps_1.2.2-1.caps   |  1 -
 tests/qemucapabilitiesdata/caps_1.6.0-1.caps   |  1 -
 tests/qemucapabilitiestest.c   | 20 -
 .../qemuxml2argv-fips-enabled.args |  6 
 .../qemuxml2argvdata/qemuxml2argv-fips-enabled.xml | 25 
 tests/qemuxml2argvtest.c   |  9 +-
 tests/qemuxmlnstest.c  |  2 +-
 12 files changed, 86 insertions(+), 48 deletions(-)
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-fips-enabled.args
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-fips-enabled.xml

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 9246813..5c3778d 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -3215,30 +3215,6 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps,
 config.data.nix.path = monpath;
 config.data.nix.listen = false;
 
-/* Qemu 1.2 and later have a binary flag -enable-fips that must be
- * used for VNC auth to obey FIPS settings; but the flag only
- * exists on Linux, and with no way to probe for it via QMP.  Our
- * solution: if FIPS mode is required, then unconditionally use
- * the flag, regardless of qemu version, for the following matrix:
- *
- *  old QEMUnew QEMU
- * FIPS enabled doesn't start   VNC auth disabled
- * FIPS disabled/missingVNC auth enabledVNC auth enabled
- *
- * Setting the flag here instead of in virQEMUCapsInitQMPMonitor
- * or virQEMUCapsInitHelp also allows the testsuite to be
- * independent of FIPS setting.
- */
-if (virFileExists(/proc/sys/crypto/fips_enabled)) {
-char *buf = NULL;
-
-if (virFileReadAll(/proc/sys/crypto/fips_enabled, 10, buf)  0)
-goto cleanup;
-if (STREQ(buf, 1\n))
-virQEMUCapsSet(qemuCaps, QEMU_CAPS_ENABLE_FIPS);
-VIR_FREE(buf);
-}
-
 VIR_DEBUG(Try to get caps via QMP qemuCaps=%p, qemuCaps);
 
 /*
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index f2e6e5a..911d026 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3293,6 +3293,35 @@ qemuCheckDiskConfig(virDomainDiskDefPtr disk)
 }
 
 
+/* Qemu 1.2 and later have a binary flag -enable-fips that must be
+ * used for VNC auth to obey FIPS settings; but the flag only
+ * exists on Linux, and with no way to probe for it via QMP.  Our
+ * solution: if FIPS mode is required, then unconditionally use
+ * the flag, regardless of qemu version, for the following matrix:
+ *
+ *  old QEMUnew QEMU
+ * FIPS enabled doesn't start   VNC auth disabled
+ * FIPS disabled/missingVNC auth enabledVNC auth enabled
+ */
+bool
+qemuCheckFips(void)
+{
+bool ret = false;
+
+if (virFileExists(/proc/sys/crypto/fips_enabled)) {
+char *buf = NULL;
+
+if (virFileReadAll(/proc/sys/crypto/fips_enabled, 10, buf)  0)
+return ret;
+if (STREQ(buf, 1\n))
+ret = true;
+VIR_FREE(buf);
+}
+
+return ret;
+}
+
+
 char *
 qemuBuildDriveStr(virConnectPtr conn,
   virDomainDiskDefPtr disk,
@@ -7543,7 +7572,8 @@ qemuBuildCommandLine(virConnectPtr conn,
  virDomainSnapshotObjPtr snapshot,
  virNetDevVPortProfileOp vmop,
  qemuBuildCommandLineCallbacksPtr callbacks,
- bool standalone)
+ bool standalone,
+ bool enableFips)
 {
 virErrorPtr originalError = NULL;
 size_t i, j;
@@ -7656,7 +7686,7 @@ qemuBuildCommandLine(virConnectPtr conn,
 if (!standalone)
 virCommandAddArg(cmd, -S); /* freeze CPU */
 
-if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_ENABLE_FIPS))
+if (enableFips)
 virCommandAddArg(cmd, -enable-fips);
 
 if (qemuBuildMachineArgStr(cmd, def, qemuCaps)  0)
diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
index 633ff71..aa40c9e 100644
--- a/src/qemu/qemu_command.h
+++ b/src/qemu/qemu_command.h
@@ -78,7 +78,8 

Re: [libvirt] [PATCH 2/3] qemu: Process the hostdev rawio setting

2014-09-18 Thread John Ferlan


On 09/18/2014 12:50 PM, Michal Privoznik wrote:
 On 10.09.2014 01:40, John Ferlan wrote:
 Mimic the Disk processing for 'rawio', but for a scsi_host hostdev
 lun device.
...snip...

 diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
 index b1d8a32..3544716 100644
 --- a/src/qemu/qemu_process.c
 +++ b/src/qemu/qemu_process.c
 @@ -3756,6 +3756,7 @@ int qemuProcessStart(virConnectPtr conn,
   struct qemuProcessHookData hookData;
   unsigned long cur_balloon;
   size_t i;
 +bool rawio_set = false;
   char *nodeset = NULL;
   virBitmapPtr nodemask = NULL;
   unsigned int stop_flags;
 @@ -4122,13 +4123,15 @@ int qemuProcessStart(virConnectPtr conn,
   virDomainDeviceDef dev;
   virDomainDiskDefPtr disk = vm-def-disks[i];

 -if (vm-def-disks[i]-rawio == 1)
 +if (vm-def-disks[i]-rawio == 1) {
   #ifdef CAP_SYS_RAWIO
   virCommandAllowCap(cmd, CAP_SYS_RAWIO);
   #else
   virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
  _(Raw I/O is not supported on this platform));
   #endif
 +rawio_set = true;
 +}
 
 Interesting. So if rawio is requested we shout an error but don't fail 
 actually. I think we are missing 'goto cleanup' here.
 

See 9a2f36ec04e0436b1ba9f0c21f9be234b25ac579

I can add a goto if desired or perhaps change it to a VIR_WARN() or
something else well.

I've copied the author of that commit to get an opinion...



   dev.type = VIR_DOMAIN_DEVICE_DISK;
   dev.data.disk = disk;
 @@ -4139,6 +4142,21 @@ int qemuProcessStart(virConnectPtr conn,
   goto cleanup;
   }

 +/* If rawio not already set, check hostdevs as well */
 +if (!rawio_set) {
 +for (i = 0; i  vm-def-nhostdevs; i++) {
 +virDomainHostdevSubsysSCSIPtr scsisrc =
 +vm-def-hostdevs[i]-source.subsys.u.scsi;
 +if (scsisrc-rawio == 1)
 +#ifdef CAP_SYS_RAWIO
 +virCommandAllowCap(cmd, CAP_SYS_RAWIO);
 +#else
 +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
 +   _(Raw I/O is not supported on this 
 platform));
 +#endif
 
 And here too then.
 

Monkey see, monkey do[o] ;-)

John

 +}
 +}
 +

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH 1/1] apparmor: use TEMPLATE.qemu for kvm

2014-09-18 Thread Serge Hallyn
virDomainVirtTypeToString() returns 'qemu' and 'kvm' separately.
Don't require a separate apparmor profile for both, rather always
look for TEMPLATE.qemu.

Signed-off-by: Serge Hallyn serge.hal...@ubuntu.com
---
 src/security/virt-aa-helper.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
index a06ba44..0447248 100644
--- a/src/security/virt-aa-helper.c
+++ b/src/security/virt-aa-helper.c
@@ -341,15 +341,19 @@ create_profile(const char *profile, const char 
*profile_name,
 int tlen, plen;
 int fd;
 int rc = -1;
+const char *virttype;
 
 if (virFileExists(profile)) {
 vah_error(NULL, 0, _(profile exists));
 goto end;
 }
 
+virttype = virDomainVirtTypeToString(virtType);
+if (strcmp(virttype, kvm) == 0)
+   virttype = qemu;
 
 if (virAsprintfQuiet(template, %s/TEMPLATE.%s, APPARMOR_DIR /libvirt,
- virDomainVirtTypeToString(virtType))  0) {
+ virttype)  0) {
 vah_error(NULL, 0, _(template name exceeds maximum length));
 goto end;
 }
-- 
2.1.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH v2] Move the FIPS detection from capabilities

2014-09-18 Thread Eric Blake
On 09/18/2014 12:01 PM, Pavel Hrdina wrote:
 We are not detecting the presence of FIPS from QEMU, but from procfs and
 that means it's not QEMU capability. It was decided that we will pass
 this flag to QEMU even if it's not supported by old QEMU binaries.
 
 This patch also reverts changes done by commit a21cfb0f to
 qemucapabilitestest and implements a new test case in qemuxml2argvtest.
 
 Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1135431
 
 Signed-off-by: Pavel Hrdina phrd...@redhat.com
 ---

 @@ -183,19 +176,16 @@ mymain(void)
  
  data.xmlopt = xmlopt;
  
 -#define DO_TEST_FULL(name, use_fips) \
 -data.base = name;\
 -data.fips = use_fips;\
 -if (virtTestRun(name, testQemuCaps, data)  0)  \
 +#define DO_TEST(name) \
 +data.base = name; \
 +if (virtTestRun(name, testQemuCaps, data)  0) \

We are not very consistent on whether multiline macros should align the
\ to the same column.

  ret = -1

Eww - we really did that in a multiline macro?  I'd much rather fix
things to use:

do {
  data.base = name;
  if (virtTestRun(name, testQemuCaps, data)  0)
ret = -1;
} while (0)

as long as we are touching the code.

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

Re: [libvirt] retiring v0.9.6-maint

2014-09-18 Thread Guido Günther
On Wed, Sep 17, 2014 at 04:24:07PM -0600, Eric Blake wrote:
 Any objections to retiring the v0.9.6-maint branch?  After all, we have
 already retired the v0.9.11-maint branch
 (http://libvirt.org/git/?p=libvirt.git;a=commit;h=cd0d348ed), and the
 only activity on v0.9.6-maint since 0.9.6.4 was released in January 2013
 was the backport of a single CVE fix.  The branch no longer builds
 cleanly on Fedora 20, and while I could identify patches to backport to
 fix the build situation, it's not worth my time if we can just retire
 the branch.

Fine for me. Debian is tracking 0.9.12.
Cheers,
 -- Guido

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH] libxl: Drop driver lock in libxlDomainDefineXML

2014-09-18 Thread Jim Fehlig
There is no need to acquire the driver-wide lock in
libxlDomainDefineXML.  When switching to jobs in the libxl
driver, most driver-wide locks were removed.  The locking here
was preserved since I mistakenly thought virDomainObjListAdd
needed protection.  This is not the case, so remove the
unnecessary locking.

Signed-off-by: Jim Fehlig jfeh...@suse.com
---
 src/libxl/libxl_driver.c | 13 +++--
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 2f2c590..d4ecd62 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -2381,25 +2381,22 @@ libxlDomainDefineXML(virConnectPtr conn, const char 
*xml)
 virObjectEventPtr event = NULL;
 virDomainDefPtr oldDef = NULL;
 
-/* Lock the driver until the virDomainObj is created and locked */
-libxlDriverLock(driver);
 if (!(def = virDomainDefParseString(xml, cfg-caps, driver-xmlopt,
 1  VIR_DOMAIN_VIRT_XEN,
 VIR_DOMAIN_XML_INACTIVE)))
-goto cleanup_unlock;
+goto cleanup;
 
 if (virDomainDefineXMLEnsureACL(conn, def)  0)
-goto cleanup_unlock;
+goto cleanup;
 
 if (!(vm = virDomainObjListAdd(driver-domains, def,
driver-xmlopt,
0,
oldDef)))
-goto cleanup_unlock;
+goto cleanup;
 
 def = NULL;
 vm-persistent = 1;
-libxlDriverUnlock(driver);
 
 if (virDomainSaveConfig(cfg-configDir,
 vm-newDef ? vm-newDef : vm-def)  0) {
@@ -2426,10 +2423,6 @@ libxlDomainDefineXML(virConnectPtr conn, const char *xml)
 libxlDomainEventQueue(driver, event);
 virObjectUnref(cfg);
 return dom;
-
- cleanup_unlock:
-libxlDriverUnlock(driver);
-goto cleanup;
 }
 
 static int
-- 
1.8.4.5

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH] Renamed internal __mon_yday into __vir_mon_yday to avoid conflicts

2014-09-18 Thread Cédric Bosdonnat
libc has another constant with the same name, which leads to
redefinition error when building against static libvirt
---
 src/util/virtime.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/util/virtime.c b/src/util/virtime.c
index acbec41..7b3ec44 100644
--- a/src/util/virtime.c
+++ b/src/util/virtime.c
@@ -105,7 +105,7 @@ int virTimeFieldsNowRaw(struct tm *fields)
 #define DIV(a, b) ((a) / (b) - ((a) % (b)  0))
 #define LEAPS_THRU_END_OF(y) (DIV (y, 4) - DIV (y, 100) + DIV (y, 400))
 
-const unsigned short int __mon_yday[2][13] = {
+const unsigned short int __vir_mon_yday[2][13] = {
 /* Normal years.  */
 { 0, 31, 59, 90, 120, 151, 181, 212, 243, 273, 304, 334, 365 },
 /* Leap years.  */
@@ -160,7 +160,7 @@ void virTimeFieldsThen(unsigned long long when, struct tm 
*fields)
 fields-tm_year = y - 1900;
 
 fields-tm_yday = days;
-ip = __mon_yday[is_leap_year(y)];
+ip = __vir_mon_yday[is_leap_year(y)];
 for (y = 11; days  (long int) ip[y]; --y)
 continue;
 days -= ip[y];
-- 
1.8.4.5

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH] Renamed internal __mon_yday into __vir_mon_yday to avoid conflicts

2014-09-18 Thread Eric Blake
On 09/18/2014 03:24 PM, Cédric Bosdonnat wrote:
 libc has another constant with the same name, which leads to
 redefinition error when building against static libvirt
 ---
  src/util/virtime.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)
 
 diff --git a/src/util/virtime.c b/src/util/virtime.c
 index acbec41..7b3ec44 100644
 --- a/src/util/virtime.c
 +++ b/src/util/virtime.c
 @@ -105,7 +105,7 @@ int virTimeFieldsNowRaw(struct tm *fields)
  #define DIV(a, b) ((a) / (b) - ((a) % (b)  0))
  #define LEAPS_THRU_END_OF(y) (DIV (y, 4) - DIV (y, 100) + DIV (y, 400))
  
 -const unsigned short int __mon_yday[2][13] = {
 +const unsigned short int __vir_mon_yday[2][13] = {

We really should stay out of the __ namespace.  Can you use a more
typical name like 'mon_yday'?  Also, can you mark it as static? Glibc
has it exported for sharing between multiple files, but we aren't doing
that.

-- 
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] LXC: add HOME environment variable docs

2014-09-18 Thread Chen Hanxiao
commit
3020594ac57c5e06e79f3db8c765f6bb18c40802
add HOME environment variable.
Add a doc for this.

Signed-off-by: Chen Hanxiao chenhanx...@cn.fujitsu.com
---
 docs/drvlxc.html.in | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/docs/drvlxc.html.in b/docs/drvlxc.html.in
index 403ce24..31da37c 100644
--- a/docs/drvlxc.html.in
+++ b/docs/drvlxc.html.in
@@ -88,6 +88,8 @@ to be provided by all container technologies on Linux.
 ddThe fixed string code/bin:/usr/bin/code/dd
 dtTERM/dt
 ddThe fixed string codelinux/code/dd
+dtHOME/dt
+ddThe fixed string code//code/dd
 /dl
 
 p
-- 
1.9.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH] libxl: Implement basic video device selection

2014-09-18 Thread Jim Fehlig
Stefan Bader wrote:
 Re-pushing this as the old thread got rather stale.

Thanks.

  Some of the
 VFB setup went in a bug fix. Not sure I missed a detail in rebasing
 bug the keyboard setting may be the only thing missing...
   

Yes, agreed.

 -Stefan

 [v2: Check return code of VIR_STRDUP and fix indentation]
 [v3: Split out VRAM fixup and return error for unsupported video type]
 [v4: Re-arrange code and move VFB setup into libxlMakeVfbList]
 [v5: Rebased against head which already had some VFB setup code]

 From b3ff8f4c658d29f15e673af88b9ae2fdfa3c1317 Mon Sep 17 00:00:00 2001
 From: Stefan Bader stefan.ba...@canonical.com
 Date: Thu, 27 Mar 2014 16:01:18 +0100
 Subject: [PATCH] libxl: Implement basic video device selection

 This started as an investigation into an issue where libvirt (using the
 libxl driver) and the Xen host, like an old couple, could not agree on
 who is responsible for selecting the VNC port to use.

 Things usually (and a bit surprisingly) did work because, just like that
 old couple, they had the same idea on what to do by default. However it
 was possible that this ended up in a big argument.

 The problem is that display information exists in two different places:
 in the vfbs list and in the build info. And for launching the device model,
 only the latter is used. But that never gets initialized from libvirt. So
 Xen allows the device model to select a default port while libvirt thinks
 it has told Xen that this is done by libvirt (though the vfbs config).

 While fixing that, I made a stab at actually evaluating the configuration
 of the video device. So that it is now possible to at least decide between
 a Cirrus or standard VGA emulation and to modify the VRAM within certain
 limits using libvirt.

 Signed-off-by: Stefan Bader stefan.ba...@canonical.com
 ---
  src/libxl/libxl_conf.c | 50 
 ++
  1 file changed, 50 insertions(+)
   

This patch suffers the same issues as the last version.  And when
commenting on that version, I promised to work on a followup to address
my concerns

https://www.redhat.com/archives/libvir-list/2014-July/msg00931.html

Your repost poked me into reworking my first attempt, the result of
which is below.  I should probably look at a sensible split-up of these
patches that would be easier to review, but in the meantime comments on
my followup would be appreciated.

With both patches, my tests are passing and my concerns are subdued :-).

Regards,
Jim
 

From 5003420c1e4d22726c596594988169a37544f867 Mon Sep 17 00:00:00 2001
From: Jim Fehlig jfeh...@suse.com
Date: Thu, 17 Jul 2014 12:00:31 -0600
Subject: [PATCH 2/2] Xen: Improve handling of video device vram

The minimum vram values supported by libxl depend upon the device
model used.  E.g. the minimum values are doubled when using QEMU_XEN,
as compared to the old QEMU_XEN_TRADITIONAL.

This patch introduces a function to detect whether the specified
emulator is QEMU_XEN or QEMU_XEN_TRADITIONAL.  Detection is based on the
string Options specific to the Xen version: in '$qemu -help' output.
AFAIK, the only qemu containing that string in help output is the
old Xen fork (aka qemu-dm).

The detection function is then used to sanity check user-provided
vram values, and set appropriate defaults when not provided.  For the
latter, virDomainVideoDefaultRAM was changed to defer setting the
default to the Xen drivers.

Note:
QEMU_XEN means a qemu that contains support for Xen.

QEMU_XEN_TRADITIONAL means Xen's old forked qemu 0.10.2

Signed-off-by: Jim Fehlig jfeh...@suse.com
---
 src/conf/domain_conf.c   |  4 +++
 src/libxl/libxl_conf.c   | 79 ++--
 src/libxl/libxl_conf.h   |  3 ++
 src/libxl/libxl_domain.c | 21 +
 src/xen/xen_driver.c | 18 +++
 5 files changed, 122 insertions(+), 3 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 3ccec1c..41be4f4 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -9683,6 +9683,10 @@ int
 virDomainVideoDefaultRAM(const virDomainDef *def,
  int type)
 {
+/* Defer setting vram to the Xen drivers */
+if (def-virtType == VIR_DOMAIN_VIRT_XEN)
+return 0;
+
 switch (type) {
 /* Weird, QEMU defaults to 9 MB ??! */
 case VIR_DOMAIN_VIDEO_TYPE_VGA:
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index ce9fafd..5505de8 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -40,6 +40,7 @@
 #include viralloc.h
 #include viruuid.h
 #include capabilities.h
+#include vircommand.h
 #include libxl_domain.h
 #include libxl_conf.h
 #include libxl_utils.h
@@ -492,6 +493,38 @@ libxlCapsInitGuests(libxl_ctx *ctx, virCapsPtr caps)
 return 0;
 }
 
+
+#define LIBXL_QEMU_DM_STR  Options specific to the Xen version:
+
+int
+libxlDomainGetEmulatorType(const virDomainDef *def)
+{
+int ret = LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN;
+virCommandPtr cmd = 

Re: [libvirt] [PATCH] libvirt-guests: wait for ntp service

2014-09-18 Thread Jim Fehlig
Any comments on this change?

Regards,
Jim

Jim Fehlig wrote:
 If an NTP server is configured on the host, it is possible for
 libvirt-guests to start before the NTP service, in which case
 guest clocks won't be synchronized to the host clock.

 Add ntp-wait.service to After in libvirt-guests systemd service
 file, ensuring NTP has synchronized the host clock before starting
 any guests.

 Signed-off-by: Jim Fehlig jfeh...@suse.com
 ---
  tools/libvirt-guests.service.in | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

 diff --git a/tools/libvirt-guests.service.in b/tools/libvirt-guests.service.in
 index d8d7adf..226b3bd 100644
 --- a/tools/libvirt-guests.service.in
 +++ b/tools/libvirt-guests.service.in
 @@ -1,6 +1,6 @@
  [Unit]
  Description=Suspend Active Libvirt Guests
 -After=network.target libvirtd.service
 +After=network.target libvirtd.service ntp-wait.service
  Documentation=man:libvirtd(8)
  Documentation=http://libvirt.org
  
   

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] Adding multiple graphics devices to a vm

2014-09-18 Thread bancfc
I was trying different video devices to correct the problem of having 
1024x768 as the maximum resolution for Debian guests. I found out that 
adding VMVGA gives the wide variety of choice of resolutions I was 
looking for and adjusts the display properly, in contrast to the 
deformed way xconf file did. However I still want to make use of SPICE 
and QXL.


Is there a way to have both vmvga and qxl attached and working on the 
same guest?


How would I go about doing this in the XML?

thanks.

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list