Re: [libvirt] LXC: Helper function for checking ownership of dir when userns enabled

[Alex Jia]

On 08/09/2013 01:53 PM, Chen Hanxiao wrote:

From: Chen Hanxiaochenhanx...@cn.fujitsu.com

  If we enable userns, the ownership of dir we provided for containers
  should match the uid/gid in idmap.
  Currently, the debug log is very implicit or misleading sometimes.
  This patch will help clarify this for us when using
  debug log or virsh.

Signed-off-by: Chen Hanxiaochenhanx...@cn.fujitsu.com
---
  src/lxc/lxc_container.c |   45 +
  1 files changed, 45 insertions(+), 0 deletions(-)

diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
index b910b10..ce17466 100644
--- a/src/lxc/lxc_container.c
+++ b/src/lxc/lxc_container.c
@@ -1815,6 +1815,48 @@ lxcNeedNetworkNamespace(virDomainDefPtr def)
  return false;
  }

+/*
+ * Helper function for helping check
+ * whether we have enough privilege
+ * to operate the source dir when userns enabled
+ * @vmDef: pointer to vm definition structure
+ * Returns 0 on success or -1 in case of error
+ */
+static int
+lxcContainerUsernsSrcOwnershipCheck(virDomainDefPtr vmDef)
+{
+struct stat buf;
+int i;
+uid_t uid;
+gid_t gid;
+
+for(i=0; i  vmDef-nfss; i++) {
+VIR_DEBUG(dst is %s, src is %s,
+vmDef-fss[i]-dst,
+vmDef-fss[i]-src);


indention issue.


+
+uid = vmDef-idmap.uidmap[0].target;
+gid = vmDef-idmap.gidmap[0].target;
+
+if (lstat(vmDef-fss[i]-src,buf)  0) {
+virReportSystemError(errno, _(Cannot access '%s'),
+ vmDef-fss[i]-src);


same as above.


+return -1;
+} else if(uid != buf.st_uid || gid != buf.st_gid) {
+VIR_DEBUG(In userns uid is %d, gid is %d\n,
+uid, gid);


same as above.


+errno = EINVAL;
+
+virReportSystemError(errno,
+[userns] Src dir \%s\ does not belong to uid/gid:%d/%d,
+vmDef-fss[i]-src, uid, gid);


same as above.


+return -1;
+}
+}
+
+return 0;
+}
+
  /**
   * lxcContainerStart:
   * @def: pointer to virtual machine structure
@@ -1866,6 +1908,9 @@ int lxcContainerStart(virDomainDefPtr def,
  if (userns_supported()) {
  VIR_DEBUG(Enable user namespace);
  cflags |= CLONE_NEWUSER;
+if(lxcContainerUsernsSrcOwnershipCheck(def)  0) {
+return -1;
+}
  } else {
  virReportSystemError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
   _(Kernel doesn't support user namespace));



libvirt/.git/rebase-apply/patch:15: trailing whitespace.
 * whether we have enough privilege
libvirt/.git/rebase-apply/patch:16: trailing whitespace.
 * to operate the source dir when userns enabled
libvirt/.git/rebase-apply/patch:45: trailing whitespace.
virReportSystemError(errno,
libvirt/.git/rebase-apply/patch:51: trailing whitespace.

warning: 4 lines add whitespace errors.

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

Re: [libvirt] LXC: Helper function for checking ownership of dir when userns enabled

[Alex Jia]

On 08/09/2013 01:53 PM, Chen Hanxiao wrote:

From: Chen Hanxiaochenhanx...@cn.fujitsu.com

  If we enable userns, the ownership of dir we provided for containers
  should match the uid/gid in idmap.
  Currently, the debug log is very implicit or misleading sometimes.
  This patch will help clarify this for us when using
  debug log or virsh.

Signed-off-by: Chen Hanxiaochenhanx...@cn.fujitsu.com
---
  src/lxc/lxc_container.c |   45 +
  1 files changed, 45 insertions(+), 0 deletions(-)

diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
index b910b10..ce17466 100644
--- a/src/lxc/lxc_container.c
+++ b/src/lxc/lxc_container.c
@@ -1815,6 +1815,48 @@ lxcNeedNetworkNamespace(virDomainDefPtr def)
  return false;
  }

+/*
+ * Helper function for helping check
+ * whether we have enough privilege
+ * to operate the source dir when userns enabled
+ * @vmDef: pointer to vm definition structure
+ * Returns 0 on success or -1 in case of error
+ */
+static int
+lxcContainerUsernsSrcOwnershipCheck(virDomainDefPtr vmDef)
+{
+struct stat buf;
+int i;
+uid_t uid;
+gid_t gid;
+
+for(i=0; i  vmDef-nfss; i++) {
+VIR_DEBUG(dst is %s, src is %s,
+vmDef-fss[i]-dst,
+vmDef-fss[i]-src);
+
+uid = vmDef-idmap.uidmap[0].target;
+gid = vmDef-idmap.gidmap[0].target;
+
+if (lstat(vmDef-fss[i]-src,buf)  0) {
+virReportSystemError(errno, _(Cannot access '%s'),
+ vmDef-fss[i]-src);
+return -1;
+} else if(uid != buf.st_uid || gid != buf.st_gid) {
+VIR_DEBUG(In userns uid is %d, gid is %d\n,
+uid, gid);
+errno = EINVAL;
+
+virReportSystemError(errno,
+[userns] Src dir \%s\ does not belong to uid/gid:%d/%d,
+vmDef-fss[i]-src, uid, gid);
+return -1;
+}
+}
+
+return 0;
+}
+
  /**
   * lxcContainerStart:
   * @def: pointer to virtual machine structure
@@ -1866,6 +1908,9 @@ int lxcContainerStart(virDomainDefPtr def,
  if (userns_supported()) {
  VIR_DEBUG(Enable user namespace);
  cflags |= CLONE_NEWUSER;
+if(lxcContainerUsernsSrcOwnershipCheck(def)  0) {
+return -1;
+}
  } else {
  virReportSystemError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
   _(Kernel doesn't support user namespace));


In addition, please run 'make syntax-check' firstly before committing 
patches.


src/lxc/lxc_container.c:1835: for(i=0; i  vmDef-nfss; i++) {
src/lxc/lxc_container.c:1847: } else if(uid != buf.st_uid || gid 
!= buf.st_gid) {
src/lxc/lxc_container.c:1913: 
if(lxcContainerUsernsSrcOwnershipCheck(def)  0) {

maint.mk: incorrect whitespace, see HACKING for rules
make: *** [bracket-spacing-check] Error 1

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

Re: [libvirt] [PATCH] Fix parallel runs of TLS test suites

[Martin Kletzander]

On 08/09/2013 12:29 AM, Eric Blake wrote:
 On 08/08/2013 04:09 PM, Daniel P. Berrange wrote:
 From: Daniel P. Berrange berra...@redhat.com

 Use a seperate keyfile name for the two TLS test suites so that
 
 s/seperate/separate/
 
 they don't clash when running tests in parallel

 Signed-off-by: Daniel P. Berrange berra...@redhat.com
 ---
  tests/virnettlscontexttest.c | 10 ++
  tests/virnettlshelpers.c |  6 ++
  tests/virnettlshelpers.h |  6 ++
  tests/virnettlssessiontest.c | 10 ++
  4 files changed, 16 insertions(+), 16 deletions(-)
 
 ACK.
 

I noticed this yesterday and fixed it in a different way, but ended up
with one more problem.  It was probably the way I fixed it combined
with one more filename changed.

Anyway, why I'm saying this is that one more filename should be renamed
in order to avoid a race (which I was unable to reproduce, though).
Consider squashing the following in before pushing.

I checked this is the last file those two tests have in common by going
through the code and the re-checked by this script:

strace -o session.trace -e open ./virnettlssessiontest
strace -o context.trace -e open ./virnettlscontexttest
sort \
 (sed -n '/^open/s/open(\([^]*\),.*$/\1/p' context.trace | sort -u)\
 (sed -n '/^open/s/open(\([^]*\),.*$/\1/p' session.trace | sort -u)\
 | uniq -d| grep '.pem$'

So it should be enough to make these tests independent of each other.

diff --git a/tests/virnettlscontexttest.c b/tests/virnettlscontexttest.c
index 53792ee..2c7d400 100644
--- a/tests/virnettlscontexttest.c
+++ b/tests/virnettlscontexttest.c
@@ -556,12 +556,12 @@ mymain(void)
 cacertlevel2areq.crt,
 };

-testTLSWriteCertChain(cacertchain.pem,
+testTLSWriteCertChain(cacertchain-ctx.pem,
   certchain,
   ARRAY_CARDINALITY(certchain));

-DO_CTX_TEST(true, cacertchain.pem, servercertlevel3areq.filename, false);
-DO_CTX_TEST(false, cacertchain.pem, clientcertlevel2breq.filename, 
false);
+DO_CTX_TEST(true, cacertchain-ctx.pem, servercertlevel3areq.filename, 
false);
+DO_CTX_TEST(false, cacertchain-ctx.pem, clientcertlevel2breq.filename, 
false);

 testTLSDiscardCert(cacertreq);
 testTLSDiscardCert(cacert1req);
@@ -617,7 +617,7 @@ mymain(void)
 testTLSDiscardCert(cacertlevel2areq);
 testTLSDiscardCert(servercertlevel3areq);
 testTLSDiscardCert(clientcertlevel2breq);
-unlink(cacertchain.pem);
+unlink(cacertchain-ctx.pem);

 testTLSCleanup(KEYFILE);

diff --git a/tests/virnettlssessiontest.c b/tests/virnettlssessiontest.c
index 9b171ed..f5f7212 100644
--- a/tests/virnettlssessiontest.c
+++ b/tests/virnettlssessiontest.c
@@ -451,11 +451,11 @@ mymain(void)
 cacertlevel2areq.crt,
 };

-testTLSWriteCertChain(cacertchain.pem,
+testTLSWriteCertChain(cacertchain-sess.pem,
   certchain,
   ARRAY_CARDINALITY(certchain));

-DO_SESS_TEST(cacertchain.pem, servercertlevel3areq.filename, 
clientcertlevel2breq.filename,
+DO_SESS_TEST(cacertchain-sess.pem, servercertlevel3areq.filename, 
clientcertlevel2breq.filename,
  false, false, libvirt.org, NULL);

 testTLSDiscardCert(clientcertreq);
@@ -474,7 +474,7 @@ mymain(void)
 testTLSDiscardCert(cacertlevel2areq);
 testTLSDiscardCert(servercertlevel3areq);
 testTLSDiscardCert(clientcertlevel2breq);
-unlink(cacertchain.pem);
+unlink(cacertchain-sess.pem);

 testTLSCleanup(KEYFILE);

--

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

[libvirt] [PATCH v2]LXC: Helper function for checking ownership of dir when userns enabled

[Chen Hanxiao]

From: Chen Hanxiao chenhanx...@cn.fujitsu.com

If we enable userns, the ownership of dir we provided for containers
should match the uid/gid in idmap.
Currently, the debug log is very implicit or misleading sometimes.
This patch will help clarify this for us when using
debug log or virsh.

v2: syntax-check clean

Signed-off-by: Chen Hanxiao chenhanx...@cn.fujitsu.com
---
 src/lxc/lxc_container.c |   46 ++
 1 files changed, 46 insertions(+), 0 deletions(-)

diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
index b910b10..2ccdc61 100644
--- a/src/lxc/lxc_container.c
+++ b/src/lxc/lxc_container.c
@@ -1815,6 +1815,49 @@ lxcNeedNetworkNamespace(virDomainDefPtr def)
 return false;
 }
 
+/*
+ * Helper function for helping check
+ * whether we have enough privilege
+ * to operate the source dir when userns enabled
+ * @vmDef: pointer to vm definition structure
+ * Returns 0 on success or -1 in case of error
+ */
+static int
+lxcContainerUsernsSrcOwnershipCheck(virDomainDefPtr vmDef)
+{
+struct stat buf;
+size_t i;
+uid_t uid;
+gid_t gid;
+
+VIR_DEBUG(vmDef-nfss %d, (int)vmDef-nfss);
+for (i = 0; i  vmDef-nfss; i++) {
+VIR_DEBUG(dst is %s, src is %s,
+  vmDef-fss[i]-dst,
+  vmDef-fss[i]-src);
+
+uid = vmDef-idmap.uidmap[0].target;
+gid = vmDef-idmap.gidmap[0].target;
+
+if (lstat(vmDef-fss[i]-src, buf)  0) {
+virReportSystemError(errno, _(Cannot access '%s'),
+ vmDef-fss[i]-src);
+return -1;
+} else if (uid != buf.st_uid || gid != buf.st_gid) {
+VIR_DEBUG(In userns uid is %d, gid is %d\n,
+  uid, gid);
+errno = EINVAL;
+
+virReportSystemError(errno,
+  _([userns] Src dir '%s' does not belong to 
uid/gid: %d/%d),
+ vmDef-fss[i]-src, uid, gid);
+return -1;
+}
+}
+
+return 0;
+}
+
 /**
  * lxcContainerStart:
  * @def: pointer to virtual machine structure
@@ -1866,6 +1909,9 @@ int lxcContainerStart(virDomainDefPtr def,
 if (userns_supported()) {
 VIR_DEBUG(Enable user namespace);
 cflags |= CLONE_NEWUSER;
+if (lxcContainerUsernsSrcOwnershipCheck(def)  0) {
+return -1;
+}
 } else {
 virReportSystemError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
  _(Kernel doesn't support user namespace));
-- 
1.7.1

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

[libvirt] Hyper-V driver API version support

[surface]

Hello

The version function is not supported by the hyperv driver:
$ virsh --connect=hyperv://hypervhost version
Compiled against library: libvirt 1.1.1
Using library: libvirt 1.1.1
Using API: Hyper-V 1.1.1
error: failed to get the hypervisor version
error: this function is not supported by the connection driver: 
virConnectGetVersion


But we need this funtion for the external/libvirt stonith plugin of 
clusterglue:


$ cat /usr/lib/stonith/plugins/libvirt | more
# get status of stonith device (*NOT* of the domain).
# If we can retrieve some info from the hypervisor
# the stonith device is OK.
libvirt_status() {
out=$($VIRSH -c $hypervisor_uri version 21)
if [ $? -eq 0 ]
then
out=`echo $out | tail -1`
ha_log.sh notice $hypervisor_uri: $out
return 0
fi

ha_log.sh err Failed to get status for $hypervisor_uri
ha_log.sh err $out
return 1
}

So, we can't implement libvirt stonith with hyperv support in our 
corosync/pacemaker cluster. Is it possible to implement the version 
function for hyperv into virConnectGetVersion? Or exist any workaround 
for this problem?


Regards
Rocco

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

[libvirt] [PATCH v4 1/2] libxl: fix node ranges in libxlNodeGetCellsFreeMemory()

[Dario Faggioli]

introduced by cs 4b9eec50fe2c23343 (libxl: implement per
NUMA node free memory reporting). What was wrong was that
libxl_get_numainfo() put in nr_nodes the actual number of
host NUMA nodes, not the highest node ID (like libnuma's
numa_max_node() does instead).

While at it, turn the failure of libxl_get_numainfo() from
a simple warning to a proper error, as requested during the
review of another patch of the original series.

Signed-off-by: Dario Faggioli dario.faggi...@citrix.com
Cc: Daniel P. Berrange berra...@redhat.com
---
 src/libxl/libxl_driver.c |   14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 9e9bc89..04058d3 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -4107,23 +4107,23 @@ libxlNodeGetCellsFreeMemory(virConnectPtr conn,
 if (virNodeGetCellsFreeMemoryEnsureACL(conn)  0)
 return -1;
 
-/* Early failure is probably worth just a warning */
 numa_info = libxl_get_numainfo(driver-ctx, nr_nodes);
 if (numa_info == NULL || nr_nodes == 0) {
-VIR_WARN(libxl_get_numainfo failed to retrieve NUMA data);
-return 0;
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+   _(libxl_get_numainfo failed));
+goto cleanup;
 }
 
 /* Check/sanitize the cell range */
-if (startCell  nr_nodes) {
+if (startCell = nr_nodes) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
_(start cell %d out of range (0-%d)),
-   startCell, nr_nodes);
+   startCell, nr_nodes - 1);
 goto cleanup;
 }
 lastCell = startCell + maxCells - 1;
