[libvirt] [PATCHv4] python: Avoid memory leaks on libvirt_virNodeGetMemoryStats

2012-03-20 Thread Alex Jia
Detected by valgrind. Leaks are introduced in commit 17c7795.

* python/libvirt-override.c (libvirt_virNodeGetMemoryStats): fix memory leaks
and improve codes return value.

For details, please see the following link:
RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=770944


Signed-off-by: Alex Jia a...@redhat.com
---
 python/libvirt-override.c |   41 +++--
 1 files changed, 31 insertions(+), 10 deletions(-)

diff --git a/python/libvirt-override.c b/python/libvirt-override.c
index 792cfa3..634e8bb 100644
--- a/python/libvirt-override.c
+++ b/python/libvirt-override.c
@@ -2550,7 +2550,9 @@ libvirt_virNodeGetCPUStats(PyObject *self 
ATTRIBUTE_UNUSED, PyObject *args)
 static PyObject *
 libvirt_virNodeGetMemoryStats(PyObject *self ATTRIBUTE_UNUSED, PyObject *args)
 {
-PyObject *ret;
+PyObject *ret = NULL;
+PyObject *key = NULL;
+PyObject *val = NULL;
 PyObject *pyobj_conn;
 virConnectPtr conn;
 unsigned int flags;
@@ -2559,7 +2561,7 @@ libvirt_virNodeGetMemoryStats(PyObject *self 
ATTRIBUTE_UNUSED, PyObject *args)
 virNodeMemoryStatsPtr stats = NULL;
 
 if (!PyArg_ParseTuple(args, (char *)Oii:virNodeGetMemoryStats, 
pyobj_conn, cellNum, flags))
-return(NULL);
+return ret;
 conn = (virConnectPtr)(PyvirConnect_Get(pyobj_conn));
 
 LIBVIRT_BEGIN_ALLOW_THREADS;
@@ -2570,7 +2572,7 @@ libvirt_virNodeGetMemoryStats(PyObject *self 
ATTRIBUTE_UNUSED, PyObject *args)
 
 if (nparams) {
 if (VIR_ALLOC_N(stats, nparams)  0)
-return VIR_PY_NONE;
+return PyErr_NoMemory();
 
 LIBVIRT_BEGIN_ALLOW_THREADS;
 c_retval = virNodeGetMemoryStats(conn, cellNum, stats, nparams, 
flags);
@@ -2580,18 +2582,37 @@ libvirt_virNodeGetMemoryStats(PyObject *self 
ATTRIBUTE_UNUSED, PyObject *args)
 return VIR_PY_NONE;
 }
 }
-if (!(ret = PyDict_New())) {
-VIR_FREE(stats);
-return VIR_PY_NONE;
-}
+
+if (!(ret = PyDict_New()))
+goto error;
+
 for (i = 0; i  nparams; i++) {
-PyDict_SetItem(ret,
-   libvirt_constcharPtrWrap(stats[i].field),
-   libvirt_ulonglongWrap(stats[i].value));
+key = libvirt_constcharPtrWrap(stats[i].field);
+val = libvirt_ulonglongWrap(stats[i].value);
+
+if (!key || !val) {
+ret = NULL;
+goto error;
+}
+
+if (PyDict_SetItem(ret, key, val)  0) {
+Py_DECREF(ret);
+ret = NULL;
+goto error;
+}
+
+Py_DECREF(key);
+Py_DECREF(val);
 }
 
 VIR_FREE(stats);
 return ret;
+
+error:
+VIR_FREE(stats);
+Py_XDECREF(key);
+Py_XDECREF(val);
+return ret;
 }
 
 static PyObject *
-- 
1.7.1

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


Re: [libvirt] [PATCH] python: make virDomainSetSchedulerParameters accpet integer argument

2012-03-20 Thread Guannan Ren

On 03/20/2012 02:11 AM, Eric Blake wrote:

On 03/19/2012 12:03 PM, Eric Blake wrote:


Why are you doing this for just VIR_TYPED_PARAM_ULLONG?  I argue that we
should be doing it for all of the integral conversions.

In fact, I'd argue that we want new helper functions in typewrappers.c,
as counterparts to our libvirt_intWrap() and friends.  Perhaps:

/* Return 0 if obj could be unwrapped into the corresponding C type, -1
with python exception set otherwise.  */
int libvirt_intUnwrap(PyObject *obj, int *val);
int libvirt_uintUnwrap(PyObject *obj, unsigned int *val);
int libvirt_longlongUnwrap(PyObject *obj, long long *val);


   About these three wrappers, we have implemented them in our code,
   the work here we gonna do is just to factor them from the codes 
to create

   wrappers for easier use in future.


int libvirt_ulonglongUnwrap(PyObject *obj, unsigned long long *val);


   This wrapper will include the fix in it.


Then the code in question simplifies to:

case VIR_TYPED_PARAM_ULLONG:
{
 unsigned long long ullong_val;
 if (libvirt_ulonglongUnwrap(value,ullong_val)  0)
 goto cleanup;
 temp-value.ul = ullong_val;



 Okay, I will do it soon

 Guannan Ren

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


Re: [libvirt] sysctl variables, FreeBSD

2012-03-20 Thread Daniel P. Berrange
On Mon, Mar 19, 2012 at 10:19:55PM -0700, Jason Helfman wrote:
 Is there a method that can be used to remove the sysctl tuning for FreeBSD
 in the pipeline?
 
 Here are the current patches I have, but would like to remove:
 http://www.freebsd.org/cgi/cvsweb.cgi/ports/devel/libvirt/files/

The installation of the sysctl files should be made conditional,
based on a check in configure.ac

eg, add an '--with-sysctl' argument, and have it auto-enabled if
uname == Linux only. Then set a AM_CONDITIONAL variable to adjust
the Makefile.am

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 v4] python: add virDomainGetCPUStats python binding API

2012-03-20 Thread Eric Blake
On 03/19/2012 10:34 PM, Guannan Ren wrote:
 dom.getCPUStats(True, 0)
   [{'cpu_time': 24699446159L, 'system_time': 1087000L, 'user_time': 
 95000L}]
 dom.getCPUStats(False, 0)
   [{'cpu_time': 8535292289L}, {'cpu_time': 1005395355L}, {'cpu_time': 
 9351766377L}, {'cpu_time': 5813545649L}]
 
 *generator.py Add a new naming rule
 *libvirt-override-api.xml The API function description
 *libvirt-override.c Implement it.
 ---
  python/generator.py |5 +-
  python/libvirt-override-api.xml |   13 
  python/libvirt-override.c   |  147 
 +++
  3 files changed, 164 insertions(+), 1 deletions(-)

ACK.

-- 
Eric Blake   ebl...@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] [PATCHv4] python: Avoid memory leaks on libvirt_virNodeGetMemoryStats

2012-03-20 Thread Eric Blake
On 03/20/2012 01:14 AM, Alex Jia wrote:
 Detected by valgrind. Leaks are introduced in commit 17c7795.
 
 * python/libvirt-override.c (libvirt_virNodeGetMemoryStats): fix memory leaks
 and improve codes return value.
 
 For details, please see the following link:
 RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=770944
 
 
 Signed-off-by: Alex Jia a...@redhat.com
 ---
  python/libvirt-override.c |   41 +++--
  1 files changed, 31 insertions(+), 10 deletions(-)

  for (i = 0; i  nparams; i++) {
 -PyDict_SetItem(ret,
 -   libvirt_constcharPtrWrap(stats[i].field),
 -   libvirt_ulonglongWrap(stats[i].value));
 +key = libvirt_constcharPtrWrap(stats[i].field);
 +val = libvirt_ulonglongWrap(stats[i].value);
 +
 +if (!key || !val) {
 +ret = NULL;
 +goto error;

Memory leak of ret.

 +}
 +
 +if (PyDict_SetItem(ret, key, val)  0) {
 +Py_DECREF(ret);
 +ret = NULL;
 +goto error;
 +}

Fix it by doing:

if (!key || !val || PyDict_SetItem(ret, key, val)  0) {
Py_DECREF(ret);
ret = NULL;
goto error;
}

ACK with that fix.

-- 
Eric Blake   ebl...@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] python: add virDomainGetCPUStats python binding API

2012-03-20 Thread Guannan Ren

On 03/20/2012 11:42 PM, Eric Blake wrote:

On 03/19/2012 10:34 PM, Guannan Ren wrote:

 dom.getCPUStats(True, 0)
   [{'cpu_time': 24699446159L, 'system_time': 1087000L, 'user_time': 
95000L}]
 dom.getCPUStats(False, 0)
   [{'cpu_time': 8535292289L}, {'cpu_time': 1005395355L}, {'cpu_time': 
9351766377L}, {'cpu_time': 5813545649L}]

 *generator.py Add a new naming rule
 *libvirt-override-api.xml The API function description
 *libvirt-override.c Implement it.
---
  python/generator.py |5 +-
  python/libvirt-override-api.xml |   13 
  python/libvirt-override.c   |  147 +++
  3 files changed, 164 insertions(+), 1 deletions(-)

ACK.



   Thanks.

   Guannan Ren

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


[libvirt] [PATCH 03/14] s/xmlURIPtr/virURIPtr/ in virURIFormat impl

2012-03-20 Thread Daniel P. Berrange
From: Daniel P. Berrange berra...@redhat.com

The parameter in the virURIFormat impl mistakenly used the
xmlURIPtr type, instead of virURIPtr. Since they will soon
cease to be identical, this needs fixing
---
 src/util/viruri.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/src/util/viruri.c b/src/util/viruri.c
index 0b25679..1d2dca4 100644
--- a/src/util/viruri.c
+++ b/src/util/viruri.c
@@ -63,7 +63,7 @@ virURIParse(const char *uri)
  * @returns the constructed uri as a string
  */
 char *