-if (lastCell  nr_nodes)
-lastCell = nr_nodes;
+if (lastCell = nr_nodes)
+lastCell = nr_nodes - 1;
 
 for (numCells = 0, n = startCell; n = lastCell; n++) {
 if (numa_info[n].size == LIBXL_NUMAINFO_INVALID_ENTRY)

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

[libvirt] [PATCH v4 0/2] libxl: implement some chuncks of the NUMA interface

[Dario Faggioli]

Hi and sorry for being so late at this. :-(

This is the leftover of my NUMA series for the libxl driver. Basically, all the
patches have been checked in but the one actually building the topology
capability, which is patch 2/2 of this series.  I think I addressed all
Daniel's comments about VIR_*ALLOC* and error handling.

Patch 1/2 is a bugfix to how the range of available NUMA nodes was being
handled in a previous patch (more in the changelog).

---

Dario Faggioli (2):
  libxl: fix node ranges in libxlNodeGetCellsFreeMemory()
  libxl: implement NUMA capabilities reporting


 src/libxl/libxl_conf.c   |  140 ++
 src/libxl/libxl_driver.c |   14 ++---
 2 files changed, 146 insertions(+), 8 deletions(-)

-- 
This happens because I choose it to happen! (Raistlin Majere)
-
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems RD Ltd., Cambridge (UK)

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

[libvirt] [PATCH v4 2/2] libxl: implement NUMA capabilities reporting

[Dario Faggioli]

Starting from Xen 4.2, libxl has all the bits and pieces in place
for retrieving an adequate amount of information about the host
NUMA topology. It is therefore possible, after a bit of shuffling,
to arrange those information in the way libvirt wants to present
them to the outside world.

Therefore, with this patch, the topology section of the host
capabilities is properly populated, when running on Xen, so that
we can figure out whether or not we're running on a NUMA host,
and what its characteristics are.

[raistlin@Zhaman ~]$ sudo virsh --connect xen:/// capabilities
capabilities
  host
cpu

topology
  cells num='2'
cell id='0'
  memory unit='KiB'6291456/memory
  cpus num='8'
cpu id='0' socket_id='1' core_id='0' siblings='0-1'/
cpu id='1' socket_id='1' core_id='0' siblings='0-1'/
cpu id='2' socket_id='1' core_id='1' siblings='2-3'/
cpu id='3' socket_id='1' core_id='1' siblings='2-3'/
cpu id='4' socket_id='1' core_id='9' siblings='4-5'/
cpu id='5' socket_id='1' core_id='9' siblings='4-5'/
cpu id='6' socket_id='1' core_id='10' siblings='6-7'/
cpu id='7' socket_id='1' core_id='10' siblings='6-7'/
  /cpus
/cell
cell id='1'
  memory unit='KiB'6881280/memory
  cpus num='8'
cpu id='8' socket_id='0' core_id='0' siblings='8-9'/
cpu id='9' socket_id='0' core_id='0' siblings='8-9'/
cpu id='10' socket_id='0' core_id='1' siblings='10-11'/
cpu id='11' socket_id='0' core_id='1' siblings='10-11'/
cpu id='12' socket_id='0' core_id='9' siblings='12-13'/
cpu id='13' socket_id='0' core_id='9' siblings='12-13'/
cpu id='14' socket_id='0' core_id='10' siblings='14-15'/
cpu id='15' socket_id='0' core_id='10' siblings='14-15'/
  /cpus
/cell
  /cells
/topology
  /host
  

Signed-off-by: Dario Faggioli dario.faggi...@citrix.com
Cc: Daniel P. Berrange berra...@redhat.com
---
Changes from v3:
 * updated VIR_*ALLOC* call sites not to invoke virReportOOMError();
 * failing at getting NUMA and CPU topology info from libxl are now
   considered proper errors, as requested during review;
 * s/out/cleanup/ as requested during review.

Changes from v2:
 * iterators turned from int to size_t;
 * fixed wrong sibling maps if on same node but different socket;
 * code motion and error handling, as requested during review.

Changes from v1:
 * fixed a typo in the commit message, as requested during review;
 * fixed coding style (one function parameters per line and no spaces
   between variable definitions), as requested during review;
 * avoid zero-filling memory after VIR_ALLOC_N(), since it does that
   already, as requested during review;
 * improved out of memory error reporting, as requested during review;
 * libxlMakeNumaCapabilities() created, accommodating all the NUMA
   related additions, instead of having them within
   libxlMakeCapabilitiesInternal(), as suggested during review.
---
 src/libxl/libxl_conf.c |  140 
 1 file changed, 139 insertions(+), 1 deletion(-)

diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 827dfdd..2c84191 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -161,6 +161,107 @@ libxlBuildCapabilities(virArch hostarch,
 }
 
 static virCapsPtr
+libxlMakeNumaCapabilities(libxl_numainfo *numa_info,
+  int nr_nodes,
+  libxl_cputopology *cpu_topo,
+  int nr_cpus,
+  virCapsPtr caps)
+{
+virCapsHostNUMACellCPUPtr *cpus = NULL;
+int *nr_cpus_node = NULL;
+bool numa_failed = true;
+size_t i;
+
+if (VIR_ALLOC_N(cpus, nr_nodes)  0)
+goto cleanup;
+
+if (VIR_ALLOC_N(nr_cpus_node, nr_nodes)  0)
+goto cleanup;
+
+/* For each node, prepare a list of CPUs belonging to that node */
+for (i = 0; i  nr_cpus; i++) {
+int node = cpu_topo[i].node;
+
+if (cpu_topo[i].core == LIBXL_CPUTOPOLOGY_INVALID_ENTRY)
+continue;
+
+nr_cpus_node[node]++;
+
+if (nr_cpus_node[node] == 1) {
+if (VIR_ALLOC(cpus[node])  0)
+goto cleanup;
+} else {
+if (VIR_REALLOC_N(cpus[node], nr_cpus_node[node])  0)
+goto cleanup;
+}
+
+/* Mapping between what libxl tells and what libvirt wants */
+cpus[node][nr_cpus_node[node]-1].id = i;
+cpus[node][nr_cpus_node[node]-1].socket_id = cpu_topo[i].socket;
+cpus[node][nr_cpus_node[node]-1].core_id = cpu_topo[i].core;
+/* Allocate the siblings maps. We will be filling them later */
+cpus[node][nr_cpus_node[node]-1].siblings = virBitmapNew(nr_cpus);
+if (!cpus[node][nr_cpus_node[node]-1].siblings) {
+

Re: [libvirt] [PATCH] Fix parallel runs of TLS test suites

[Daniel P. Berrange]

On Fri, Aug 09, 2013 at 09:53:30AM +0200, Martin Kletzander wrote:
 On 08/09/2013 12:29 AM, Eric Blake wrote:
  On 08/08/2013 04:09 PM, Daniel P. Berrange wrote:
  From: Daniel P. Berrange berra...@redhat.com
 
  Use a seperate keyfile name for the two TLS test suites so that
  
  s/seperate/separate/
  
  they don't clash when running tests in parallel
 
  Signed-off-by: Daniel P. Berrange berra...@redhat.com
  ---
   tests/virnettlscontexttest.c | 10 ++
   tests/virnettlshelpers.c |  6 ++
   tests/virnettlshelpers.h |  6 ++
   tests/virnettlssessiontest.c | 10 ++
   4 files changed, 16 insertions(+), 16 deletions(-)
  
  ACK.
  
 
 I noticed this yesterday and fixed it in a different way, but ended up
 with one more problem.  It was probably the way I fixed it combined
 with one more filename changed.
 
 Anyway, why I'm saying this is that one more filename should be renamed
 in order to avoid a race (which I was unable to reproduce, though).
 Consider squashing the following in before pushing.
 
 I checked this is the last file those two tests have in common by going
 through the code and the re-checked by this script:
 
 strace -o session.trace -e open ./virnettlssessiontest
 strace -o context.trace -e open ./virnettlscontexttest
 sort \
  (sed -n '/^open/s/open(\([^]*\),.*$/\1/p' context.trace | sort -u)\
  (sed -n '/^open/s/open(\([^]*\),.*$/\1/p' session.trace | sort -u)\
  | uniq -d| grep '.pem$'
 
 So it should be enough to make these tests independent of each other.
 
 diff --git a/tests/virnettlscontexttest.c b/tests/virnettlscontexttest.c
 index 53792ee..2c7d400 100644
 --- a/tests/virnettlscontexttest.c
 +++ b/tests/virnettlscontexttest.c
 @@ -556,12 +556,12 @@ mymain(void)
  cacertlevel2areq.crt,
  };
 
 -testTLSWriteCertChain(cacertchain.pem,
 +testTLSWriteCertChain(cacertchain-ctx.pem,
certchain,
ARRAY_CARDINALITY(certchain));
 
 -DO_CTX_TEST(true, cacertchain.pem, servercertlevel3areq.filename, 
 false);
 -DO_CTX_TEST(false, cacertchain.pem, clientcertlevel2breq.filename, 
 false);
 +DO_CTX_TEST(true, cacertchain-ctx.pem, servercertlevel3areq.filename, 
 false);
 +DO_CTX_TEST(false, cacertchain-ctx.pem, clientcertlevel2breq.filename, 
 false);
 
  testTLSDiscardCert(cacertreq);
  testTLSDiscardCert(cacert1req);
 @@ -617,7 +617,7 @@ mymain(void)
  testTLSDiscardCert(cacertlevel2areq);
  testTLSDiscardCert(servercertlevel3areq);
  testTLSDiscardCert(clientcertlevel2breq);
 -unlink(cacertchain.pem);
 +unlink(cacertchain-ctx.pem);
 
  testTLSCleanup(KEYFILE);
 
 diff --git a/tests/virnettlssessiontest.c b/tests/virnettlssessiontest.c
 index 9b171ed..f5f7212 100644
 --- a/tests/virnettlssessiontest.c
 +++ b/tests/virnettlssessiontest.c
 @@ -451,11 +451,11 @@ mymain(void)
  cacertlevel2areq.crt,
  };
 
 -testTLSWriteCertChain(cacertchain.pem,
 +testTLSWriteCertChain(cacertchain-sess.pem,
certchain,
ARRAY_CARDINALITY(certchain));
 
 -DO_SESS_TEST(cacertchain.pem, servercertlevel3areq.filename, 
 clientcertlevel2breq.filename,
 +DO_SESS_TEST(cacertchain-sess.pem, servercertlevel3areq.filename, 
 clientcertlevel2breq.filename,
   false, false, libvirt.org, NULL);
 
  testTLSDiscardCert(clientcertreq);
 @@ -474,7 +474,7 @@ mymain(void)
  testTLSDiscardCert(cacertlevel2areq);
  testTLSDiscardCert(servercertlevel3areq);
  testTLSDiscardCert(clientcertlevel2breq);
 -unlink(cacertchain.pem);
 +unlink(cacertchain-sess.pem);
 
  testTLSCleanup(KEYFILE);

ACK, yes this is also needed ontop of my patch.


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] libxl: Create per-domain log file

[Dario Faggioli]

On gio, 2013-08-08 at 17:41 -0600, Jim Fehlig wrote:
 Currently, only one log file is created by the libxl driver, with
 all output from libxl for all domains going to this one file.
 
 Create a per-domain log file based on domain name, making sifting
 through the logs a bit easier.

That is actually a really nice idea.

I gave it a quick spin, and it worked well, so I think you can add (if
you want, and if it is customary here in libvir) something like the
below:

Tested-by: Dario Faggioli dario.faggi...@citrix.com

Regards,
Dario

-- 
This happens because I choose it to happen! (Raistlin Majere)
-
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems RD Ltd., Cambridge (UK)



signature.asc
Description: This is a digitally signed message part
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [test-API][PATCH] Add network update test case

[hongming zhang]

The patch add network update test case to cover network update api.
modified: cases/basic_network.conf
- The test suite covers add/modify/delete ip-dhcp-host in network. For other 
test
scenario, they can be test via customing the conf file.
new file: repos/network/update.py
- So far the network update test case only checking when flag is live or 
current.
new file: repos/network/xmls/ip-dhcp-host.xml
- Create the ip-dhcp-host sample xml file for adding/deleting ip-dhcp-host
new file: repos/network/xmls/modify-ip-dhcp-host.xml
- Create the ip-dhcp-host sample xml file for modifying ip-dhcp-host
Known bug:
- Bug 985782 - Some flag values of method are missing in libvirt-python bindings
---
 cases/basic_network.conf   |   28 ++
 repos/network/update.py|   82 
 repos/network/xmls/ip-dhcp-host.xml|1 +
 repos/network/xmls/modify-ip-dhcp-host.xml |1 +
 4 files changed, 112 insertions(+), 0 deletions(-)
 create mode 100644 repos/network/update.py
 create mode 100644 repos/network/xmls/ip-dhcp-host.xml
 create mode 100644 repos/network/xmls/modify-ip-dhcp-host.xml

diff --git a/cases/basic_network.conf b/cases/basic_network.conf
index 91d7f21..e9abd57 100644
--- a/cases/basic_network.conf
+++ b/cases/basic_network.conf
@@ -32,6 +32,34 @@ network:autostart
 autostart
 enable
 
+network:update
+networkname
+   $defaultnetname
+command
+add-first
+section
+ip-dhcp-host
+
+network:update
+networkname
+   $defaultnetname
+command
+modify
+section
+ip-dhcp-host
+xml
+   xmls/modify-ip-dhcp-host.xml
+
+network:update
+networkname
+   $defaultnetname
+command
+delete
+section
+ip-dhcp-host
+xml
+   xmls/modify-ip-dhcp-host.xml
+
 network:destroy
 networkname
 $defaultnetname
diff --git a/repos/network/update.py b/repos/network/update.py
new file mode 100644
index 000..0024a5e
--- /dev/null
+++ b/repos/network/update.py
@@ -0,0 +1,82 @@
+#!/usr/bin/evn python
+#Update a network
+
+import libvirt
+from libvirt import libvirtError
+from src import sharedmod
+
+COMMANDDICT = {none:0, modify:1, delete:2, add-first:4}
+SECTIONDICT = {none:0, bridge:1, domain:2, ip:3, ip-dhcp-host:4, \
+   ip-dhcp-range:5, forward:6, forward-interface:7,\
+   forward-pf:8, portgroup:9, dns-host:10, dns-txt:11,\
+   dns-srv:12}
+FLAGSDICT = {current:0, live:1, config: 2}
+
+required_params = ('networkname', )
+optional_params = {
+   'command': 'add-first',
+   'section': 'ip-dhcp-host',
+   'parentIndex': 0,
+   'xml': 'xmls/ip-dhcp-host.xml',
+   'flag': 'current',
+  }
+
+def update(params):
+Update a network from xml
+global logger
+logger = params['logger']
+networkname = params['networkname']
+conn = sharedmod.libvirtobj['conn']
+
+command = params['command']
+logger.info(The specified command is %s % command)
+section = params['section']
+logger.info(The specified section is %s % section)
+parentIndex = int(params.get('parentIndex', 0))
+logger.info(The specified parentIndex is %d % parentIndex)
+xmlstr = params.get('xml', 'xmls/ip-dhcp-host.xml').replace('\','\'')
+logger.info(The specified updatexml is %s % xmlstr)
+flag = params.get('flag', 'current')
+logger.info(The specified flag is %s % flag)
+
+command_val = 0
+section_val = 0
+flag_val = 0
+if COMMANDDICT.has_key(command):
+command_val = COMMANDDICT.get(command)
+if SECTIONDICT.has_key(section):
+section_val = SECTIONDICT.get(section)
+if FLAGSDICT.has_key(flag):
+flag_val = FLAGSDICT.get(flag)
+
+try:
+network = conn.networkLookupByName(networkname)
+logger.info(The original network xml is %s % network.XMLDesc(0))
+network.update(command_val, section_val, parentIndex, xmlstr, \
+   flag_val)
+updated_netxml = network.XMLDesc(0)
+logger.info(The updated network xml is %s % updated_netxml)
+#The check only works when flag isn't set as config
+if flag_val !=2:
+if command_val == 0 or command_val == 2:
+if xmlstr not in updated_netxml:
+logger.info(Successfully update network)
+return 0
+else:
+logger.error(Failed to update network)
+return 1
+
+elif command_val == 1 or command_val == 4:
+if xmlstr in updated_netxml:
+logger.info(Successfully update network)
+return 0
+else:
+logger.error(Failed to update network)
+return 1
+
+except libvirtError, e:
+logger.error(API error message: %s, 

Re: [libvirt] Hyper-V driver API version support

[Daniel P. Berrange]

On Fri, Aug 09, 2013 at 10:13:03AM +0200, surf...@me.is-a-linux-user.org wrote:
 Hello
 
 The version function is not supported by the hyperv driver:
 $ virsh --connect=hyperv://hypervhost version
 Compiled against library: libvirt 1.1.1
 Using library: libvirt 1.1.1
 Using API: Hyper-V 1.1.1
 error: failed to get the hypervisor version
 error: this function is not supported by the connection driver:
 virConnectGetVersion
 
 But we need this funtion for the external/libvirt stonith plugin
 of clusterglue:
 
 $ cat /usr/lib/stonith/plugins/libvirt | more
 # get status of stonith device (*NOT* of the domain).
 # If we can retrieve some info from the hypervisor
 # the stonith device is OK.
 libvirt_status() {
 out=$($VIRSH -c $hypervisor_uri version 21)
 if [ $? -eq 0 ]
 then
 out=`echo $out | tail -1`
 ha_log.sh notice $hypervisor_uri: $out
 return 0
 fi
 
 ha_log.sh err Failed to get status for $hypervisor_uri
 ha_log.sh err $out
 return 1
 }
 
 So, we can't implement libvirt stonith with hyperv support in our
 corosync/pacemaker cluster. Is it possible to implement the
 version function for hyperv into virConnectGetVersion? Or exist
 any workaround for this problem?

I'm sure its possible, but it needs someone with knowledge of the
Hyper-V apis to write a patch and there's only a couple of people
in libvirt comunity who can do that. Patches welcome from any able
person...

Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

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

[libvirt] [libvirt-sandbox][PATCH 0/2] Fix some issues in virt-sandbox-service

[Alex Jia]

Alex Jia (2):
  Fix logical judgement in get_name
  Raise clear error message if no legacy configuration

 bin/virt-sandbox-service | 11 +--
 1 file changed, 9 insertions(+), 2 deletions(-)

-- 
1.8.3.1

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

[libvirt] [libvirt-sandbox][PATCH 1/2] Fix logical judgement in get_name

[Alex Jia]

Signed-off-by: Alex Jia a...@redhat.com
---
 bin/virt-sandbox-service | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/bin/virt-sandbox-service b/bin/virt-sandbox-service
index 03873c9..26b4a40 100755
--- a/bin/virt-sandbox-service
+++ b/bin/virt-sandbox-service
@@ -453,8 +453,12 @@ WantedBy=multi-user.target
 
 def get_name(self):
 if self.config:
-return self.config.get_name()
-raise ValueError([_(Name not configured)])
+name = self.config.get_name()
+if not name:
+raise ValueError([_(Name not configured)])
+return name
+sys.stderr.write(The configuration %s does not exist\n % self.config)
+sys.exit(1)
 
 def set_copy(self, copy):
 self.copy = copy
-- 
1.8.3.1

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

[libvirt] [libvirt-sandbox][PATCH 2/2] Raise clear error message if no legacy configuration

[Alex Jia]

Signed-off-by: Alex Jia a...@redhat.com
---
 bin/virt-sandbox-service | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/bin/virt-sandbox-service b/bin/virt-sandbox-service
index 26b4a40..cb40f6a 100755
--- a/bin/virt-sandbox-service
+++ b/bin/virt-sandbox-service
@@ -965,6 +965,9 @@ def upgrade_config(args):
 configfile = get_legacy_config_path(args.name)
 if os.path.exists(configfile):
 upgrade_config_legacy(configfile)
+else:
+sys.stderr.write(No legacy '%s' configuration\n % args.name)
+sys.exit(1)
 
 
 def upgrade_filesystem(args):
-- 
1.8.3.1

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

Re: [libvirt] [libvirt-sandbox][PATCH 1/2] Fix logical judgement in get_name

[Daniel P. Berrange]

On Fri, Aug 09, 2013 at 06:26:46PM +0800, Alex Jia wrote:

Please explain the scenario where you hit the flaw in the commit
message. I can see what you've changed, but I don't see why you
have changed it. The commit message must describe the 'why'.

 Signed-off-by: Alex Jia a...@redhat.com
 ---
  bin/virt-sandbox-service | 8 ++--
  1 file changed, 6 insertions(+), 2 deletions(-)
 
 diff --git a/bin/virt-sandbox-service b/bin/virt-sandbox-service
 index 03873c9..26b4a40 100755
 --- a/bin/virt-sandbox-service
 +++ b/bin/virt-sandbox-service
 @@ -453,8 +453,12 @@ WantedBy=multi-user.target
  
  def get_name(self):
  if self.config:
 -return self.config.get_name()
 -raise ValueError([_(Name not configured)])
 +name = self.config.get_name()
 +if not name:
 +raise ValueError([_(Name not configured)])
 +return name
 +sys.stderr.write(The configuration %s does not exist\n % 
 self.config)
 +sys.exit(1)
  
  def set_copy(self, copy):
  self.copy = copy

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] [libvirt-sandbox][PATCH 2/2] Raise clear error message if no legacy configuration

[Daniel P. Berrange]

On Fri, Aug 09, 2013 at 06:26:47PM +0800, Alex Jia wrote:
 Signed-off-by: Alex Jia a...@redhat.com
 ---
  bin/virt-sandbox-service | 3 +++
  1 file changed, 3 insertions(+)
 
 diff --git a/bin/virt-sandbox-service b/bin/virt-sandbox-service
 index 26b4a40..cb40f6a 100755
 --- a/bin/virt-sandbox-service
 +++ b/bin/virt-sandbox-service
 @@ -965,6 +965,9 @@ def upgrade_config(args):
  configfile = get_legacy_config_path(args.name)
  if os.path.exists(configfile):
  upgrade_config_legacy(configfile)
 +else:
 +sys.stderr.write(No legacy '%s' configuration\n % args.name)
 +sys.exit(1)

This isn't desired. This command is intended to be a no-op if nothing
needs changing. It is not just about upgrading from this legacy
config file layout. In the future I expet us to add more code here
as we make further changes. So it is right to silently exit with
success here, not report an error.

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] [libvirt-sandbox][PATCH 2/2] Raise clear error message if no legacy configuration

[Alex Jia]

On 08/09/2013 06:30 PM, Daniel P. Berrange wrote:

On Fri, Aug 09, 2013 at 06:26:47PM +0800, Alex Jia wrote:

Signed-off-by: Alex Jiaa...@redhat.com
---
  bin/virt-sandbox-service | 3 +++
  1 file changed, 3 insertions(+)

diff --git a/bin/virt-sandbox-service b/bin/virt-sandbox-service
index 26b4a40..cb40f6a 100755
--- a/bin/virt-sandbox-service
+++ b/bin/virt-sandbox-service
@@ -965,6 +965,9 @@ def upgrade_config(args):
  configfile = get_legacy_config_path(args.name)
  if os.path.exists(configfile):
  upgrade_config_legacy(configfile)
+else:
+sys.stderr.write(No legacy '%s' configuration\n % args.name)
+sys.exit(1)

This isn't desired. This command is intended to be a no-op if nothing
needs changing. It is not just about upgrading from this legacy
config file layout. In the future I expet us to add more code here
as we make further changes. So it is right to silently exit with
success here, not report an error.


Ok, got it and thanks for your review.


Daniel


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

Re: [libvirt] [libvirt-sandbox][PATCH 1/2] Fix logical judgement in get_name

[Alex Jia]

On 08/09/2013 06:29 PM, Daniel P. Berrange wrote:

On Fri, Aug 09, 2013 at 06:26:46PM +0800, Alex Jia wrote:

Please explain the scenario where you hit the flaw in the commit
message. I can see what you've changed, but I don't see why you
have changed it. The commit message must describe the 'why'.


ok, I will explain it in v2.


Signed-off-by: Alex Jiaa...@redhat.com
---
  bin/virt-sandbox-service | 8 ++--
  1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/bin/virt-sandbox-service b/bin/virt-sandbox-service
index 03873c9..26b4a40 100755
--- a/bin/virt-sandbox-service
+++ b/bin/virt-sandbox-service
@@ -453,8 +453,12 @@ WantedBy=multi-user.target

  def get_name(self):
  if self.config:
-return self.config.get_name()
-raise ValueError([_(Name not configured)])
+name = self.config.get_name()
+if not name:
+raise ValueError([_(Name not configured)])
+return name
+sys.stderr.write(The configuration %s does not exist\n % self.config)
+sys.exit(1)

  def set_copy(self, copy):
  self.copy = copy

Daniel


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

[libvirt] [libvirt-sandbox][PATCH v2] Fix logical judgement in get_name

[Alex Jia]

As usual, this issue can't be hit, but from codes point of view,
if deliberately remove 'name' in the configuration, and then the
'Name not congfigured' error message can't be raised unless the
configuration file doesn't exist, in fact, the get_name() will 
directly return None without expected error.

Signed-off-by: Alex Jia a...@redhat.com
---
 bin/virt-sandbox-service | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/bin/virt-sandbox-service b/bin/virt-sandbox-service
index 03873c9..26b4a40 100755
--- a/bin/virt-sandbox-service
+++ b/bin/virt-sandbox-service
@@ -453,8 +453,12 @@ WantedBy=multi-user.target
 
 def get_name(self):
 if self.config:
-return self.config.get_name()
-raise ValueError([_(Name not configured)])
+name = self.config.get_name()
+if not name:
+raise ValueError([_(Name not configured)])
+return name
+sys.stderr.write(The configuration %s does not exist\n % self.config)
+sys.exit(1)
 
 def set_copy(self, copy):
 self.copy = copy
-- 
1.8.3.1

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

Re: [libvirt] Updated patch for virt-login-shell for joing libvirt lxc containers.

[John Ferlan]

On 08/02/2013 11:22 AM, Daniel J Walsh wrote:
 -BEGIN PGP SIGNED MESSAGE-
 Hash: SHA1
 
 THis patch fixes all of Eric's and Daniels comments.
 
 [PATCH] virt-login-shell joins users into lxc container.
 -BEGIN PGP SIGNATURE-
 Version: GnuPG v1.4.14 (GNU/Linux)
 Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/
 
 iEYEARECAAYFAlH7zp8ACgkQrlYvE4MpobNx3gCbBtxw7T4fzIfHSyEEKKyjojXR
 BUUAoOToptiTOi+RC6Bdcp+zvg/xzfRh
 =7zpw
 -END PGP SIGNATURE-
 
 
 
The overnight run of Coverity found two issues with this module. I've 
cut-n-pasted the relevant portions of the error/traces:

#1: DEADCODE: virLoginShellAllowedUser()


86  if (pp-str[0] == '%') {

(1) Event assignment:   Assigning: ptr = pp-str + 1.
Also see events:[notnull][dead_error_condition][dead_error_line]

87  ptr = pp-str[1];

(2) Event notnull:  At condition ptr, the value of ptr cannot be NULL.
(3) Event dead_error_condition: The condition !ptr cannot be true.
Also see events:[assignment][dead_error_line]

88  if (!ptr)

(4) Event dead_error_line:  Execution cannot reach this statement 
continue;.
Also see events:[assignment][notnull][dead_error_condition]

89  continue;

Did you perhaps mean (!*ptr) (eg, the contents not the value?)


#2: USE_AFTER_FREE: main()

252 if ((ngroups = virGetGroupList(uid, gid, groups))  0)
253 goto cleanup;
254 

(18) Event freed_arg:   virLoginShellAllowedUser(virConfPtr, char const *, 
gid_t *) frees groups. [details]
(19) Event cond_false:  Condition virLoginShellAllowedUser(conf, name, groups) 
 0, taking false branch
Also see events:[double_free]

255 if (virLoginShellAllowedUser(conf, name, groups)  0)
256 goto cleanup;
257 


Which is true:

...

61  
62  static int virLoginShellAllowedUser(virConfPtr conf,
63  const char *name,
64  gid_t *groups)
65  {
...
(6) Event label:Reached label cleanup

110 cleanup:
111 VIR_FREE(gname);

(7) Event freed_arg:virFree(void *) frees parameter groups. [details]

112 VIR_FREE(groups);
113 return ret;
114 }

...

Then back in main()

338 

(24) Event label:   Reached label cleanup

339 cleanup:
340 virConfFree(conf);
341 virLoginShellFini(conn, dom);
342 virStringFreeList(shargv);
343 VIR_FREE(name);
344 VIR_FREE(homedir);
345 VIR_FREE(seclabel);
346 VIR_FREE(secmodel);

(25) Event double_free: Calling virFree(void *) frees pointer 
groups which has already been freed. [details]
Also see events:[freed_arg]

347 VIR_FREE(groups);


Not quite sure how you want to approach fixing the second error.  There are
two VIR_FREE(groups) in virLoginShellAllowedUser() and both would need to 
be handled properly.  Additionally code in main() seems to perhaps use
groups() after the call - so freeing it could have other consequences too.


John

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

[libvirt] [PATCH] Fix double-free and broken logic in virt-login-shell

[Daniel P. Berrange]

From: Daniel P. Berrange berra...@redhat.com

The virLoginShellAllowedUser method must not free the 'groups'
parameter it is given, as that is owned by the caller.

The virLoginShellAllowedUser method should be checking
'!*ptr' (ie empty string) rather than '!ptr' (NULL string)
since the latter cannot be true.

Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
 tools/virt-login-shell.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/tools/virt-login-shell.c b/tools/virt-login-shell.c
index b8f1a28..b27e44f 100644
--- a/tools/virt-login-shell.c
+++ b/tools/virt-login-shell.c
@@ -85,7 +85,7 @@ static int virLoginShellAllowedUser(virConfPtr conf,
 */
 if (pp-str[0] == '%') {
 ptr = pp-str[1];
-if (!ptr)
+if (!*ptr)
 continue;
 for (i = 0; groups[i]; i++) {
 if (!(gname = virGetGroupName(groups[i])))
@@ -96,7 +96,6 @@ static int virLoginShellAllowedUser(virConfPtr conf,
 }
 VIR_FREE(gname);
 }
-VIR_FREE(groups);
 continue;
 }
 if (fnmatch(pp-str, name, 0) == 0) {
@@ -109,7 +108,6 @@ static int virLoginShellAllowedUser(virConfPtr conf,
 virReportSystemError(EPERM, _(%s not listed as an allowed_users in %s), 
name, conf_file);
 cleanup:
 VIR_FREE(gname);
-VIR_FREE(groups);
 return ret;
 }
 
-- 
1.8.3.1

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

Re: [libvirt] [PATCH] Fix double-free and broken logic in virt-login-shell

[John Ferlan]

On 08/09/2013 07:01 AM, Daniel P. Berrange wrote:
 From: Daniel P. Berrange berra...@redhat.com
 
 The virLoginShellAllowedUser method must not free the 'groups'
 parameter it is given, as that is owned by the caller.
 
 The virLoginShellAllowedUser method should be checking
 '!*ptr' (ie empty string) rather than '!ptr' (NULL string)
 since the latter cannot be true.
 
 Signed-off-by: Daniel P. Berrange berra...@redhat.com
 ---
  tools/virt-login-shell.c | 4 +---
  1 file changed, 1 insertion(+), 3 deletions(-)
 

ACK


John

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

Re: [libvirt] Updated patch for virt-login-shell for joing libvirt lxc containers.

[Daniel P. Berrange]

On Fri, Aug 09, 2013 at 06:51:25AM -0400, John Ferlan wrote:
 On 08/02/2013 11:22 AM, Daniel J Walsh wrote:
  -BEGIN PGP SIGNED MESSAGE-
  Hash: SHA1
  
  THis patch fixes all of Eric's and Daniels comments.
  
  [PATCH] virt-login-shell joins users into lxc container.
  -BEGIN PGP SIGNATURE-
  Version: GnuPG v1.4.14 (GNU/Linux)
  Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/
  
  iEYEARECAAYFAlH7zp8ACgkQrlYvE4MpobNx3gCbBtxw7T4fzIfHSyEEKKyjojXR
  BUUAoOToptiTOi+RC6Bdcp+zvg/xzfRh
  =7zpw
  -END PGP SIGNATURE-
  
  
  
 The overnight run of Coverity found two issues with this module. I've 
 cut-n-pasted the relevant portions of the error/traces:
 
 #1: DEADCODE: virLoginShellAllowedUser()
 
 
 86if (pp-str[0] == '%') {
 
 (1) Event assignment: Assigning: ptr = pp-str + 1.
 Also see events:  [notnull][dead_error_condition][dead_error_line]
 
 87ptr = pp-str[1];
 
 (2) Event notnull:At condition ptr, the value of ptr cannot be NULL.
 (3) Event dead_error_condition:   The condition !ptr cannot be true.
 Also see events:  [assignment][dead_error_line]
 
 88if (!ptr)
 
 (4) Event dead_error_line:Execution cannot reach this statement 
 continue;.
 Also see events:  [assignment][notnull][dead_error_condition]
 
 89continue;
 
 Did you perhaps mean (!*ptr) (eg, the contents not the value?)

Yes, that should have been   !*ptr.


 #2: USE_AFTER_FREE: main()
 
 252   if ((ngroups = virGetGroupList(uid, gid, groups))  0)
 253   goto cleanup;
 254   
 
 (18) Event freed_arg: virLoginShellAllowedUser(virConfPtr, char 
 const *, gid_t *) frees groups. [details]
 (19) Event cond_false:Condition virLoginShellAllowedUser(conf, name, 
 groups)  0, taking false branch
 Also see events:  [double_free]
 
 255   if (virLoginShellAllowedUser(conf, name, groups)  0)
 256   goto cleanup;
 257   
 
 
 Which is true:
 
 ...
 
 61
 62static int virLoginShellAllowedUser(virConfPtr conf,
 63const char *name,
 64gid_t *groups)
 65{
 ...
 (6) Event label:  Reached label cleanup
 
 110   cleanup:
 111   VIR_FREE(gname);
 
 (7) Event freed_arg:  virFree(void *) frees parameter groups. [details]
 
 112   VIR_FREE(groups);
 113   return ret;
 114   }
 
 ...
 
 Then back in main()
 
 338   
 
 (24) Event label: Reached label cleanup
 
 339   cleanup:
 340   virConfFree(conf);
 341   virLoginShellFini(conn, dom);
 342   virStringFreeList(shargv);
 343   VIR_FREE(name);
 344   VIR_FREE(homedir);
 345   VIR_FREE(seclabel);
 346   VIR_FREE(secmodel);
 
 (25) Event double_free:   Calling virFree(void *) frees pointer 
 groups which has already been freed. [details]
 Also see events:  [freed_arg]
 
 347   VIR_FREE(groups);
 
 
 Not quite sure how you want to approach fixing the second error.  There are
 two VIR_FREE(groups) in virLoginShellAllowedUser() and both would need to 
 be handled properly.  Additionally code in main() seems to perhaps use
 groups() after the call - so freeing it could have other consequences too.

The virLoginShellAllowedUser() method has absolutely no business free'ing
the 'groups' parameter it is given. That pointer is owned by the caller
who will free it when needed.


I've sent a patch which should fix both these issues.

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] virnettlscontext: Resolve Coverity warnings (UNINIT)

[John Ferlan]

Coverity complained about the usage of the uninitialized cacerts in the
event(s) that access(certFile, R_OK) and/or access(cacertFile, R_OK)
fail the for loop used to fill in the certs will have indeterminate data
as well as the possibility that both failures would result in the
gnutls_x509_crt_deinit() call having a similar fate.

Initializing cacerts only would resolve the issue; however, it still
would leave the indeterminate action, so rather add a parameter to
the virNetTLSContextLoadCACertListFromFile() to pass the max size rather
then overloading the returned count parameter. If the the call is never
made, then we won't go through the for loops referencing the empty
cacerts
---
 src/rpc/virnettlscontext.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/src/rpc/virnettlscontext.c b/src/rpc/virnettlscontext.c
index 2beee8f..7cee27c 100644
--- a/src/rpc/virnettlscontext.c
+++ b/src/rpc/virnettlscontext.c
@@ -545,12 +545,12 @@ cleanup:
 
 static int virNetTLSContextLoadCACertListFromFile(const char *certFile,
   gnutls_x509_crt_t *certs,
+  unsigned int certMax,
   size_t *ncerts)
 {
 gnutls_datum_t data;
 char *buf = NULL;
 int ret = -1;
-unsigned int certMax = *ncerts;
 
 *ncerts = 0;
 VIR_DEBUG(certFile %s, certFile);
@@ -584,15 +584,17 @@ static int virNetTLSContextSanityCheckCredentials(bool 
isServer,
 {
 gnutls_x509_crt_t cert = NULL;
 gnutls_x509_crt_t cacerts[MAX_CERTS];
-size_t ncacerts = MAX_CERTS;
+size_t ncacerts = 0;
 size_t i;
 int ret = -1;
 
+memset(cacerts, 0, sizeof(cacerts));
 if ((access(certFile, R_OK) == 0) 
 !(cert = virNetTLSContextLoadCertFromFile(certFile, isServer)))
 goto cleanup;
 if ((access(cacertFile, R_OK) == 0) 
-virNetTLSContextLoadCACertListFromFile(cacertFile, cacerts, ncacerts) 
 0)
+virNetTLSContextLoadCACertListFromFile(cacertFile, cacerts,
+   MAX_CERTS, ncacerts)  0)
 goto cleanup;
 
 if (cert 
-- 
1.8.3.1

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

Re: [libvirt] [PATCH] virnettlscontext: Resolve Coverity warnings (UNINIT)

[Daniel P. Berrange]

On Fri, Aug 09, 2013 at 07:19:18AM -0400, John Ferlan wrote:
 Coverity complained about the usage of the uninitialized cacerts in the
 event(s) that access(certFile, R_OK) and/or access(cacertFile, R_OK)
 fail the for loop used to fill in the certs will have indeterminate data
 as well as the possibility that both failures would result in the
 gnutls_x509_crt_deinit() call having a similar fate.
 
 Initializing cacerts only would resolve the issue; however, it still
 would leave the indeterminate action, so rather add a parameter to
 the virNetTLSContextLoadCACertListFromFile() to pass the max size rather
 then overloading the returned count parameter. If the the call is never
 made, then we won't go through the for loops referencing the empty
 cacerts
 ---
  src/rpc/virnettlscontext.c | 8 +---
  1 file changed, 5 insertions(+), 3 deletions(-)
 
 diff --git a/src/rpc/virnettlscontext.c b/src/rpc/virnettlscontext.c
 index 2beee8f..7cee27c 100644
 --- a/src/rpc/virnettlscontext.c
 +++ b/src/rpc/virnettlscontext.c
 @@ -545,12 +545,12 @@ cleanup:
  
  static int virNetTLSContextLoadCACertListFromFile(const char *certFile,
gnutls_x509_crt_t *certs,
 +  unsigned int certMax,
size_t *ncerts)
  {
  gnutls_datum_t data;
  char *buf = NULL;
  int ret = -1;
 -unsigned int certMax = *ncerts;
  
  *ncerts = 0;
  VIR_DEBUG(certFile %s, certFile);
 @@ -584,15 +584,17 @@ static int virNetTLSContextSanityCheckCredentials(bool 
 isServer,
  {
  gnutls_x509_crt_t cert = NULL;
  gnutls_x509_crt_t cacerts[MAX_CERTS];
 -size_t ncacerts = MAX_CERTS;
 +size_t ncacerts = 0;
  size_t i;
  int ret = -1;
  
 +memset(cacerts, 0, sizeof(cacerts));
  if ((access(certFile, R_OK) == 0) 
  !(cert = virNetTLSContextLoadCertFromFile(certFile, isServer)))
  goto cleanup;
  if ((access(cacertFile, R_OK) == 0) 
 -virNetTLSContextLoadCACertListFromFile(cacertFile, cacerts, 
 ncacerts)  0)
 +virNetTLSContextLoadCACertListFromFile(cacertFile, cacerts,
 +   MAX_CERTS, ncacerts)  0)
  goto cleanup;
  
  if (cert 

ACK


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] Fix double-free and broken logic in virt-login-shell

[Eric Blake]

On 08/09/2013 05:01 AM, Daniel P. Berrange wrote:
 From: Daniel P. Berrange berra...@redhat.com
 
 The virLoginShellAllowedUser method must not free the 'groups'
 parameter it is given, as that is owned by the caller.
 
 The virLoginShellAllowedUser method should be checking
 '!*ptr' (ie empty string) rather than '!ptr' (NULL string)
 since the latter cannot be true.

This only fixes the blatant errors that I called out, but there are
still more (cosmetic) fixes needed:
https://www.redhat.com/archives/libvir-list/2013-August/msg00398.html

-- 
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] Fix double-free and broken logic in virt-login-shell

[Daniel P. Berrange]

On Fri, Aug 09, 2013 at 06:27:36AM -0600, Eric Blake wrote:
 On 08/09/2013 05:01 AM, Daniel P. Berrange wrote:
  From: Daniel P. Berrange berra...@redhat.com
  
  The virLoginShellAllowedUser method must not free the 'groups'
  parameter it is given, as that is owned by the caller.
  
  The virLoginShellAllowedUser method should be checking
  '!*ptr' (ie empty string) rather than '!ptr' (NULL string)
  since the latter cannot be true.
 
 This only fixes the blatant errors that I called out, but there are
 still more (cosmetic) fixes needed:
 https://www.redhat.com/archives/libvir-list/2013-August/msg00398.html

Ah, I didn't see your reply - i was just responding to John's
coverity report. I'll put together a patch for the other fixes
too.


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] Updated patch for virt-login-shell for joing libvirt lxc containers.

[Eric Blake]

On 08/09/2013 04:51 AM, John Ferlan wrote:

 
 88if (!ptr)
 
 (4) Event dead_error_line:Execution cannot reach this statement 
 continue;.
 Also see events:  [assignment][notnull][dead_error_condition]
 
 89continue;
 
 Did you perhaps mean (!*ptr) (eg, the contents not the value?)

Ha - I caught that one earlier in the day by manual code inspection :)
https://www.redhat.com/archives/libvir-list/2013-August/msg00398.html

I also caught other instances of dead code not listed by Coverity,
although with less impact.

 
 
 #2: USE_AFTER_FREE: main()

But I missed that one, so Coverity is still proving useful.

-- 
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] Add an example config file for virtlockd

[Eric Blake]

On 08/08/2013 09:08 AM, Daniel P. Berrange wrote:
 From: Daniel P. Berrange berra...@redhat.com
 
 The virtlockd daemon supports an /etc/libvirt/virtlockd.conf
 config file, but we never installed a default config, nor
 created any augeas scripts. This change addresses that omission.
 
 Signed-off-by: Daniel P. Berrange berra...@redhat.com
 ---
  .gitignore|  1 +
  libvirt.spec.in   |  3 ++
  src/Makefile.am   | 20 -
  src/locking/test_virtlockd.aug.in | 12 
  src/locking/virtlockd.aug | 44 
  src/locking/virtlockd.conf| 60 
 +++
  6 files changed, 139 insertions(+), 1 deletion(-)
  create mode 100644 src/locking/test_virtlockd.aug.in
  create mode 100644 src/locking/virtlockd.aug
  create mode 100644 src/locking/virtlockd.conf

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] [PATCH] libxl: Create per-domain log file

[Michal Privoznik]

On 09.08.2013 01:41, Jim Fehlig wrote:
 Currently, only one log file is created by the libxl driver, with
 all output from libxl for all domains going to this one file.
 
 Create a per-domain log file based on domain name, making sifting
 through the logs a bit easier.  This required deferring libxl_ctx
 allocation until starting the domain, which is fine since the
 ctx is not used when the domain is inactive.
 ---
  src/libxl/libxl_conf.h   |  5 +--
  src/libxl/libxl_driver.c | 88 
 +---
  2 files changed, 57 insertions(+), 36 deletions(-)
 

ACK

Michal

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

Re: [libvirt] [PATCH] Add documentation for access control system

[Michal Privoznik]

On 08.08.2013 13:27, Daniel P. Berrange wrote:
 From: Daniel P. Berrange berra...@redhat.com
 
 This adds two new pages to the website, acl.html describing
 the general access control framework and permissions models,
 and aclpolkit.html describing the use of polkit as an
 access control driver.
 
 page.xsl is modified to support a new syntax
 
   div id=include filename=somefile.htmlinc/
 
 which will cause the XSL transform to replace that div
 with the contents of 'somefile.htmlinc'. We use this in
 the acl.html.in file, to pull the table of permissions
 for each libvirt object. This table is autogenerated
 from the enums in src/access/viraccessperms.h by the
 genaclperms.pl script.
 
 newapi.xsl is modified so that the list of permissions
 checks shown against each API will link to the description
 of the permissions in acl.html
 
 Signed-off-by: Daniel P. Berrange berra...@redhat.com
 ---
  .gitignore |   1 +
  docs/Makefile.am   |  12 +-
  docs/acl.html.in   | 100 
  docs/aclpolkit.html.in | 414 
 +
  docs/auth.html.in  |   6 +-
  docs/genaclperms.pl| 124 +++
  docs/newapi.xsl|   4 +-
  docs/page.xsl  |  11 ++
  docs/sitemap.html.in   |  10 ++
  9 files changed, 677 insertions(+), 5 deletions(-)
  create mode 100644 docs/acl.html.in
  create mode 100644 docs/aclpolkit.html.in
  create mode 100644 docs/genaclperms.pl

ACL, I mean ACK :)

Michal

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

[libvirt] [PATCH] qemu: Drop qemuDomainMemoryLimit

[Michal Privoznik]

This function is to guess the correct limit for maximal memory
usage by qemu for given domain. This can never be guessed
correctly, not to mention all the pains and sleepless nights this
code has caused. Once somebody discovers algorithm to solve the
Halting Problem, we can compute the limit algorithmically. But
till then, this code should never see the light of the release
again.
---
 src/qemu/qemu_cgroup.c  |  3 +--
 src/qemu/qemu_command.c |  2 +-
 src/qemu/qemu_domain.c  | 49 -
 src/qemu/qemu_domain.h  |  2 --
 src/qemu/qemu_hotplug.c |  2 +-
 5 files changed, 3 insertions(+), 55 deletions(-)

diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index dc949db..9673e8e 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -428,8 +428,7 @@ qemuSetupMemoryCgroup(virDomainObjPtr vm)
 }
 }
 
-if (virCgroupSetMemoryHardLimit(priv-cgroup,
-qemuDomainMemoryLimit(vm-def))  0)
+if (virCgroupSetMemoryHardLimit(priv-cgroup, vm-def-mem.hard_limit)  0)
 return -1;
 
 if (vm-def-mem.soft_limit != 0 
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index b811e1d..a0a1773 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -9220,7 +9220,7 @@ qemuBuildCommandLine(virConnectPtr conn,
 }
 
 if (mlock)
-virCommandSetMaxMemLock(cmd, qemuDomainMemoryLimit(def) * 1024);
+virCommandSetMaxMemLock(cmd, def-mem.hard_limit * 1024);
 
 virObjectUnref(cfg);
 return cmd;
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 393af6b..7f4d17d 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -2306,55 +2306,6 @@ cleanup:
 return ret;
 }
 
-
-unsigned long long
-qemuDomainMemoryLimit(virDomainDefPtr def)
-{
-unsigned long long mem;
-size_t i;
-
-if (def-mem.hard_limit) {
-mem = def-mem.hard_limit;
-} else {
-/* If there is no hard_limit set, compute a reasonable one to avoid
- * system thrashing caused by exploited qemu.  A 'reasonable
- * limit' has been chosen:
- * (1 + k) * (domain memory + total video memory) + (32MB for
- * cache per each disk) + F
- * where k = 0.5 and F = 400MB.  The cache for disks is important as
- * kernel cache on the host side counts into the RSS limit.
- * Moreover, VFIO requires some amount for IO space. Alex Williamson
- * suggested adding 1GiB for IO space just to be safe (some finer
- * tuning might be nice, though).
- *
- * Technically, the disk cache does not have to be included in
- * RLIMIT_MEMLOCK but it doesn't hurt as it's just an upper limit and
- * it makes this function and its usage simpler.
- */
-mem = def-mem.max_balloon;
-for (i = 0; i  def-nvideos; i++)
-mem += def-videos[i]-vram;
-mem *= 1.5;
-mem += def-ndisks * 32768;
-mem += 409600;
-
-for (i = 0; i  def-nhostdevs; i++) {
-virDomainHostdevDefPtr hostdev = def-hostdevs[i];
-if (hostdev-mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS 
-hostdev-source.subsys.type ==
-VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI 
-hostdev-source.subsys.u.pci.backend ==
-VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO) {
-mem += 1024 * 1024;
-break;
-}
-}
-}
-
-return mem;
-}
-
-
 int
 qemuDomainUpdateDeviceList(virQEMUDriverPtr driver,
virDomainObjPtr vm)
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 0a4a51e..21f116c 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -365,8 +365,6 @@ extern virDomainXMLPrivateDataCallbacks 
virQEMUDriverPrivateDataCallbacks;
 extern virDomainXMLNamespace virQEMUDriverDomainXMLNamespace;
 extern virDomainDefParserConfig virQEMUDriverDomainDefParserConfig;
 
-unsigned long long qemuDomainMemoryLimit(virDomainDefPtr def);
-
 int qemuDomainUpdateDeviceList(virQEMUDriverPtr driver,
virDomainObjPtr vm);
 
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index c9748d9..fa64dd7 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1030,7 +1030,7 @@ int qemuDomainAttachHostPciDevice(virQEMUDriverPtr driver,
  */
 vm-def-hostdevs[vm-def-nhostdevs++] = hostdev;
 virProcessSetMaxMemLock(vm-pid,
-qemuDomainMemoryLimit(vm-def) * 1024);
+vm-def-mem.hard_limit * 1024);
 vm-def-hostdevs[vm-def-nhostdevs--] = NULL;
 }
 
-- 
1.8.1.5

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

Re: [libvirt] [PATCH] qemu: Drop qemuDomainMemoryLimit

[Eric Blake]

On 08/09/2013 06:56 AM, Michal Privoznik wrote:
 This function is to guess the correct limit for maximal memory
 usage by qemu for given domain. This can never be guessed
 correctly, not to mention all the pains and sleepless nights this
 code has caused. Once somebody discovers algorithm to solve the
 Halting Problem, we can compute the limit algorithmically. But
 till then, this code should never see the light of the release
 again.
 ---
  src/qemu/qemu_cgroup.c  |  3 +--
  src/qemu/qemu_command.c |  2 +-
  src/qemu/qemu_domain.c  | 49 
 -
  src/qemu/qemu_domain.h  |  2 --
  src/qemu/qemu_hotplug.c |  2 +-
  5 files changed, 3 insertions(+), 55 deletions(-)

ACK.  Users that put an explicit limit in their XML are taking on their
own risk at guessing correctly; all other users should not be forced to
suffer from a bad guess on our part killing their domain.

-- 
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] qemu: Drop qemuDomainMemoryLimit

[Daniel P. Berrange]

On Fri, Aug 09, 2013 at 07:13:58AM -0600, Eric Blake wrote:
 On 08/09/2013 06:56 AM, Michal Privoznik wrote:
  This function is to guess the correct limit for maximal memory
  usage by qemu for given domain. This can never be guessed
  correctly, not to mention all the pains and sleepless nights this
  code has caused. Once somebody discovers algorithm to solve the
  Halting Problem, we can compute the limit algorithmically. But
  till then, this code should never see the light of the release
  again.
  ---
   src/qemu/qemu_cgroup.c  |  3 +--
   src/qemu/qemu_command.c |  2 +-
   src/qemu/qemu_domain.c  | 49 
  -
   src/qemu/qemu_domain.h  |  2 --
   src/qemu/qemu_hotplug.c |  2 +-
   5 files changed, 3 insertions(+), 55 deletions(-)
 
 ACK.  Users that put an explicit limit in their XML are taking on their
 own risk at guessing correctly; all other users should not be forced to
 suffer from a bad guess on our part killing their domain.

If we don't understand how to calculate a default limit that works,
how are users with even less knowledge than us, suppose to calculate
an explicit level of their own ?

This limit was designed so that the hosts are not vulnerable to DOS
attack from a compromised QEMU, so removing this is arguably introducing
a security weakness in our default deployment.

I think I'd like to see some feedback / agreement from QEMU developers
that this problem really can't be solved, before we remove it.

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] qemu: Drop qemuDomainMemoryLimit

[Michal Privoznik]

This function is to guess the correct limit for maximal memory
usage by qemu for given domain. This can never be guessed
correctly, not to mention all the pains and sleepless nights this
code has caused. Once somebody discovers algorithm to solve the
Halting Problem, we can compute the limit algorithmically. But
till then, this code should never see the light of the release
again.
---
 src/qemu/qemu_cgroup.c  |  3 +--
 src/qemu/qemu_command.c |  2 +-
 src/qemu/qemu_domain.c  | 49 -
 src/qemu/qemu_domain.h  |  2 --
 src/qemu/qemu_hotplug.c |  2 +-
 5 files changed, 3 insertions(+), 55 deletions(-)

diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index dc949db..9673e8e 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -428,8 +428,7 @@ qemuSetupMemoryCgroup(virDomainObjPtr vm)
 }
 }
 
-if (virCgroupSetMemoryHardLimit(priv-cgroup,
-qemuDomainMemoryLimit(vm-def))  0)
+if (virCgroupSetMemoryHardLimit(priv-cgroup, vm-def-mem.hard_limit)  0)
 return -1;
 
 if (vm-def-mem.soft_limit != 0 
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index b811e1d..a0a1773 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -9220,7 +9220,7 @@ qemuBuildCommandLine(virConnectPtr conn,
 }
 
 if (mlock)
-virCommandSetMaxMemLock(cmd, qemuDomainMemoryLimit(def) * 1024);
+virCommandSetMaxMemLock(cmd, def-mem.hard_limit * 1024);
 
 virObjectUnref(cfg);
 return cmd;
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 393af6b..7f4d17d 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -2306,55 +2306,6 @@ cleanup:
 return ret;
 }
 
-
-unsigned long long
-qemuDomainMemoryLimit(virDomainDefPtr def)
-{
-unsigned long long mem;
-size_t i;
-
-if (def-mem.hard_limit) {
-mem = def-mem.hard_limit;
-} else {
-/* If there is no hard_limit set, compute a reasonable one to avoid
- * system thrashing caused by exploited qemu.  A 'reasonable
- * limit' has been chosen:
- * (1 + k) * (domain memory + total video memory) + (32MB for
- * cache per each disk) + F
- * where k = 0.5 and F = 400MB.  The cache for disks is important as
- * kernel cache on the host side counts into the RSS limit.
- * Moreover, VFIO requires some amount for IO space. Alex Williamson
- * suggested adding 1GiB for IO space just to be safe (some finer
- * tuning might be nice, though).
- *
- * Technically, the disk cache does not have to be included in
- * RLIMIT_MEMLOCK but it doesn't hurt as it's just an upper limit and
- * it makes this function and its usage simpler.
- */
-mem = def-mem.max_balloon;
-for (i = 0; i  def-nvideos; i++)
-mem += def-videos[i]-vram;
-mem *= 1.5;
-mem += def-ndisks * 32768;
-mem += 409600;
-
-for (i = 0; i  def-nhostdevs; i++) {
-virDomainHostdevDefPtr hostdev = def-hostdevs[i];
-if (hostdev-mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS 
-hostdev-source.subsys.type ==
-VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI 
-hostdev-source.subsys.u.pci.backend ==
-VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO) {
-mem += 1024 * 1024;
-break;
-}
-}
-}
-
-return mem;
-}
-
-
 int
 qemuDomainUpdateDeviceList(virQEMUDriverPtr driver,
virDomainObjPtr vm)
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 0a4a51e..21f116c 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -365,8 +365,6 @@ extern virDomainXMLPrivateDataCallbacks 
virQEMUDriverPrivateDataCallbacks;
 extern virDomainXMLNamespace virQEMUDriverDomainXMLNamespace;
 extern virDomainDefParserConfig virQEMUDriverDomainDefParserConfig;
 