-virURIFormat(xmlURIPtr uri)
+virURIFormat(virURIPtr uri)
 {
 char *backupserver = NULL;
 char *tmpserver = NULL;
-- 
1.7.7.6

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


[libvirt] [PATCH 02/14] Use virURIFree instead of xmlFreeURI

2012-03-20 Thread Daniel P. Berrange
From: Daniel P. Berrange berra...@redhat.com

Since we defined a custom virURIPtr type, we should use a
virURIFree method instead of assuming it will always be
a typedef for xmlURIPtr

* src/util/viruri.c, src/util/viruri.h, src/libvirt_private.syms:
  Add a virURIFree method
* src/datatypes.c, src/esx/esx_driver.c, src/libvirt.c,
  src/qemu/qemu_migration.c, src/vmx/vmx.c, src/xen/xend_internal.c,
  tests/viruritest.c: s/xmlFreeURI/virURIFree/
---
 src/datatypes.c   |2 +-
 src/esx/esx_driver.c  |2 +-
 src/libvirt.c |4 ++--
 src/libvirt_private.syms  |1 +
 src/qemu/qemu_migration.c |2 +-
 src/util/viruri.c |   12 
 src/util/viruri.h |2 ++
 src/vmx/vmx.c |2 +-
 src/xen/xend_internal.c   |8 
 tests/viruritest.c|2 +-
 10 files changed, 26 insertions(+), 11 deletions(-)

diff --git a/src/datatypes.c b/src/datatypes.c
index f0b5025..5ad988b 100644
--- a/src/datatypes.c
+++ b/src/datatypes.c
@@ -117,7 +117,7 @@ virReleaseConnect(virConnectPtr conn) {
 
 virResetError(conn-err);
 
-xmlFreeURI(conn-uri);
+virURIFree(conn-uri);
 
 virMutexUnlock(conn-lock);
 virMutexDestroy(conn-lock);
diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
index 6943534..dbf95f4 100644
--- a/src/esx/esx_driver.c
+++ b/src/esx/esx_driver.c
@@ -4072,7 +4072,7 @@ esxDomainMigratePerform(virDomainPtr domain,
 result = 0;
 
   cleanup:
-xmlFreeURI(parsedUri);
+virURIFree(parsedUri);
 esxVI_ObjectContent_Free(virtualMachine);
 esxVI_Event_Free(eventList);
 esxVI_ManagedObjectReference_Free(task);
diff --git a/src/libvirt.c b/src/libvirt.c
index 7f8d42c..f58623a 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -5072,10 +5072,10 @@ virDomainMigratePeer2Peer (virDomainPtr domain,
 if (!tempuri-server || STRPREFIX(tempuri-server, localhost)) {
 virLibConnError(VIR_ERR_INVALID_ARG, __FUNCTION__);
 virDispatchError(domain-conn);
-xmlFreeURI(tempuri);
+virURIFree(tempuri);
 return -1;
 }
-xmlFreeURI(tempuri);
+virURIFree(tempuri);
 
 /* Perform the migration.  The driver isn't supposed to return
  * until the migration is complete.
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index e40c80e..7cd6a96 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1469,6 +1469,7 @@ virTypedParameterAssign;
 
 
 # viruri.h
+virURIFree;
 virURIFormat;
 virURIParse;
 
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 81b2d5b..6f274ba 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -1875,7 +1875,7 @@ static int doNativeMigrate(struct qemud_driver *driver,
 if (spec.destType == MIGRATION_DEST_FD)
 VIR_FORCE_CLOSE(spec.dest.fd.qemu);
 
-xmlFreeURI(uribits);
+virURIFree(uribits);
 
 return ret;
 }
diff --git a/src/util/viruri.c b/src/util/viruri.c
index 6c4dfe3..0b25679 100644
--- a/src/util/viruri.c
+++ b/src/util/viruri.c
@@ -91,3 +91,15 @@ virURIFormat(xmlURIPtr uri)
 
 return ret;
 }
+
+
+/**
+ * virURIFree:
+ * @uri: uri to free
+ *
+ * Frees the URI
+ */
+void virURIFree(virURIPtr uri)
+{
+xmlFreeURI(uri);
+}
diff --git a/src/util/viruri.h b/src/util/viruri.h
index 5215e42..80369be 100644
--- a/src/util/viruri.h
+++ b/src/util/viruri.h
@@ -19,4 +19,6 @@ typedef xmlURIPtr virURIPtr;
 virURIPtr virURIParse(const char *uri);
 char *virURIFormat(virURIPtr uri);
 
+void virURIFree(virURIPtr uri);
+
 #endif /* __VIR_URI_H__ */
diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c
index cc426da..3179688 100644
--- a/src/vmx/vmx.c
+++ b/src/vmx/vmx.c
@@ -2696,7 +2696,7 @@ virVMXParseSerial(virVMXContext *ctx, virConfPtr conf, 
int port,
 VIR_FREE(fileType);
 VIR_FREE(fileName);
 VIR_FREE(network_endPoint);
-xmlFreeURI(parsedUri);
+virURIFree(parsedUri);
 
 return result;
 
diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c
index a19d055..df6b1bb 100644
--- a/src/xen/xend_internal.c
+++ b/src/xen/xend_internal.c
@@ -3234,25 +3234,25 @@ xenDaemonDomainMigratePerform (virDomainPtr domain,
 virXendError(VIR_ERR_INVALID_ARG,
   %s, _(xenDaemonDomainMigrate: only xenmigr://
  migrations are supported by Xen));
-xmlFreeURI (uriptr);
+virURIFree (uriptr);
 return -1;
 }
 if (!uriptr-server) {
 virXendError(VIR_ERR_INVALID_ARG,
   %s, _(xenDaemonDomainMigrate: a hostname must be
  specified in the URI));
-xmlFreeURI (uriptr);
+virURIFree (uriptr);
 return -1;
 }
 hostname = strdup (uriptr-server);
 if (!hostname) {
 virReportOOMError();
-xmlFreeURI (uriptr);
+virURIFree (uriptr);
 return -1;
 }
 if 

[libvirt] [PATCH 04/14] Centralize error reporting for URI parsing/formatting problems

2012-03-20 Thread Daniel P. Berrange
From: Daniel P. Berrange berra...@redhat.com

Move error reporting out of the callers, into virURIParse
and virURIFormat, to get consistency.

* include/libvirt/virterror.h, src/util/virterror.c: Add VIR_FROM_URI
* src/util/viruri.c, src/util/viruri.h: Add error reporting
* src/esx/esx_driver.c, src/libvirt.c, src/libxl/libxl_driver.c,
  src/lxc/lxc_driver.c, src/openvz/openvz_driver.c,
  src/qemu/qemu_driver.c, src/qemu/qemu_migration.c,
  src/remote/remote_driver.c, src/uml/uml_driver.c,
  src/vbox/vbox_tmpl.c, src/vmx/vmx.c, src/xen/xen_driver.c,
  src/xen/xend_internal.c, tests/viruritest.c: Remove error
  reporting
---
 include/libvirt/virterror.h |1 +
 src/esx/esx_driver.c|6 +-
 src/libvirt.c   |   16 
 src/libxl/libxl_driver.c|5 +
 src/lxc/lxc_driver.c|5 +
 src/openvz/openvz_driver.c  |5 +
 src/qemu/qemu_driver.c  |9 +++--
 src/qemu/qemu_migration.c   |5 +
 src/remote/remote_driver.c  |   18 --
 src/uml/uml_driver.c|9 +++--
 src/util/virterror.c|3 +++
 src/util/viruri.c   |   26 ++
 src/util/viruri.h   |6 --
 src/vbox/vbox_tmpl.c|   10 +++---
 src/vmx/vmx.c   |6 +-
 src/xen/xen_driver.c|5 +
 src/xen/xend_internal.c |8 +++-
 tests/viruritest.c  |8 ++--
 18 files changed, 63 insertions(+), 88 deletions(-)

diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h
index 38ac15e..c8ec8d4 100644
--- a/include/libvirt/virterror.h
+++ b/include/libvirt/virterror.h
@@ -85,6 +85,7 @@ typedef enum {
 VIR_FROM_LOCKING = 42, /* Error from lock manager */
 VIR_FROM_HYPERV = 43,  /* Error from Hyper-V driver */
 VIR_FROM_CAPABILITIES = 44, /* Error from capabilities */
+VIR_FROM_URI = 45,  /* Error from URI handling */
 } virErrorDomain;
 
 
diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
index dbf95f4..7e41fa3 100644
--- a/src/esx/esx_driver.c
+++ b/src/esx/esx_driver.c
@@ -3977,12 +3977,8 @@ esxDomainMigratePerform(virDomainPtr domain,
 }
 
 /* Parse migration URI */
-parsedUri = virURIParse(uri);
-
-if (parsedUri == NULL) {
-virReportOOMError();
+if (!(parsedUri = virURIParse(uri)))
 return -1;
-}
 
 if (parsedUri-scheme == NULL || STRCASENEQ(parsedUri-scheme, vpxmigr)) 
{
 ESX_ERROR(VIR_ERR_INVALID_ARG, %s,
diff --git a/src/libvirt.c b/src/libvirt.c
index f58623a..f7590e0 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -1166,11 +1166,7 @@ do_open (const char *name,
 virConnectOpenResolveURIAlias(conf, name, alias)  0)
 goto failed;
 
-ret-uri = virURIParse (alias ? alias : name);
-if (!ret-uri) {
-virLibConnError(VIR_ERR_INVALID_ARG,
-_(could not parse connection URI %s),
-alias ? alias : name);
+if (!(ret-uri = virURIParse (alias ? alias : name))) {
 VIR_FREE(alias);
 goto failed;
 }
@@ -1771,11 +1767,9 @@ virConnectGetURI (virConnectPtr conn)
 return NULL;
 }
 
-name = virURIFormat(conn-uri);
-if (!name) {
-virReportOOMError();
+if (!(name = virURIFormat(conn-uri)))
 goto error;
-}
+
 return name;
 
 error:
@@ -5062,9 +5056,7 @@ virDomainMigratePeer2Peer (virDomainPtr domain,
 return -1;
 }
 
-tempuri = virURIParse(dconnuri);
-if (!tempuri) {
-virLibConnError(VIR_ERR_INVALID_ARG, __FUNCTION__);
+if (!(tempuri = virURIParse(dconnuri))) {
 virDispatchError(domain-conn);
 return -1;
 }
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 177b218..eccd198 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -1044,11 +1044,8 @@ libxlOpen(virConnectPtr conn,
 if (libxl_driver == NULL)
 return VIR_DRV_OPEN_DECLINED;
 
-conn-uri = virURIParse(xen:///);
-if (!conn-uri) {
-virReportOOMError();
+if (!(conn-uri = virURIParse(xen:///)))
 return VIR_DRV_OPEN_ERROR;
-}
 } else {
 /* Only xen scheme */
 if (conn-uri-scheme == NULL || STRNEQ(conn-uri-scheme, xen))
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 3af8084..29842a5 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -140,11 +140,8 @@ static virDrvOpenStatus lxcOpen(virConnectPtr conn,
 if (lxc_driver == NULL)
 return VIR_DRV_OPEN_DECLINED;
 
-conn-uri = virURIParse(lxc:///);
-if (!conn-uri) {
-virReportOOMError();
+if (!(conn-uri = virURIParse(lxc:///)))
 return VIR_DRV_OPEN_ERROR;
-}
 } else {
 if (conn-uri-scheme == NULL ||
 STRNEQ(conn-uri-scheme, lxc))
diff --git 

[libvirt] [PATCH 06/14] Store parsed query parameters directly in the virURIPtr struct

2012-03-20 Thread Daniel P. Berrange
From: Daniel P. Berrange berra...@redhat.com

Avoid the need for each driver to parse query parameters itself
by storing them directly in the virURIPtr struct. The parsing
code is a copy of that from src/util/qparams.c  The latter will
be removed in a later patch

* src/util/viruri.h: Add query params to virURIPtr
* src/util/viruri.c: Parse query parameters when creating virURIPtr
* tests/viruritest.c: Expand test to cover params
---
 src/libvirt_private.syms |1 +
 src/util/viruri.c|  139 ++
 src/util/viruri.h|   15 +
 tests/viruritest.c   |   46 +---
 4 files changed, 193 insertions(+), 8 deletions(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 7cd6a96..49fb2ee 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1471,6 +1471,7 @@ virTypedParameterAssign;
 # viruri.h
 virURIFree;
 virURIFormat;
+virURIFormatQuery;
 virURIParse;
 
 
diff --git a/src/util/viruri.c b/src/util/viruri.c
index d8618d1..f5adca5 100644
--- a/src/util/viruri.c
+++ b/src/util/viruri.c
@@ -13,6 +13,7 @@
 #include memory.h
 #include util.h
 #include virterror_internal.h
+#include buf.h
 
 #define VIR_FROM_THIS VIR_FROM_URI
 
@@ -21,6 +22,117 @@
  __FUNCTION__, __LINE__, __VA_ARGS__)
 
 
+static int
+virURIParamAppend(virURIPtr uri,
+  const char *name,
+  const char *value)
+{
+char *pname = NULL;
+char *pvalue = NULL;
+
+if (!(pname = strdup(name)))
+goto no_memory;
+if (!(pvalue = strdup (value)))
+goto no_memory;
+
+if (VIR_RESIZE_N(uri-params, uri-paramsAlloc, uri-paramsCount, 1)  0)
+goto no_memory;
+
+uri-params[uri-paramsCount].name = pname;
+uri-params[uri-paramsCount].value = pvalue;
+uri-params[uri-paramsCount].ignore = 0;
+uri-paramsCount++;
+
+return 0;
+
+no_memory:
+VIR_FREE(pname);
+VIR_FREE(pvalue);
+virReportOOMError();
+return -1;
+}
+
+
+static int
+virURIParseParams(virURIPtr uri)
+{
+const char *end, *eq;
+const char *query = uri-query;
+
+if (!query || query[0] == '\0')
+return 0;
+
+while (*query) {
+char *name = NULL, *value = NULL;
+
+/* Find the next separator, or end of the string. */
+end = strchr (query, '');
+if (!end)
+end = strchr (query, ';');
+if (!end)
+end = query + strlen (query);
+
+/* Find the first '=' character between here and end. */
+eq = strchr (query, '=');
+if (eq  eq = end) eq = NULL;
+
+/* Empty section (eg. ). */
+if (end == query)
+goto next;
+
+/* If there is no '=' character, then we have just name
+ * and consistent with CGI.pm we assume value is .
+ */
+else if (!eq) {
+name = xmlURIUnescapeString (query, end - query, NULL);
+if (!name) goto no_memory;
+}
+/* Or if we have name= here (works around annoying
+ * problem when calling xmlURIUnescapeString with len = 0).
+ */
+else if (eq+1 == end) {
+name = xmlURIUnescapeString (query, eq - query, NULL);
+if (!name) goto no_memory;
+}
+/* If the '=' character is at the beginning then we have
+ * =value and consistent with CGI.pm we _ignore_ this.
+ */
+else if (query == eq)
+goto next;
+
+/* Otherwise it's name=value. */
+else {
+name = xmlURIUnescapeString (query, eq - query, NULL);
+if (!name)
+goto no_memory;
+value = xmlURIUnescapeString (eq+1, end - (eq+1), NULL);
+if (!value) {
+VIR_FREE(name);
+goto no_memory;
+}
+}
+
+/* Append to the parameter set. */
+if (virURIParamAppend(uri, name, value ? value : )  0) {
+VIR_FREE(name);
+VIR_FREE(value);
+goto no_memory;
+}
+VIR_FREE(name);
+VIR_FREE(value);
+
+next:
+query = end;
+if (*query) query ++; /* skip '' separator */
+}
+
+return 0;
+
+ no_memory:
+virReportOOMError();
+return -1;
+}
+
 /**
  * virURIParse:
  * @uri: URI to parse
@@ -92,12 +204,16 @@ virURIParse(const char *uri)
  * the uri with xmlFreeURI() */
 }
 
+if (virURIParseParams(ret)  0)
+goto error;
+
 xmlFreeURI(xmluri);
 
 return ret;
 
 no_memory:
 virReportOOMError();
+error:
 xmlFreeURI(xmluri);
 virURIFree(ret);
 return NULL;
@@ -153,6 +269,29 @@ cleanup:
 }
 
 
+char *virURIFormatQuery(virURIPtr uri)
+{
+virBuffer buf = VIR_BUFFER_INITIALIZER;
+int i, amp = 0;
+
+for (i = 0; i  uri-paramsCount; ++i) {
+if (!uri-params[i].ignore) {
+if (amp) virBufferAddChar (buf, '');
+virBufferStrcat (buf, uri-params[i].name, 

[libvirt] [PATCH 07/14] Convert drivers over to use virURIPtr for query params

2012-03-20 Thread Daniel P. Berrange
From: Daniel P. Berrange berra...@redhat.com

Convert drivers currently using the qparams APIs, to instead
use the virURIPtr query parameters directly.

* src/esx/esx_util.c, src/hyperv/hyperv_util.c,
  src/remote/remote_driver.c, src/xenapi/xenapi_utils.c: Remove
  use of qparams
* src/util/qparams.h, src/util/qparams.c: Delete
* src/Makefile.am, src/libvirt_private.syms: Remove qparams
---
 src/Makefile.am|1 -
 src/esx/esx_util.c |   17 +---
 src/hyperv/hyperv_util.c   |   17 +---
 src/libvirt_private.syms   |6 -
 src/remote/remote_driver.c |   17 +---
 src/util/qparams.c |  265 
 src/util/qparams.h |   58 --
 src/xenapi/xenapi_utils.c  |   20 +---
 8 files changed, 9 insertions(+), 392 deletions(-)
 delete mode 100644 src/util/qparams.c
 delete mode 100644 src/util/qparams.h

diff --git a/src/Makefile.am b/src/Makefile.am
index e57eca2..39076cc 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -70,7 +70,6 @@ UTIL_SOURCES =
\
util/pci.c util/pci.h   \
util/processinfo.c util/processinfo.h   \
util/hostusb.c util/hostusb.h   \
-   util/qparams.c util/qparams.h   \
util/sexpr.c util/sexpr.h   \
util/stats_linux.c util/stats_linux.h   \
util/storage_file.c util/storage_file.h \
diff --git a/src/esx/esx_util.c b/src/esx/esx_util.c
index 67b07b7..a08ca19 100644
--- a/src/esx/esx_util.c
+++ b/src/esx/esx_util.c
@@ -28,7 +28,6 @@
 
 #include internal.h
 #include datatypes.h
-#include qparams.h
 #include util.h
 #include memory.h
 #include logging.h
@@ -45,8 +44,6 @@ int
 esxUtil_ParseUri(esxUtil_ParsedUri **parsedUri, virURIPtr uri)
 {
 int result = -1;
-struct qparam_set *queryParamSet = NULL;
-struct qparam *queryParam = NULL;
 int i;
 int noVerify;
 int autoAnswer;
@@ -62,14 +59,8 @@ esxUtil_ParseUri(esxUtil_ParsedUri **parsedUri, virURIPtr 
uri)
 return -1;
 }
 
-queryParamSet = qparam_query_parse(uri-query);
-
-if (queryParamSet == NULL) {
-goto cleanup;
-}
-
-for (i = 0; i  queryParamSet-n; i++) {
-queryParam = queryParamSet-p[i];
+for (i = 0; i  uri-paramsCount; i++) {
+virURIParamPtr queryParam = uri-params[i];
 
 if (STRCASEEQ(queryParam-name, transport)) {
 VIR_FREE((*parsedUri)-transport);
@@ -204,10 +195,6 @@ esxUtil_ParseUri(esxUtil_ParsedUri **parsedUri, virURIPtr 
uri)
 esxUtil_FreeParsedUri(parsedUri);
 }
 
-if (queryParamSet != NULL) {
-free_qparam_set(queryParamSet);
-}
-
 return result;
 }
 
diff --git a/src/hyperv/hyperv_util.c b/src/hyperv/hyperv_util.c
index 63c761b..81c087e 100644
--- a/src/hyperv/hyperv_util.c
+++ b/src/hyperv/hyperv_util.c
@@ -24,7 +24,6 @@
 
 #include internal.h
 #include datatypes.h
-#include qparams.h
 #include util.h
 #include memory.h
 #include logging.h
@@ -40,8 +39,6 @@ int
 hypervParseUri(hypervParsedUri **parsedUri, virURIPtr uri)
 {
 int result = -1;
-struct qparam_set *queryParamSet = NULL;
-struct qparam *queryParam = NULL;
 int i;
 
 if (parsedUri == NULL || *parsedUri != NULL) {
@@ -54,14 +51,8 @@ hypervParseUri(hypervParsedUri **parsedUri, virURIPtr uri)
 return -1;
 }
 
-queryParamSet = qparam_query_parse(uri-query);
-
-if (queryParamSet == NULL) {
-goto cleanup;
-}
-
-for (i = 0; i  queryParamSet-n; i++) {
-queryParam = queryParamSet-p[i];
+for (i = 0; i  uri-paramsCount; i++) {
+virURIParamPtr queryParam = uri-params[i];
 
 if (STRCASEEQ(queryParam-name, transport)) {
 VIR_FREE((*parsedUri)-transport);
@@ -103,10 +94,6 @@ hypervParseUri(hypervParsedUri **parsedUri, virURIPtr uri)
 hypervFreeParsedUri(parsedUri);
 }
 
-if (queryParamSet != NULL) {
-free_qparam_set(queryParamSet);
-}
-
 return result;
 }
 
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 49fb2ee..8a14838 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -917,12 +917,6 @@ virProcessInfoGetAffinity;
 virProcessInfoSetAffinity;
 
 
-# qparams.h
-free_qparam_set;
-qparam_get_query;
-qparam_query_parse;
-
-
 # secret_conf.h
 virSecretDefFormat;
 virSecretDefFree;
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index 9de966f..bc6fea2 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -35,7 +35,6 @@
 #include domain_event.h
 #include driver.h
 #include buf.h
-#include qparams.h
 #include remote_driver.h
 #include remote_protocol.h
 #include qemu_protocol.h
@@ -311,7 +310,6 @@ doRemoteOpen (virConnectPtr conn,
   virConnectAuthPtr auth ATTRIBUTE_UNUSED,
   unsigned int 

[libvirt] [PATCH 09/14] Rename src/util/authhelper.[ch] to src/util/virauth.[ch]

2012-03-20 Thread Daniel P. Berrange
From: Daniel P. Berrange berra...@redhat.com

To follow latest naming conventions, rename src/util/authhelper.[ch]
to src/util/virauth.[ch].

* src/util/authhelper.[ch]: Rename to src/util/virauth.[ch]
* src/esx/esx_driver.c, src/hyperv/hyperv_driver.c,
  src/phyp/phyp_driver.c, src/xenapi/xenapi_driver.c: Update
  for renamed include files
---
 po/POTFILES.in   |2 +-
 src/Makefile.am  |2 +-
 src/esx/esx_driver.c |2 +-
 src/hyperv/hyperv_driver.c   |2 +-
 src/phyp/phyp_driver.c   |2 +-
 src/util/{authhelper.c = virauth.c} |5 ++---
 src/util/{authhelper.h = virauth.h} |9 -
 src/xenapi/xenapi_driver.c   |2 +-
 8 files changed, 12 insertions(+), 14 deletions(-)
 rename src/util/{authhelper.c = virauth.c} (97%)
 rename src/util/{authhelper.h = virauth.h} (87%)

diff --git a/po/POTFILES.in b/po/POTFILES.in
index 8354c09..8eaa8ad 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -106,7 +106,6 @@ src/storage/storage_driver.c
 src/test/test_driver.c
 src/uml/uml_conf.c
 src/uml/uml_driver.c
-src/util/authhelper.c
 src/util/cgroup.c
 src/util/command.c
 src/util/conf.c
@@ -124,6 +123,7 @@ src/util/stats_linux.c
 src/util/storage_file.c
 src/util/sysinfo.c
 src/util/util.c
+src/util/virauth.c
 src/util/viraudit.c
 src/util/virfile.c
 src/util/virhash.c
diff --git a/src/Makefile.am b/src/Makefile.am
index 07d7faa..3cbf9d7 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -52,7 +52,6 @@ augeastest_DATA =
 # These files are not related to driver APIs. Simply generic
 # helper APIs for various purposes
 UTIL_SOURCES = \
-   util/authhelper.c util/authhelper.h \
util/bitmap.c util/bitmap.h \
util/buf.c util/buf.h   \
util/command.c util/command.h   \
@@ -81,6 +80,7 @@ UTIL_SOURCES =
\
util/uuid.c util/uuid.h \
util/util.c util/util.h \
util/viraudit.c util/viraudit.h \
+   util/virauth.c util/virauth.h   \
util/virfile.c util/virfile.h   \
util/virnodesuspend.c util/virnodesuspend.h \
util/virpidfile.c util/virpidfile.h \
diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
index 7e41fa3..51bd5b2 100644
--- a/src/esx/esx_driver.c
+++ b/src/esx/esx_driver.c
@@ -26,7 +26,7 @@
 
 #include internal.h
 #include domain_conf.h
-#include authhelper.h
+#include virauth.h
 #include util.h
 #include memory.h
 #include logging.h
diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c
index 27c8747..5ca20cf 100644
--- a/src/hyperv/hyperv_driver.c
+++ b/src/hyperv/hyperv_driver.c
@@ -26,7 +26,7 @@
 #include internal.h
 #include datatypes.h
 #include domain_conf.h
-#include authhelper.h
+#include virauth.h
 #include util.h
 #include memory.h
 #include logging.h
diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c
index bdf4a7b..4b99465 100644
--- a/src/phyp/phyp_driver.c
+++ b/src/phyp/phyp_driver.c
@@ -44,7 +44,7 @@
 #include domain_event.h
 
 #include internal.h
-#include authhelper.h
+#include virauth.h
 #include util.h
 #include datatypes.h
 #include buf.h
diff --git a/src/util/authhelper.c b/src/util/virauth.c
similarity index 97%
rename from src/util/authhelper.c
rename to src/util/virauth.c
index 9398cb3..a3c6b94 100644
--- a/src/util/authhelper.c
+++ b/src/util/virauth.c
@@ -1,6 +1,5 @@
-
 /*
- * authhelper.c: authentication related utility functions
+ * virauth.c: authentication related utility functions
  *
  * Copyright (C) 2010 Matthias Bolte matthias.bo...@googlemail.com
  *
@@ -22,7 +21,7 @@
 
 #include config.h
 
-#include authhelper.h
+#include virauth.h
 #include util.h
 #include memory.h
 
diff --git a/src/util/authhelper.h b/src/util/virauth.h
similarity index 87%
rename from src/util/authhelper.h
rename to src/util/virauth.h
index ca45d16..2c8d80f 100644
--- a/src/util/authhelper.h
+++ b/src/util/virauth.h
@@ -1,6 +1,5 @@
-
 /*
- * authhelper.h: authentication related utility functions
+ * virauth.h: authentication related utility functions
  *
  * Copyright (C) 2010 Matthias Bolte matthias.bo...@googlemail.com
  *
@@ -20,8 +19,8 @@
  *
  */
 
-#ifndef __VIR_AUTHHELPER_H__
-# define __VIR_AUTHHELPER_H__
+#ifndef __VIR_AUTH_H__
+# define __VIR_AUTH_H__
 
 # include internal.h
 
@@ -30,4 +29,4 @@ char *virRequestUsername(virConnectAuthPtr auth, const char 
*defaultUsername,
 char *virRequestPassword(virConnectAuthPtr auth, const char *username,
  const char *hostname);
 
-#endif /* __VIR_AUTHHELPER_H__ */
+#endif /* __VIR_AUTH_H__ */
diff --git a/src/xenapi/xenapi_driver.c 

[libvirt] [PATCH 14/14] Lookup auth credentials in config file before prompting

2012-03-20 Thread Daniel P. Berrange
From: Daniel P. Berrange berra...@redhat.com

When SASL requests auth credentials, try to look them up in the
config file first. If any are found, remove them from the list
that the user is prompted for
---
 src/esx/esx_driver.c   |   58 +-
 src/hyperv/hyperv_driver.c |4 +-
 src/phyp/phyp_driver.c |4 +-
 src/remote/remote_driver.c |  138 
 src/util/virauth.c |   69 +-
 src/util/virauth.h |   10 +++-
 src/xenapi/xenapi_driver.c |4 +-
 7 files changed, 224 insertions(+), 63 deletions(-)

diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
index 7689306..6962832 100644
--- a/src/esx/esx_driver.c
+++ b/src/esx/esx_driver.c
@@ -667,10 +667,8 @@ esxCapsInit(esxPrivate *priv)
 
 
 static int
-esxConnectToHost(esxPrivate *priv, virConnectAuthPtr auth,
- const char *hostname, int port,
- const char *predefinedUsername,
- esxVI_ProductVersion expectedProductVersion,
+esxConnectToHost(virConnectPtr conn,
+ virConnectAuthPtr auth,
  char **vCenterIpAddress)
 {
 int result = -1;
@@ -682,25 +680,29 @@ esxConnectToHost(esxPrivate *priv, virConnectAuthPtr auth,
 esxVI_String *propertyNameList = NULL;
 esxVI_ObjectContent *hostSystem = NULL;
 esxVI_Boolean inMaintenanceMode = esxVI_Boolean_Undefined;
+esxPrivate *priv = conn-privateData;
+esxVI_ProductVersion expectedProductVersion = STRCASEEQ(conn-uri-scheme, 
esx)
+? esxVI_ProductVersion_ESX
+: esxVI_ProductVersion_GSX;
 
 if (vCenterIpAddress == NULL || *vCenterIpAddress != NULL) {
 ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR, %s, _(Invalid argument));
 return -1;
 }
 
-if (esxUtil_ResolveHostname(hostname, ipAddress, NI_MAXHOST)  0) {
+if (esxUtil_ResolveHostname(conn-uri-server, ipAddress, NI_MAXHOST)  0) 
{
 return -1;
 }
 
-if (predefinedUsername != NULL) {
-username = strdup(predefinedUsername);
+if (conn-uri-user != NULL) {
+username = strdup(conn-uri-user);
 
 if (username == NULL) {
 virReportOOMError();
 goto cleanup;
 }
 } else {
-username = virAuthGetUsername(auth, root, hostname);
+username = virAuthGetUsername(conn, auth, esx, root, 
conn-uri-server);
 
 if (username == NULL) {
 ESX_ERROR(VIR_ERR_AUTH_FAILED, %s, _(Username request failed));
@@ -708,7 +710,7 @@ esxConnectToHost(esxPrivate *priv, virConnectAuthPtr auth,
 }
 }
 
-unescapedPassword = virAuthGetPassword(auth, username, hostname);
+unescapedPassword = virAuthGetPassword(conn, auth, esx, username, 
conn-uri-server);
 
 if (unescapedPassword == NULL) {
 ESX_ERROR(VIR_ERR_AUTH_FAILED, %s, _(Password request failed));
@@ -722,7 +724,7 @@ esxConnectToHost(esxPrivate *priv, virConnectAuthPtr auth,
 }
 
 if (virAsprintf(url, %s://%s:%d/sdk, priv-parsedUri-transport,
-hostname, port)  0) {
+conn-uri-server, conn-uri-port)  0) {
 virReportOOMError();
 goto cleanup;
 }
@@ -743,13 +745,13 @@ esxConnectToHost(esxPrivate *priv, virConnectAuthPtr auth,
 priv-host-productVersion != esxVI_ProductVersion_ESX5x) {
 ESX_ERROR(VIR_ERR_INTERNAL_ERROR,
   _(%s is neither an ESX 3.5, 4.x nor 5.x host),
-  hostname);
+  conn-uri-server);
 goto cleanup;
 }
 } else { /* GSX */
 if (priv-host-productVersion != esxVI_ProductVersion_GSX20) {
 ESX_ERROR(VIR_ERR_INTERNAL_ERROR,
-  _(%s isn't a GSX 2.0 host), hostname);
+  _(%s isn't a GSX 2.0 host), conn-uri-server);
 goto cleanup;
 }
 }
@@ -799,9 +801,9 @@ esxConnectToHost(esxPrivate *priv, virConnectAuthPtr auth,
 
 
 static int
-esxConnectToVCenter(esxPrivate *priv, virConnectAuthPtr auth,
-const char *hostname, int port,
-const char *predefinedUsername,
+esxConnectToVCenter(virConnectPtr conn,
+virConnectAuthPtr auth,
+const char *hostname,
 const char *hostSystemIpAddress)
 {
 int result = -1;
@@ -810,6 +812,7 @@ esxConnectToVCenter(esxPrivate *priv, virConnectAuthPtr 
auth,
 char *unescapedPassword = NULL;
 char *password = NULL;
 char *url = NULL;
+esxPrivate *priv = conn-privateData;
 
 if (hostSystemIpAddress == NULL 
 (priv-parsedUri-path == NULL || STREQ(priv-parsedUri-path, /))) {
@@ -822,15 +825,15 @@ esxConnectToVCenter(esxPrivate *priv, virConnectAuthPtr 
auth,
 return -1;
 }
 
-if (predefinedUsername != NULL) {
-username = strdup(predefinedUsername);
+if (conn-uri-user != NULL) {
+username = strdup(conn-uri-user);
 
 if 

[libvirt] [PATCH 12/14] Add APIs for handling lookup of auth credentials from config file

2012-03-20 Thread Daniel P. Berrange
From: Daniel P. Berrange berra...@redhat.com

This defines the format for the auth credential config file and
provides APIs to access the data. The config file contains
one or more named 'credential' sets

  [credentials-$NAME]
  credname1=value1
  credname2=value2

eg

  [credentials-test]
  authname=fred
  password=123456

  [credentials-prod]
  authname=bar
  password=letmein

There are then one or more 'auth' sets which match services/hosts
and map to credential sets.

  [auth-$SERVICE-$HOSTNAME]
  credentials=$CREDENTIALS

eg

  [auth-libvirt-test1.example.com]
  credentials=test

  [auth-libvirt-test2.example.com]
  credentials=test

  [auth-libvirt-demo3.example.com]
  credentials=test

  [auth-libvirt-prod1.example.com]
  credentials=prod

* docs/auth.html.in: Document use of client auth config files
* src/Makefile.am, src/libvirt_private.syms,
  src/util/virauthconfig.c, src/util/virauthconfig.h: Add
  APIs for processing auth.conf file
---
 docs/auth.html.in |  118 ++-
 po/POTFILES.in|1 +
 src/Makefile.am   |1 +
 src/libvirt_private.syms  |7 ++
 src/util/virauthconfig.c  |  175 +
 src/util/virauthconfig.h  |   45 
 tests/Makefile.am |9 ++-
 tests/virauthconfigtest.c |  140 
 8 files changed, 494 insertions(+), 2 deletions(-)
 create mode 100644 src/util/virauthconfig.c
 create mode 100644 src/util/virauthconfig.h
 create mode 100644 tests/virauthconfigtest.c

diff --git a/docs/auth.html.in b/docs/auth.html.in
index 2163959..ecff0fc 100644
--- a/docs/auth.html.in
+++ b/docs/auth.html.in
@@ -1,7 +1,7 @@
 ?xml version=1.0?
 html
   body
-h1 Access control/h1
+h1 Authentication amp; access control/h1
 p
   When connecting to libvirt, some connections may require client
   authentication before allowing use of the APIs. The set of possible
@@ -11,6 +11,122 @@
 
 ul id=toc/ul
 
+h2a name=Auth_client_configClient configuration/a/h2
+
+p
+  When connecting to a remote hypervisor which requires authentication,
+most libvirt applications will prompt the user for the credentials. It is
+also possible to provide a client configuration file containing all the
+authentication credentials, avoiding any interaction. Libvirt will look
+for the authentication file using the following sequence:
+/p
+ol
+  liThe file path specified by the $LIBVIRT_AUTH_FILE environment
+variable./li
+  liThe file path specified by the authfile=/some/file URI
+query parameter/li
+  liThe file $HOME/.libvirt/auth.conf/li
+  liThe file /etc/libvirt/auth.conf/li
+/ol
+
+p
+  The auth configuration file uses the traditional code.ini/code
+  style syntax. There are two types of groups that can be present in
+  the config. First there are one or more strongcredential/strong
+  sets, which provide the actual authentication credentials. The keys
+  within the group may be:
+/p
+
+ul
+  licodeusername/code: the user login name to act as. This
+is relevant for ESX, Xen, HyperV and SSH, but probably not
+the one you want to libvirtd with SASL./li
+  licodeauthname/code: the name to authorize as. This is
+what is commonly required for libvirtd with SASL./li
+  licodepassword/code: the secret password/li
+  licoderealm/code: the domain realm for SASL, mostly
+unused/li
+/ul
+
+p
+  Each set of credentials has a name, which is part of the group
+  entry name. Overall the syntax is
+/p
+
+pre
+[credentials-$NAME]
+credname1=value1
+credname2=value2/pre
+
+p
+  For example, to define two sets of credentials used for production
+  and test machines, using libvirtd, and a further ESX server for dev:
+/p
+pre
+[credentials-test]
+authname=fred
+password=123456
+
+[credentials-prod]
+authname=bar
+password=letmein
+
+[credentials-dev]
+username=joe
+password=hello/pre
+
+p
+  The second set of groups provide mappings of credentials to
+  specific machine services. The config file group names compromise
+  the service type and host:
+/p
+
+pre
+[auth-$SERVICE-$HOSTNAME]
+credentials=$CREDENTIALS/pre
+
+p
+  For example, following the previous example, here is how to
+  list some machines
+/p
+
+pre
+[auth-libvirt-test1.example.com]
+credentials=test
+
+[auth-libvirt-test2.example.com]
+credentials=test
+
+[auth-libvirt-demo3.example.com]
+credentials=test
+
+[auth-libvirt-prod1.example.com]
+credentials=prod
+
+[auth-esx-dev1.example.com]
+credentials=dev/pre
+
+p
+  The following service types are known to libvirt
+/p
+
+ol
+  licodelibvirt/code - used for connections to a libvirtd
+server, which is configured with SASL auth/li
+  licodessh/code - used for connections to a Phyp server
+over SSH/li
+  

[libvirt] [PATCH 10/14] Rename virRequest{Username, Password} to virAuthGet{Username, Password}

2012-03-20 Thread Daniel P. Berrange
From: Daniel P. Berrange berra...@redhat.com

Ensure that the functions in virauth.h have names matching the file
prefix, by renaming  virRequest{Username,Password} to
virAuthGet{Username,Password}
---
 src/esx/esx_driver.c   |8 
 src/hyperv/hyperv_driver.c |4 ++--
 src/libvirt_private.syms   |5 +
 src/phyp/phyp_driver.c |4 ++--
 src/util/virauth.c |4 ++--
 src/util/virauth.h |4 ++--
 src/xenapi/xenapi_driver.c |4 ++--
 7 files changed, 19 insertions(+), 14 deletions(-)

diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
index 51bd5b2..7689306 100644
--- a/src/esx/esx_driver.c
+++ b/src/esx/esx_driver.c
@@ -700,7 +700,7 @@ esxConnectToHost(esxPrivate *priv, virConnectAuthPtr auth,
 goto cleanup;
 }
 } else {
-username = virRequestUsername(auth, root, hostname);
+username = virAuthGetUsername(auth, root, hostname);
 
 if (username == NULL) {
 ESX_ERROR(VIR_ERR_AUTH_FAILED, %s, _(Username request failed));
@@ -708,7 +708,7 @@ esxConnectToHost(esxPrivate *priv, virConnectAuthPtr auth,
 }
 }
 
-unescapedPassword = virRequestPassword(auth, username, hostname);
+unescapedPassword = virAuthGetPassword(auth, username, hostname);
 
 if (unescapedPassword == NULL) {
 ESX_ERROR(VIR_ERR_AUTH_FAILED, %s, _(Password request failed));
@@ -830,7 +830,7 @@ esxConnectToVCenter(esxPrivate *priv, virConnectAuthPtr 
auth,
 goto cleanup;
 }
 } else {
-username = virRequestUsername(auth, administrator, hostname);
+username = virAuthGetUsername(auth, administrator, hostname);
 
 if (username == NULL) {
 ESX_ERROR(VIR_ERR_AUTH_FAILED, %s, _(Username request failed));
@@ -838,7 +838,7 @@ esxConnectToVCenter(esxPrivate *priv, virConnectAuthPtr 
auth,
 }
 }
 
-unescapedPassword = virRequestPassword(auth, username, hostname);
+unescapedPassword = virAuthGetPassword(auth, username, hostname);
 
 if (unescapedPassword == NULL) {
 ESX_ERROR(VIR_ERR_AUTH_FAILED, %s, _(Password request failed));
diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c
index 5ca20cf..0469e2e 100644
--- a/src/hyperv/hyperv_driver.c
+++ b/src/hyperv/hyperv_driver.c
@@ -147,7 +147,7 @@ hypervOpen(virConnectPtr conn, virConnectAuthPtr auth, 
unsigned int flags)
 goto cleanup;
 }
 } else {
-username = virRequestUsername(auth, administrator, 
conn-uri-server);
+username = virAuthGetUsername(auth, administrator, 
conn-uri-server);
 
 if (username == NULL) {
 HYPERV_ERROR(VIR_ERR_AUTH_FAILED, %s, _(Username request 
failed));
@@ -155,7 +155,7 @@ hypervOpen(virConnectPtr conn, virConnectAuthPtr auth, 
unsigned int flags)
 }
 }
 
-password = virRequestPassword(auth, username, conn-uri-server);
+password = virAuthGetPassword(auth, username, conn-uri-server);
 
 if (password == NULL) {
 HYPERV_ERROR(VIR_ERR_AUTH_FAILED, %s, _(Password request failed));
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 3f69ec1..07302cd 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1165,6 +1165,11 @@ virUUIDGenerate;
 virUUIDParse;
 
 
+# virauth.h
+virAuthGetUsername;
+virAuthGetPassword;
+
+
 # viraudit.h
 virAuditClose;
 virAuditEncode;
diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c
index 4b99465..470706d 100644
--- a/src/phyp/phyp_driver.c
+++ b/src/phyp/phyp_driver.c
@@ -1006,7 +1006,7 @@ openSSHSession(virConnectPtr conn, virConnectAuthPtr auth,
 goto err;
 }
 
-username = virRequestUsername(auth, NULL, conn-uri-server);
+username = virAuthGetUsername(auth, NULL, conn-uri-server);
 
 if (username == NULL) {
 PHYP_ERROR(VIR_ERR_AUTH_FAILED, %s,
@@ -1087,7 +1087,7 @@ keyboard_interactive:
 goto disconnect;
 }
 
-password = virRequestPassword(auth, username, conn-uri-server);
+password = virAuthGetPassword(auth, username, conn-uri-server);
 
 if (password == NULL) {
 PHYP_ERROR(VIR_ERR_AUTH_FAILED, %s,
diff --git a/src/util/virauth.c b/src/util/virauth.c
index a3c6b94..d7375e9 100644
--- a/src/util/virauth.c
+++ b/src/util/virauth.c
@@ -27,7 +27,7 @@
 
 
 char *
-virRequestUsername(virConnectAuthPtr auth, const char *defaultUsername,
+virAuthGetUsername(virConnectAuthPtr auth, const char *defaultUsername,
const char *hostname)
 {
 unsigned int ncred;
@@ -74,7 +74,7 @@ virRequestUsername(virConnectAuthPtr auth, const char 
*defaultUsername,
 
 
 char *
-virRequestPassword(virConnectAuthPtr auth, const char *username,
+virAuthGetPassword(virConnectAuthPtr auth, const char *username,
const char *hostname)
 {
 unsigned int ncred;
diff --git a/src/util/virauth.h b/src/util/virauth.h
index 2c8d80f..8856701 100644
--- 

[libvirt] [PATCH 00/14] Support reading auth credentials from a config file

2012-03-20 Thread Daniel P. Berrange
When connecting to a remote libvirtd, or hypervisor that requires
authentication we have callbacks which can be used to prompt for
various credentials. This is tedious if you are looking to automate
stuff though. While we could let each application implement a
custom auth callback for fetching credentials from a config file,
it is nicer if we provide this ability directly in libvirt, so that
all apps can benefit from it.

So this series introduces a new config file in which authentication
credentials can be placed. It supports setting multiple sets of
credentials, of varying types in one file. I use the '.ini' config
file syntax, since this makes it easy to reuse the same config file
in virt-viewer  similar apps to store VNC / SPICE credentials.

To find the config file we check for each of

 - Path from LIBVIRT_AUTH_FILE env var
 - Query param 'authfile=PATH' in the URI params
 - /etc/libvirt/auth.conf
 - $HOME/.libvirt/auth.conf

if no config file is present, everything should carry on as normal.
If the config is present, then it is consulted to fill in credentials
first. Only if there are still missing credentials, will the auth
callbacks then be invoked.

See the docs in patch 12 of this series for the config file
syntax examples.

This is supported for all drivers that do auth, remote, esx,
xenapi, hyperv and phyp, though I have only tested it with the
remote driver. I would appreciate someone giving it a go with
the other drivers.

The first 7 patches are actually all related to the viruri.[ch]
file, expanding its functionality  adding tests. This is to
make sure the auth config patches can have easy access to the
query params.

The last 7 patches are related to the auth config file support
again including tests

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


[libvirt] [PATCH 01/14] Add test case for virURIPtr classs

2012-03-20 Thread Daniel P. Berrange
From: Daniel P. Berrange berra...@redhat.com

To ensure we properly escape  unescape IPv6 numeric addresses,
add a test case

* tests/Makefile.am, tests/viruritest.c: URI parsing test
---
 tests/Makefile.am  |7 ++-
 tests/viruritest.c |  139 
 2 files changed, 145 insertions(+), 1 deletions(-)
 create mode 100644 tests/viruritest.c

diff --git a/tests/Makefile.am b/tests/Makefile.am
index 0c8cf37..035c8c6 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -96,7 +96,7 @@ check_PROGRAMS = virshtest conftest sockettest \
commandtest commandhelper seclabeltest \
virhashtest virnetmessagetest virnetsockettest ssh \
utiltest virnettlscontexttest shunloadtest \
-   virtimetest
+   virtimetest viruritest
 
 check_LTLIBRARIES = libshunload.la
 
@@ -219,6 +219,7 @@ TESTS = virshtest \
virnetsockettest \
virnettlscontexttest \
virtimetest \
+viruritest \
shunloadtest \
utiltest \
$(test_scripts)
@@ -506,6 +507,10 @@ virtimetest_SOURCES = \
 virtimetest_CFLAGS = -Dabs_builddir=\$(abs_builddir)\ $(AM_CFLAGS)
 virtimetest_LDADD = ../src/libvirt-net-rpc.la $(LDADDS)
 
+viruritest_SOURCES = \
+   viruritest.c testutils.h testutils.c
+viruritest_CFLAGS = -Dabs_builddir=\$(abs_builddir)\ $(AM_CFLAGS)
+viruritest_LDADD = ../src/libvirt-net-rpc.la $(LDADDS)
 
 seclabeltest_SOURCES = \
seclabeltest.c
diff --git a/tests/viruritest.c b/tests/viruritest.c
new file mode 100644
index 000..a5de50a
--- /dev/null
+++ b/tests/viruritest.c
@@ -0,0 +1,139 @@
+/*
+ * Copyright (C) 2011 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307  USA
+ *
+ * Author: Daniel P. Berrange berra...@redhat.com
+ */
+
+#include config.h
+
+#include stdlib.h
+#include signal.h
+
+#include testutils.h
+#include util.h
+#include virterror_internal.h
+#include memory.h
+#include logging.h
+
+#include viruri.h
+
+#define VIR_FROM_THIS VIR_FROM_RPC
+
+struct URIParseData {
+const char *uri;
+const char *scheme;
+const char *server;
+int port;
+const char *path;
+const char *query;
+const char *fragment;
+};
+
+static int testURIParse(const void *args)
+{
+int ret = -1;
+virURIPtr uri = NULL;
+const struct URIParseData *data = args;
+char *uristr;
+
+if (!(uri = virURIParse(data-uri))) {
+virReportOOMError();
+goto cleanup;
+}
+
+if (!(uristr = virURIFormat(uri))) {
+virReportOOMError();
+goto cleanup;
+}
+
+if (!STREQ(uristr, data-uri)) {
+VIR_DEBUG(URI did not roundtrip, expect '%s', actual '%s',
+  data-uri, uristr);
+goto cleanup;
+}
+
+if (!STREQ(uri-scheme, data-scheme)) {
+VIR_DEBUG(Expected scheme '%s', actual '%s',
+  data-scheme, uri-scheme);
+goto cleanup;
+}
+
+if (!STREQ(uri-server, data-server)) {
+VIR_DEBUG(Expected server '%s', actual '%s',
+  data-server, uri-server);
+goto cleanup;
+}
+
+if (uri-port != data-port) {
+VIR_DEBUG(Expected port '%d', actual '%d',
+  data-port, uri-port);
+goto cleanup;
+}
+
+if (!STREQ_NULLABLE(uri-path, data-path)) {
+VIR_DEBUG(Expected path '%s', actual '%s',
+  data-path, uri-path);
+goto cleanup;
+}
+
+if (!STREQ_NULLABLE(uri-query, data-query)) {
+VIR_DEBUG(Expected query '%s', actual '%s',
+  data-query, uri-query);
+goto cleanup;
+}
+
+if (!STREQ_NULLABLE(uri-fragment, data-fragment)) {
+VIR_DEBUG(Expected fragment '%s', actual '%s',
+  data-fragment, uri-fragment);
+goto cleanup;
+}
+
+ret = 0;
+cleanup:
+VIR_FREE(uristr);
+xmlFreeURI(uri);
+return ret;
+}
+
+
+static int
+mymain(void)
+{
+int ret = 0;
+
+signal(SIGPIPE, SIG_IGN);
+
+#define TEST_PARSE(uri, scheme, server, port, path, query, fragment)\
+do  {   \
+conststruct URIParseData data = {   \
+uri, scheme, server, port, path, query, fragment\
+}; 

[libvirt] [PATCH 05/14] Use a libvirt custom struct for virURIPtr

2012-03-20 Thread Daniel P. Berrange
From: Daniel P. Berrange berra...@redhat.com

Instead of just typedef'ing the xmlURIPtr struct for virURIPtr,
use a custom libvirt struct. This allows us to fix various
problems with libxml2. This initially just fixes the query vs
query_raw handling problems.
---
 src/esx/esx_util.c |4 --
 src/hyperv/hyperv_util.c   |4 --
 src/libvirt.c  |5 +--
 src/remote/remote_driver.c |   16 +
 src/util/viruri.c  |   79 +++
 src/util/viruri.h  |   14 +++-
 6 files changed, 78 insertions(+), 44 deletions(-)

diff --git a/src/esx/esx_util.c b/src/esx/esx_util.c
index 7d4b908..67b07b7 100644
--- a/src/esx/esx_util.c
+++ b/src/esx/esx_util.c
@@ -62,11 +62,7 @@ esxUtil_ParseUri(esxUtil_ParsedUri **parsedUri, virURIPtr 
uri)
 return -1;
 }
 
-#ifdef HAVE_XMLURI_QUERY_RAW
-queryParamSet = qparam_query_parse(uri-query_raw);
-#else
 queryParamSet = qparam_query_parse(uri-query);
-#endif
 
 if (queryParamSet == NULL) {
 goto cleanup;
diff --git a/src/hyperv/hyperv_util.c b/src/hyperv/hyperv_util.c
index 2e6a2d4..63c761b 100644
--- a/src/hyperv/hyperv_util.c
+++ b/src/hyperv/hyperv_util.c
@@ -54,11 +54,7 @@ hypervParseUri(hypervParsedUri **parsedUri, virURIPtr uri)
 return -1;
 }
 
-#ifdef HAVE_XMLURI_QUERY_RAW
-queryParamSet = qparam_query_parse(uri-query_raw);
-#else
 queryParamSet = qparam_query_parse(uri-query);
-#endif
 
 if (queryParamSet == NULL) {
 goto cleanup;
diff --git a/src/libvirt.c b/src/libvirt.c
index f7590e0..fb7885f 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -1173,15 +1173,12 @@ do_open (const char *name,
 
 VIR_DEBUG(name \%s\ to URI components:\n
 scheme %s\n
-opaque %s\n
-authority %s\n
 server %s\n
 user %s\n
 port %d\n
 path %s\n,
   alias ? alias : name,
-  NULLSTR(ret-uri-scheme), NULLSTR(ret-uri-opaque),
-  NULLSTR(ret-uri-authority), NULLSTR(ret-uri-server),
+  NULLSTR(ret-uri-scheme), NULLSTR(ret-uri-server),
   NULLSTR(ret-uri-user), ret-uri-port,
   NULLSTR(ret-uri-path));
 
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index c6c5809..9de966f 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -417,15 +417,9 @@ doRemoteOpen (virConnectPtr conn,
  */
 struct qparam *var;
 int i;
-char *query;
 
 if (conn-uri) {
-#ifdef HAVE_XMLURI_QUERY_RAW
-query = conn-uri-query_raw;
-#else
-query = conn-uri-query;
-#endif
-vars = qparam_query_parse (query);
+vars = qparam_query_parse (conn-uri-query);
 if (vars == NULL) goto failed;
 
 for (i = 0; i  vars-n; i++) {
@@ -490,11 +484,7 @@ doRemoteOpen (virConnectPtr conn,
 } else {
 virURI tmpuri = {
 .scheme = conn-uri-scheme,
-#ifdef HAVE_XMLURI_QUERY_RAW
-.query_raw = qparam_get_query (vars),
-#else
 .query = qparam_get_query (vars),
-#endif
 .path = conn-uri-path,
 .fragment = conn-uri-fragment,
 };
@@ -507,11 +497,7 @@ doRemoteOpen (virConnectPtr conn,
 
 name = virURIFormat(tmpuri);
 
-#ifdef HAVE_XMLURI_QUERY_RAW
-VIR_FREE(tmpuri.query_raw);
-#else
 VIR_FREE(tmpuri.query);
-#endif
 
 /* Restore transport scheme */
 if (transport_str)
diff --git a/src/util/viruri.c b/src/util/viruri.c
index bbd8742..d8618d1 100644
--- a/src/util/viruri.c
+++ b/src/util/viruri.c
@@ -36,15 +36,45 @@
 virURIPtr
 virURIParse(const char *uri)
 {
-virURIPtr ret = xmlParseURI(uri);
+xmlURIPtr xmluri;
+virURIPtr ret = NULL;
 
-if (!ret) {
+xmluri = xmlParseURI(uri);
+
+if (!uri) {
 /* libxml2 does not tell us what failed. Grr :-( */
 virURIReportError(VIR_ERR_INTERNAL_ERROR,
   Unable to parse URI %s, uri);
 return NULL;
 }
 
+if (VIR_ALLOC(ret)  0)
+goto no_memory;
+
+if (xmluri-scheme 
+!(ret-scheme = strdup(xmluri-scheme)))
+goto no_memory;
+if (xmluri-server 
+!(ret-server = strdup(xmluri-server)))
+goto no_memory;
+ret-port = xmluri-port;
+if (xmluri-path 
+!(ret-path = strdup(xmluri-path)))
+goto no_memory;
+#ifdef HAVE_XMLURI_QUERY_RAW
+if (xmluri-query_raw 
+!(ret-query = strdup(xmluri-query_raw)))
+goto no_memory;
+#else
+if (xmluri-query 
+!(ret-query = strdup(xmluri-query)))
+goto no_memory;
+#endif
+if (xmluri-fragment 
+!(ret-fragment = strdup(xmluri-fragment)))
+goto no_memory;
+
+
 /* First check: does it even make 

[libvirt] [PATCH 08/14] Add a virKeyfilePtr object for parsing '.ini' files

2012-03-20 Thread Daniel P. Berrange
From: Daniel P. Berrange berra...@redhat.com

The '.ini' file format is a useful alternative to the existing
config file style, when you need to have config files which
are hashes of hashes. The 'virKeyFilePtr' object provides a
way to parse these file types.

* src/Makefile.am, src/util/virkeyfile.c,
  src/util/virkeyfile.h: Add .ini file parser
* tests/Makefile.am, tests/virkeyfiletest.c: Test
  basic parsing capabilities
---
 po/POTFILES.in   |1 +
 src/Makefile.am  |1 +
 src/libvirt_private.syms |   10 ++
 src/util/virkeyfile.c|  367 ++
 src/util/virkeyfile.h|   64 
 tests/Makefile.am|8 +-
 tests/virkeyfiletest.c   |  123 
 7 files changed, 573 insertions(+), 1 deletions(-)
 create mode 100644 src/util/virkeyfile.c
 create mode 100644 src/util/virkeyfile.h
 create mode 100644 tests/virkeyfiletest.c

diff --git a/po/POTFILES.in b/po/POTFILES.in
index 16a3f9e..8354c09 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -127,6 +127,7 @@ src/util/util.c
 src/util/viraudit.c
 src/util/virfile.c
 src/util/virhash.c
+src/util/virkeyfile.c
 src/util/virnetdev.c
 src/util/virnetdevbridge.c
 src/util/virnetdevmacvlan.c
diff --git a/src/Makefile.am b/src/Makefile.am
index 39076cc..07d7faa 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -90,6 +90,7 @@ UTIL_SOURCES =
\
util/virhash.c util/virhash.h   \
util/virhashcode.c util/virhashcode.h   \
util/virkeycode.c util/virkeycode.h \
+   util/virkeyfile.c util/virkeyfile.h \
util/virkeymaps.h   \
util/virmacaddr.h util/virmacaddr.c \
util/virnetdev.h util/virnetdev.c   \
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 8a14838..3f69ec1 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1198,6 +1198,16 @@ virKeycodeValueFromString;
 virKeycodeValueTranslate;
 
 
+# virkeyfile.h
+virKeyFileNew;
+virKeyFileLoadFile;
+virKeyFileLoadData;
+virKeyFileFree;
+virKeyFileHasValue;
+virKeyFileHasGroup;
+virKeyFileGetValueString;
+
+
 # virmacaddr.h
 virMacAddrCompare;
 virMacAddrFormat;
diff --git a/src/util/virkeyfile.c b/src/util/virkeyfile.c
new file mode 100644
index 000..3dd4960
--- /dev/null
+++ b/src/util/virkeyfile.c
@@ -0,0 +1,367 @@
+/*
+ * virkeyfile.c: ini-style configuration file handling
+ *
+ * Copyright (C) 2012 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307  USA
+ *
+ * Authors:
+ * Daniel P. Berrange berra...@redhat.com
+ */
+
+#include config.h
+
+#include stdio.h
+
+#include c-ctype.h
+#include logging.h
+#include memory.h
+#include util.h
+#include virhash.h
+#include virkeyfile.h
+#include virterror_internal.h
+
+#define VIR_FROM_THIS VIR_FROM_CONF
+
+typedef struct _virKeyFileGroup virKeyFileGroup;
+typedef virKeyFileGroup *virKeyFileGroupPtr;
+
+typedef struct _virKeyFileParserCtxt virKeyFileParserCtxt;
+typedef virKeyFileParserCtxt *virKeyFileParserCtxtPtr;
+
+struct _virKeyFile {
+virHashTablePtr groups;
+};
+
+struct _virKeyFileParserCtxt {
+virKeyFilePtr conf;
+
+const char *filename;
+
+const char *base;
+const char *cur;
+const char *end;
+size_t line;
+
+char *groupname;
+virHashTablePtr group;
+};
+
+/*
+ * The grammar for the keyfile
+ *
+ * KEYFILE = (GROUP | COMMENT | BLANK )*
+ *
+ * COMMENT = ('#' | ';') [^\n]* '\n'
+ * BLANK = (' ' | '\t' )* '\n'
+ *
+ * GROUP = '[' GROUPNAME ']' '\n' (ENTRY ) *
+ * GROUPNAME = [^[]\n]+
+ *
+ * ENTRY = KEYNAME '=' VALUE
+ * VALUE = [^\n]* '\n'
+ * KEYNAME = [-a-zA-Z0-9]+
+ */
+
+#define IS_EOF (ctxt-cur = ctxt-end)
+#define IS_EOL(c) (((c) == '\n') || ((c) == '\r'))
+#define CUR (*ctxt-cur)
+#define NEXT if (!IS_EOF) ctxt-cur++;
+
+
+#define virKeyFileError(ctxt, error, info) \
+virKeyFileErrorHelper(__FILE__, __FUNCTION__, __LINE__, ctxt, error, info)
+static void
+virKeyFileErrorHelper(const char *file, const char *func, size_t line,
+  virKeyFileParserCtxtPtr ctxt,
+  virErrorNumber error, const char 

[libvirt] [PATCH 11/14] Add helper API for finding auth file path

2012-03-20 Thread Daniel P. Berrange
From: Daniel P. Berrange berra...@redhat.com

* src/util/virauth.c, src/util/virauth.h: Add virAuthGetConfigFilePath
* include/libvirt/virterror.h, src/util/virterror.c: Add
  VIR_FROM_AUTH error domain
---
 include/libvirt/virterror.h |1 +
 src/libvirt_private.syms|1 +
 src/util/virauth.c  |   74 +++
 src/util/virauth.h  |3 ++
 src/util/virterror.c|3 ++
 5 files changed, 82 insertions(+), 0 deletions(-)

diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h
index c8ec8d4..e04d29e 100644
--- a/include/libvirt/virterror.h
+++ b/include/libvirt/virterror.h
@@ -86,6 +86,7 @@ typedef enum {
 VIR_FROM_HYPERV = 43,  /* Error from Hyper-V driver */
 VIR_FROM_CAPABILITIES = 44, /* Error from capabilities */
 VIR_FROM_URI = 45,  /* Error from URI handling */
+VIR_FROM_AUTH = 46, /* Error from auth handling */
 } virErrorDomain;
 
 
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 07302cd..6f53aa8 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1168,6 +1168,7 @@ virUUIDParse;
 # virauth.h
 virAuthGetUsername;
 virAuthGetPassword;
+virAuthGetConfigFilePath;
 
 
 # viraudit.h
diff --git a/src/util/virauth.c b/src/util/virauth.c
index d7375e9..150b8e7 100644
--- a/src/util/virauth.c
+++ b/src/util/virauth.c
@@ -21,9 +21,83 @@
 
 #include config.h
 
+#include stdlib.h
+
 #include virauth.h
 #include util.h
 #include memory.h
+#include logging.h
+#include datatypes.h
+#include virterror_internal.h
+#include configmake.h
+
+#define VIR_FROM_THIS VIR_FROM_AUTH
+
+
+int virAuthGetConfigFilePath(virConnectPtr conn,
+ char **path)
+{
+int ret = -1;
+size_t i;
+const char *authenv = getenv(LIBVIRT_AUTH_FILE);
+char *userdir = NULL;
+
+*path = NULL;
+
+VIR_DEBUG(Determining auth config file path);
+
+if (authenv) {
+VIR_DEBUG(Using path from env '%s', authenv);
+if (!(*path = strdup(authenv)))
+goto no_memory;
+return 0;
+}
+
+for (i = 0 ; i  conn-uri-paramsCount ; i++) {
+if (STREQ_NULLABLE(conn-uri-params[i].name, authfile) 
+conn-uri-params[i].value) {
+VIR_DEBUG(Using path from URI '%s',
+  conn-uri-params[i].value);
+if (!(*path = strdup(conn-uri-params[i].value)))
+goto no_memory;
+return 0;
+}
+}
+
+if (!(userdir = virGetUserDirectory(geteuid(
+goto cleanup;
+
+if (virAsprintf(path, %s/.libvirt/auth.conf, userdir)  0)
+goto no_memory;
+
+VIR_DEBUG(Checking for readability of '%s', *path);
+if (access(*path, R_OK) == 0)
+goto done;
+
+VIR_FREE(*path);
+
+if (!(*path = strdup(SYSCONFDIR /libvirt/auth.conf)))
+goto no_memory;
+
+VIR_DEBUG(Checking for readability of '%s', *path);
+if (access(*path, R_OK) == 0)
+goto done;
+
+VIR_FREE(*path);
+
+done:
+ret = 0;
+
+VIR_DEBUG(Using auth file '%s', NULLSTR(*path));
+cleanup:
+VIR_FREE(userdir);
+
+return ret;
+
+no_memory:
+virReportOOMError();
+goto cleanup;
+}
 
 
 char *
diff --git a/src/util/virauth.h b/src/util/virauth.h
index 8856701..7f43bee 100644
--- a/src/util/virauth.h
+++ b/src/util/virauth.h
@@ -24,6 +24,9 @@
 
 # include internal.h
 
+int virAuthGetConfigFilePath(virConnectPtr conn,
+ char **path);
+
 char *virAuthGetUsername(virConnectAuthPtr auth, const char *defaultUsername,
  const char *hostname);
 char *virAuthGetPassword(virConnectAuthPtr auth, const char *username,
diff --git a/src/util/virterror.c b/src/util/virterror.c
index e1fe522..9dc40a8 100644
--- a/src/util/virterror.c
+++ b/src/util/virterror.c
@@ -181,6 +181,9 @@ static const char *virErrorDomainName(virErrorDomain 
domain) {
 case VIR_FROM_URI:
 dom = URI ;
 break;
+case VIR_FROM_AUTH:
+dom = Auth ;
+break;
 }
 return(dom);
 }
-- 
1.7.7.6

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


[libvirt] [PATCH 13/14] Refactor code prompting for SASL credentials

2012-03-20 Thread Daniel P. Berrange
From: Daniel P. Berrange berra...@redhat.com

SASL may prompt for credentials after either a 'start' or 'step'
invocation. In both cases the code to handle this is the same.
Refactor this code into a separate method to reduce the duplication,
since the complexity is about to grow

* src/remote/remote_driver.c: Refactor interaction with SASL
---
 src/remote/remote_driver.c |  135 +--
 1 files changed, 78 insertions(+), 57 deletions(-)

diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index bc6fea2..1faaf9e 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -2863,7 +2863,8 @@ static sasl_callback_t *remoteAuthMakeCallbacks(int 
*credtype, int ncredtype)
  * are basically a 1-to-1 copy of each other.
  */
 static int remoteAuthMakeCredentials(sasl_interact_t *interact,
- virConnectCredentialPtr *cred)
+ virConnectCredentialPtr *cred,
+ size_t *ncred)
 {
 int ninteract;
 if (!cred)
@@ -2889,16 +2890,8 @@ static int remoteAuthMakeCredentials(sasl_interact_t 
*interact,
 (*cred)[ninteract].result = NULL;
 }
 
-return ninteract;
-}
-
-static void remoteAuthFreeCredentials(virConnectCredentialPtr cred,
-  int ncred)
-{
-int i;
-for (i = 0 ; i  ncred ; i++)
-VIR_FREE(cred[i].result);
-VIR_FREE(cred);
+*ncred = ninteract;
+return 0;
 }
 
 
@@ -2919,6 +2912,69 @@ static void 
remoteAuthFillInteract(virConnectCredentialPtr cred,
 }
 }
 
+
+struct remoteAuthInteractState {
+sasl_interact_t *interact;
+virConnectCredentialPtr cred;
+size_t ncred;
+};
+
+
+static void remoteAuthInteractStateClear(struct remoteAuthInteractState *state)
+{
+size_t i;
+if (!state)
+return;
+
+for (i = 0 ; i  state-ncred ; i++)
+VIR_FREE(state-cred[i].result);
+VIR_FREE(state-cred);
+state-ncred = 0;
+}
+
+
+static int remoteAuthInteract(struct remoteAuthInteractState *state,
+  virConnectAuthPtr auth)
+{
+int ret = -1;
+
+remoteAuthInteractStateClear(state);
+
+if (remoteAuthMakeCredentials(state-interact, state-cred, 
state-ncred)  0) {
+remoteError(VIR_ERR_AUTH_FAILED, %s,
+_(Failed to make auth credentials));
+goto cleanup;
+}
+
+/* Run the authentication callback */
+if (!auth || !auth-cb) {
+remoteError(VIR_ERR_AUTH_FAILED, %s,
+_(No authentication callback available));
+goto cleanup;
+}
+
+if ((*(auth-cb))(state-cred, state-ncred, auth-cbdata)  0) {
+remoteError(VIR_ERR_AUTH_FAILED, %s,
+_(Failed to collect auth credentials));
+goto cleanup;
+}
+
+remoteAuthFillInteract(state-cred, state-interact);
+/*
+ * 'interact' now has pointers to strings in 'state-cred'
+ * so we must not free state-cred until the *next*
+ * sasl_start/step function is complete. Hence we
+ * call remoteAuthInteractStateClear() at the *start*
+ * of this method, rather than the end.
+ */
+
+ret = 0;
+
+cleanup:
+return ret;
+}
+
+
 /* Perform the SASL authentication process
  */
 static int
@@ -2937,13 +2993,13 @@ remoteAuthSASL (virConnectPtr conn, struct private_data 
*priv,
 int err, complete;
 int ssf;
 sasl_callback_t *saslcb = NULL;
-sasl_interact_t *interact = NULL;
-virConnectCredentialPtr cred = NULL;
-int ncred = 0;
 int ret = -1;
 const char *mechlist;
 virNetSASLContextPtr saslCtxt;
 virNetSASLSessionPtr sasl = NULL;
+struct remoteAuthInteractState state;
+
+memset(state, 0, sizeof(state));
 
 VIR_DEBUG(Client initialize SASL authentication);
 
@@ -3010,7 +3066,7 @@ remoteAuthSASL (virConnectPtr conn, struct private_data 
*priv,
 VIR_DEBUG(Client start negotiation mechlist '%s', mechlist);
 if ((err = virNetSASLSessionClientStart(sasl,
 mechlist,
-interact,
+state.interact,
 clientout,
 clientoutlen,
 mech))  0)
@@ -3018,29 +3074,11 @@ remoteAuthSASL (virConnectPtr conn, struct private_data 
*priv,
 
 /* Need to gather some credentials from the client */
 if (err == VIR_NET_SASL_INTERACT) {
-const char *msg;
-if (cred) {
-remoteAuthFreeCredentials(cred, ncred);
-cred = NULL;
-}
-if ((ncred = remoteAuthMakeCredentials(interact, cred))  0) {
-remoteError(VIR_ERR_AUTH_FAILED, %s,
-_(Failed to make auth credentials));
+if (remoteAuthInteract(state, auth)  0) {
 

Re: [libvirt] Modern CPU models cannot be used with libvirt

2012-03-20 Thread Eduardo Habkost

I added a summary of the changes I am planning, at:
http://wiki.qemu.org/Features/CPUModels#Current_Issues_and_proposed_changes

I'm sure I forgot lots of details, so feel free to send me questions and
suggestions, or even edit the wiki page directly.


On Fri, Mar 09, 2012 at 05:56:52PM -0300, Eduardo Habkost wrote:
 Resurrecting an old thread:
 
[...]

-- 
Eduardo

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


[libvirt] [RFC][PATCH] nwfilter: Add support for ipset

2012-03-20 Thread Stefan Berger

Sending this as an RFC for now...

This patch adds support for the recent ipset iptables extension
to libvirt's nwfilter subsystem. Ipset allows to maintain 'sets'
of IP addresses, ports and other packet parameters and allows for
faster lookup and ('chunked') rule evaluation to achieve higher
throughput than what can be achieved with individual iptables rules.

On the command line iptables supports ipset using

iptables ... -m set --match-set ipset name flags -j ...

where 'ipset name' is the name of a previously created ipset and
flags is a comma-separated list of up to 6 flags. Flags use 'src' and 'dst'
for selecting IP addresses, ports etc. from the source or
destination part of a packet. So a concrete example may look like this:

iptables -A INPUT -m set --match-set test src,src -j ACCEPT

Since ipset management is quite complex, the idea was to leave ipset
management outside of libvirt but still allow users to reference an ipset.
The user would have to make sure the ipset is available once the VM is
started so that the iptables rules can be created.

Using XML to describe an ipset in an nwfilter rule would then look as
follows:

rule action='accept' direction='in'
all ipset='test:src,src'/
/rule

The two parameters on the command line become one parameter in the XML
attribute 'ipset' and are separated by a colon (':').


FYI: Here is the man page for ipset:

https://ipset.netfilter.org/ipset.man.html

If you have any comments, please let me know.

Regards,
Stefan

---
 docs/formatnwfilter.html.in   |   34 ++
 docs/schemas/nwfilter.rng |   11 +++
 src/conf/nwfilter_conf.c  |   98 
++

 src/conf/nwfilter_conf.h  |   11 +++
 src/nwfilter/nwfilter_ebiptables_driver.c |   64 ++-
 tests/nwfilterxml2xmlin/ipset-test.xml|   12 +++
 tests/nwfilterxml2xmlout/ipset-test.xml   |   12 +++
 tests/nwfilterxml2xmltest.c   |2
 8 files changed, 240 insertions(+), 4 deletions(-)

Index: libvirt/src/conf/nwfilter_conf.c
===
--- libvirt.orig/src/conf/nwfilter_conf.c
+++ libvirt/src/conf/nwfilter_conf.c
@@ -183,6 +183,7 @@ static const char dstportstart_str[] = 
 static const char dstportend_str[]   = dstportend;
 static const char dscp_str[] = dscp;
 static const char state_str[]= state;
+static const char ipset_str[]= ipset;

 #define SRCMACADDRsrcmacaddr_str
 #define SRCMACMASKsrcmacmask_str
@@ -206,6 +207,7 @@ static const char state_str[]= 
 #define DSTPORTENDdstportend_str
 #define DSCP  dscp_str
 #define STATE state_str
+#define IPSET ipset_str


 /**
@@ -980,6 +982,94 @@ tcpFlagsFormatter(virBufferPtr buf,
 return true;
 }

+static bool
+ipsetValidator(enum attrDatatype datatype ATTRIBUTE_UNUSED, union data 
*val,

+   virNWFilterRuleDefPtr nwf ATTRIBUTE_UNUSED,
+   nwItemDesc *item)
+{
+unsigned int idx;
+const char *errmsg = NULL;
+char *col = strrchr(val-c, ':');
+size_t pos = col - val-c;
+
+if (!col || pos == 0) {
+errmsg = _(malformed ipset description);
+goto arg_err_exit;
+}
+
+if (pos  sizeof(item-u.ipset.setname) - 1) {
+errmsg = _(ipset name exceeds 31 characters);
+goto arg_err_exit;
+}
+
+if (virStrncpy(item-u.ipset.setname, val-c, pos,
+   sizeof(item-u.ipset.setname)) == NULL) {
+errmsg = _(could not copy ipset name);
+goto err_exit;
+}
+
+if (item-u.ipset.setname[strspn(item-u.ipset.setname,
+ VALID_IPSETNAME)] != 0) {
+errmsg = _(ipset name contains invalid characters);
+goto arg_err_exit;
+}
+
+idx = pos + 1;
+item-u.ipset.numFlags = 0;
+item-u.ipset.flags = 0;
+
+errmsg = _(malformed ipset flags);
+
+while (item-u.ipset.numFlags  6) {
+if (STRCASEEQLEN(val-c[idx], src, 3)) {
+item-u.ipset.flags |= (1  item-u.ipset.numFlags);
+} else if (!STRCASEEQLEN(val-c[idx], dst, 3)) {
+goto arg_err_exit;
+}
+item-u.ipset.numFlags++;
+idx += 3;
+if (val-c[idx] != ',')
+break;
+idx++;
+}
+
+if (val-c[idx] != '\0')
+goto arg_err_exit;
+
+return true;
+
+arg_err_exit:
+virNWFilterReportError(VIR_ERR_INVALID_ARG,
+   %s, errmsg);
+return false;
+
+err_exit:
+virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
+   %s, errmsg);
+return false;
+}
+
+static bool
+ipsetFormatter(virBufferPtr buf,
+   virNWFilterRuleDefPtr nwf ATTRIBUTE_UNUSED,
+   nwItemDesc *item)
+{
+uint8_t ctr;
+
+virBufferAdd(buf, item-u.ipset.setname, -1);
+virBufferAddLit(buf, :);
+
+for (ctr = 0; ctr  item-u.ipset.numFlags; ctr++) {
+if (ctr != 0)
+

Re: [libvirt] Unknown KVM internal error on 3.2.1

2012-03-20 Thread Doug Goldstein
On Wed, Mar 7, 2012 at 8:19 AM, Avi Kivity a...@redhat.com wrote:
 On 03/07/2012 01:00 PM, Gleb Natapov wrote:
 
   KVM internal error. Suberror: 1
   emulation failure
   EAX=8004003b EBX=38d54633 ECX=c0460a7e EDX=8005003b
   ESI=e49329a8 EDI=f7c98d60 EBP=0286 ESP=f7fecf68
   EIP=f91d1778 EFL=0282 [--S] CPL=0 II=0 A20=1 SMM=0 HLT=0
   ES =007b   00c0f300 DPL=3 DS   [-WA]
   CS =0060   00c09b00 DPL=0 CS32 [-RA]
   SS =0068   00c09300 DPL=0 DS   [-WA]
   DS =007b   00c0f300 DPL=3 DS   [-WA]
   FS =   
   GS = b7f526c0  
   LDT=0088 c074a020 0027 8200 DPL=0 LDT
   TR =0080 c180a7c4 2073 8b00 DPL=0 TSS32-busy
   GDT=     f7c9f000 00ff
   IDT=     c06fa000 07ff
   CR0=8005003b CR2=0046b044 CR3=3100d000 CR4=06d0
   DR0= DR1= DR2=
   DR3=
   DR6=0ff0 DR7=0400
   EFER=
   Code=?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ??
   ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ??
   ?? ?? ?? ?? ?? ??
 
  What are all these ?? doing here?  Usually they indicate the bad code,
  but here they don't, this is strange.
 
 I think it tries to execute code from mmio.


 Likely.  But let's be sure.

 When it happens again, please keep the guest alive so we can examine it
 via qemu monitor commands.

 --
 error compiling committee.c: too many arguments to function


Shortly after I sent the original e-mail I told libvirt to use the
host CPU. As discussed in another thread since libvirt uses
-nodefconfig, it doesn't really have the correct CPU. I believe the
original issue is from the fact that I had an AMD K10 based processor
with the errata that was previously causing some issues when it wasn't
respected. The default qemu64 processor that libvirt had the guest use
did not take into account this errata and causes this issue.

I have reverted these CPU settings and will get the monitor
information as soon as it occurs again.

Providing all the background just to be sure.


The host is:

processor   : 63
vendor_id   : AuthenticAMD
cpu family  : 21
model   : 1
model name  : AMD Opteron(TM) Processor 6272
stepping: 2
microcode   : 0x6000613
cpu MHz : 2099.875
cache size  : 2048 KB
physical id : 2
siblings: 16
core id : 7
cpu cores   : 8
apicid  : 79
initial apicid  : 79
fpu : yes
fpu_exception   : yes
cpuid level : 13
wp  : yes
flags   : fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov
pat pse36 clflush mmx fxsr sse sse2 ht syscall nx mmxext fxsr_opt
pdpe1gb rdtscp lm constant_tsc rep_good nopl nonstop_tsc extd_apicid
amd_dcm aperfmperf pni pclmulqdq monitor ssse3 cx16 sse4_1 sse4_2
popcnt aes xsave avx lahf_lm cmp_legacy svm extapic cr8_legacy abm
sse4a misalignsse 3dnowprefetch osvw ibs xop skinit wdt lwp fma4
nodeid_msr topoext perfctr_core arat cpb npt lbrv svm_lock nrip_save
tsc_scale vmcb_clean flushbyasid decodeassists pausefilter pfthreshold
bogomips: 4200.08
TLB size: 1536 4K pages
clflush size: 64
cache_alignment : 64
address sizes   : 48 bits physical, 48 bits virtual
power management: ts ttp tm 100mhzsteps hwpstate [9]

The guest is:

processor   : 1
vendor_id   : AuthenticAMD
cpu family  : 6
model   : 15
model name  : Intel(R) Core(TM)2 Duo CPU T7700  @ 2.40GHz
stepping: 11
cpu MHz : 2100.397
cache size  : 512 KB
fdiv_bug: no
hlt_bug : no
f00f_bug: no
coma_bug: no
fpu : yes
fpu_exception   : yes
cpuid level : 10
wp  : yes
flags   : fpu vme de pse tsc msr pae mce cx8 apic mtrr pge mca cmov pat
pse36 clflush mmx fxsr sse sse2 syscall nx mmxext fxsr_opt pdpe1gb lm
pni cx16 popcnt lahf_lm cmp_legacy svm cr8legacy abm sse4a misalignsse
3dnowprefetch
bogomips: 4200.01

The QEMU command line contains the following:

-cpu 
core2duo,+wdt,+skinit,+osvw,+3dnowprefetch,+misalignsse,+sse4a,+abm,+cr8legacy,+extapic,+svm,+cmp_legacy,+lahf_lm,+rdtscp,+pdpe1gb,+fxsr_opt,+mmxext,+aes,+popcnt,+sse4.2,+sse4.1,+cx16,+ht

While libvirt's XML contains:

  cpu match='exact'
modelOpteron_G3/model
vendorAMD/vendor
feature policy='require' name='aes'/
feature policy='require' name='skinit'/
feature policy='require' name='vme'/
feature policy='require' name='mmxext'/
feature policy='require' name='fxsr_opt'/
feature policy='require' name='cr8legacy'/
feature policy='require' name='ht'/
feature policy='require' name='3dnowprefetch'/
feature policy='require' name='ssse3'/
feature policy='require' name='wdt'/
feature policy='require' name='extapic'/
feature policy='require' name='pdpe1gb'/
feature policy='require' name='osvw'/

[libvirt] [PATCH] atomic snapshots, round 3

2012-03-20 Thread Eric Blake
Depends on round 2 being applied first:
https://www.redhat.com/archives/libvir-list/2012-March/msg00761.html

This is the next part of my overall RFC:
https://www.redhat.com/archives/libvir-list/2012-March/msg00578.html

Eric Blake (1):
  snapshot: improve qemu handling of reused snapshot targets

 src/libvirt.c|   11 +++
 src/qemu/qemu_driver.c   |   33 ++---
 src/qemu/qemu_monitor.c  |   14 --
 src/qemu/qemu_monitor.h  |4 +++-
 src/qemu/qemu_monitor_json.c |6 +-
 src/qemu/qemu_monitor_json.h |4 +++-
 6 files changed, 48 insertions(+), 24 deletions(-)

-- 
1.7.7.6

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


[libvirt] [PATCH] snapshot: improve qemu handling of reused snapshot targets

2012-03-20 Thread Eric Blake
The oVirt developers have stated that the real reasons they want
to have qemu reuse existing volumes when creating a snapshot are:
1. the management framework is set up so that creation has to be
done from a central node for proper resource tracking, and having
libvirt and/or qemu create things violates the framework, and
2. qemu defaults to creating snapshots with an absolute path to
the backing file, but oVirt wants to manage a backing chain that
uses just relative names, to allow for easier migration of a chain
across storage locations.

When 0.9.10 added VIR_DOMAIN_SNAPSHOT_CREATE_REUSE_EXT (commit
4e9953a4), it only addressed point 1, but libvirt was still using
O_TRUNC which violates point 2.  Meanwhile, the new qemu
'transaction' monitor command includes a new optional mode argument
that will force qemu to reuse the metadata of the file it just
opened (with the burden on the caller to have valid metadata there
in the first place).  So, this tweaks the meaning of the flag to
cover both points as intended for use by oVirt.  It is not strictly
backward-compatible to 0.9.10 behavior, but it can be argued that
the O_TRUNC of 0.9.10 was a bug.

Note that this flag is all-or-nothing, and only selects between
'existing' and the default 'absolute-paths'.  A more flexible
approach that would allow per-disk selections, as well as adding
support for the 'no-backing-file' mode, would be possible by
extending the domainsnapshot xml to have a per-disk mode, but
until we have a management application expressing a need for that
additional complexity, it is not worth doing.

* src/libvirt.c (virDomainSnapshotCreateXML): Tweak documentation.
* src/qemu/qemu_monitor.h (qemuMonitorDiskSnapshot): Add
parameters.
* src/qemu/qemu_monitor_json.h (qemuMonitorJSONDiskSnapshot):
Likewise.
* src/qemu/qemu_monitor.c (qemuMonitorDiskSnapshot): Pass them
through.
* src/qemu/qemu_monitor_json.c (qemuMonitorJSONDiskSnapshot): Use
new monitor command arguments.
* src/qemu/qemu_driver.c (qemuDomainSnapshotCreateDiskActive)
(qemuDomainSnapshotCreateSingleDiskActive): Adjust callers.
(qemuDomainSnapshotDiskPrepare): Allow qed, modify rules on reuse.
---
 src/libvirt.c|   11 +++
 src/qemu/qemu_driver.c   |   33 ++---
 src/qemu/qemu_monitor.c  |   14 --
 src/qemu/qemu_monitor.h  |4 +++-
 src/qemu/qemu_monitor_json.c |6 +-
 src/qemu/qemu_monitor_json.h |4 +++-
 6 files changed, 48 insertions(+), 24 deletions(-)

diff --git a/src/libvirt.c b/src/libvirt.c
index 534c8ac..7df3667 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -17113,10 +17113,13 @@ virDomainSnapshotGetConnect(virDomainSnapshotPtr 
snapshot)
  * VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY to be passed as well.
  *
  * By default, if the snapshot involves external files, and any of the
- * destination files already exist as a regular file, the snapshot is
- * rejected to avoid losing contents of those files.  However, if
- * @flags includes VIR_DOMAIN_SNAPSHOT_CREATE_REUSE_EXT, then existing
- * destination files are instead truncated and reused.
+ * destination files already exist as a non-empty regular file, the
+ * snapshot is rejected to avoid losing contents of those files.
+ * However, if @flags includes VIR_DOMAIN_SNAPSHOT_CREATE_REUSE_EXT,
+ * then the destination files must already exist and contain content
+ * identical to the source files (this allows a management app to
+ * pre-create files with relative backing file names, rather than the
+ * default of creating with absolute backing file names).
  *
  * Be aware that although libvirt prefers to report errors up front with
  * no other effect, some hypervisors have certain types of failures where
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 2bc8d5d..bc21df0 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -9725,6 +9725,12 @@ qemuDomainSnapshotDiskPrepare(virDomainObjPtr vm, 
virDomainSnapshotDefPtr def,
 int external = 0;
 qemuDomainObjPrivatePtr priv = vm-privateData;

+if (allow_reuse  !qemuCapsGet(priv-qemuCaps, QEMU_CAPS_TRANSACTION)) {
+qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+_(reuse is not supported with this QEMU binary));
+goto cleanup;
+}
+
 for (i = 0; i  def-ndisks; i++) {
 virDomainSnapshotDiskDefPtr disk = def-disks[i];

@@ -9755,8 +9761,8 @@ qemuDomainSnapshotDiskPrepare(virDomainObjPtr vm, 
virDomainSnapshotDefPtr def,
 virReportOOMError();
 goto cleanup;
 }
-} else if (STRNEQ(disk-driverType, qcow2)) {
-/* XXX We should also support QED */
+} else if (STRNEQ(disk-driverType, qcow2) 
+   STRNEQ(disk-driverType, qed)) {
 qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
 _(external snapshot format for disk %s 
  

Re: [libvirt] [PATCH 1/5] qemu: Add support for domain cleanup callbacks

2012-03-20 Thread Eric Blake
On 03/19/2012 10:18 AM, Jiri Denemark wrote:
 Add support for registering cleanup callbacks to be run when a domain
 transitions to shutoff state.
 ---
  src/qemu/qemu_domain.c  |   73 
 +++
  src/qemu/qemu_domain.h  |   15 +
  src/qemu/qemu_process.c |2 +
  3 files changed, 90 insertions(+), 0 deletions(-)
 

 +int
 +qemuDomainCleanupAdd(virDomainObjPtr vm,
 + qemuDomainCleanupCallback cb)
 +{

No opaque 'void *' parameter to pass to the cleanup?  I guess we can add
it later, if we find ourselves needing it, and didn't spot anything else
fishy in this patch, so:

ACK.

-- 
Eric Blake   ebl...@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 2/5] qemu: Avoid dangling migration-in job on shutoff domains

2012-03-20 Thread Eric Blake
On 03/19/2012 10:18 AM, Jiri Denemark wrote:
 Destination daemon should not rely on the client or source daemon
 (depending on the type of migration) to call Finish when migration
 fails, because the client may crash before it can do so. The domain
 prepared for incoming migration is set to be destroyed (and migration
 job cleaned up) when connection with the client closes but this is not
 enough. If the associated qemu process crashes after Prepare step and
 the domain is cleaned up before the connection gets closed, autodestroy
 is not called for the domain and migration jobs remains set. In case the
 domain is defined on destination host (i.e., it is not completely
 removed once destroyed) we keep the job set for ever. To fix this, we
 register a cleanup callback which is responsible to clean migration-in
 job when a domain dies anywhere between Prepare and Finish steps. Note
 that we can't blindly clean any job when spotting EOF on monitor since
 normally an API is running at that time.
 ---
  src/qemu/qemu_domain.c|2 --
  src/qemu/qemu_domain.h|2 ++
  src/qemu/qemu_migration.c |   22 ++
  3 files changed, 24 insertions(+), 2 deletions(-)

I'm restating my understanding of the bug, to make sure I am sure why
your patch helps:

- src requests a migration
- dest starts a qemu process using information from the src, but the
destination happens to be running an older qemu that can't support the
full migration
- qemu dies, but the destination hasn't seen a 'Finish' from the source,
so the job remains open and the domain remains
- connection is broken, but the open job prevents reclaiming the
autodestroy domain on the destination
- new connection is made, but source can't migrate because destination
is already locked up on the stale attempt

and the fix is adding a new callback, which says if qemu dies while the
callback is registered, we cancel the migration job; therefore, even
without a 'Finish' from the source, the autodestroy can now kick in

ACK.

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



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

Re: [libvirt] [PATCH 3/5] qemu: Add connection close callbacks

2012-03-20 Thread Eric Blake
On 03/19/2012 10:18 AM, Jiri Denemark wrote:
 Add support for registering arbitrary callback to be called for a domain
 when a connection gets closed.
 ---
  src/qemu/qemu_conf.c   |  172 
 
  src/qemu/qemu_conf.h   |   27 
  src/qemu/qemu_driver.c |5 ++
  3 files changed, 204 insertions(+), 0 deletions(-)
 

  
 +/* Mapping of 'char *uuidstr' - qemuDriverCloseDefPtr of domains
 + * which want a specific cleanup to be done when a connection is
 + * closed. Such cleanup may be to automatically destroy  the

Are the two spaces intentional?

Other than that, ACK.

-- 
Eric Blake   ebl...@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 4/5] qemu: Make autodestroy utilize connection close callbacks

2012-03-20 Thread Eric Blake
On 03/19/2012 10:18 AM, Jiri Denemark wrote:
 ---
  src/qemu/qemu_conf.h|5 --
  src/qemu/qemu_driver.c  |5 --
  src/qemu/qemu_process.c |  107 ++
  3 files changed, 24 insertions(+), 93 deletions(-)

Good!  I was thinking about this very cleanup myself as I reviewed 3/5,
and I agree with your decision to separate it into different patches.

ACK.

-- 
Eric Blake   ebl...@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 5/5] qemu: Avoid dangling migration-out job when client dies

2012-03-20 Thread Eric Blake
On 03/19/2012 10:19 AM, Jiri Denemark wrote:
 When a client which started non-p2p migration dies in a bad time, the
 source libvirtd never clears the migration job and almost nothing can be
 done with the domain without restarting the daemon. This patch makes use
 of connection close callbacks and ensures that migration job is properly
 discarded when the client disconnects.
 ---
  src/qemu/qemu_driver.c|4 +++
  src/qemu/qemu_migration.c |   66 
 +
  src/qemu/qemu_migration.h |4 +++
  3 files changed, 74 insertions(+), 0 deletions(-)

Can't have been a fun bug to track down, but I'm glad you found it.

  
 +/* This is called for outgoing non-p2p migrations when a connection to the
 + * client which initiated the migration was closed and we are waiting for it

s/and we are waiting/but we were waiting/


 +
 +VIR_DEBUG(The connection which started outgoing migration of domain %s
 +   was closed; cancelling the migration,

I think that in user-visible messages, we have been favoring US spelling
(canceling) over UK spelling (cancelling); but VIR_DEBUG is borderline
on whether it is user-visible.

ACK.

-- 
Eric Blake   ebl...@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] python: always include config.h first

2012-03-20 Thread Eric Blake
On RHEL 5.7, I got this compilation failure:

In file included from /usr/include/python2.4/pyport.h:98,
 from /usr/include/python2.4/Python.h:55,
 from libvirt.c:3:
../gnulib/lib/time.h:468: error: expected ';', ',' or ')' before '__timer'

Turns out that our '#define restrict __restrict' from config.h wasn't
being picked up.  Gnulib _requires_ that all .c files include config.h
first, otherwise the gnulib header overrides tend to misbehave.

* python/generator.py (buildStubs): Include config.h first.
---

Pushing under the build-breaker rule.

 python/generator.py |3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/python/generator.py b/python/generator.py
index 98072f0..e641c31 100755
--- a/python/generator.py
+++ b/python/generator.py
@@ -757,7 +757,8 @@ def buildStubs(module):
 export.write(/* Generated */\n\n)

 wrapper = open(wrapper_file, w)
-wrapper.write(/* Generated */\n\n)
+wrapper.write(/* Generated by generator.py */\n\n)
+wrapper.write(#include config.h\n)
 wrapper.write(#include Python.h\n)
 wrapper.write(#include libvirt/ + module + .h\n)
 wrapper.write(#include \typewrappers.h\\n)
-- 
1.7.7.6

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


[libvirt] [PATCH] build: drop a painfully long gnulib test

2012-03-20 Thread Eric Blake
On machines with massive amounts of CPUs, the gnulib 'test-lock'
could take minutes, or even appear to deadlock, because of timing
interactions between multiple cores.

See https://bugzilla.redhat.com/show_bug.cgi?id=797284.
For precedence, note that iwhd has done the same:
https://lists.gnu.org/archive/html/bug-gnulib/2012-01/msg00311.html

We can re-enable things if gnulib ever analyzes and improves the
situation.

* bootstrap.conf (gnulib_tool_option_extras): Avoid lock-tests.
---

Personally, I haven't run into the problem on my 2-core laptop;
where do I sign up for one of the 2048-core machines in that BZ? :)
But since I haven't seen it, I can't push this under the trivial
rule, so I'll wait for a review.

 bootstrap.conf |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/bootstrap.conf b/bootstrap.conf
index 03da267..c6f7fd9 100644
--- a/bootstrap.conf
+++ b/bootstrap.conf
@@ -174,6 +174,7 @@ gnulib_tool_option_extras=\
  --with-tests\
  --makefile-name=gnulib.mk\
  --avoid=pt_chown\
+ --avoid=lock-tests\
 
 local_gl_dir=gnulib/local

-- 
1.7.7.6

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


Re: [libvirt] [libvirt-glib] Remove unneeded call in libvirt-gconfig test

2012-03-20 Thread Zeeshan Ali (Khattak)
On Fri, Mar 16, 2012 at 6:43 PM, Christophe Fergeau cferg...@redhat.com wrote:
 Because of a copy and paste error, the test program is making an
 unneeded call to gvir_config_domain_channel_set_target_type
 when setting up its USB redirection device.

ACK :)

-- 
Regards,

Zeeshan Ali (Khattak)
FSF member#5124

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


Re: [libvirt] [PATCH libvirt-glib] Fix generation of docs in a VPATH builder

2012-03-20 Thread Zeeshan Ali (Khattak)
On Wed, Mar 7, 2012 at 1:14 PM, Daniel P. Berrange berra...@redhat.com wrote:
 From: Daniel P. Berrange berra...@redhat.com

 The DOC_SOURCE_DIR variable was missing the $(top_srcdir) variable
 so it could not find the source files when run from a VPATH build.
 Empirically the previous comment saying that $(top_srcdir) was not
 needed is wrong.

Don't know how exactly this stuff works but seems harmless so if you
have tested it, ACK!

-- 
Regards,

Zeeshan Ali (Khattak)
FSF member#5124

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