-unsigned long long qemuDomainMemoryLimit(virDomainDefPtr def);
-
 int qemuDomainUpdateDeviceList(virQEMUDriverPtr driver,
virDomainObjPtr vm);
 
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index c9748d9..fa64dd7 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1030,7 +1030,7 @@ int qemuDomainAttachHostPciDevice(virQEMUDriverPtr driver,
  */
 vm-def-hostdevs[vm-def-nhostdevs++] = hostdev;
 virProcessSetMaxMemLock(vm-pid,
-qemuDomainMemoryLimit(vm-def) * 1024);
+vm-def-mem.hard_limit * 1024);
 vm-def-hostdevs[vm-def-nhostdevs--] = NULL;
 }
 
-- 
1.8.1.5

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

Re: [libvirt] [PATCH] qemu: Drop qemuDomainMemoryLimit

[Michal Privoznik]

[CC'ing qemu-devel list]
On 09.08.2013 15:17, Daniel P. Berrange wrote:
 On Fri, Aug 09, 2013 at 07:13:58AM -0600, Eric Blake wrote:
 On 08/09/2013 06:56 AM, Michal Privoznik wrote:
 This function is to guess the correct limit for maximal memory
 usage by qemu for given domain. This can never be guessed
 correctly, not to mention all the pains and sleepless nights this
 code has caused. Once somebody discovers algorithm to solve the
 Halting Problem, we can compute the limit algorithmically. But
 till then, this code should never see the light of the release
 again.
 ---
  src/qemu/qemu_cgroup.c  |  3 +--
  src/qemu/qemu_command.c |  2 +-
  src/qemu/qemu_domain.c  | 49 
 -
  src/qemu/qemu_domain.h  |  2 --
  src/qemu/qemu_hotplug.c |  2 +-
  5 files changed, 3 insertions(+), 55 deletions(-)

 ACK.  Users that put an explicit limit in their XML are taking on their
 own risk at guessing correctly; all other users should not be forced to
 suffer from a bad guess on our part killing their domain.
 
 If we don't understand how to calculate a default limit that works,
 how are users with even less knowledge than us, suppose to calculate
 an explicit level of their own ?
 
 This limit was designed so that the hosts are not vulnerable to DOS
 attack from a compromised QEMU, so removing this is arguably introducing
 a security weakness in our default deployment.
 
 I think I'd like to see some feedback / agreement from QEMU developers
 that this problem really can't be solved, before we remove it.
 
 Daniel
 

In libvirt I've introduced a heuristic to guess the maximum limit for a
memory for a given VM definition. The rationale was it's better to be
safe by default and not let leaking qemu trash the host. The heuristic
is only used if user has not configured any limit himself. However, over
the time the number of users reporting OOM kills due to my heuristic has
grown. Finally, I've full nose of this problem so I've made a patch [1]
that removes this 'functionality' completely (I'd say it's bug after
all). In the patch you can see the heuristic we've converged to. But Dan
has his point. If libvirt  qemu devels aren't able to come up with
proper heuristic, how can an ordinary user (who doesn't have any
knowledge of code) do so? So before I apply my patch, I want to ask you
guys, what do you think about it.

Michal

1: https://www.redhat.com/archives/libvir-list/2013-August/msg00437.html

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

[libvirt] [PATCH] tests: avoid too-large constants

[Eric Blake]

Compiling with gcc 4.1.2 (RHEL 5) complains:

virdbustest.c: In function 'testMessageSimple':
virdbustest.c:61: warning: integer constant is too large for 'long' type
virdbustest.c:62: warning: integer constant is too large for 'long' type
virdbustest.c: In function 'testMessageArray':
virdbustest.c:183: warning: this decimal constant is unsigned only in ISO C90
virdbustest.c: In function 'testMessageStruct':
virdbustest.c:239: warning: integer constant is too large for 'long' type
virdbustest.c:240: warning: integer constant is too large for 'long' type

* tests/virdbustest.c (testMessageSiple, testMessageArray)
(testMessageStruct): Don't violate C89 constant constraints.

Signed-off-by: Eric Blake ebl...@redhat.com
---

Pushing under the build-breaker rule.

 tests/virdbustest.c | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/tests/virdbustest.c b/tests/virdbustest.c
index 61de937..528342b 100644
--- a/tests/virdbustest.c
+++ b/tests/virdbustest.c
@@ -58,8 +58,8 @@ static int testMessageSimple(const void *args 
ATTRIBUTE_UNUSED)
 unsigned short in_uint16 = 32000, out_uint16 = 0;
 int in_int32 = 1, out_int32 = 0;
 unsigned int in_uint32 = 2, out_uint32 = 0;
-long long in_int64 = 1, out_int64 = 0;
-unsigned long long in_uint64 = 2, out_uint64 = 0;
+long long in_int64 = 1LL, out_int64 = 0;
+unsigned long long in_uint64 = 2LL, out_uint64 = 0;
 double in_double = 3.14159265359, out_double = 0;;
 const char *in_string = Hello World;
 char *out_string = NULL;
@@ -178,9 +178,9 @@ static int testMessageArray(const void *args 
ATTRIBUTE_UNUSED)
 DBusMessage *msg = NULL;
 int ret = -1;
 const char *in_str1 = Hello;
-int in_int32a = 10, out_int32a = 0;
-int in_int32b = 20, out_int32b = 0;
-int in_int32c = 30, out_int32c = 0;
+int in_int32a = 1, out_int32a = 0;
+int in_int32b = 2, out_int32b = 0;
+int in_int32c = 3, out_int32c = 0;
 const char *in_str2 = World;
 char *out_str1 = NULL, *out_str2 = NULL;

@@ -236,8 +236,8 @@ static int testMessageStruct(const void *args 
ATTRIBUTE_UNUSED)
 unsigned short in_uint16 = 32000, out_uint16 = 0;
 int in_int32 = 1, out_int32 = 0;
 unsigned int in_uint32 = 2, out_uint32 = 0;
-long long in_int64 = 1, out_int64 = 0;
-unsigned long long in_uint64 = 2, out_uint64 = 0;
+long long in_int64 = 1LL, out_int64 = 0;
+unsigned long long in_uint64 = 2LL, out_uint64 = 0;
 double in_double = 3.14159265359, out_double = 0;;
 const char *in_string = Hello World;
 char *out_string = NULL;
-- 
1.8.3.1

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

Re: [libvirt] [PATCH] tests: avoid too-large constants

[Daniel P. Berrange]

On Fri, Aug 09, 2013 at 07:45:16AM -0600, Eric Blake wrote:
 Compiling with gcc 4.1.2 (RHEL 5) complains:
 
 virdbustest.c: In function 'testMessageSimple':
 virdbustest.c:61: warning: integer constant is too large for 'long' type
 virdbustest.c:62: warning: integer constant is too large for 'long' type
 virdbustest.c: In function 'testMessageArray':
 virdbustest.c:183: warning: this decimal constant is unsigned only in ISO C90
 virdbustest.c: In function 'testMessageStruct':
 virdbustest.c:239: warning: integer constant is too large for 'long' type
 virdbustest.c:240: warning: integer constant is too large for 'long' type
 
 * tests/virdbustest.c (testMessageSiple, testMessageArray)
 (testMessageStruct): Don't violate C89 constant constraints.
 
 Signed-off-by: Eric Blake ebl...@redhat.com
 ---
 
 Pushing under the build-breaker rule.
 
  tests/virdbustest.c | 14 +++---
  1 file changed, 7 insertions(+), 7 deletions(-)
 
 diff --git a/tests/virdbustest.c b/tests/virdbustest.c
 index 61de937..528342b 100644
 --- a/tests/virdbustest.c
 +++ b/tests/virdbustest.c
 @@ -58,8 +58,8 @@ static int testMessageSimple(const void *args 
 ATTRIBUTE_UNUSED)
  unsigned short in_uint16 = 32000, out_uint16 = 0;
  int in_int32 = 1, out_int32 = 0;
  unsigned int in_uint32 = 2, out_uint32 = 0;
 -long long in_int64 = 1, out_int64 = 0;
 -unsigned long long in_uint64 = 2, out_uint64 = 0;
 +long long in_int64 = 1LL, out_int64 = 0;
 +unsigned long long in_uint64 = 2LL, out_uint64 = 0;
  double in_double = 3.14159265359, out_double = 0;;
  const char *in_string = Hello World;
  char *out_string = NULL;
 @@ -178,9 +178,9 @@ static int testMessageArray(const void *args 
 ATTRIBUTE_UNUSED)
  DBusMessage *msg = NULL;
  int ret = -1;
  const char *in_str1 = Hello;
 -int in_int32a = 10, out_int32a = 0;
 -int in_int32b = 20, out_int32b = 0;
 -int in_int32c = 30, out_int32c = 0;
 +int in_int32a = 1, out_int32a = 0;
 +int in_int32b = 2, out_int32b = 0;
 +int in_int32c = 3, out_int32c = 0;

I actually intentionally choose 3 as a value that would
be above MAX_INT32 (2147483647). I guess what I really should
have done was use something like -2147483640 instead, so we
didn't rely on wrapping of 30. 

Could you change this test to use a large -ve number for the
3rd int, rather than stripping a 0 from all 3.

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] Proposing Archive / Unarchive VM Guest functionality.

[Daniel P. Berrange]

On Fri, Aug 09, 2013 at 06:39:34PM +0530, Suhas Mane wrote:
 Hello Team,
 
 Me and few of my friends are planning to add VM Guest archival
 functionality.
 What we are proposing is:
 1.  Below diagram shows: possible states of VM guest (*archived new state
 introduced*)
 2.  For any reason, User / Admin expects to archive the VM guest to remote
 storage space, so that storage space consumed by VM guest (disk image...
 .vmdk / .img etc..) will be relived. Rest configuration files will be
 intact.
 3.  Later on when so called archived VM guest is to be booted, libvirt
 should unarchive the VM guest to local place, and booting should proceed as
 usual.

I'm not really convinced this is something we need / should do in
libvirt itself. When we talking about archiving VMs, I think that
in many cases apps are going to want to archive to some kind of
low cost cloud storage. If we do this archiving capability in libvrt,
then libvirt is going to have to learn about 1000's of differnet
cloud storage APIs.  As such, I think this concept is better done in
applications above libvirt, such as oVirt or OpenStack, where it can
be more easily integrated into their own storage architectures.

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] tests: avoid too-large constants

[Eric Blake]

On 08/09/2013 07:53 AM, Daniel P. Berrange wrote:
 On Fri, Aug 09, 2013 at 07:45:16AM -0600, Eric Blake wrote:
 Compiling with gcc 4.1.2 (RHEL 5) complains:

 virdbustest.c: In function 'testMessageSimple':
 virdbustest.c:61: warning: integer constant is too large for 'long' type
 virdbustest.c:62: warning: integer constant is too large for 'long' type
 virdbustest.c: In function 'testMessageArray':
 virdbustest.c:183: warning: this decimal constant is unsigned only in ISO C90
 virdbustest.c: In function 'testMessageStruct':
 virdbustest.c:239: warning: integer constant is too large for 'long' type
 virdbustest.c:240: warning: integer constant is too large for 'long' type

 * tests/virdbustest.c (testMessageSiple, testMessageArray)
 (testMessageStruct): Don't violate C89 constant constraints.

 Signed-off-by: Eric Blake ebl...@redhat.com
 ---

 Pushing under the build-breaker rule.


 @@ -178,9 +178,9 @@ static int testMessageArray(const void *args 
 ATTRIBUTE_UNUSED)
  DBusMessage *msg = NULL;
  int ret = -1;
  const char *in_str1 = Hello;
 -int in_int32a = 10, out_int32a = 0;
 -int in_int32b = 20, out_int32b = 0;
 -int in_int32c = 30, out_int32c = 0;
 +int in_int32a = 1, out_int32a = 0;
 +int in_int32b = 2, out_int32b = 0;
 +int in_int32c = 3, out_int32c = 0;
 
 I actually intentionally choose 3 as a value that would
 be above MAX_INT32 (2147483647). I guess what I really should
 have done was use something like -2147483640 instead, so we
 didn't rely on wrapping of 30. 
 
 Could you change this test to use a large -ve number for the
 3rd int, rather than stripping a 0 from all 3.

Will do.

-- 
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] libxl: Create per-domain log file

[Jim Fehlig]

Michal Privoznik wrote:
 On 09.08.2013 01:41, Jim Fehlig wrote:
   
 Currently, only one log file is created by the libxl driver, with
 all output from libxl for all domains going to this one file.

 Create a per-domain log file based on domain name, making sifting
 through the logs a bit easier.  This required deferring libxl_ctx
 allocation until starting the domain, which is fine since the
 ctx is not used when the domain is inactive.
 ---
  src/libxl/libxl_conf.h   |  5 +--
  src/libxl/libxl_driver.c | 88 
 +---
  2 files changed, 57 insertions(+), 36 deletions(-)

 

 ACK
   

Thanks Michal.  Pushed now.

Regards,
Jim

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

Re: [libvirt] [PATCH] libxl: Create per-domain log file

[Jim Fehlig]

Dario Faggioli wrote:
 On gio, 2013-08-08 at 17:41 -0600, Jim Fehlig wrote:
   
 Currently, only one log file is created by the libxl driver, with
 all output from libxl for all domains going to this one file.

 Create a per-domain log file based on domain name, making sifting
 through the logs a bit easier.

 
 That is actually a really nice idea.

 I gave it a quick spin, and it worked well, so I think you can add (if
 you want, and if it is customary here in libvir) something like the
 below:

 Tested-by: Dario Faggioli dario.faggi...@citrix.com
   

Ok.  Thanks for testing Dario!

Regards,
Jim

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

Re: [libvirt] Hyper-V driver API version support

[surface]

On Fri, Aug 09, 2013 at 10:13:03AM +0200, 
surf...@me.is-a-linux-user.org wrote:

Hello

The version function is not supported by the hyperv driver:
$ virsh --connect=hyperv://hypervhost version
Compiled against library: libvirt 1.1.1
Using library: libvirt 1.1.1
Using API: Hyper-V 1.1.1
error: failed to get the hypervisor version
error: this function is not supported by the connection driver:
virConnectGetVersion

But we need this funtion for the external/libvirt stonith plugin
of clusterglue:

$ cat /usr/lib/stonith/plugins/libvirt | more
# get status of stonith device (*NOT* of the domain).
# If we can retrieve some info from the hypervisor
# the stonith device is OK.
libvirt_status() {
out=$($VIRSH -c $hypervisor_uri version 21)
if [ $? -eq 0 ]
then
out=`echo $out | tail -1`
ha_log.sh notice $hypervisor_uri: $out
return 0
fi

ha_log.sh err Failed to get status for $hypervisor_uri
ha_log.sh err $out
return 1
}

So, we can't implement libvirt stonith with hyperv support in our
corosync/pacemaker cluster. Is it possible to implement the
version function for hyperv into virConnectGetVersion? Or exist
any workaround for this problem?


I'm sure its possible, but it needs someone with knowledge of the
Hyper-V apis to write a patch and there's only a couple of people
in libvirt comunity who can do that. Patches welcome from any able
person...

Daniel


Hi Daniel

I don't know which api is used for the driver handling, maybe you mean 
this one here: http://msdn.microsoft.com/en-us/library/aa155227.aspx


I tested the following command on our micro$oft server:
PS C:\Users\administrator gwmi -namespace root\virtualization 
Msvm_VirtualSystemManagementService



__GENUS : 2
__CLASS : Msvm_VirtualSystemManagementService
__SUPERCLASS: CIM_VirtualSystemManagementService
__DYNASTY   : CIM_ManagedElement
__RELPATH   : 
Msvm_VirtualSystemManagementService.CreationClassName=Msvm_VirtualSystemManagementService,N
  
ame=vmms,SystemCreationClassName=Msvm_ComputerSystem,SystemName=VSRV1

__PROPERTY_COUNT: 21
__DERIVATION: {CIM_VirtualSystemManagementService, 
CIM_Service, CIM_EnabledLogicalElement,

  CIM_LogicalElement...}
__SERVER: VSRV1
__NAMESPACE : root\virtualization
__PATH  : 
\\VSRV1\root\virtualization:Msvm_VirtualSystemManagementService.CreationClassName=Msvm_
  
VirtualSystemManagementService,Name=vmms,SystemCreationClassName=Msvm_ComputerSystem,Sys

  temName=VSRV1
Caption : Hyper-V Virtual System Management Service
CreationClassName   : Msvm_VirtualSystemManagementService
Description : Service for creating, manipulating, and 
managing virtual systems

ElementName : Hyper-V Virtual System Management Service
EnabledDefault  : 2
EnabledState: 2
HealthState : 5
InstallDate : 20130717120539.00-000
Name: vmms
OperationalStatus   : {2}
OtherEnabledState   :
PrimaryOwnerContact :
PrimaryOwnerName:
RequestedState  : 12
Started : True
StartMode   :
Status  : OK
StatusDescriptions  : {The service is running normally}
SystemCreationClassName : Msvm_ComputerSystem
SystemName  : VSRV1
TimeOfLastStateChange   : 20130801095437.00-000
PSComputerName  : VSRV1

so I think we can query the state like this:
PS C:\Users\administrator gwmi -namespace root\virtualization 
Msvm_VirtualSystemManagementService | select Status



Status
--
OK

It's not the version, but we need something to let libvirt stonith know, 
that the stonith device can connect to hyperv and the hyperv service is 
running..



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

Re: [libvirt] [PATCHv3] build: avoid -lgcrypt with newer gnutls

[Eric Blake]

On 08/07/2013 08:15 AM, Eric Blake wrote:
 On 08/07/2013 07:29 AM, Doug Goldstein wrote:
 On Tue, Jul 30, 2013 at 3:45 PM, Eric Blake ebl...@redhat.com wrote:
 https://bugzilla.redhat.com/show_bug.cgi?id=951637

 Newer gnutls uses nettle, rather than gcrypt, which is a lot nicer
 regarding initialization.  Yet we were unconditionally initializing
 gcrypt even when gnutls wouldn't be using it, and having two crypto
 libraries linked into libvirt.so is pointless, but mostly harmless
 (it doesn't crash, but does interfere with certification efforts).

 

 v3: configure logic is enhanced to try harder to get the
 right answer for gentoo.  I tested with Fedora 19 and RHEL 6,
 but need feedback from a gentoo tester before this can go in.

 

 I can ACK this by visual code inspection. I haven't explicitly tested
 this patch but its a cleaned up version of the patch we previously
 discussed it. If you need me to test it, I can do that tonight.
 
 I'd feel better with an explicit test; we're in no rush, so I'll wait
 for your test results on gentoo.

Did you ever re-run this?

-- 
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] [PATCHv3] build: more workarounds for if_bridge.h

[Daniel P. Berrange]

On Wed, Aug 07, 2013 at 05:10:26PM -0600, Eric Blake wrote:
 This is a second attempt at fixing the problem first attempted
 in commit 2df8d99; basically undoing the fact that it was
 reverted in commit 43cee32f, plus fixing two more issues: the
 code in configure.ac has to EXACTLY match virnetdevbridge.c
 with regards to declaring in6 types before using if_bridge.h,
 and the fact that RHEL 5 has even more conflicts:
 
 In file included from util/virnetdevbridge.c:49:
 /usr/include/linux/in6.h:47: error: conflicting types for 'in6addr_any'
 /usr/include/netinet/in.h:206: error: previous declaration of 'in6addr_any' 
 was here
 /usr/include/linux/in6.h:49: error: conflicting types for 'in6addr_loopback'
 /usr/include/netinet/in.h:207: error: previous declaration of 
 'in6addr_loopback' was here
 
 The rest of this commit message borrows from the original try
 of 2df8d99:
 
 A fresh checkout on a RHEL 6 machine with these packages:
 kernel-headers-2.6.32-405.el6.x86_64
 glibc-2.12-1.128.el6.x86_64
 failed to configure with this message:
 checking for linux/if_bridge.h... no
 configure: error: You must install kernel-headers in order to compile libvirt 
 with QEMU or LXC support
 
 Digging in config.log, we see that the problem is identical to
 what we fixed earlier in commit d12c2811:
 
 configure:98831: checking for linux/if_bridge.h
 configure:98853: gcc -std=gnu99 -c -g -O2  conftest.c 5
 In file included from /usr/include/linux/if_bridge.h:17,
  from conftest.c:559:
 /usr/include/linux/in6.h:31: error: redefinition of 'struct in6_addr'
 /usr/include/linux/in6.h:48: error: redefinition of 'struct sockaddr_in6'
 /usr/include/linux/in6.h:56: error: redefinition of 'struct ipv6_mreq'
 configure:98860: $? = 1
 
 I had not hit it earlier because I was using incremental builds,
 where config.cache had shielded me from the kernel-headers breakage.
 
 * configure.ac (if_bridge.h): Avoid conflicting type definitions.
 * src/util/virnetdevbridge.c (includes): Also sanitize for RHEL 5.
 
 Signed-off-by: Eric Blake ebl...@redhat.com
 ---
 
 Good thing I'm waiting for a review - that waiting time was enough
 for me to test on RHEL 5 and find another issue.
 v3: also work around RHEL 5 - yet ANOTHER symptom of broken if_bridge.h
 
  configure.ac   | 12 +++-
  src/util/virnetdevbridge.c |  4 
  2 files changed, 15 insertions(+), 1 deletion(-)

ACK, verified on F19.

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] [PATCHv3] build: avoid -lgcrypt with newer gnutls

[Daniel P. Berrange]

On Fri, Aug 09, 2013 at 09:31:39AM -0600, Eric Blake wrote:
 On 08/07/2013 08:15 AM, Eric Blake wrote:
  On 08/07/2013 07:29 AM, Doug Goldstein wrote:
  On Tue, Jul 30, 2013 at 3:45 PM, Eric Blake ebl...@redhat.com wrote:
  https://bugzilla.redhat.com/show_bug.cgi?id=951637
 
  Newer gnutls uses nettle, rather than gcrypt, which is a lot nicer
  regarding initialization.  Yet we were unconditionally initializing
  gcrypt even when gnutls wouldn't be using it, and having two crypto
  libraries linked into libvirt.so is pointless, but mostly harmless
  (it doesn't crash, but does interfere with certification efforts).
 
  
 
  v3: configure logic is enhanced to try harder to get the
  right answer for gentoo.  I tested with Fedora 19 and RHEL 6,
  but need feedback from a gentoo tester before this can go in.
 
  
 
  I can ACK this by visual code inspection. I haven't explicitly tested
  this patch but its a cleaned up version of the patch we previously
  discussed it. If you need me to test it, I can do that tonight.
  
  I'd feel better with an explicit test; we're in no rush, so I'll wait
  for your test results on gentoo.
 
 Did you ever re-run this?

ACK from my tests on F19.

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] Add documentation for access control system

[Eric Blake]

On 08/08/2013 05:27 AM, Daniel P. Berrange wrote:
 From: Daniel P. Berrange berra...@redhat.com
 
 This adds two new pages to the website, acl.html describing
 the general access control framework and permissions models,
 and aclpolkit.html describing the use of polkit as an
 access control driver.
 
 page.xsl is modified to support a new syntax
 
   div id=include filename=somefile.htmlinc/
 
 which will cause the XSL transform to replace that div
 with the contents of 'somefile.htmlinc'. We use this in
 the acl.html.in file, to pull the table of permissions
 for each libvirt object. This table is autogenerated
 from the enums in src/access/viraccessperms.h by the
 genaclperms.pl script.
 
 newapi.xsl is modified so that the list of permissions
 checks shown against each API will link to the description
 of the permissions in acl.html
 
 Signed-off-by: Daniel P. Berrange berra...@redhat.com
 ---
  .gitignore |   1 +
  docs/Makefile.am   |  12 +-
  docs/acl.html.in   | 100 
  docs/aclpolkit.html.in | 414 
 +
  docs/auth.html.in  |   6 +-
  docs/genaclperms.pl| 124 +++
  docs/newapi.xsl|   4 +-
  docs/page.xsl  |  11 ++
  docs/sitemap.html.in   |  10 ++
  9 files changed, 677 insertions(+), 5 deletions(-)
  create mode 100644 docs/acl.html.in
  create mode 100644 docs/aclpolkit.html.in
  create mode 100644 docs/genaclperms.pl

[I still need to look into why, in a clean checkout, 'make distcheck'
fails when 'make all distcheck' passes, but that shoudln't hold up this
patch, whether or not this patch aggravates the issue]


 +aclperms.htmlinc: $(top_srcdir)/src/access/viraccessperm.h \
 +genaclperms.pl Makefile.am
 + $(PERL) genaclperms.pl $  $@

Did you test a VPATH build?


 +p
 +  In a default configuration, the libvirtd daemon have three levels

s/have/has/

 +  of access control. All connections start off in an unauthenticated
 +  state, where the only API operations allowed are those required
 +  to complete authentication. After successful authentication, a
 +  connection either has full, unrestricted access to all libvirt
 +  API calls, or is locked down to only read only operations,
 +  according to what socket a client connection originated on.
 +/p
 +
 +p
 +  The access control framework allows authenticated connections to
 +  have fine grained permission rules to be defined by the administrator.
 +  Every API call in libvirt has a set of permissions that will
 +  be validated against the object being used. For example, the
 +  codevirDomainSetSchedulerParametersFlags/code method will
 +  check whether the client user has the codewrite/code
 +  permission on the codedomain/code object instance passed
 +  in as a parameter. Further permissions will also be checked
 +  if certain flags are set in the API call. In addition to
 +  checks on the object passed into an API call, some methods

s/into/in to/

 +  will filter their results. For example the 
 codevirConnectListAllDomains/code
 +  method will check the codesearch_domains/code on the 
 codeconnect/code
 +  object, but will also filter the returned codedomain/code
 +  objects to only those on which the client user has the
 +  codegetattr/code permission.
 +/p
 +
 +h2a name=driversAccess control drivers/a/h2
 +
 +p
 +  The access control framework is designed as a pluggable
 +  system to enable future integration with arbitrary access
 +  control technologies. By default, the codenone/code
 +  driver is used, which does not access controll checks at

s/not/no/; s/controll/control/

 +  all. At this time, libvirt ships with support for using
 +  a href=http://www.freedesktop.org/wiki/Software/polkit/;polkit/a 
 as a real access
 +  control driver. To learn how to use the polkit access
 +  driver consult a href=aclpolkit.htmlthe configuration
 +  docs/a.
 +/p
 +
 +p
 +  The access driver is configured in the codelibvirtd.conf/code
 +  configuration file, using the codeaccess_drivers/code
 +  parameter. This parameter accepts an array of access control
 +  driver names. If more than one access driver is requested,
 +  then all must succeed in order for access to be granted.
 +  To enable 'polkit' as the driver
 +/p
 +
 +pre
 +# augtool -s set '/files/etc/libvirt/libvirtd.conf/access_drivers[1]' polkit
 +/pre
 +
 +p
 +  And to reset back to the default (no-op) driver
 +/p
 +
 +
 +pre
 +# augtool -s rm /files/etc/libvirt/libvirtd.conf/access_drivers
 +/pre
 +
 +p
 +  strongNote:/strong changes to libvirtd.conf require that
 +  the libvirtd daemon be restarted.

Isn't sending SIGHUP sufficient, or does it have to be a full restart?

 +/p
 +
 +h2a name=permsObjects and permissions/a/h2
 +
 +p
 + 

Re: [libvirt] [PATCH] Add documentation for access control system

[Daniel P. Berrange]

On Fri, Aug 09, 2013 at 09:47:46AM -0600, Eric Blake wrote:
 On 08/08/2013 05:27 AM, Daniel P. Berrange wrote:
  +aclperms.htmlinc: $(top_srcdir)/src/access/viraccessperm.h \
  +genaclperms.pl Makefile.am
  +   $(PERL) genaclperms.pl $  $@
 
 Did you test a VPATH build?

No, but I will do.


  +p
  +  And to reset back to the default (no-op) driver
  +/p
  +
  +
  +pre
  +# augtool -s rm /files/etc/libvirt/libvirtd.conf/access_drivers
  +/pre
  +
  +p
  +  strongNote:/strong changes to libvirtd.conf require that
  +  the libvirtd daemon be restarted.
 
 Isn't sending SIGHUP sufficient, or does it have to be a full restart?

No, SIGHUP only reloads .xml files.

  +pre
  +polkit.addRule(function(action, subject) {
  +if (action.id == org.libvirt.api.connect.getattr amp;amp;
  +subject.user == berrange) {
  +  if (action._detail_connect_driver == 'QEMU') {
  +return polkit.Result.YES;
  +  } else {
  +return polkit.Result.NO;
  +  }
  +}
 
 This function has no return statement when the initial 'if' is not
 satisfied; is that valid?

Yeah, it will just carry on with other polkit rules that are defined
in other files, eventually fallback back to the default policy
defined for the action.


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] Add documentation for access control system

[Eric Blake]

On 08/09/2013 09:51 AM, Daniel P. Berrange wrote:


 This function has no return statement when the initial 'if' is not
 satisfied; is that valid?
 
 Yeah, it will just carry on with other polkit rules that are defined
 in other files, eventually fallback back to the default policy
 defined for the action.

Ah, a tri-state sort of thing - explicit return yes to end the search
with permission granted, explicit return no to end the search with
permission denied, and no return to fall through to any other checkers.

-- 
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] [Qemu-devel] [PATCH] qemu: Drop qemuDomainMemoryLimit

[Anthony Liguori]

Michal Privoznik mpriv...@redhat.com writes:

 [CC'ing qemu-devel list]
 On 09.08.2013 15:17, Daniel P. Berrange wrote:
 On Fri, Aug 09, 2013 at 07:13:58AM -0600, Eric Blake wrote:
 On 08/09/2013 06:56 AM, Michal Privoznik wrote:
 This function is to guess the correct limit for maximal memory
 usage by qemu for given domain. This can never be guessed
 correctly, not to mention all the pains and sleepless nights this
 code has caused. Once somebody discovers algorithm to solve the
 Halting Problem, we can compute the limit algorithmically. But
 till then, this code should never see the light of the release
 again.
 ---
  src/qemu/qemu_cgroup.c  |  3 +--
  src/qemu/qemu_command.c |  2 +-
  src/qemu/qemu_domain.c  | 49 
 -
  src/qemu/qemu_domain.h  |  2 --
  src/qemu/qemu_hotplug.c |  2 +-
  5 files changed, 3 insertions(+), 55 deletions(-)

 ACK.  Users that put an explicit limit in their XML are taking on their
 own risk at guessing correctly; all other users should not be forced to
 suffer from a bad guess on our part killing their domain.
 
 If we don't understand how to calculate a default limit that works,
 how are users with even less knowledge than us, suppose to calculate
 an explicit level of their own ?
 
 This limit was designed so that the hosts are not vulnerable to DOS
 attack from a compromised QEMU, so removing this is arguably introducing
 a security weakness in our default deployment.
 
 I think I'd like to see some feedback / agreement from QEMU developers
 that this problem really can't be solved, before we remove it.
 
 Daniel
 

 In libvirt I've introduced a heuristic to guess the maximum limit for a
 memory for a given VM definition. The rationale was it's better to be
 safe by default and not let leaking qemu trash the host. The heuristic
 is only used if user has not configured any limit himself. However, over
 the time the number of users reporting OOM kills due to my heuristic has
 grown. Finally, I've full nose of this problem so I've made a patch [1]
 that removes this 'functionality' completely (I'd say it's bug after
 all). In the patch you can see the heuristic we've converged to. But Dan
 has his point. If libvirt  qemu devels aren't able to come up with
 proper heuristic, how can an ordinary user (who doesn't have any
 knowledge of code) do so? So before I apply my patch, I want to ask you
 guys, what do you think about it.

Even if we had an algorithm for calculating memory overhead (we don't),
glibc will still introduce uncertainty since malloc(size) doesn't
translate to allocating size bytes from the kernel.  When you throw in
fragmentation too it becomes extremely hard to predict.

The only practical way of doing this would be to have QEMU gracefully
handle malloc() == NULL so that you could set a limit and gracefully
degrade.  We don't though so setting a limit is likely to get you in
trouble.

Regards,

Anthony Liguori


 Michal

 1: https://www.redhat.com/archives/libvir-list/2013-August/msg00437.html

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

Re: [libvirt] [PATCHv3] build: more workarounds for if_bridge.h

[Eric Blake]

On 08/09/2013 09:38 AM, Daniel P. Berrange wrote:
 On Wed, Aug 07, 2013 at 05:10:26PM -0600, Eric Blake wrote:
 This is a second attempt at fixing the problem first attempted
 in commit 2df8d99; basically undoing the fact that it was
 reverted in commit 43cee32f, plus fixing two more issues: the
 code in configure.ac has to EXACTLY match virnetdevbridge.c
 with regards to declaring in6 types before using if_bridge.h,
 and the fact that RHEL 5 has even more conflicts:


 
 ACK, verified on F19.

Thanks; pushed.

-- 
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 v4 1/2] libxl: fix node ranges in libxlNodeGetCellsFreeMemory()

[Jim Fehlig]

Dario Faggioli wrote:
 introduced by cs 4b9eec50fe2c23343 (libxl: implement per
 NUMA node free memory reporting). What was wrong was that
 libxl_get_numainfo() put in nr_nodes the actual number of
 host NUMA nodes, not the highest node ID (like libnuma's
 numa_max_node() does instead).
   

Ok, makes sense.

 While at it, turn the failure of libxl_get_numainfo() from
 a simple warning to a proper error, as requested during the
 review of another patch of the original series.
   

ACK and pushed.  Thanks!

Regards,
Jim

 Signed-off-by: Dario Faggioli dario.faggi...@citrix.com
 Cc: Daniel P. Berrange berra...@redhat.com
 ---
  src/libxl/libxl_driver.c |   14 +++---
  1 file changed, 7 insertions(+), 7 deletions(-)

 diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
 index 9e9bc89..04058d3 100644
 --- a/src/libxl/libxl_driver.c
 +++ b/src/libxl/libxl_driver.c
 @@ -4107,23 +4107,23 @@ libxlNodeGetCellsFreeMemory(virConnectPtr conn,
  if (virNodeGetCellsFreeMemoryEnsureACL(conn)  0)
  return -1;
  
 -/* Early failure is probably worth just a warning */
  numa_info = libxl_get_numainfo(driver-ctx, nr_nodes);
  if (numa_info == NULL || nr_nodes == 0) {
 -VIR_WARN(libxl_get_numainfo failed to retrieve NUMA data);
 -return 0;
 +virReportError(VIR_ERR_INTERNAL_ERROR, %s,
 +   _(libxl_get_numainfo failed));
 +goto cleanup;
  }
  
  /* Check/sanitize the cell range */
 -if (startCell  nr_nodes) {
 +if (startCell = nr_nodes) {
  virReportError(VIR_ERR_INTERNAL_ERROR,
 _(start cell %d out of range (0-%d)),
 -   startCell, nr_nodes);
 +   startCell, nr_nodes - 1);
  goto cleanup;
  }
  lastCell = startCell + maxCells - 1;
 -if (lastCell  nr_nodes)
 -lastCell = nr_nodes;
 +if (lastCell = nr_nodes)
 +lastCell = nr_nodes - 1;
  
  for (numCells = 0, n = startCell; n = lastCell; n++) {
  if (numa_info[n].size == LIBXL_NUMAINFO_INVALID_ENTRY)



   

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

Re: [libvirt] Hyper-V driver API version support

[Matthias Bolte]

2013/8/9  surf...@me.is-a-linux-user.org:
 On Fri, Aug 09, 2013 at 10:13:03AM +0200, surf...@me.is-a-linux-user.org
 wrote:

 Hello

 The version function is not supported by the hyperv driver:
 $ virsh --connect=hyperv://hypervhost version
 Compiled against library: libvirt 1.1.1
 Using library: libvirt 1.1.1
 Using API: Hyper-V 1.1.1
 error: failed to get the hypervisor version
 error: this function is not supported by the connection driver:
 virConnectGetVersion

 But we need this funtion for the external/libvirt stonith plugin
 of clusterglue:

 $ cat /usr/lib/stonith/plugins/libvirt | more
 # get status of stonith device (*NOT* of the domain).
 # If we can retrieve some info from the hypervisor
 # the stonith device is OK.
 libvirt_status() {
 out=$($VIRSH -c $hypervisor_uri version 21)
 if [ $? -eq 0 ]
 then
 out=`echo $out | tail -1`
 ha_log.sh notice $hypervisor_uri: $out
 return 0
 fi

 ha_log.sh err Failed to get status for $hypervisor_uri
 ha_log.sh err $out
 return 1
 }

 So, we can't implement libvirt stonith with hyperv support in our
 corosync/pacemaker cluster. Is it possible to implement the
 version function for hyperv into virConnectGetVersion? Or exist
 any workaround for this problem?


 I'm sure its possible, but it needs someone with knowledge of the
 Hyper-V apis to write a patch and there's only a couple of people
 in libvirt comunity who can do that. Patches welcome from any able
 person...

 Daniel


 Hi Daniel

 I don't know which api is used for the driver handling, maybe you mean this
 one here: http://msdn.microsoft.com/en-us/library/aa155227.aspx

 I tested the following command on our micro$oft server:
 PS C:\Users\administrator gwmi -namespace root\virtualization
 Msvm_VirtualSystemManagementService


 __GENUS : 2
 __CLASS : Msvm_VirtualSystemManagementService
 __SUPERCLASS: CIM_VirtualSystemManagementService
 __DYNASTY   : CIM_ManagedElement
 __RELPATH   :
 Msvm_VirtualSystemManagementService.CreationClassName=Msvm_VirtualSystemManagementService,N

 ame=vmms,SystemCreationClassName=Msvm_ComputerSystem,SystemName=VSRV1
 __PROPERTY_COUNT: 21
 __DERIVATION: {CIM_VirtualSystemManagementService, CIM_Service,
 CIM_EnabledLogicalElement,
   CIM_LogicalElement...}
 __SERVER: VSRV1
 __NAMESPACE : root\virtualization
 __PATH  :
 \\VSRV1\root\virtualization:Msvm_VirtualSystemManagementService.CreationClassName=Msvm_

 VirtualSystemManagementService,Name=vmms,SystemCreationClassName=Msvm_ComputerSystem,Sys
   temName=VSRV1
 Caption : Hyper-V Virtual System Management Service
 CreationClassName   : Msvm_VirtualSystemManagementService
 Description : Service for creating, manipulating, and managing
 virtual systems
 ElementName : Hyper-V Virtual System Management Service
 EnabledDefault  : 2
 EnabledState: 2
 HealthState : 5
 InstallDate : 20130717120539.00-000
 Name: vmms
 OperationalStatus   : {2}
 OtherEnabledState   :
 PrimaryOwnerContact :
 PrimaryOwnerName:
 RequestedState  : 12
 Started : True
 StartMode   :
 Status  : OK
 StatusDescriptions  : {The service is running normally}
 SystemCreationClassName : Msvm_ComputerSystem
 SystemName  : VSRV1
 TimeOfLastStateChange   : 20130801095437.00-000
 PSComputerName  : VSRV1

 so I think we can query the state like this:
 PS C:\Users\administrator gwmi -namespace root\virtualization
 Msvm_VirtualSystemManagementService | select Status


 Status
 --
 OK

 It's not the version, but we need something to let libvirt stonith know,
 that the stonith device can connect to hyperv and the hyperv service is
 running..

The Hyper-V driver in libvirt already checks if the host has the
Hyper-V role installed on connect. It does this by checking for the
existance of an Msvm_ComputerSystem object, if there is non then
virsh connect fails. This means if virsh connect succeeds then the
URI you specified points to an Hyper-V server. No need to abuse the
version command for such a check.

If you think that it is useful to check that an
Msvm_VirtualSystemManagementService object exists on the host then
this could be done in addition to the existing checks.

-- 
Matthias Bolte
http://photron.blogspot.com

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

[libvirt] [PATCH] Fix VPATH build of ACL docs

[Daniel P. Berrange]

From: Daniel P. Berrange berra...@redhat.com

The aclperms.htmlinc file must be generated in $(srcdir) to
let includes work when doing a VPATH build

Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
 docs/Makefile.am | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/docs/Makefile.am b/docs/Makefile.am
index 9057432..0b0d2d4 100644
--- a/docs/Makefile.am
+++ b/docs/Makefile.am
@@ -141,13 +141,13 @@ EXTRA_DIST=   \
 
 BUILT_SOURCES += aclperms.htmlinc
 
-CLEANFILES = aclperms.htmlinc
+CLEANFILES = $(srcdir)/aclperms.htmlinc
 
-acl.html:: aclperms.htmlinc
+acl.html:: $(srcdir)/aclperms.htmlinc
 
-aclperms.htmlinc: $(top_srcdir)/src/access/viraccessperm.h \
-genaclperms.pl Makefile.am
-   $(PERL) genaclperms.pl $  $@
+$(srcdir)/aclperms.htmlinc: $(top_srcdir)/src/access/viraccessperm.h \
+$(srcdir)/genaclperms.pl Makefile.am
+   $(PERL) $(srcdir)/genaclperms.pl $  $@
 
 MAINTAINERCLEANFILES = \
   $(addprefix $(srcdir)/,$(dot_html)) \
-- 
1.8.1.4

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

Re: [libvirt] Hyper-V driver API version support

[Daniel P. Berrange]

On Fri, Aug 09, 2013 at 06:08:38PM +0200, Matthias Bolte wrote:
 2013/8/9  surf...@me.is-a-linux-user.org:
  On Fri, Aug 09, 2013 at 10:13:03AM +0200, surf...@me.is-a-linux-user.org
  wrote:
 
  Hello
 
  The version function is not supported by the hyperv driver:
  $ virsh --connect=hyperv://hypervhost version
  Compiled against library: libvirt 1.1.1
  Using library: libvirt 1.1.1
  Using API: Hyper-V 1.1.1
  error: failed to get the hypervisor version
  error: this function is not supported by the connection driver:
  virConnectGetVersion
 
  But we need this funtion for the external/libvirt stonith plugin
  of clusterglue:
 
  $ cat /usr/lib/stonith/plugins/libvirt | more
  # get status of stonith device (*NOT* of the domain).
  # If we can retrieve some info from the hypervisor
  # the stonith device is OK.
  libvirt_status() {
  out=$($VIRSH -c $hypervisor_uri version 21)
  if [ $? -eq 0 ]
  then
  out=`echo $out | tail -1`
  ha_log.sh notice $hypervisor_uri: $out
  return 0
  fi
 
  ha_log.sh err Failed to get status for $hypervisor_uri
  ha_log.sh err $out
  return 1
  }
 
  So, we can't implement libvirt stonith with hyperv support in our
  corosync/pacemaker cluster. Is it possible to implement the
  version function for hyperv into virConnectGetVersion? Or exist
  any workaround for this problem?
 
 
  I'm sure its possible, but it needs someone with knowledge of the
  Hyper-V apis to write a patch and there's only a couple of people
  in libvirt comunity who can do that. Patches welcome from any able
  person...
 
  Daniel
 
 
  Hi Daniel
 
  I don't know which api is used for the driver handling, maybe you mean this
  one here: http://msdn.microsoft.com/en-us/library/aa155227.aspx
 
  I tested the following command on our micro$oft server:
  PS C:\Users\administrator gwmi -namespace root\virtualization
  Msvm_VirtualSystemManagementService
 
 
  __GENUS : 2
  __CLASS : Msvm_VirtualSystemManagementService
  __SUPERCLASS: CIM_VirtualSystemManagementService
  __DYNASTY   : CIM_ManagedElement
  __RELPATH   :
  Msvm_VirtualSystemManagementService.CreationClassName=Msvm_VirtualSystemManagementService,N
 
  ame=vmms,SystemCreationClassName=Msvm_ComputerSystem,SystemName=VSRV1
  __PROPERTY_COUNT: 21
  __DERIVATION: {CIM_VirtualSystemManagementService, CIM_Service,
  CIM_EnabledLogicalElement,
CIM_LogicalElement...}
  __SERVER: VSRV1
  __NAMESPACE : root\virtualization
  __PATH  :
  \\VSRV1\root\virtualization:Msvm_VirtualSystemManagementService.CreationClassName=Msvm_
 
  VirtualSystemManagementService,Name=vmms,SystemCreationClassName=Msvm_ComputerSystem,Sys
temName=VSRV1
  Caption : Hyper-V Virtual System Management Service
  CreationClassName   : Msvm_VirtualSystemManagementService
  Description : Service for creating, manipulating, and managing
  virtual systems
  ElementName : Hyper-V Virtual System Management Service
  EnabledDefault  : 2
  EnabledState: 2
  HealthState : 5
  InstallDate : 20130717120539.00-000
  Name: vmms
  OperationalStatus   : {2}
  OtherEnabledState   :
  PrimaryOwnerContact :
  PrimaryOwnerName:
  RequestedState  : 12
  Started : True
  StartMode   :
  Status  : OK
  StatusDescriptions  : {The service is running normally}
  SystemCreationClassName : Msvm_ComputerSystem
  SystemName  : VSRV1
  TimeOfLastStateChange   : 20130801095437.00-000
  PSComputerName  : VSRV1
 
  so I think we can query the state like this:
  PS C:\Users\administrator gwmi -namespace root\virtualization
  Msvm_VirtualSystemManagementService | select Status
 
 
  Status
  --
  OK
 
  It's not the version, but we need something to let libvirt stonith know,
  that the stonith device can connect to hyperv and the hyperv service is
  running..
 
 The Hyper-V driver in libvirt already checks if the host has the
 Hyper-V role installed on connect. It does this by checking for the
 existance of an Msvm_ComputerSystem object, if there is non then
 virsh connect fails. This means if virsh connect succeeds then the
 URI you specified points to an Hyper-V server. No need to abuse the
 version command for such a check.
 
 If you think that it is useful to check that an
 Msvm_VirtualSystemManagementService object exists on the host then
 this could be done in addition to the existing checks.

What they really want, per the first mail, is for 

  virConnectGetVersion

to be implemented for hyper-v

Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: 

Re: [libvirt] [PATCH] Fix VPATH build of ACL docs

[Eric Blake]

On 08/09/2013 10:09 AM, Daniel P. Berrange wrote:
 From: Daniel P. Berrange berra...@redhat.com
 
 The aclperms.htmlinc file must be generated in $(srcdir) to
 let includes work when doing a VPATH build
 
 Signed-off-by: Daniel P. Berrange berra...@redhat.com
 ---
  docs/Makefile.am | 10 +-
  1 file changed, 5 insertions(+), 5 deletions(-)
 
 diff --git a/docs/Makefile.am b/docs/Makefile.am
 index 9057432..0b0d2d4 100644
 --- a/docs/Makefile.am
 +++ b/docs/Makefile.am
 @@ -141,13 +141,13 @@ EXTRA_DIST= \
  
  BUILT_SOURCES += aclperms.htmlinc
  
 -CLEANFILES = aclperms.htmlinc
 +CLEANFILES = $(srcdir)/aclperms.htmlinc

Hmm - this says that in order to build from a tarball, a user must have
the tools to generate the htmlinc file (in this case perl).  But I guess
we already require the user to have perl installed for other things.

  
 -acl.html:: aclperms.htmlinc
 +acl.html:: $(srcdir)/aclperms.htmlinc
  
 -aclperms.htmlinc: $(top_srcdir)/src/access/viraccessperm.h \
 -genaclperms.pl Makefile.am
 - $(PERL) genaclperms.pl $  $@
 +$(srcdir)/aclperms.htmlinc: $(top_srcdir)/src/access/viraccessperm.h \
 +$(srcdir)/genaclperms.pl Makefile.am

The $(srcdir)/ on genaclperms.pl should not be necessary in this line
(that is, make should be able to find it via VPATH rules), but it
doesn't hurt given that...

 + $(PERL) $(srcdir)/genaclperms.pl $  $@

...it IS necessary on this line (perl doesn't know what VPATH make is
using).

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] [PATCH] Fix VPATH build of ACL docs

[Daniel P. Berrange]

On Fri, Aug 09, 2013 at 05:09:34PM +0100, Daniel P. Berrange wrote:
 From: Daniel P. Berrange berra...@redhat.com
 
 The aclperms.htmlinc file must be generated in $(srcdir) to
 let includes work when doing a VPATH build
 
 Signed-off-by: Daniel P. Berrange berra...@redhat.com
 ---
  docs/Makefile.am | 10 +-
  1 file changed, 5 insertions(+), 5 deletions(-)
 
 diff --git a/docs/Makefile.am b/docs/Makefile.am
 index 9057432..0b0d2d4 100644
 --- a/docs/Makefile.am
 +++ b/docs/Makefile.am
 @@ -141,13 +141,13 @@ EXTRA_DIST= \
  
  BUILT_SOURCES += aclperms.htmlinc
  
 -CLEANFILES = aclperms.htmlinc
 +CLEANFILES = $(srcdir)/aclperms.htmlinc
  
 -acl.html:: aclperms.htmlinc
 +acl.html:: $(srcdir)/aclperms.htmlinc
  
 -aclperms.htmlinc: $(top_srcdir)/src/access/viraccessperm.h \
 -genaclperms.pl Makefile.am
 - $(PERL) genaclperms.pl $  $@
 +$(srcdir)/aclperms.htmlinc: $(top_srcdir)/src/access/viraccessperm.h \
 +$(srcdir)/genaclperms.pl Makefile.am
 + $(PERL) $(srcdir)/genaclperms.pl $  $@
  
  MAINTAINERCLEANFILES = \
$(addprefix $(srcdir)/,$(dot_html)) \

Oh, ignore this. I forgot I hadn't pushed the patch that adds the
flaw yet, so I'll squash it in with that.


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] [Qemu-devel] [PATCH] qemu: Drop qemuDomainMemoryLimit

[Daniel P. Berrange]

On Fri, Aug 09, 2013 at 10:58:55AM -0500, Anthony Liguori wrote:
 Michal Privoznik mpriv...@redhat.com writes:
 
  [CC'ing qemu-devel list]
  On 09.08.2013 15:17, Daniel P. Berrange wrote:
  On Fri, Aug 09, 2013 at 07:13:58AM -0600, Eric Blake wrote:
  On 08/09/2013 06:56 AM, Michal Privoznik wrote:
  This function is to guess the correct limit for maximal memory
  usage by qemu for given domain. This can never be guessed
  correctly, not to mention all the pains and sleepless nights this
  code has caused. Once somebody discovers algorithm to solve the
  Halting Problem, we can compute the limit algorithmically. But
  till then, this code should never see the light of the release
  again.
  ---
   src/qemu/qemu_cgroup.c  |  3 +--
   src/qemu/qemu_command.c |  2 +-
   src/qemu/qemu_domain.c  | 49 
  -
   src/qemu/qemu_domain.h  |  2 --
   src/qemu/qemu_hotplug.c |  2 +-
   5 files changed, 3 insertions(+), 55 deletions(-)
 
  ACK.  Users that put an explicit limit in their XML are taking on their
  own risk at guessing correctly; all other users should not be forced to
  suffer from a bad guess on our part killing their domain.
  
  If we don't understand how to calculate a default limit that works,
  how are users with even less knowledge than us, suppose to calculate
  an explicit level of their own ?
  
  This limit was designed so that the hosts are not vulnerable to DOS
  attack from a compromised QEMU, so removing this is arguably introducing
  a security weakness in our default deployment.
  
  I think I'd like to see some feedback / agreement from QEMU developers
  that this problem really can't be solved, before we remove it.
  
  Daniel
  
 
  In libvirt I've introduced a heuristic to guess the maximum limit for a
  memory for a given VM definition. The rationale was it's better to be
  safe by default and not let leaking qemu trash the host. The heuristic
  is only used if user has not configured any limit himself. However, over
  the time the number of users reporting OOM kills due to my heuristic has
  grown. Finally, I've full nose of this problem so I've made a patch [1]
  that removes this 'functionality' completely (I'd say it's bug after
  all). In the patch you can see the heuristic we've converged to. But Dan
  has his point. If libvirt  qemu devels aren't able to come up with
  proper heuristic, how can an ordinary user (who doesn't have any
  knowledge of code) do so? So before I apply my patch, I want to ask you
  guys, what do you think about it.
 
 Even if we had an algorithm for calculating memory overhead (we don't),
 glibc will still introduce uncertainty since malloc(size) doesn't
 translate to allocating size bytes from the kernel.  When you throw in
 fragmentation too it becomes extremely hard to predict.
 
 The only practical way of doing this would be to have QEMU gracefully
 handle malloc() == NULL so that you could set a limit and gracefully
 degrade.  We don't though so setting a limit is likely to get you in
 trouble.

So you're saying there's no way we can define a reasonable limit
on a QEMU guest to prevent a compomised QEMU exhausting all host
memory ? It rather sucks if that's the position we're in :-(

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] [Qemu-devel] [PATCH] qemu: Drop qemuDomainMemoryLimit

[Andreas Färber]

Am 09.08.2013 17:58, schrieb Anthony Liguori:
 Even if we had an algorithm for calculating memory overhead (we don't),
 glibc will still introduce uncertainty since malloc(size) doesn't
 translate to allocating size bytes from the kernel.  When you throw in
 fragmentation too it becomes extremely hard to predict.
 
 The only practical way of doing this would be to have QEMU gracefully
 handle malloc() == NULL so that you could set a limit and gracefully
 degrade.  We don't though so setting a limit is likely to get you in
 trouble.

FWIW my QOM realize work is targetted at reducing likelihood that
device_add blows up QEMU due to OOM in object_new(). But before I can
change qdev-monitor.c I still need to tweak core QOM to either get at
TypeImpl::instance_size or to introduce an object_try_new() function
using g_try_malloc0() rather than g_malloc0(). That's where proper child
struct composition comes into play.

The major variance in runtime memory consumption was so far attributed
to block and network I/O, without ever getting exact proof points...

Regards,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

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

Re: [libvirt] [PATCH] libxl: Create per-domain log file

[Daniel P. Berrange]

On Thu, Aug 08, 2013 at 05:41:26PM -0600, Jim Fehlig wrote:
 Currently, only one log file is created by the libxl driver, with
 all output from libxl for all domains going to this one file.
 
 Create a per-domain log file based on domain name, making sifting
 through the logs a bit easier.  This required deferring libxl_ctx
 allocation until starting the domain, which is fine since the
 ctx is not used when the domain is inactive.
 ---
  src/libxl/libxl_conf.h   |  5 +--
  src/libxl/libxl_driver.c | 88 
 +---
  2 files changed, 57 insertions(+), 36 deletions(-)

Startup of libvirtd SEGVs with this change in GIT

==4706== For counts of detected and suppressed errors, rerun with: -v
==4706== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 2 from 2)
==4601== Thread 12:
==4601== Invalid read of size 8
==4601==at 0x1E57964F: xtl_logv (in /usr/lib64/libxenctrl.so.4.2.0)
==4601==by 0x1E5796D3: xtl_log (in /usr/lib64/libxenctrl.so.4.2.0)
==4601==by 0x1E33258A: ??? (in /usr/lib64/libxenlight.so.2.0.0)
==4601==by 0x1E332641: ??? (in /usr/lib64/libxenlight.so.2.0.0)
==4601==by 0x1E317CD7: libxl_ctx_alloc (in /usr/lib64/libxenlight.so.2.0.0)
==4601==by 0x1E0E3016: libxlStateInitialize.part.12 (libxl_driver.c:1319)
==4601==by 0x514AA29: virStateInitialize (libvirt.c:832)
==4601==by 0x11C42A: daemonRunStateInit (libvirtd.c:906)
==4601==by 0x50C273D: virThreadHelper (virthreadpthread.c:161)
==4601==by 0x82D0C52: start_thread (pthread_create.c:308)
==4601==by 0x89E213C: clone (clone.S:113)
==4601==  Address 0x0 is not stack'd, malloc'd or (recently) free'd
==4601== 
Caught Segmentation violation dumping internal log buffer:


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] Hyper-V driver API version support

[Matthias Bolte]

2013/8/9 Daniel P. Berrange berra...@redhat.com:
 On Fri, Aug 09, 2013 at 06:08:38PM +0200, Matthias Bolte wrote:
 2013/8/9  surf...@me.is-a-linux-user.org:
  On Fri, Aug 09, 2013 at 10:13:03AM +0200, surf...@me.is-a-linux-user.org
  wrote:
 
  Hello
 
  The version function is not supported by the hyperv driver:
  $ virsh --connect=hyperv://hypervhost version
  Compiled against library: libvirt 1.1.1
  Using library: libvirt 1.1.1
  Using API: Hyper-V 1.1.1
  error: failed to get the hypervisor version
  error: this function is not supported by the connection driver:
  virConnectGetVersion
 
  But we need this funtion for the external/libvirt stonith plugin
  of clusterglue:
 
  $ cat /usr/lib/stonith/plugins/libvirt | more
  # get status of stonith device (*NOT* of the domain).
  # If we can retrieve some info from the hypervisor
  # the stonith device is OK.
  libvirt_status() {
  out=$($VIRSH -c $hypervisor_uri version 21)
  if [ $? -eq 0 ]
  then
  out=`echo $out | tail -1`
  ha_log.sh notice $hypervisor_uri: $out
  return 0
  fi
 
  ha_log.sh err Failed to get status for $hypervisor_uri
  ha_log.sh err $out
  return 1
  }
 
  So, we can't implement libvirt stonith with hyperv support in our
  corosync/pacemaker cluster. Is it possible to implement the
  version function for hyperv into virConnectGetVersion? Or exist
  any workaround for this problem?
 
 
  I'm sure its possible, but it needs someone with knowledge of the
  Hyper-V apis to write a patch and there's only a couple of people
  in libvirt comunity who can do that. Patches welcome from any able
  person...
 
  Daniel
 
 
  Hi Daniel
 
  I don't know which api is used for the driver handling, maybe you mean this
  one here: http://msdn.microsoft.com/en-us/library/aa155227.aspx
 
  I tested the following command on our micro$oft server:
  PS C:\Users\administrator gwmi -namespace root\virtualization
  Msvm_VirtualSystemManagementService
 
 
  __GENUS : 2
  __CLASS : Msvm_VirtualSystemManagementService
  __SUPERCLASS: CIM_VirtualSystemManagementService
  __DYNASTY   : CIM_ManagedElement
  __RELPATH   :
  Msvm_VirtualSystemManagementService.CreationClassName=Msvm_VirtualSystemManagementService,N
 
  ame=vmms,SystemCreationClassName=Msvm_ComputerSystem,SystemName=VSRV1
  __PROPERTY_COUNT: 21
  __DERIVATION: {CIM_VirtualSystemManagementService, CIM_Service,
  CIM_EnabledLogicalElement,
CIM_LogicalElement...}
  __SERVER: VSRV1
  __NAMESPACE : root\virtualization
  __PATH  :
  \\VSRV1\root\virtualization:Msvm_VirtualSystemManagementService.CreationClassName=Msvm_
 
  VirtualSystemManagementService,Name=vmms,SystemCreationClassName=Msvm_ComputerSystem,Sys
temName=VSRV1
  Caption : Hyper-V Virtual System Management Service
  CreationClassName   : Msvm_VirtualSystemManagementService
  Description : Service for creating, manipulating, and managing
  virtual systems
  ElementName : Hyper-V Virtual System Management Service
  EnabledDefault  : 2
  EnabledState: 2
  HealthState : 5
  InstallDate : 20130717120539.00-000
  Name: vmms
  OperationalStatus   : {2}
  OtherEnabledState   :
  PrimaryOwnerContact :
  PrimaryOwnerName:
  RequestedState  : 12
  Started : True
  StartMode   :
  Status  : OK
  StatusDescriptions  : {The service is running normally}
  SystemCreationClassName : Msvm_ComputerSystem
  SystemName  : VSRV1
  TimeOfLastStateChange   : 20130801095437.00-000
  PSComputerName  : VSRV1
 
  so I think we can query the state like this:
  PS C:\Users\administrator gwmi -namespace root\virtualization
  Msvm_VirtualSystemManagementService | select Status
 
 
  Status
  --
  OK
 
  It's not the version, but we need something to let libvirt stonith know,
  that the stonith device can connect to hyperv and the hyperv service is
  running..

 The Hyper-V driver in libvirt already checks if the host has the
 Hyper-V role installed on connect. It does this by checking for the
 existance of an Msvm_ComputerSystem object, if there is non then
 virsh connect fails. This means if virsh connect succeeds then the
 URI you specified points to an Hyper-V server. No need to abuse the
 version command for such a check.

 If you think that it is useful to check that an
 Msvm_VirtualSystemManagementService object exists on the host then
 this could be done in addition to the existing checks.

 What they really want, per the first mail, is for

   virConnectGetVersion

 to be implemented for hyper-v

I understood that :)

But he doesn't actually need the version information, as far as I
understood his problem. The point 

Re: [libvirt] [PATCH] libxl: Create per-domain log file

[Daniel P. Berrange]

On Fri, Aug 09, 2013 at 10:48:52AM -0600, Jim Fehlig wrote:
 Daniel P. Berrange wrote:
  On Thu, Aug 08, 2013 at 05:41:26PM -0600, Jim Fehlig wrote:

  Currently, only one log file is created by the libxl driver, with
  all output from libxl for all domains going to this one file.
 
  Create a per-domain log file based on domain name, making sifting
  through the logs a bit easier.  This required deferring libxl_ctx
  allocation until starting the domain, which is fine since the
  ctx is not used when the domain is inactive.
  ---
   src/libxl/libxl_conf.h   |  5 +--
   src/libxl/libxl_driver.c | 88 
  +---
   2 files changed, 57 insertions(+), 36 deletions(-)
  
 
  Startup of libvirtd SEGVs with this change in GIT
 
  ==4706== For counts of detected and suppressed errors, rerun with: -v
  ==4706== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 2 from 2)
  ==4601== Thread 12:
  ==4601== Invalid read of size 8
  ==4601==at 0x1E57964F: xtl_logv (in /usr/lib64/libxenctrl.so.4.2.0)
  ==4601==by 0x1E5796D3: xtl_log (in /usr/lib64/libxenctrl.so.4.2.0)
  ==4601==by 0x1E33258A: ??? (in /usr/lib64/libxenlight.so.2.0.0)
  ==4601==by 0x1E332641: ??? (in /usr/lib64/libxenlight.so.2.0.0)
  ==4601==by 0x1E317CD7: libxl_ctx_alloc (in 
  /usr/lib64/libxenlight.so.2.0.0)

 
 I should have looked at the xen code closer.  Seems libxl doesn't cope
 well with a NULL logger :(.
 
 Hmm, should the logger for this driver-wide ctx (used for getting libxl
 version and the like, no domain ops) just dump messages to /dev/null or
 should they go to a driver-wide log file?

Depends if you think there's any useful info to be had from the
driver wide context object ? If so, then probably best to have a
driver-wide log file for those messages


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 v2]LXC: Helper function for checking ownership of dir when userns enabled

[Daniel P. Berrange]

On Fri, Aug 09, 2013 at 04:05:58PM +0800, Chen Hanxiao wrote:
 From: Chen Hanxiao chenhanx...@cn.fujitsu.com
 
 If we enable userns, the ownership of dir we provided for containers
 should match the uid/gid in idmap.
 Currently, the debug log is very implicit or misleading sometimes.
 This patch will help clarify this for us when using
 debug log or virsh.

I do recall hitting some permission issue once, but can't remember
just what it was. Can you describe exactly how to reproduce the
problem ?

 Signed-off-by: Chen Hanxiao chenhanx...@cn.fujitsu.com
 ---
  src/lxc/lxc_container.c |   46 ++
  1 files changed, 46 insertions(+), 0 deletions(-)
 
 diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
 index b910b10..2ccdc61 100644
 --- a/src/lxc/lxc_container.c
 +++ b/src/lxc/lxc_container.c
 @@ -1815,6 +1815,49 @@ lxcNeedNetworkNamespace(virDomainDefPtr def)
  return false;
  }
  
 +/*
 + * Helper function for helping check
 + * whether we have enough privilege
 + * to operate the source dir when userns enabled
 + * @vmDef: pointer to vm definition structure
 + * Returns 0 on success or -1 in case of error
 + */
 +static int
 +lxcContainerUsernsSrcOwnershipCheck(virDomainDefPtr vmDef)
 +{
 +struct stat buf;
 +size_t i;
 +uid_t uid;
 +gid_t gid;
 +
 +VIR_DEBUG(vmDef-nfss %d, (int)vmDef-nfss);
 +for (i = 0; i  vmDef-nfss; i++) {
 +VIR_DEBUG(dst is %s, src is %s,
 +  vmDef-fss[i]-dst,
 +  vmDef-fss[i]-src);
 +
 +uid = vmDef-idmap.uidmap[0].target;
 +gid = vmDef-idmap.gidmap[0].target;
 +
 +if (lstat(vmDef-fss[i]-src, buf)  0) {
 +virReportSystemError(errno, _(Cannot access '%s'),
 + vmDef-fss[i]-src);
 +return -1;
 +} else if (uid != buf.st_uid || gid != buf.st_gid) {
 +VIR_DEBUG(In userns uid is %d, gid is %d\n,
 +  uid, gid);
 +errno = EINVAL;
 +
 +virReportSystemError(errno,
 +  _([userns] Src dir '%s' does not belong 
 to uid/gid: %d/%d),
 + vmDef-fss[i]-src, uid, gid);
 +return -1;
 +}
 +}
 +
 +return 0;
 +}
 +
  /**
   * lxcContainerStart:
   * @def: pointer to virtual machine structure
 @@ -1866,6 +1909,9 @@ int lxcContainerStart(virDomainDefPtr def,
  if (userns_supported()) {
  VIR_DEBUG(Enable user namespace);
  cflags |= CLONE_NEWUSER;
 +if (lxcContainerUsernsSrcOwnershipCheck(def)  0) {
 +return -1;
 +}
  } else {
  virReportSystemError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
   _(Kernel doesn't support user namespace));


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] libxl: Create per-domain log file

[Eric Blake]

On 08/09/2013 10:50 AM, Daniel P. Berrange wrote:

 I should have looked at the xen code closer.  Seems libxl doesn't cope
 well with a NULL logger :(.

 Hmm, should the logger for this driver-wide ctx (used for getting libxl
 version and the like, no domain ops) just dump messages to /dev/null or
 should they go to a driver-wide log file?
 
 Depends if you think there's any useful info to be had from the
 driver wide context object ? If so, then probably best to have a
 driver-wide log file for those messages

Or even just reply the data into the main libvirtd.log (ie. use existing
virLog functionality, rather than creating a new file), since ideally
the log will be relatively sparse: in my case, it would be just one
message stating that libxl is not available, at which point the libxl
driver no longer does anything else during the life of libvirtd.

-- 
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 01/10] test: Split object parsing into their own functions

[Daniel P. Berrange]

On Wed, Aug 07, 2013 at 07:28:55PM -0400, Cole Robinson wrote:
 The function that parses custom driver XML was getting pretty unruly,
 split the object parsing into their own functions. Rename some variables
 to be consistent across each function. This should be functionally
 identical.
 ---
  src/test/test_driver.c | 463 
 +
  1 file changed, 276 insertions(+), 187 deletions(-)

ACK

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 02/10] test: Simplify args passed to testDomainStartState

[Daniel P. Berrange]

On Wed, Aug 07, 2013 at 07:28:56PM -0400, Cole Robinson wrote:
 Passing virConnectPtr is redundant, just pass testConnPtr and simplify
 certain callers.
 ---
  src/test/test_driver.c | 36 +---
  1 file changed, 17 insertions(+), 19 deletions(-)

ACK

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 03/10] test: Unify object XML parsing

[Daniel P. Berrange]

On Wed, Aug 07, 2013 at 07:28:57PM -0400, Cole Robinson wrote:
 Right now things are split a bit between parsing from a relative file
 path or parsing from inline XML. Unify it. This will simplify upcoming
 bits.
 ---
  src/test/test_driver.c | 236 
 +
  1 file changed, 103 insertions(+), 133 deletions(-)

ACK

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 04/10] test: Allow specifying object runstate in driver XML

[Daniel P. Berrange]

On Wed, Aug 07, 2013 at 07:28:58PM -0400, Cole Robinson wrote:
 When passing in custom driver XML, allow a block like
 
 domain
   ...
   testdriver
 runstate3/runstate
   /testdriver
 /domain
 
 This is only read at initial driver start time, and sets the initial
 run state of the object. This is handy for UI testing.
 
 Wire it up for domains, networks, pools, and interfaces. The latter
 3 only have a notion of active and inactive, which map to runstate
 1 and 0 respectively.
 
 We also take care to unlink this testdriver block before passing the
 XML to the object parse function. Right now nothing complains, but if
 XML parsing became stricter in the future we don't want it to choke
 on this custom element.

We have the ability to register custom parser/formatting callbacks
for namespaces that we use for QEMU cli arg passthrough.

I'm thinking we'd be better off defining a custom namespace for
the test driver  using that for this element.

Of course we'd have to extend these namespace hooks to the other
types of objects, since we only have it for domains currently.

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 07/10] snapshot_conf: Allow parsing an XML node

[Daniel P. Berrange]

On Wed, Aug 07, 2013 at 07:29:01PM -0400, Cole Robinson wrote:
 Similar to how other objects arrange their parse APIs. This will be
 used by the test driver.
 ---
  src/conf/snapshot_conf.c | 85 
 ++--
  src/conf/snapshot_conf.h |  6 
  2 files changed, 67 insertions(+), 24 deletions(-)

ACK

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 00/10] test: Mock snapshot APIs and misc improvements

[Daniel P. Berrange]

On Wed, Aug 07, 2013 at 07:28:54PM -0400, Cole Robinson wrote:
 This series implements snapshot APIs for the test driver, and adds
 some misc improvements, like specifying domain state in the passed
 in driver XML.

Good addition, the test driver would be usefully put to work in a
unit test to validate operation of all virsh commands too, so we
will want to make sure it covers every API eventually.

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] libxl: Create per-domain log file

[Daniel P. Berrange]

On Fri, Aug 09, 2013 at 10:56:41AM -0600, Eric Blake wrote:
 On 08/09/2013 10:50 AM, Daniel P. Berrange wrote:
 
  I should have looked at the xen code closer.  Seems libxl doesn't cope
  well with a NULL logger :(.
 
  Hmm, should the logger for this driver-wide ctx (used for getting libxl
  version and the like, no domain ops) just dump messages to /dev/null or
  should they go to a driver-wide log file?
  
  Depends if you think there's any useful info to be had from the
  driver wide context object ? If so, then probably best to have a
  driver-wide log file for those messages
 
 Or even just reply the data into the main libvirtd.log (ie. use existing
 virLog functionality, rather than creating a new file), since ideally
 the log will be relatively sparse: in my case, it would be just one
 message stating that libxl is not available, at which point the libxl
 driver no longer does anything else during the life of libvirtd.

I was going to suggest that, but IIUC, the libxl logger API requires
you to give it a FILE * instance to which it writes directly. To feed
it into our normal logs, we'd want it to let us give it a callback
for writing log messages.

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] libxl: Create per-domain log file

[Jim Fehlig]

Daniel P. Berrange wrote:
 On Thu, Aug 08, 2013 at 05:41:26PM -0600, Jim Fehlig wrote:
   
 Currently, only one log file is created by the libxl driver, with
 all output from libxl for all domains going to this one file.

 Create a per-domain log file based on domain name, making sifting
 through the logs a bit easier.  This required deferring libxl_ctx
 allocation until starting the domain, which is fine since the
 ctx is not used when the domain is inactive.
 ---
  src/libxl/libxl_conf.h   |  5 +--
  src/libxl/libxl_driver.c | 88 
 +---
  2 files changed, 57 insertions(+), 36 deletions(-)
 

 Startup of libvirtd SEGVs with this change in GIT

 ==4706== For counts of detected and suppressed errors, rerun with: -v
 ==4706== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 2 from 2)
 ==4601== Thread 12:
 ==4601== Invalid read of size 8
 ==4601==at 0x1E57964F: xtl_logv (in /usr/lib64/libxenctrl.so.4.2.0)
 ==4601==by 0x1E5796D3: xtl_log (in /usr/lib64/libxenctrl.so.4.2.0)
 ==4601==by 0x1E33258A: ??? (in /usr/lib64/libxenlight.so.2.0.0)
 ==4601==by 0x1E332641: ??? (in /usr/lib64/libxenlight.so.2.0.0)
 ==4601==by 0x1E317CD7: libxl_ctx_alloc (in 
 /usr/lib64/libxenlight.so.2.0.0)
   

I should have looked at the xen code closer.  Seems libxl doesn't cope
well with a NULL logger :(.

Hmm, should the logger for this driver-wide ctx (used for getting libxl
version and the like, no domain ops) just dump messages to /dev/null or
should they go to a driver-wide log file?

Regards,
Jim

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

[libvirt] [PATCH] Make check for /dev/loop device names stricter to avoid /dev/loop-control

[Daniel P. Berrange]

From: Daniel P. Berrange berra...@redhat.com

Recentish (2011) kernels introduced a new device called /dev/loop-control,
which causes libvirt's detection of loop devices to get confused
since it only checks for a prefix of 'loop'. Also check that the
next character is a digit

Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
 src/util/virfile.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/src/util/virfile.c b/src/util/virfile.c
index 8f0eec3..2b07ac9 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -546,7 +546,11 @@ static int virFileLoopDeviceOpen(char **dev_name)
 
 errno = 0;
 while ((de = readdir(dh)) != NULL) {
-if (!STRPREFIX(de-d_name, loop))
+/* Checking 'loop' prefix is insufficient, since
+ * new kernels have a dev named 'loop-control'
+ */
+if (!STRPREFIX(de-d_name, loop) ||
+!c_isdigit(de-d_name[4]))
 continue;
 
 if (virAsprintf(looppath, /dev/%s, de-d_name)  0)
-- 
1.8.3.1

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

Re: [libvirt] [PATCH] libxl: Create per-domain log file

[Jim Fehlig]

Daniel P. Berrange wrote:
 On Fri, Aug 09, 2013 at 10:56:41AM -0600, Eric Blake wrote:
   
 On 08/09/2013 10:50 AM, Daniel P. Berrange wrote:

 
 I should have looked at the xen code closer.  Seems libxl doesn't cope
 well with a NULL logger :(.

 Hmm, should the logger for this driver-wide ctx (used for getting libxl
 version and the like, no domain ops) just dump messages to /dev/null or
 should they go to a driver-wide log file?
 
 Depends if you think there's any useful info to be had from the
 driver wide context object ? If so, then probably best to have a
 driver-wide log file for those messages
   
 Or even just reply the data into the main libvirtd.log (ie. use existing
 virLog functionality, rather than creating a new file), since ideally
 the log will be relatively sparse: in my case, it would be just one
 message stating that libxl is not available, at which point the libxl
 driver no longer does anything else during the life of libvirtd.
 

 I was going to suggest that, but IIUC, the libxl logger API requires
 you to give it a FILE * instance to which it writes directly.

Right.

  To feed
 it into our normal logs, we'd want it to let us give it a callback
 for writing log messages.
   

I was going to pass the logger create function 'stderr' for the
driver-wide ctx.  Would that be safe?

Regards,
Jim

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

Re: [libvirt] [PATCH] libxl: Create per-domain log file

[Eric Blake]

On 08/09/2013 11:04 AM, Daniel P. Berrange wrote:
 On Fri, Aug 09, 2013 at 10:56:41AM -0600, Eric Blake wrote:
 On 08/09/2013 10:50 AM, Daniel P. Berrange wrote:

 I should have looked at the xen code closer.  Seems libxl doesn't cope
 well with a NULL logger :(.

 Hmm, should the logger for this driver-wide ctx (used for getting libxl
 version and the like, no domain ops) just dump messages to /dev/null or
 should they go to a driver-wide log file?

 Depends if you think there's any useful info to be had from the
 driver wide context object ? If so, then probably best to have a
 driver-wide log file for those messages

 Or even just reply the data into the main libvirtd.log (ie. use existing
 virLog functionality, rather than creating a new file), since ideally
 the log will be relatively sparse: in my case, it would be just one
 message stating that libxl is not available, at which point the libxl
 driver no longer does anything else during the life of libvirtd.
 
 I was going to suggest that, but IIUC, the libxl logger API requires
 you to give it a FILE * instance to which it writes directly. To feed
 it into our normal logs, we'd want it to let us give it a callback
 for writing log messages.

Can we rely on open_memstream() being present on any platform with
libxl?  If so, that would give us a FILE* interface to libxl, while
still giving us access to the strings it prints which we can then turn
around and hand to our logger.

-- 
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] Make check for /dev/loop device names stricter to avoid /dev/loop-control

[Eric Blake]

On 08/09/2013 10:45 AM, Daniel P. Berrange wrote:
 From: Daniel P. Berrange berra...@redhat.com
 
 Recentish (2011) kernels introduced a new device called /dev/loop-control,
 which causes libvirt's detection of loop devices to get confused
 since it only checks for a prefix of 'loop'. Also check that the
 next character is a digit
 
 Signed-off-by: Daniel P. Berrange berra...@redhat.com
 ---
  src/util/virfile.c | 6 +-
  1 file changed, 5 insertions(+), 1 deletion(-)

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

[libvirt] [PATCH] virsh: nicer abort of blockcopy

[Eric Blake]

I attempted 'virsh blockcopy $dom vda $path --wait --verbose', then
hit Ctrl-C; I was a bit surprised to see this error message:

Block Copy: [  3 %]error: failed to query job for disk vda

when I had been expecting:

Block Copy: [  3 %]
Copy aborted

* tools/virsh-domain.c (cmdBlockCopy): Print graceful exit message
rather than error when ctrl-c interrupts job.

Signed-off-by: Eric Blake ebl...@redhat.com
---
 tools/virsh-domain.c | 10 +++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 8cafce4..4081451 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -1810,10 +1810,13 @@ cmdBlockCopy(vshControl *ctl, const vshCmd *cmd)
 result = virDomainGetBlockJobInfo(dom, path, info, 0);
 pthread_sigmask(SIG_SETMASK, oldsigmask, NULL);

-if (result = 0) {
+if (result  0) {
 vshError(ctl, _(failed to query job for disk %s), path);
 goto cleanup;
 }
+if (result == 0)
+break;
+
 if (verbose)
 print_job_progress(_(Block Copy), info.end - info.cur, info.end);
 if (info.cur == info.end)
@@ -1840,13 +1843,14 @@ cmdBlockCopy(vshControl *ctl, const vshCmd *cmd)
 }
 }

-if (pivot) {
+if (!quit  pivot) {
 abort_flags |= VIR_DOMAIN_BLOCK_JOB_ABORT_PIVOT;
 if (virDomainBlockJobAbort(dom, path, abort_flags)  0) {
 vshError(ctl, _(failed to pivot job for disk %s), path);
 goto cleanup;
 }
-} else if (finish  virDomainBlockJobAbort(dom, path, abort_flags)  0) {
+} else if (finish  !quit 
+   virDomainBlockJobAbort(dom, path, abort_flags)  0) {
 vshError(ctl, _(failed to finish job for disk %s), path);
 goto cleanup;
 }
-- 
1.8.3.1

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

[libvirt] [PATCH] libxl: fix libvirtd segfault

[Jim Fehlig]

Commit d72ef888 introduced a bug in the libxl driver that will
segfault libvirtd if libxl reports an error message, e.g. when
attempting to initialize the driver on a non-Xen system.  I
assumed it was valid to pass a NULL logger to libxl_ctx_alloc(),
but that is not the case since any errors associated with the ctx
that are emitted by libxl will dereference the logger and crash
libvirtd.

Errors associated with the libxl driver-wide ctx could be useful
for debugging anyway, so create a 'libxl-driver.log' to capture
these errors.
---
 src/libxl/libxl_conf.h   |  3 +++
 src/libxl/libxl_driver.c | 30 --
 2 files changed, 31 insertions(+), 2 deletions(-)

diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h
index 78133b9..9d72b72 100644
--- a/src/libxl/libxl_conf.h
+++ b/src/libxl/libxl_conf.h
@@ -57,6 +57,9 @@ struct _libxlDriverPrivate {
 virDomainXMLOptionPtr xmlopt;
 unsigned int version;
 
+/* log stream for driver-wide libxl ctx */
+FILE *logger_file;
+xentoollog_logger *logger;
 /* libxl ctx for driver wide ops; getVersion, getNodeInfo, ... */
 libxl_ctx *ctx;
 
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index cc9e772..9dc7261 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -1180,6 +1180,9 @@ libxlStateCleanup(void)
 virObjectUnref(libxl_driver-xmlopt);
 virObjectUnref(libxl_driver-domains);
 libxl_ctx_free(libxl_driver-ctx);
+xtl_logger_destroy(libxl_driver-logger);
+if (libxl_driver-logger_file)
+VIR_FORCE_FCLOSE(libxl_driver-logger_file);
 
 virObjectUnref(libxl_driver-reservedVNCPorts);
 
@@ -1229,6 +1232,7 @@ libxlStateInitialize(bool privileged,
  void *opaque ATTRIBUTE_UNUSED)
 {
 const libxl_version_info *ver_info;
+char *log_file = NULL;
 virCommandPtr cmd;
 int status, ret = 0;
 unsigned int free_mem;
@@ -1308,6 +1312,17 @@ libxlStateInitialize(bool privileged,
 goto error;
 }
 
+if (virAsprintf(log_file, %s/libxl-driver.log, libxl_driver-logDir)  
0)
+goto error;
+
+if ((libxl_driver-logger_file = fopen(log_file, a)) == NULL)  {
+virReportSystemError(errno,
+ _(failed to create logfile %s),
+ log_file);
+goto error;
+}
+VIR_FREE(log_file);
+
 /* read the host sysinfo */
 if (privileged)
 libxl_driver-hostsysinfo = virSysinfoRead();
@@ -1316,8 +1331,18 @@ libxlStateInitialize(bool privileged,
 if (!libxl_driver-domainEventState)
 goto error;
 
-if (libxl_ctx_alloc(libxl_driver-ctx, LIBXL_VERSION, 0, NULL)) {
-VIR_INFO(cannot initialize libxenlight context, probably not running 
in a Xen Dom0, disabling driver);
+libxl_driver-logger =
+(xentoollog_logger 
*)xtl_createlogger_stdiostream(libxl_driver-logger_file, XTL_DEBUG, 0);
+if (!libxl_driver-logger) {
+VIR_INFO(cannot create logger for libxenlight, disabling driver);
+goto fail;
+}
+
+if (libxl_ctx_alloc(libxl_driver-ctx,
+   LIBXL_VERSION, 0,
+   libxl_driver-logger)) {
+VIR_INFO(cannot initialize libxenlight context, probably not running 
+ in a Xen Dom0, disabling driver);
 goto fail;
 }
 
@@ -1383,6 +1408,7 @@ libxlStateInitialize(bool privileged,
 error:
 ret = -1;
 fail:
+VIR_FREE(log_file);
 if (libxl_driver)
 libxlDriverUnlock(libxl_driver);
 libxlStateCleanup();
-- 
1.8.1.4

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

Re: [libvirt] [PATCH] libxl: Create per-domain log file

[Jim Fehlig]

Daniel P. Berrange wrote:
 On Thu, Aug 08, 2013 at 05:41:26PM -0600, Jim Fehlig wrote:
   
 Currently, only one log file is created by the libxl driver, with
 all output from libxl for all domains going to this one file.

 Create a per-domain log file based on domain name, making sifting
 through the logs a bit easier.  This required deferring libxl_ctx
 allocation until starting the domain, which is fine since the
 ctx is not used when the domain is inactive.
 ---
  src/libxl/libxl_conf.h   |  5 +--
  src/libxl/libxl_driver.c | 88 
 +---
  2 files changed, 57 insertions(+), 36 deletions(-)
 

 Startup of libvirtd SEGVs with this change in GIT
   

I should have tested this change on a system with libxl, but not booted
to Xen.  libxl would have then complained, and I would have seen the
crash.  Lesson learned...

I've sent a patch to fix the issue

https://www.redhat.com/archives/libvir-list/2013-August/msg00484.html

Regards,
Jim

 ==4706== For counts of detected and suppressed errors, rerun with: -v
 ==4706== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 2 from 2)
 ==4601== Thread 12:
 ==4601== Invalid read of size 8
 ==4601==at 0x1E57964F: xtl_logv (in /usr/lib64/libxenctrl.so.4.2.0)
 ==4601==by 0x1E5796D3: xtl_log (in /usr/lib64/libxenctrl.so.4.2.0)
 ==4601==by 0x1E33258A: ??? (in /usr/lib64/libxenlight.so.2.0.0)
 ==4601==by 0x1E332641: ??? (in /usr/lib64/libxenlight.so.2.0.0)
 ==4601==by 0x1E317CD7: libxl_ctx_alloc (in 
 /usr/lib64/libxenlight.so.2.0.0)
 ==4601==by 0x1E0E3016: libxlStateInitialize.part.12 (libxl_driver.c:1319)
 ==4601==by 0x514AA29: virStateInitialize (libvirt.c:832)
 ==4601==by 0x11C42A: daemonRunStateInit (libvirtd.c:906)
 ==4601==by 0x50C273D: virThreadHelper (virthreadpthread.c:161)
 ==4601==by 0x82D0C52: start_thread (pthread_create.c:308)
 ==4601==by 0x89E213C: clone (clone.S:113)
 ==4601==  Address 0x0 is not stack'd, malloc'd or (recently) free'd
 ==4601== 
 Caught Segmentation violation dumping internal log buffer:


 Daniel
   

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

Re: [libvirt] [PATCH] libxl: fix libvirtd segfault

[Dave Allan]

Functional ack; this patch fixes the crash on my system.

On Fri, Aug 09, 2013 at 05:58:45PM -0600, Jim Fehlig wrote:
 Commit d72ef888 introduced a bug in the libxl driver that will
 segfault libvirtd if libxl reports an error message, e.g. when
 attempting to initialize the driver on a non-Xen system.  I
 assumed it was valid to pass a NULL logger to libxl_ctx_alloc(),
 but that is not the case since any errors associated with the ctx
 that are emitted by libxl will dereference the logger and crash
 libvirtd.
 
 Errors associated with the libxl driver-wide ctx could be useful
 for debugging anyway, so create a 'libxl-driver.log' to capture
 these errors.
 ---
  src/libxl/libxl_conf.h   |  3 +++
  src/libxl/libxl_driver.c | 30 --
  2 files changed, 31 insertions(+), 2 deletions(-)
 
 diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h
 index 78133b9..9d72b72 100644
 --- a/src/libxl/libxl_conf.h
 +++ b/src/libxl/libxl_conf.h
 @@ -57,6 +57,9 @@ struct _libxlDriverPrivate {
  virDomainXMLOptionPtr xmlopt;
  unsigned int version;
  
 +/* log stream for driver-wide libxl ctx */
 +FILE *logger_file;
 +xentoollog_logger *logger;
  /* libxl ctx for driver wide ops; getVersion, getNodeInfo, ... */
  libxl_ctx *ctx;
  
 diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
 index cc9e772..9dc7261 100644
 --- a/src/libxl/libxl_driver.c
 +++ b/src/libxl/libxl_driver.c
 @@ -1180,6 +1180,9 @@ libxlStateCleanup(void)
  virObjectUnref(libxl_driver-xmlopt);
  virObjectUnref(libxl_driver-domains);
  libxl_ctx_free(libxl_driver-ctx);
 +xtl_logger_destroy(libxl_driver-logger);
 +if (libxl_driver-logger_file)
 +VIR_FORCE_FCLOSE(libxl_driver-logger_file);
  
  virObjectUnref(libxl_driver-reservedVNCPorts);
  
 @@ -1229,6 +1232,7 @@ libxlStateInitialize(bool privileged,
   void *opaque ATTRIBUTE_UNUSED)
  {
  const libxl_version_info *ver_info;
 +char *log_file = NULL;
  virCommandPtr cmd;
  int status, ret = 0;
  unsigned int free_mem;
 @@ -1308,6 +1312,17 @@ libxlStateInitialize(bool privileged,
  goto error;
  }
  
 +if (virAsprintf(log_file, %s/libxl-driver.log, libxl_driver-logDir) 
  0)
 +goto error;
 +
 +if ((libxl_driver-logger_file = fopen(log_file, a)) == NULL)  {
 +virReportSystemError(errno,
 + _(failed to create logfile %s),
 + log_file);
 +goto error;
 +}
 +VIR_FREE(log_file);
 +
  /* read the host sysinfo */
  if (privileged)
  libxl_driver-hostsysinfo = virSysinfoRead();
 @@ -1316,8 +1331,18 @@ libxlStateInitialize(bool privileged,
  if (!libxl_driver-domainEventState)
  goto error;
  
 -if (libxl_ctx_alloc(libxl_driver-ctx, LIBXL_VERSION, 0, NULL)) {
 -VIR_INFO(cannot initialize libxenlight context, probably not 
 running in a Xen Dom0, disabling driver);
 +libxl_driver-logger =
 +(xentoollog_logger 
 *)xtl_createlogger_stdiostream(libxl_driver-logger_file, XTL_DEBUG, 0);
 +if (!libxl_driver-logger) {
 +VIR_INFO(cannot create logger for libxenlight, disabling driver);
 +goto fail;
 +}
 +
 +if (libxl_ctx_alloc(libxl_driver-ctx,
 +   LIBXL_VERSION, 0,
 +   libxl_driver-logger)) {
 +VIR_INFO(cannot initialize libxenlight context, probably not 
 running 
 + in a Xen Dom0, disabling driver);
  goto fail;
  }
  
 @@ -1383,6 +1408,7 @@ libxlStateInitialize(bool privileged,
  error:
  ret = -1;
  fail:
 +VIR_FREE(log_file);
  if (libxl_driver)
  libxlDriverUnlock(libxl_driver);
  libxlStateCleanup();
 -- 
 1.8.1.4
 
 --
 libvir-list mailing list
 libvir-list@redhat.com
 https://www.redhat.com/mailman/listinfo/libvir-list

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

Re: [libvirt] [PATCH] remote: Fix a segfault in remoteDomainCreateWithFlags

[Doug Goldstein]

On Thu, Aug 8, 2013 at 10:05 PM, Alex Jia a...@redhat.com wrote:
 Martin, I pushed this now.

 --
 Regards,
 Alex

Pushed to v1.1.1-maint

-- 
Doug Goldstein

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

Re: [libvirt] [PATCHv3] build: avoid -lgcrypt with newer gnutls

[Doug Goldstein]

On Fri, Aug 9, 2013 at 10:31 AM, Eric Blake ebl...@redhat.com wrote:
 On 08/07/2013 08:15 AM, Eric Blake wrote:
 On 08/07/2013 07:29 AM, Doug Goldstein wrote:
 On Tue, Jul 30, 2013 at 3:45 PM, Eric Blake ebl...@redhat.com wrote:
 https://bugzilla.redhat.com/show_bug.cgi?id=951637

 Newer gnutls uses nettle, rather than gcrypt, which is a lot nicer
 regarding initialization.  Yet we were unconditionally initializing
 gcrypt even when gnutls wouldn't be using it, and having two crypto
 libraries linked into libvirt.so is pointless, but mostly harmless
 (it doesn't crash, but does interfere with certification efforts).



 v3: configure logic is enhanced to try harder to get the
 right answer for gentoo.  I tested with Fedora 19 and RHEL 6,
 but need feedback from a gentoo tester before this can go in.



 I can ACK this by visual code inspection. I haven't explicitly tested
 this patch but its a cleaned up version of the patch we previously
 discussed it. If you need me to test it, I can do that tonight.

 I'd feel better with an explicit test; we're in no rush, so I'll wait
 for your test results on gentoo.

 Did you ever re-run this?

Sorry completely forgot. I just did and it worked great. So ACK.

-- 
Doug Goldstein

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

Re: [libvirt] [PATCH] libxl: fix libvirtd segfault

[Jim Fehlig]

Dave Allan wrote:
 Functional ack; this patch fixes the crash on my system.
   

I've pushed this patch since it fixes a crash affecting everyone that
has built the libxl driver but not actually running Xen.  Any additional
review comments can be addressed with a follow-up patch.

Regards,
Jim

 On Fri, Aug 09, 2013 at 05:58:45PM -0600, Jim Fehlig wrote:
   
 Commit d72ef888 introduced a bug in the libxl driver that will
 segfault libvirtd if libxl reports an error message, e.g. when
 attempting to initialize the driver on a non-Xen system.  I
 assumed it was valid to pass a NULL logger to libxl_ctx_alloc(),
 but that is not the case since any errors associated with the ctx
 that are emitted by libxl will dereference the logger and crash
 libvirtd.

 Errors associated with the libxl driver-wide ctx could be useful
 for debugging anyway, so create a 'libxl-driver.log' to capture
 these errors.
 ---
  src/libxl/libxl_conf.h   |  3 +++
  src/libxl/libxl_driver.c | 30 --
  2 files changed, 31 insertions(+), 2 deletions(-)

 diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h
 index 78133b9..9d72b72 100644
 --- a/src/libxl/libxl_conf.h
 +++ b/src/libxl/libxl_conf.h
 @@ -57,6 +57,9 @@ struct _libxlDriverPrivate {
  virDomainXMLOptionPtr xmlopt;
  unsigned int version;
  
 +/* log stream for driver-wide libxl ctx */
 +FILE *logger_file;
 +xentoollog_logger *logger;
  /* libxl ctx for driver wide ops; getVersion, getNodeInfo, ... */
  libxl_ctx *ctx;
  
 diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
 index cc9e772..9dc7261 100644
 --- a/src/libxl/libxl_driver.c
 +++ b/src/libxl/libxl_driver.c
 @@ -1180,6 +1180,9 @@ libxlStateCleanup(void)
  virObjectUnref(libxl_driver-xmlopt);
  virObjectUnref(libxl_driver-domains);
  libxl_ctx_free(libxl_driver-ctx);
 +xtl_logger_destroy(libxl_driver-logger);
 +if (libxl_driver-logger_file)
 +VIR_FORCE_FCLOSE(libxl_driver-logger_file);
  
  virObjectUnref(libxl_driver-reservedVNCPorts);
  
 @@ -1229,6 +1232,7 @@ libxlStateInitialize(bool privileged,
   void *opaque ATTRIBUTE_UNUSED)
  {
  const libxl_version_info *ver_info;
 +char *log_file = NULL;
  virCommandPtr cmd;
  int status, ret = 0;
  unsigned int free_mem;
 @@ -1308,6 +1312,17 @@ libxlStateInitialize(bool privileged,
  goto error;
  }
  
 +if (virAsprintf(log_file, %s/libxl-driver.log, libxl_driver-logDir) 
  0)
 +goto error;
 +
 +if ((libxl_driver-logger_file = fopen(log_file, a)) == NULL)  {
 +virReportSystemError(errno,
 + _(failed to create logfile %s),
 + log_file);
 +goto error;
 +}
 +VIR_FREE(log_file);
 +
  /* read the host sysinfo */
  if (privileged)
  libxl_driver-hostsysinfo = virSysinfoRead();
 @@ -1316,8 +1331,18 @@ libxlStateInitialize(bool privileged,
  if (!libxl_driver-domainEventState)
  goto error;
  
 -if (libxl_ctx_alloc(libxl_driver-ctx, LIBXL_VERSION, 0, NULL)) {
 -VIR_INFO(cannot initialize libxenlight context, probably not 
 running in a Xen Dom0, disabling driver);
 +libxl_driver-logger =
 +(xentoollog_logger 
 *)xtl_createlogger_stdiostream(libxl_driver-logger_file, XTL_DEBUG, 0);
 +if (!libxl_driver-logger) {
 +VIR_INFO(cannot create logger for libxenlight, disabling driver);
 +goto fail;
 +}
 +
 +if (libxl_ctx_alloc(libxl_driver-ctx,
 +   LIBXL_VERSION, 0,
 +   libxl_driver-logger)) {
 +VIR_INFO(cannot initialize libxenlight context, probably not 
 running 
 + in a Xen Dom0, disabling driver);
  goto fail;
  }
  
 @@ -1383,6 +1408,7 @@ libxlStateInitialize(bool privileged,
  error:
  ret = -1;
  fail:
 +VIR_FREE(log_file);
  if (libxl_driver)
  libxlDriverUnlock(libxl_driver);
  libxlStateCleanup();
 

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