Re: [libvirt] [PATCH 0/8 v5] Summary on block IO throttle
Am 18.01.2012 17:22, schrieb Eric Blake: On 11/23/2011 02:18 AM, Stefan Hajnoczi wrote: Notes: Now all the planed features were implemented (#1#2 were implemented by Zhi Yong Wu), the previous comments were all fixed up too. And the qemu part patches have been accepted upstream and are expected to be part of the QEMU 1.1 release, git tree from Zhi Yong: http://repo.or.cz/w/qemu/kevin.git/shortlog/refs/heads/block I just realized that the block_set_io_throttle monitor command is not yet in released upstream qemu. I will continue reviewing this series, but it's harder to justify including it into libvirt 0.9.8 unless we have a good feel that the design for qemu is stable and unlikely to I strongly think that the syntax of this qemu monitor command will be not changed later. :) Moreover, it has not been changed since the patches for qemu block I/O throttling is published. It's good to be careful about adding libvirt features that are not yet in QEMU. I got burnt by this with image streaming and have had to work extra hard to finally get the QEMU side merged with the libvirt side already set in stone. However, Kevin has merged the QEMU I/O throttling patches into the block tree. They are waiting for the 1.1 merge window because qemu.git has only been taking QEMU 1.0-rc patches for the past few weeks. QEMU I/O throttling has been through review and accepted by the community: http://patchwork.ozlabs.org/patch/124266/ How can libvirt reliably tell whether block throttling is supported? Does that patch series alter the help output of 'qemu -h' to list something like '-drive ...[,bps=...]'? Is there a magic 'qemu -driver xxx,?' call where we can see that the new bps attribute is settable? The help output has changed, you can use that. There is no such thing as -drive ? Kevin -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Add two NUMA tuning python bindings APIs
- Original Message -
From: Alex Jia a...@redhat.com
To: Guan Nan Ren g...@redhat.com
Cc: libvir-list@redhat.com
Sent: Thursday, January 19, 2012 1:39:58 PM
Subject: Re: [libvirt] [PATCH] Add two NUMA tuning python bindings APIs
On 01/19/2012 11:29 AM, Alex Jia wrote:
On 01/19/2012 09:38 AM, Guan Nan Ren wrote:
Hi
Anybody could give a hand on reviewing this patch,
I appreciate it.
Chinese New Year is coming, best wishes to this community :)
Guannan Ren
Happy New Year!
- Original Message -
From: Guannan Reng...@redhat.com
To: libvir-list@redhat.com
Sent: Monday, January 16, 2012 6:58:06 PM
Subject: [libvirt] [PATCH] Add two NUMA tuning python bindings APIs
*virDomainSetNumaParameters
*virDomainGetNumaParameters
---
python/libvirt-override-api.xml | 13 +++
python/libvirt-override.c | 186
+++
2 files changed, 199 insertions(+), 0 deletions(-)
diff --git a/python/libvirt-override-api.xml
b/python/libvirt-override-api.xml
index 704fee9..e09290c 100644
--- a/python/libvirt-override-api.xml
+++ b/python/libvirt-override-api.xml
@@ -248,6 +248,19 @@
arg name='domain' type='virDomainPtr' info='pointer to domain object'/
arg name='flags' type='int' info='an ORapos;ed set of
virDomainModificationImpact'/
/function
+function name='virDomainSetNumaParameters' file='python'
+infoChange the NUMA tunables/info
+return type='int' info='-1 in case of error, 0 in case of success.'/
+arg name='domain' type='virDomainPtr' info='pointer to domain
object'/
+arg name='params' type='virTypedParameterPtr' info='pointer to
memory tunable objects'/
A copy-paste error, s/memory/numa/.
+arg name='flags' type='int' info='an ORapos;ed set of
virDomainModificationImpact'/
+/function
+function name='virDomainGetNumaParameters' file='python'
+infoGet the NUMA parameters/info
+return type='virTypedParameterPtr' info='the I/O tunables value or
None in case of error'/
Save as above, s/I/O/numa/.
+arg name='domain' type='virDomainPtr' info='pointer to domain
object'/
+arg name='flags' type='int' info='an ORapos;ed set of
virDomainModificationImpact'/
+/function
function name='virConnectListStoragePools' file='python'
infolist the storage pools, stores the pointers to the names in
@names/info
arg name='conn' type='virConnectPtr' info='pointer to the hypervisor
connection'/
diff --git a/python/libvirt-override.c b/python/libvirt-override.c
index d2aad0f..27ad1d8 100644
--- a/python/libvirt-override.c
+++ b/python/libvirt-override.c
@@ -1000,6 +1000,190 @@ libvirt_virDomainGetMemoryParameters(PyObject
*self ATTRIBUTE_UNUSED,
}
static PyObject *
+libvirt_virDomainSetNumaParameters(PyObject *self ATTRIBUTE_UNUSED,
+ PyObject *args) {
+virDomainPtr domain;
+PyObject *pyobj_domain, *info;
+int i_retval;
+int nparams = 0, i;
+unsigned int flags;
+virTypedParameterPtr params;
+
+if (!PyArg_ParseTuple(args,
+ (char *)OOi:virDomainSetNumaParameters,
+pyobj_domain,info,flags))
+return(NULL);
+domain = (virDomainPtr) PyvirDomain_Get(pyobj_domain);
+
+LIBVIRT_BEGIN_ALLOW_THREADS;
+i_retval = virDomainGetNumaParameters(domain, NULL,nparams,
flags);
+LIBVIRT_END_ALLOW_THREADS;
+
+if (i_retval 0)
+return VIR_PY_INT_FAIL;
+
+if ((params = malloc(sizeof(*params)*nparams)) == NULL)
+return VIR_PY_INT_FAIL;
+
+LIBVIRT_BEGIN_ALLOW_THREADS;
+i_retval = virDomainGetNumaParameters(domain, params,nparams,
flags);
+LIBVIRT_END_ALLOW_THREADS;
+
+if (i_retval 0) {
+free(params);
+return VIR_PY_INT_FAIL;
+}
+
+/* convert to a Python tuple of long objects */
+for (i = 0; i nparams; i++) {
+PyObject *key, *val;
+key = libvirt_constcharPtrWrap(params[i].field);
+val = PyDict_GetItem(info, key);
+Py_DECREF(key);
+
+if (val == NULL)
+continue;
+
+switch (params[i].type) {
+case VIR_TYPED_PARAM_INT:
+params[i].value.i = (int)PyInt_AS_LONG(val);
+break;
+
+case VIR_TYPED_PARAM_UINT:
+params[i].value.ui = (unsigned int)PyInt_AS_LONG(val);
+break;
+
+case VIR_TYPED_PARAM_LLONG:
+params[i].value.l = (long long)PyLong_AsLongLong(val);
+break;
+
+case VIR_TYPED_PARAM_ULLONG:
+params[i].value.ul = (unsigned long
long)PyLong_AsLongLong(val);
+break;
+
+case VIR_TYPED_PARAM_DOUBLE:
+params[i].value.d = (double)PyFloat_AsDouble(val);
+break;
+
+case VIR_TYPED_PARAM_BOOLEAN:
+{
+/* Hack - Python's definition of Py_True breaks strict
+ * aliasing rules, so can't directly compare
[libvirt] [PATCH v2] Add two NUMA tuning python bindings APIs
*virDomainSetNumaParameters
*virDomainGetNumaParameters
---
python/libvirt-override-api.xml | 13 +++
python/libvirt-override.c | 186 +++
2 files changed, 199 insertions(+), 0 deletions(-)
diff --git a/python/libvirt-override-api.xml b/python/libvirt-override-api.xml
index 704fee9..e09290c 100644
--- a/python/libvirt-override-api.xml
+++ b/python/libvirt-override-api.xml
@@ -248,6 +248,19 @@
arg name='domain' type='virDomainPtr' info='pointer to domain object'/
arg name='flags' type='int' info='an ORapos;ed set of
virDomainModificationImpact'/
/function
+function name='virDomainSetNumaParameters' file='python'
+ infoChange the NUMA tunables/info
+ return type='int' info='-1 in case of error, 0 in case of success.'/
+ arg name='domain' type='virDomainPtr' info='pointer to domain object'/
+ arg name='params' type='virTypedParameterPtr' info='pointer to numa
tunable objects'/
+ arg name='flags' type='int' info='an ORapos;ed set of
virDomainModificationImpact'/
+/function
+function name='virDomainGetNumaParameters' file='python'
+ infoGet the NUMA parameters/info
+ return type='virTypedParameterPtr' info='the numa tunables value or
None in case of error'/
+ arg name='domain' type='virDomainPtr' info='pointer to domain object'/
+ arg name='flags' type='int' info='an ORapos;ed set of
virDomainModificationImpact'/
+/function
function name='virConnectListStoragePools' file='python'
infolist the storage pools, stores the pointers to the names in
@names/info
arg name='conn' type='virConnectPtr' info='pointer to the hypervisor
connection'/
diff --git a/python/libvirt-override.c b/python/libvirt-override.c
index d2aad0f..27ad1d8 100644
--- a/python/libvirt-override.c
+++ b/python/libvirt-override.c
@@ -1000,6 +1000,190 @@ libvirt_virDomainGetMemoryParameters(PyObject *self
ATTRIBUTE_UNUSED,
}
static PyObject *
+libvirt_virDomainSetNumaParameters(PyObject *self ATTRIBUTE_UNUSED,
+ PyObject *args) {
+virDomainPtr domain;
+PyObject *pyobj_domain, *info;
+int i_retval;
+int nparams = 0, i;
+unsigned int flags;
+virTypedParameterPtr params;
+
+if (!PyArg_ParseTuple(args,
+ (char *)OOi:virDomainSetNumaParameters,
+ pyobj_domain, info, flags))
+return(NULL);
+domain = (virDomainPtr) PyvirDomain_Get(pyobj_domain);
+
+LIBVIRT_BEGIN_ALLOW_THREADS;
+i_retval = virDomainGetNumaParameters(domain, NULL, nparams, flags);
+LIBVIRT_END_ALLOW_THREADS;
+
+if (i_retval 0)
+return VIR_PY_INT_FAIL;
+
+if ((params = malloc(sizeof(*params)*nparams)) == NULL)
+return VIR_PY_INT_FAIL;
+
+LIBVIRT_BEGIN_ALLOW_THREADS;
+i_retval = virDomainGetNumaParameters(domain, params, nparams, flags);
+LIBVIRT_END_ALLOW_THREADS;
+
+if (i_retval 0) {
+free(params);
+return VIR_PY_INT_FAIL;
+}
+
+/* convert to a Python tuple of long objects */
+for (i = 0; i nparams; i++) {
+PyObject *key, *val;
+key = libvirt_constcharPtrWrap(params[i].field);
+val = PyDict_GetItem(info, key);
+Py_DECREF(key);
+
+if (val == NULL)
+continue;
+
+switch (params[i].type) {
+case VIR_TYPED_PARAM_INT:
+params[i].value.i = (int)PyInt_AS_LONG(val);
+break;
+
+case VIR_TYPED_PARAM_UINT:
+params[i].value.ui = (unsigned int)PyInt_AS_LONG(val);
+break;
+
+case VIR_TYPED_PARAM_LLONG:
+params[i].value.l = (long long)PyLong_AsLongLong(val);
+break;
+
+case VIR_TYPED_PARAM_ULLONG:
+params[i].value.ul = (unsigned long long)PyLong_AsLongLong(val);
+break;
+
+case VIR_TYPED_PARAM_DOUBLE:
+params[i].value.d = (double)PyFloat_AsDouble(val);
+break;
+
+case VIR_TYPED_PARAM_BOOLEAN:
+{
+/* Hack - Python's definition of Py_True breaks strict
+ * aliasing rules, so can't directly compare
+ */
+PyObject *hacktrue = PyBool_FromLong(1);
+params[i].value.b = hacktrue == val ? 1: 0;
+Py_DECREF(hacktrue);
+}
+break;
+
+case VIR_TYPED_PARAM_STRING:
+params[i].value.s = PyString_AsString(val);
+break;
+
+default:
+free(params);
+return VIR_PY_INT_FAIL;
+}
+}
+
+LIBVIRT_BEGIN_ALLOW_THREADS;
+i_retval = virDomainSetNumaParameters(domain, params, nparams, flags);
+LIBVIRT_END_ALLOW_THREADS;
+if (i_retval 0) {
+free(params);
+return VIR_PY_INT_FAIL;
+}
+
+free(params);
+return VIR_PY_INT_SUCCESS;
+}
+
+static PyObject *
Re: [libvirt] [PATCH] Add two NUMA tuning python bindings APIs
On 01/19/2012 04:54 PM, Guan Nan Ren wrote:
- Original Message -
From: Alex Jiaa...@redhat.com
To: Guan Nan Reng...@redhat.com
Cc: libvir-list@redhat.com
Sent: Thursday, January 19, 2012 1:39:58 PM
Subject: Re: [libvirt] [PATCH] Add two NUMA tuning python bindings APIs
On 01/19/2012 11:29 AM, Alex Jia wrote:
On 01/19/2012 09:38 AM, Guan Nan Ren wrote:
Hi
Anybody could give a hand on reviewing this patch,
I appreciate it.
Chinese New Year is coming, best wishes to this community :)
Guannan Ren
Happy New Year!
- Original Message -
From: Guannan Reng...@redhat.com
To: libvir-list@redhat.com
Sent: Monday, January 16, 2012 6:58:06 PM
Subject: [libvirt] [PATCH] Add two NUMA tuning python bindings APIs
*virDomainSetNumaParameters
*virDomainGetNumaParameters
---
python/libvirt-override-api.xml | 13 +++
python/libvirt-override.c | 186
+++
2 files changed, 199 insertions(+), 0 deletions(-)
diff --git a/python/libvirt-override-api.xml
b/python/libvirt-override-api.xml
index 704fee9..e09290c 100644
--- a/python/libvirt-override-api.xml
+++ b/python/libvirt-override-api.xml
@@ -248,6 +248,19 @@
arg name='domain' type='virDomainPtr' info='pointer to domain object'/
arg name='flags' type='int' info='an ORapos;ed set of
virDomainModificationImpact'/
/function
+function name='virDomainSetNumaParameters' file='python'
+infoChange the NUMA tunables/info
+return type='int' info='-1 in case of error, 0 in case of success.'/
+arg name='domain' type='virDomainPtr' info='pointer to domain
object'/
+arg name='params' type='virTypedParameterPtr' info='pointer to
memory tunable objects'/
A copy-paste error, s/memory/numa/.
+arg name='flags' type='int' info='an ORapos;ed set of
virDomainModificationImpact'/
+/function
+function name='virDomainGetNumaParameters' file='python'
+infoGet the NUMA parameters/info
+return type='virTypedParameterPtr' info='the I/O tunables value or
None in case of error'/
Save as above, s/I/O/numa/.
+arg name='domain' type='virDomainPtr' info='pointer to domain
object'/
+arg name='flags' type='int' info='an ORapos;ed set of
virDomainModificationImpact'/
+/function
function name='virConnectListStoragePools' file='python'
infolist the storage pools, stores the pointers to the names in
@names/info
arg name='conn' type='virConnectPtr' info='pointer to the hypervisor
connection'/
diff --git a/python/libvirt-override.c b/python/libvirt-override.c
index d2aad0f..27ad1d8 100644
--- a/python/libvirt-override.c
+++ b/python/libvirt-override.c
@@ -1000,6 +1000,190 @@ libvirt_virDomainGetMemoryParameters(PyObject
*self ATTRIBUTE_UNUSED,
}
static PyObject *
+libvirt_virDomainSetNumaParameters(PyObject *self ATTRIBUTE_UNUSED,
+ PyObject *args) {
+virDomainPtr domain;
+PyObject *pyobj_domain, *info;
+int i_retval;
+int nparams = 0, i;
+unsigned int flags;
+virTypedParameterPtr params;
+
+if (!PyArg_ParseTuple(args,
+ (char *)OOi:virDomainSetNumaParameters,
+pyobj_domain,info,flags))
+return(NULL);
+domain = (virDomainPtr) PyvirDomain_Get(pyobj_domain);
+
+LIBVIRT_BEGIN_ALLOW_THREADS;
+i_retval = virDomainGetNumaParameters(domain, NULL,nparams,
flags);
+LIBVIRT_END_ALLOW_THREADS;
+
+if (i_retval 0)
+return VIR_PY_INT_FAIL;
+
+if ((params = malloc(sizeof(*params)*nparams)) == NULL)
+return VIR_PY_INT_FAIL;
+
+LIBVIRT_BEGIN_ALLOW_THREADS;
+i_retval = virDomainGetNumaParameters(domain, params,nparams,
flags);
+LIBVIRT_END_ALLOW_THREADS;
+
+if (i_retval 0) {
+free(params);
+return VIR_PY_INT_FAIL;
+}
+
+/* convert to a Python tuple of long objects */
+for (i = 0; i nparams; i++) {
+PyObject *key, *val;
+key = libvirt_constcharPtrWrap(params[i].field);
+val = PyDict_GetItem(info, key);
+Py_DECREF(key);
+
+if (val == NULL)
+continue;
+
+switch (params[i].type) {
+case VIR_TYPED_PARAM_INT:
+params[i].value.i = (int)PyInt_AS_LONG(val);
+break;
+
+case VIR_TYPED_PARAM_UINT:
+params[i].value.ui = (unsigned int)PyInt_AS_LONG(val);
+break;
+
+case VIR_TYPED_PARAM_LLONG:
+params[i].value.l = (long long)PyLong_AsLongLong(val);
+break;
+
+case VIR_TYPED_PARAM_ULLONG:
+params[i].value.ul = (unsigned long
long)PyLong_AsLongLong(val);
+break;
+
+case VIR_TYPED_PARAM_DOUBLE:
+params[i].value.d = (double)PyFloat_AsDouble(val);
+break;
+
+case VIR_TYPED_PARAM_BOOLEAN:
+{
+/* Hack - Python's definition of Py_True breaks strict
+ * aliasing rules, so can't directly compare
+ */
+PyObject *hacktrue = PyBool_FromLong(1);
+
Re: [libvirt] [PATCH 1/5 V3] libvirt: Add new public API virDomainGetPcpusUsage
On Wed, Jan 18, 2012 at 05:07:16PM -0700, Eric Blake wrote: If others like my thoughts, can you respin your patch series to adjust your API accordingly? Sounds good to me. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones libguestfs lets you edit virtual machines. Supports shell scripting, bindings from many languages. http://libguestfs.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] ALERT Virtualization Automatic Builder
On 18.01.2012 19:56, berra...@redhat.com wrote: Overall status: failed Start date: Wed Jan 18 2012 Start time: 18:32:52 UTC / 18:32:52 GMT Build counter: 1326911571 Build timestamp: 1326911571 URL: http://dhcp-0-241.camlab.fab.redhat.com/index.html Module: libvirt Status: failed URL: http://dhcp-0-241.camlab.fab.redhat.com/module-libvirt.html This is because of WARNING: no socket to connect to string being emitted by GnuTLS. I guess that gnutls was upgraded, wasn't it? Because on the older one (2.12.7) everything works. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] ALERT Virtualization Automatic Builder
On Thu, Jan 19, 2012 at 11:07:54AM +0100, Michal Privoznik wrote: On 18.01.2012 19:56, berra...@redhat.com wrote: Overall status: failed Start date: Wed Jan 18 2012 Start time: 18:32:52 UTC / 18:32:52 GMT Build counter: 1326911571 Build timestamp: 1326911571 URL: http://dhcp-0-241.camlab.fab.redhat.com/index.html Module: libvirt Status: failed URL: http://dhcp-0-241.camlab.fab.redhat.com/module-libvirt.html This is because of WARNING: no socket to connect to string being emitted by GnuTLS. I guess that gnutls was upgraded, wasn't it? Because on the older one (2.12.7) everything works. Yeha, there is something odd going on in either gnutls or gnome-keyring that I'm investigating. It is absolutely wrong of it to print this kind of junk to stderr Also, sorry about the non-accessible URLs in this mail - it wasn't supposed to be sent to the public list, since this is just a test builder I'm configuring Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/8 v5] Summary on block IO throttle
On Wed, Jan 18, 2012 at 09:22:01AM -0700, Eric Blake wrote: On 11/23/2011 02:18 AM, Stefan Hajnoczi wrote: Notes: Now all the planed features were implemented (#1#2 were implemented by Zhi Yong Wu), the previous comments were all fixed up too. And the qemu part patches have been accepted upstream and are expected to be part of the QEMU 1.1 release, git tree from Zhi Yong: http://repo.or.cz/w/qemu/kevin.git/shortlog/refs/heads/block I just realized that the block_set_io_throttle monitor command is not yet in released upstream qemu. I will continue reviewing this series, but it's harder to justify including it into libvirt 0.9.8 unless we have a good feel that the design for qemu is stable and unlikely to I strongly think that the syntax of this qemu monitor command will be not changed later. :) Moreover, it has not been changed since the patches for qemu block I/O throttling is published. It's good to be careful about adding libvirt features that are not yet in QEMU. I got burnt by this with image streaming and have had to work extra hard to finally get the QEMU side merged with the libvirt side already set in stone. However, Kevin has merged the QEMU I/O throttling patches into the block tree. They are waiting for the 1.1 merge window because qemu.git has only been taking QEMU 1.0-rc patches for the past few weeks. QEMU I/O throttling has been through review and accepted by the community: http://patchwork.ozlabs.org/patch/124266/ How can libvirt reliably tell whether block throttling is supported? Does that patch series alter the help output of 'qemu -h' to list something like '-drive ...[,bps=...]'? Is there a magic 'qemu -driver xxx,?' call where we can see that the new bps attribute is settable? I ask, because we need to alter libvirt to properly detect whether qemu supports this new feature, so as to gracefully reject attempts to use the XML if qemu does not support the feature. See also: https://www.redhat.com/archives/libvir-list/2012-January/msg00727.html The most robust method is to use QMP query-commands or HMP help to look for the block I/O throttling commands. Scraping qemu -h is possible too but fragile, note the last line of help output: -drive [file=file][,if=type][,bus=n][,unit=m][,media=d][,index=i] [,cyls=c,heads=h,secs=s[,trans=t]][,snapshot=on|off] [,cache=writethrough|writeback|none|directsync|unsafe][,format=f] [,serial=s][,addr=A][,id=name][,aio=threads|native] [,readonly=on|off][,copy-on-read=on|off] [[,bps=b]|[[,bps_rd=r][,bps_wr=w]]][[,iops=i]|[[,iops_rd=r][,iops_wr=w]] Here's a hack that really probes. Block I/O throttling supported: $ qemu-system-x86_64 -drive bps=1,bps_rd=1 qemu-system-x86_64: -drive bps=1,bps_rd=1: bps(iops) and bps_rd/bps_wr(iops_rd/iops_wr) cannot be used at the same time Block I/O throttling *not* supported: $ qemu-system-x86_64 -drive bps=1,bps_rd=1 qemu-system-x86_64: -drive bps=1,bps_rd=1: Invalid parameter 'bps' Stefan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] ALERT Virtualization Automatic Builder
Overall status: failed Start date: Thu Jan 19 2012 Start time: 12:26:32 UTC / 12:26:32 GMT Build counter: 1326975992 Build timestamp: 1326975992 URL: http://t500wlan.home.berrange.com/index.html Module: libvirt Status: failed URL: http://t500wlan.home.berrange.com/module-libvirt.html -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH RFCv2 4/5] libssh2_transport: Use libssh2 driver code in remote driver
On Wed, Jan 18, 2012 at 05:28:47PM +0100, Michal Privoznik wrote:
On 04.01.2012 00:47, Peter Krempa wrote:
-if (verr verr-code == VIR_ERR_NO_SUPPORT) {
-/* Missing RPC - old server - ignore */
-virResetLastError();
-return 0;
+if ((verr = virGetLastError())) {
+if (verr-code == VIR_ERR_NO_SUPPORT) {
+/* Missing RPC - old server - ignore */
+virResetLastError();
+return 0;
+}
+
+if (verr-code == VIR_ERR_LIBSSH_REMOTE_COMMAND) {
+virResetLastError();
+remoteError(VIR_ERR_LIBSSH_REMOTE_COMMAND, %s,
+_(Remote daemon is not running or remote
command has failed));
+}
}
return -1;
}
Related to 1st patch in the set:
Some users might be using ssh-agent, however, want to select different
auth mechanisms. I'd suggest to allow users to select which auth
mechanism they want to use. If we don't parse ssh configs, we should let
user to choose if he wants keyboard-interactive or ssh-agent or ...;
Otherwise we end up trying to sign in with keys provided by ssh-agent
which doesn't really must have the right ones.
For example, /me uses ssh-agent for git+ssh://libvirt.org but use public
keys for other machines and even keyboard-interactive :)
For the libssh2 driver, I think *not* using .ssh/config is actually
a good feature. The main benefit of libssh2, over forking ssh, is
that libvirt can provide applications direct control over all settings
and interactions. So any bits that we think need to be configurable
should all be done as query parameters in the URI
At least i see us wanting
- use agent - yes|no
- auth list (ie keyboard-interactive | gssapi-with-mic | public-key ... etc)
- public key paths
Daniel
--
|: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] errors: Improve error reporting to log multiple errors instead of just the last one
This patch introduces a new structure called virErrorsPtr which can get all
the errors that occurred since the connection open. The error callback function
is being used as many times as necessary. The new public function called
virGetAllErrors() has been introduced to get all the errors that occurred.
Also, a new test called errorreport has been written to test the functionality
of error logging for multiple error occurrences.
Signed-off-by: Michal Novotny minov...@redhat.com
---
include/libvirt/virterror.h | 14 ++
python/generator.py |1 +
src/util/virterror.c | 99
src/util/virterror_internal.h |1 +
tests/Makefile.am |9 +++-
tests/errorreport.c | 74 ++
6 files changed, 186 insertions(+), 12 deletions(-)
create mode 100644 tests/errorreport.c
diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h
index e896d67..c7a8018 100644
--- a/include/libvirt/virterror.h
+++ b/include/libvirt/virterror.h
@@ -118,6 +118,19 @@ struct _virError {
see note above */
};
+/*
+ * virErrors:
+ *
+ * A libvirt Errors array instance
+ */
+typedef struct _virErrors virErrors;
+typedef virErrors *virErrorsPtr;
+struct _virErrors {
+virErrorPtr lastError;
+unsigned int nerrors;
+virErrorPtr errors;
+};
+
/**
* virErrorNumber:
*
@@ -261,6 +274,7 @@ typedef void (*virErrorFunc) (void *userData, virErrorPtr
error);
*/
virErrorPtrvirGetLastError (void);
+virErrorsPtr virGetAllErrors (void);
virErrorPtrvirSaveLastError(void);
void virResetLastError (void);
void virResetError (virErrorPtr err);
diff --git a/python/generator.py b/python/generator.py
index 6fee3a4..315eb51 100755
--- a/python/generator.py
+++ b/python/generator.py
@@ -356,6 +356,7 @@ skip_impl = (
'virDomainSnapshotListChildrenNames',
'virConnGetLastError',
'virGetLastError',
+'virGetAllErrors',
'virDomainGetInfo',
'virDomainGetState',
'virDomainGetControlInfo',
diff --git a/src/util/virterror.c b/src/util/virterror.c
index 380dc56..b03ae7c 100644
--- a/src/util/virterror.c
+++ b/src/util/virterror.c
@@ -190,11 +190,17 @@ static const char *virErrorDomainName(virErrorDomain
domain) {
static void
virLastErrFreeData(void *data)
{
-virErrorPtr err = data;
-if (!err)
+virErrorsPtr errs = data;
+if (!errs)
return;
-virResetError(err);
-VIR_FREE(err);
+
+if (errs-lastError) {
+virResetError(errs-lastError);
+VIR_FREE(errs-lastError);
+}
+if (errs-errors)
+VIR_FREE(errs-errors);
+VIR_FREE(errs);
}
@@ -262,14 +268,18 @@ virCopyError(virErrorPtr from,
static virErrorPtr
virLastErrorObject(void)
{
-virErrorPtr err;
-err = virThreadLocalGet(virLastErr);
-if (!err) {
-if (VIR_ALLOC(err) 0)
+virErrorsPtr errs = NULL;
+
+errs = virThreadLocalGet(virLastErr);
+if (!errs) {
+if (VIR_ALLOC(errs) 0)
+return NULL;
+if (VIR_ALLOC(errs-lastError) 0)
return NULL;
-virThreadLocalSet(virLastErr, err);
+virThreadLocalSet(virLastErr, errs);
}
-return err;
+
+return errs-lastError;
}
@@ -292,6 +302,27 @@ virGetLastError(void)
return err;
}
+/*
+ * virGetAllErrors:
+ *
+ * Provide a pointer to all errors caught at the library level
+ *
+ * The error object is kept in thread local storage, so separate
+ * threads can safely access this concurrently.
+ *
+ * Returns a pointer to all errors caught or NULL if none occurred.
+ */
+virErrorsPtr
+virGetAllErrors(void)
+{
+virErrorsPtr errs;
+
+errs = virThreadLocalGet(virLastErr);
+if (!errs || errs-nerrors == 0)
+return NULL;
+return errs;
+}
+
/**
* virSetError:
* @newerr: previously saved error object
@@ -316,11 +347,56 @@ virSetError(virErrorPtr newerr)
virResetError(err);
ret = virCopyError(newerr, err);
+
+/* Add error into the error array */
+virAddError(newerr);
cleanup:
errno = saved_errno;
return ret;
}
+/*
+ * virAddError:
+ * @err: error pointer
+ *
+ * Add the error to the array of error pointer
+ */
+void
+virAddError(virErrorPtr err)
+{
+virErrorsPtr errs;
+
+/* Discard all error codes that mean 'no error' */
+if (!err || err-code == VIR_ERR_OK)
+return;
+
+errs = virThreadLocalGet(virLastErr);
+
+/* Shouldn't happen but doesn't hurt to check */
+if (!errs)
+return;
+
+if (errs-errors == NULL) {
+if (VIR_ALLOC(errs-errors) 0)
+return;
+errs-nerrors = 0;
+}
+else {
+if (VIR_REALLOC_N(errs-errors, errs-nerrors + 1) 0)
+return;
+}
+
+/* Insert data into errors array element */
+
Re: [libvirt] ALERT Virtualization Automatic Builder
On 01/19/2012 03:07 AM, Michal Privoznik wrote: On 18.01.2012 19:56, berra...@redhat.com wrote: Overall status: failed Start date: Wed Jan 18 2012 Start time: 18:32:52 UTC / 18:32:52 GMT Build counter: 1326911571 Build timestamp: 1326911571 URL: http://dhcp-0-241.camlab.fab.redhat.com/index.html Module: libvirt Status: failed URL: http://dhcp-0-241.camlab.fab.redhat.com/module-libvirt.html This is because of WARNING: no socket to connect to string being emitted by GnuTLS. I guess that gnutls was upgraded, wasn't it? Because on the older one (2.12.7) everything works. gnome-keyring is the culprit: https://bugzilla.redhat.com/show_bug.cgi?id=782725 and a fix is available https://admin.fedoraproject.org/updates/FEDORA-2012-0658/gnome-keyring-3.2.1-3.fc16?_csrf_token=49e6e94694b1c1076f7f37102af21a2002e016db -- 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] errors: Improve error reporting to log multiple errors instead of just the last one
Oops, I'm sorry as this didn't pass the syntax check.
To pass the syntax check the new add-on patch is necessary:
diff --git a/src/util/virterror.c b/src/util/virterror.c
index b03ae7c..55269b9 100644
--- a/src/util/virterror.c
+++ b/src/util/virterror.c
@@ -198,8 +198,7 @@ virLastErrFreeData(void *data)
virResetError(errs-lastError);
VIR_FREE(errs-lastError);
}
-if (errs-errors)
-VIR_FREE(errs-errors);
+VIR_FREE(errs-errors);
VIR_FREE(errs);
}
With this applied on top of my patch it's working fine to pass all of
the tests (make check) and also the syntax check.
Also, when trying to debug the tests the gnulib tests are really
annoying. Would somebody consider creating some alias like `make
vircheck` to just do the libvirt tests, i.e. with bypassing the gnulib
tests. If you debug some test then it's pretty annoying to run through
the gnulib tests everytime you invoke `make check`.
Michal
On 01/19/2012 02:13 PM, Michal Novotny wrote:
This patch introduces a new structure called virErrorsPtr which can get all
the errors that occurred since the connection open. The error callback
function
is being used as many times as necessary. The new public function called
virGetAllErrors() has been introduced to get all the errors that occurred.
Also, a new test called errorreport has been written to test the functionality
of error logging for multiple error occurrences.
Signed-off-by: Michal Novotny minov...@redhat.com
---
include/libvirt/virterror.h | 14 ++
python/generator.py |1 +
src/util/virterror.c | 99
src/util/virterror_internal.h |1 +
tests/Makefile.am |9 +++-
tests/errorreport.c | 74 ++
6 files changed, 186 insertions(+), 12 deletions(-)
create mode 100644 tests/errorreport.c
diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h
index e896d67..c7a8018 100644
--- a/include/libvirt/virterror.h
+++ b/include/libvirt/virterror.h
@@ -118,6 +118,19 @@ struct _virError {
see note above */
};
+/*
+ * virErrors:
+ *
+ * A libvirt Errors array instance
+ */
+typedef struct _virErrors virErrors;
+typedef virErrors *virErrorsPtr;
+struct _virErrors {
+virErrorPtr lastError;
+unsigned int nerrors;
+virErrorPtr errors;
+};
+
/**
* virErrorNumber:
*
@@ -261,6 +274,7 @@ typedef void (*virErrorFunc) (void *userData, virErrorPtr
error);
*/
virErrorPtr virGetLastError (void);
+virErrorsPtr virGetAllErrors (void);
virErrorPtr virSaveLastError(void);
void virResetLastError (void);
void virResetError (virErrorPtr err);
diff --git a/python/generator.py b/python/generator.py
index 6fee3a4..315eb51 100755
--- a/python/generator.py
+++ b/python/generator.py
@@ -356,6 +356,7 @@ skip_impl = (
'virDomainSnapshotListChildrenNames',
'virConnGetLastError',
'virGetLastError',
+'virGetAllErrors',
'virDomainGetInfo',
'virDomainGetState',
'virDomainGetControlInfo',
diff --git a/src/util/virterror.c b/src/util/virterror.c
index 380dc56..b03ae7c 100644
--- a/src/util/virterror.c
+++ b/src/util/virterror.c
@@ -190,11 +190,17 @@ static const char *virErrorDomainName(virErrorDomain
domain) {
static void
virLastErrFreeData(void *data)
{
-virErrorPtr err = data;
-if (!err)
+virErrorsPtr errs = data;
+if (!errs)
return;
-virResetError(err);
-VIR_FREE(err);
+
+if (errs-lastError) {
+virResetError(errs-lastError);
+VIR_FREE(errs-lastError);
+}
+if (errs-errors)
+VIR_FREE(errs-errors);
+VIR_FREE(errs);
}
@@ -262,14 +268,18 @@ virCopyError(virErrorPtr from,
static virErrorPtr
virLastErrorObject(void)
{
-virErrorPtr err;
-err = virThreadLocalGet(virLastErr);
-if (!err) {
-if (VIR_ALLOC(err) 0)
+virErrorsPtr errs = NULL;
+
+errs = virThreadLocalGet(virLastErr);
+if (!errs) {
+if (VIR_ALLOC(errs) 0)
+return NULL;
+if (VIR_ALLOC(errs-lastError) 0)
return NULL;
-virThreadLocalSet(virLastErr, err);
+virThreadLocalSet(virLastErr, errs);
}
-return err;
+
+return errs-lastError;
}
@@ -292,6 +302,27 @@ virGetLastError(void)
return err;
}
+/*
+ * virGetAllErrors:
+ *
+ * Provide a pointer to all errors caught at the library level
+ *
+ * The error object is kept in thread local storage, so separate
+ * threads can safely access this concurrently.
+ *
+ * Returns a pointer to all errors caught or NULL if none occurred.
+ */
+virErrorsPtr
+virGetAllErrors(void)
+{
+virErrorsPtr errs;
+
+errs = virThreadLocalGet(virLastErr);
Re: [libvirt] [PATCH] errors: Improve error reporting to log multiple errors instead of just the last one
On Thu, Jan 19, 2012 at 02:13:59PM +0100, Michal Novotny wrote: This patch introduces a new structure called virErrorsPtr which can get all the errors that occurred since the connection open. The error callback function is being used as many times as necessary. The new public function called virGetAllErrors() has been introduced to get all the errors that occurred. This impl is effectively an unbounded memory leak, if you consider that applications will keep the same virConnectPtr open more or less forever. In addition any libvirt API that raises multiple errors should be considered broken, so I don't think we should have any such API for querying multiple errors. What is the situation that motivated this new API ? Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] errors: Improve error reporting to log multiple errors instead of just the last one
On 01/19/2012 02:20 PM, Daniel P. Berrange wrote: On Thu, Jan 19, 2012 at 02:13:59PM +0100, Michal Novotny wrote: This patch introduces a new structure called virErrorsPtr which can get all the errors that occurred since the connection open. The error callback function is being used as many times as necessary. The new public function called virGetAllErrors() has been introduced to get all the errors that occurred. This impl is effectively an unbounded memory leak, if you consider that applications will keep the same virConnectPtr open more or less forever. In addition any libvirt API that raises multiple errors should be considered broken, so I don't think we should have any such API for querying multiple errors. What is the situation that motivated this new API ? This is simple. When you have situation with e.g. disk or domain creation. It fails but you don't know why. Sometimes the disk creation may fail on insufficient permissions and sometimes there may be not enough space etc... The same for domain creation. Also, we've been discussing this with Peter and we've both agreed that it would be a nice feature of libvirt to have some API like this. I'm CCing Peter for his feedback on this (not the patch rather than the reason of implementation) as I have to admit that this patch would like some tweaking most likely but the issue here is not different as you're asking whether we really need this. Peter, could you please provide Daniel any other example why you would like this implemented? Thanks, Michal -- Michal Novotny minov...@redhat.com, RHCE, Red Hat Virtualization | libvirt-php bindings | php-virt-control.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] errors: Improve error reporting to log multiple errors instead of just the last one
On Thu, Jan 19, 2012 at 02:25:55PM +0100, Michal Novotny wrote: On 01/19/2012 02:20 PM, Daniel P. Berrange wrote: On Thu, Jan 19, 2012 at 02:13:59PM +0100, Michal Novotny wrote: This patch introduces a new structure called virErrorsPtr which can get all the errors that occurred since the connection open. The error callback function is being used as many times as necessary. The new public function called virGetAllErrors() has been introduced to get all the errors that occurred. This impl is effectively an unbounded memory leak, if you consider that applications will keep the same virConnectPtr open more or less forever. In addition any libvirt API that raises multiple errors should be considered broken, so I don't think we should have any such API for querying multiple errors. What is the situation that motivated this new API ? This is simple. When you have situation with e.g. disk or domain creation. It fails but you don't know why. Sometimes the disk creation may fail on insufficient permissions and sometimes there may be not enough space etc... The same for domain creation. But for any single API call into libvirt, there is only one error that is relevant upon failure. If you are making a sequence of multiple API calls, you should check each one for an error, before trying the next call. Not doing so is simply an application bug 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] errors: Improve error reporting to log multiple errors instead of just the last one
On 01/19/2012 02:28 PM, Daniel P. Berrange wrote: On Thu, Jan 19, 2012 at 02:25:55PM +0100, Michal Novotny wrote: On 01/19/2012 02:20 PM, Daniel P. Berrange wrote: On Thu, Jan 19, 2012 at 02:13:59PM +0100, Michal Novotny wrote: This patch introduces a new structure called virErrorsPtr which can get all the errors that occurred since the connection open. The error callback function is being used as many times as necessary. The new public function called virGetAllErrors() has been introduced to get all the errors that occurred. This impl is effectively an unbounded memory leak, if you consider that applications will keep the same virConnectPtr open more or less forever. In addition any libvirt API that raises multiple errors should be considered broken, so I don't think we should have any such API for querying multiple errors. What is the situation that motivated this new API ? This is simple. When you have situation with e.g. disk or domain creation. It fails but you don't know why. Sometimes the disk creation may fail on insufficient permissions and sometimes there may be not enough space etc... The same for domain creation. But for any single API call into libvirt, there is only one error that is relevant upon failure. If you are making a sequence of multiple API calls, you should check each one for an error, before trying the next call. Not doing so is simply an application bug Daniel I can see your point Daniel and not I don't recall what we were talking about with Peter exactly. Maybe he can recall so I'll leave it to him but the truth is that I've been struggling with the reason of failure and finally just the logging using the LIBVIRT_DEBUG=1 when running the daemon helped me and I've been stuck before using it since the error was masked by something AFAIK. Michal -- Michal Novotny minov...@redhat.com, RHCE, Red Hat Virtualization | libvirt-php bindings | php-virt-control.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/4] Remove duplicate call to virNetSASLSessionGetIdentity
From: Daniel P. Berrange berra...@redhat.com * daemon/remote.c: remoteSASLFinish called the method virNetSASLSessionGetIdentity twice, remove second call --- daemon/remote.c |1 - 1 files changed, 0 insertions(+), 1 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index 80a2c1f..d917786 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -2198,7 +2198,6 @@ remoteSASLFinish(virNetServerClientPtr client) VIR_DEBUG(Authentication successful %d, virNetServerClientGetFD(client)); -identity = virNetSASLSessionGetIdentity(priv-sasl); PROBE(RPC_SERVER_CLIENT_AUTH_ALLOW, client=%p auth=%d identity=%s, client, REMOTE_AUTH_SASL, identity); -- 1.7.7.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/4] Misc patches to support RBAC work
The following patches aren't really needed on their own, rather they are part of the role based access control work I'm doing. In order to keep my patch queue small, I'm sending these out now, since there is no harm in merging them -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/4] Rename APIs for fetching UNIX socket credentials
From: Daniel P. Berrange berra...@redhat.com
To avoid a namespace clash with forthcoming identity APIs,
rename the virNet*GetLocalIdentity() APIs to have the form
virNet*GetUNIXIdentity()
* daemon/remote.c, src/libvirt_private.syms: Update
for renamed APIs
* src/rpc/virnetserverclient.c, src/rpc/virnetserverclient.h,
src/rpc/virnetsocket.c, src/rpc/virnetsocket.h: s/LocalIdentity/UNIXIdentity/
---
daemon/remote.c |6 +++---
src/libvirt_private.syms |2 +-
src/rpc/virnetserverclient.c |6 +++---
src/rpc/virnetserverclient.h |4 ++--
src/rpc/virnetsocket.c | 16
src/rpc/virnetsocket.h |8
6 files changed, 21 insertions(+), 21 deletions(-)
diff --git a/daemon/remote.c b/daemon/remote.c
index d917786..3b8ccd9 100644
--- a/daemon/remote.c
+++ b/daemon/remote.c
@@ -2038,7 +2038,7 @@ remoteDispatchAuthList(virNetServerPtr server
ATTRIBUTE_UNUSED,
* some piece of polkit isn't present/running
*/
if (auth == VIR_NET_SERVER_SERVICE_AUTH_POLKIT) {
-if (virNetServerClientGetLocalIdentity(client, callerUid, callerGid,
callerPid) 0) {
+if (virNetServerClientGetUNIXIdentity(client, callerUid, callerGid,
callerPid) 0) {
/* Don't do anything on error - it'll be validated at next
* phase of auth anyway */
virResetLastError();
@@ -2494,7 +2494,7 @@ remoteDispatchAuthPolkit(virNetServerPtr server
ATTRIBUTE_UNUSED,
goto authfail;
}
-if (virNetServerClientGetLocalIdentity(client, callerUid, callerGid,
callerPid) 0) {
+if (virNetServerClientGetUNIXIdentity(client, callerUid, callerGid,
callerPid) 0) {
goto authfail;
}
@@ -2592,7 +2592,7 @@ remoteDispatchAuthPolkit(virNetServerPtr server,
goto authfail;
}
-if (virNetServerClientGetLocalIdentity(client, callerUid, callerGid,
callerPid) 0) {
+if (virNetServerClientGetUNIXIdentity(client, callerUid, callerGid,
callerPid) 0) {
VIR_ERROR(_(cannot get peer socket identity));
goto authfail;
}
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 8301c78..cf17ed1 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1291,7 +1291,7 @@ virNetServerClientDelayedClose;
virNetServerClientFree;
virNetServerClientGetAuth;
virNetServerClientGetFD;
-virNetServerClientGetLocalIdentity;
+virNetServerClientGetUNIXIdentity;
virNetServerClientGetPrivateData;
virNetServerClientGetReadonly;
virNetServerClientGetTLSKeySize;
diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c
index ed08e40..67600fd 100644
--- a/src/rpc/virnetserverclient.c
+++ b/src/rpc/virnetserverclient.c
@@ -447,13 +447,13 @@ int virNetServerClientGetFD(virNetServerClientPtr client)
return fd;
}
-int virNetServerClientGetLocalIdentity(virNetServerClientPtr client,
- uid_t *uid, gid_t *gid, pid_t *pid)
+int virNetServerClientGetUNIXIdentity(virNetServerClientPtr client,
+ uid_t *uid, gid_t *gid, pid_t *pid)
{
int ret = -1;
virNetServerClientLock(client);
if (client-sock)
-ret = virNetSocketGetLocalIdentity(client-sock, uid, gid, pid);
+ret = virNetSocketGetUNIXIdentity(client-sock, uid, gid, pid);
virNetServerClientUnlock(client);
return ret;
}
diff --git a/src/rpc/virnetserverclient.h b/src/rpc/virnetserverclient.h
index 2dd01c5..154a160 100644
--- a/src/rpc/virnetserverclient.h
+++ b/src/rpc/virnetserverclient.h
@@ -70,8 +70,8 @@ int virNetServerClientSetIdentity(virNetServerClientPtr
client,
const char *identity);
const char *virNetServerClientGetIdentity(virNetServerClientPtr client);
-int virNetServerClientGetLocalIdentity(virNetServerClientPtr client,
- uid_t *uid, gid_t *gid, pid_t *pid);
+int virNetServerClientGetUNIXIdentity(virNetServerClientPtr client,
+ uid_t *uid, gid_t *gid, pid_t *pid);
void virNetServerClientRef(virNetServerClientPtr client);
diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c
index 8178ac3..67d33b7 100644
--- a/src/rpc/virnetsocket.c
+++ b/src/rpc/virnetsocket.c
@@ -824,10 +824,10 @@ int virNetSocketGetPort(virNetSocketPtr sock)
#ifdef SO_PEERCRED
-int virNetSocketGetLocalIdentity(virNetSocketPtr sock,
- uid_t *uid,
- gid_t *gid,
- pid_t *pid)
+int virNetSocketGetUNIXIdentity(virNetSocketPtr sock,
+uid_t *uid,
+gid_t *gid,
+pid_t *pid)
{
struct ucred cr;
socklen_t cr_len = sizeof (cr);
@@ -848,10 +848,10 @@ int virNetSocketGetLocalIdentity(virNetSocketPtr sock,
return 0;
}
#else
-int virNetSocketGetLocalIdentity(virNetSocketPtr
[libvirt] [PATCH 3/4] Add virGetGroupName to convert from GID to group name
From: Daniel P. Berrange berra...@redhat.com
---
src/libvirt_private.syms |1 +
src/util/util.c | 55 ++
src/util/util.h |1 +
3 files changed, 57 insertions(+), 0 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 0dfd4c1..8301c78 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1112,6 +1112,7 @@ virFindFileInPath;
virFormatMacAddr;
virGenerateMacAddr;
virGetGroupID;
+virGetGroupName;
virGetHostname;
virGetUserDirectory;
virGetUserID;
diff --git a/src/util/util.c b/src/util/util.c
index 8663c4d..fdfceaa 100644
--- a/src/util/util.c
+++ b/src/util/util.c
@@ -2187,6 +2187,56 @@ static char *virGetUserEnt(uid_t uid,
return ret;
}
+static char *virGetGroupEnt(gid_t gid)
+{
+char *strbuf;
+char *ret;
+struct group grbuf;
+struct group *gr = NULL;
+long val = sysconf(_SC_GETGR_R_SIZE_MAX);
+size_t strbuflen = val;
+int rc;
+
+/* sysconf is a hint; if it fails, fall back to a reasonable size */
+if (val 0)
+strbuflen = 1024;
+
+if (VIR_ALLOC_N(strbuf, strbuflen) 0) {
+virReportOOMError();
+return NULL;
+}
+
+/*
+ * From the manpage (terrifying but true):
+ *
+ * ERRORS
+ * 0 or ENOENT or ESRCH or EBADF or EPERM or ...
+ *The given name or gid was not found.
+ */
+while ((rc = getgrgid_r(gid, grbuf, strbuf, strbuflen, gr)) == ERANGE) {
+if (VIR_RESIZE_N(strbuf, strbuflen, strbuflen, strbuflen) 0) {
+virReportOOMError();
+VIR_FREE(strbuf);
+return NULL;
+}
+}
+if (rc != 0 || gr == NULL) {
+virReportSystemError(rc,
+ _(Failed to find group record for gid '%u'),
+ (unsigned int) gid);
+VIR_FREE(strbuf);
+return NULL;
+}
+
+ret = strdup(gr-gr_name);
+
+VIR_FREE(strbuf);
+if (!ret)
+virReportOOMError();
+
+return ret;
+}
+
char *virGetUserDirectory(uid_t uid)
{
return virGetUserEnt(uid, VIR_USER_ENT_DIRECTORY);
@@ -2197,6 +2247,11 @@ char *virGetUserName(uid_t uid)
return virGetUserEnt(uid, VIR_USER_ENT_NAME);
}
+char *virGetGroupName(gid_t gid)
+{
+return virGetGroupEnt(gid);
+}
+
int virGetUserID(const char *name,
uid_t *uid)
diff --git a/src/util/util.h b/src/util/util.h
index 977ab6c..a380144 100644
--- a/src/util/util.h
+++ b/src/util/util.h
@@ -236,6 +236,7 @@ int virKillProcess(pid_t pid, int sig);
char *virGetUserDirectory(uid_t uid);
char *virGetUserName(uid_t uid);
+char *virGetGroupName(gid_t gid);
int virGetUserID(const char *name,
uid_t *uid) ATTRIBUTE_RETURN_CHECK;
int virGetGroupID(const char *name,
--
1.7.7.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/4] Also retrieve GID from SO_PEERCRED
From: Daniel P. Berrange berra...@redhat.com
* daemon/remote.c, src/rpc/virnetserverclient.c,
src/rpc/virnetserverclient.h, src/rpc/virnetsocket.c,
src/rpc/virnetsocket.h: Add gid parameter
---
daemon/remote.c |9 ++---
src/rpc/virnetserverclient.c |4 ++--
src/rpc/virnetserverclient.h |2 +-
src/rpc/virnetsocket.c |3 +++
src/rpc/virnetsocket.h |1 +
5 files changed, 13 insertions(+), 6 deletions(-)
diff --git a/daemon/remote.c b/daemon/remote.c
index a28a754..80a2c1f 100644
--- a/daemon/remote.c
+++ b/daemon/remote.c
@@ -2030,6 +2030,7 @@ remoteDispatchAuthList(virNetServerPtr server
ATTRIBUTE_UNUSED,
int rv = -1;
int auth = virNetServerClientGetAuth(client);
uid_t callerUid;
+gid_t callerGid;
pid_t callerPid;
/* If the client is root then we want to bypass the
@@ -2037,7 +2038,7 @@ remoteDispatchAuthList(virNetServerPtr server
ATTRIBUTE_UNUSED,
* some piece of polkit isn't present/running
*/
if (auth == VIR_NET_SERVER_SERVICE_AUTH_POLKIT) {
-if (virNetServerClientGetLocalIdentity(client, callerUid, callerPid)
0) {
+if (virNetServerClientGetLocalIdentity(client, callerUid, callerGid,
callerPid) 0) {
/* Don't do anything on error - it'll be validated at next
* phase of auth anyway */
virResetLastError();
@@ -2463,6 +2464,7 @@ remoteDispatchAuthPolkit(virNetServerPtr server
ATTRIBUTE_UNUSED,
remote_auth_polkit_ret *ret)
{
pid_t callerPid = -1;
+gid_t callerGid = -1;
uid_t callerUid = -1;
const char *action;
int status = -1;
@@ -2493,7 +2495,7 @@ remoteDispatchAuthPolkit(virNetServerPtr server
ATTRIBUTE_UNUSED,
goto authfail;
}
-if (virNetServerClientGetLocalIdentity(client, callerUid, callerPid)
0) {
+if (virNetServerClientGetLocalIdentity(client, callerUid, callerGid,
callerPid) 0) {
goto authfail;
}
@@ -2563,6 +2565,7 @@ remoteDispatchAuthPolkit(virNetServerPtr server,
remote_auth_polkit_ret *ret)
{
pid_t callerPid;
+gid_t callerGid;
uid_t callerUid;
PolKitCaller *pkcaller = NULL;
PolKitAction *pkaction = NULL;
@@ -2590,7 +2593,7 @@ remoteDispatchAuthPolkit(virNetServerPtr server,
goto authfail;
}
-if (virNetServerClientGetLocalIdentity(client, callerUid, callerPid)
0) {
+if (virNetServerClientGetLocalIdentity(client, callerUid, callerGid,
callerPid) 0) {
VIR_ERROR(_(cannot get peer socket identity));
goto authfail;
}
diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c
index cb07dd9..ed08e40 100644
--- a/src/rpc/virnetserverclient.c
+++ b/src/rpc/virnetserverclient.c
@@ -448,12 +448,12 @@ int virNetServerClientGetFD(virNetServerClientPtr client)
}
int virNetServerClientGetLocalIdentity(virNetServerClientPtr client,
- uid_t *uid, pid_t *pid)
+ uid_t *uid, gid_t *gid, pid_t *pid)
{
int ret = -1;
virNetServerClientLock(client);
if (client-sock)
-ret = virNetSocketGetLocalIdentity(client-sock, uid, pid);
+ret = virNetSocketGetLocalIdentity(client-sock, uid, gid, pid);
virNetServerClientUnlock(client);
return ret;
}
diff --git a/src/rpc/virnetserverclient.h b/src/rpc/virnetserverclient.h
index a201dca..2dd01c5 100644
--- a/src/rpc/virnetserverclient.h
+++ b/src/rpc/virnetserverclient.h
@@ -71,7 +71,7 @@ int virNetServerClientSetIdentity(virNetServerClientPtr
client,
const char *virNetServerClientGetIdentity(virNetServerClientPtr client);
int virNetServerClientGetLocalIdentity(virNetServerClientPtr client,
- uid_t *uid, pid_t *pid);
+ uid_t *uid, gid_t *gid, pid_t *pid);
void virNetServerClientRef(virNetServerClientPtr client);
diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c
index af4fc5e..8178ac3 100644
--- a/src/rpc/virnetsocket.c
+++ b/src/rpc/virnetsocket.c
@@ -826,6 +826,7 @@ int virNetSocketGetPort(virNetSocketPtr sock)
#ifdef SO_PEERCRED
int virNetSocketGetLocalIdentity(virNetSocketPtr sock,
uid_t *uid,
+ gid_t *gid,
pid_t *pid)
{
struct ucred cr;
@@ -841,6 +842,7 @@ int virNetSocketGetLocalIdentity(virNetSocketPtr sock,
*pid = cr.pid;
*uid = cr.uid;
+*gid = cr.gid;
virMutexUnlock(sock-lock);
return 0;
@@ -848,6 +850,7 @@ int virNetSocketGetLocalIdentity(virNetSocketPtr sock,
#else
int virNetSocketGetLocalIdentity(virNetSocketPtr sock ATTRIBUTE_UNUSED,
uid_t *uid ATTRIBUTE_UNUSED,
+ gid_t *gid ATTRIBUTE_UNUSED,
pid_t *pid ATTRIBUTE_UNUSED)
{
/* XXX Many more OS
[libvirt] [PATCH] Fix rpc generator to anchor matches for method names
From: Daniel P. Berrange berra...@redhat.com
The RPC generator transforms methods matching certain
patterns like 'id' or 'uuid', etc but does not anchor
its matches to the end of the word. So if a method
contains 'id' in the middle (eg virIdentity) then the
RPC generator munges that.
* src/rpc/gendispatch.pl: Anchor matches
---
src/rpc/gendispatch.pl |8
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl
index b36ca69..0460fca 100755
--- a/src/rpc/gendispatch.pl
+++ b/src/rpc/gendispatch.pl
@@ -34,10 +34,10 @@ sub name_to_ProcName {
my $name = shift;
my @elems = split /_/, $name;
@elems = map ucfirst, @elems;
-@elems = map { $_ =~ s/Nwfilter/NWFilter/; $_ =~ s/Xml/XML/;
- $_ =~ s/Uri/URI/; $_ =~ s/Uuid/UUID/; $_ =~ s/Id/ID/;
- $_ =~ s/Mac/MAC/; $_ =~ s/Cpu/CPU/; $_ =~ s/Os/OS/;
- $_ =~ s/Nmi/NMI/; $_ } @elems;
+@elems = map { $_ =~ s/Nwfilter/NWFilter/; $_ =~ s/Xml$/XML/;
+ $_ =~ s/Uri$/URI/; $_ =~ s/Uuid$/UUID/; $_ =~ s/Id$/ID/;
+ $_ =~ s/Mac$/MAC/; $_ =~ s/Cpu$/CPU/; $_ =~ s/Os$/OS/;
+ $_ =~ s/Nmi$/NMI/; $_ } @elems;
join , @elems
}
--
1.7.7.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/4] Rename APIs for fetching UNIX socket credentials
On 01/19/2012 06:51 AM, Daniel P. Berrange wrote: From: Daniel P. Berrange berra...@redhat.com To avoid a namespace clash with forthcoming identity APIs, rename the virNet*GetLocalIdentity() APIs to have the form virNet*GetUNIXIdentity() * daemon/remote.c, src/libvirt_private.syms: Update for renamed APIs * src/rpc/virnetserverclient.c, src/rpc/virnetserverclient.h, src/rpc/virnetsocket.c, src/rpc/virnetsocket.h: s/LocalIdentity/UNIXIdentity/ +++ b/src/libvirt_private.syms @@ -1291,7 +1291,7 @@ virNetServerClientDelayedClose; virNetServerClientFree; virNetServerClientGetAuth; virNetServerClientGetFD; -virNetServerClientGetLocalIdentity; +virNetServerClientGetUNIXIdentity; virNetServerClientGetPrivateData; This rename implies a different sorting order. Other than that nit, ACK to the series. -- 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] Fix rpc generator to anchor matches for method names
On 01/19/2012 07:21 AM, Daniel P. Berrange wrote:
From: Daniel P. Berrange berra...@redhat.com
The RPC generator transforms methods matching certain
patterns like 'id' or 'uuid', etc but does not anchor
its matches to the end of the word. So if a method
contains 'id' in the middle (eg virIdentity) then the
RPC generator munges that.
* src/rpc/gendispatch.pl: Anchor matches
---
src/rpc/gendispatch.pl |8
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl
index b36ca69..0460fca 100755
--- a/src/rpc/gendispatch.pl
+++ b/src/rpc/gendispatch.pl
@@ -34,10 +34,10 @@ sub name_to_ProcName {
my $name = shift;
my @elems = split /_/, $name;
@elems = map ucfirst, @elems;
-@elems = map { $_ =~ s/Nwfilter/NWFilter/; $_ =~ s/Xml/XML/;
- $_ =~ s/Uri/URI/; $_ =~ s/Uuid/UUID/; $_ =~ s/Id/ID/;
- $_ =~ s/Mac/MAC/; $_ =~ s/Cpu/CPU/; $_ =~ s/Os/OS/;
- $_ =~ s/Nmi/NMI/; $_ } @elems;
+@elems = map { $_ =~ s/Nwfilter/NWFilter/; $_ =~ s/Xml$/XML/;
+ $_ =~ s/Uri$/URI/; $_ =~ s/Uuid$/UUID/; $_ =~ s/Id$/ID/;
+ $_ =~ s/Mac$/MAC/; $_ =~ s/Cpu$/CPU/; $_ =~ s/Os$/OS/;
+ $_ =~ s/Nmi$/NMI/; $_ } @elems;
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] pibvirt php help
sir
i am trying to create ssh connention and i am able to connect with out
password from terminal using
virsh -c qemu+ssh://punit@192.168.102.133/system
but this is not working with libvirt-php binding
error :for libvirt-php ssh connection
Libvirt last error: cannot recv data: Resource temporarily unavailable
my php code is
?php
$res1=libvirt_connect(qemu+ssh://punit@192.168.102.133/system,false);
print_r($res1); printf (\n);
if ($res1==false)
{
echo (Libvirt last error: .libvirt_get_last_error());
}
?
--
Punit Gupta
M-Tect (jiit noida)
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] can not get screen shot using libvirt-php
sir i am able to connect to local system an gett all the parameters but i am not able to get screen shots it gives error Cannot find gvnccapture binary -- Punit Gupta M-Tect (jiit noida) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] can not get screen shot using libvirt-php
Hi, you have to install gvnc-tools package on your Fedora box (as you mentioned you're using Fedora). This is having the gvnccapture binary. Michal On 01/19/2012 04:43 PM, punit gupta wrote: sir i am able to connect to local system an gett all the parameters but i am not able to get screen shots it gives error Cannot find gvnccapture binary -- Punit Gupta M-Tect (jiit noida) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list -- Michal Novotny minov...@redhat.com, RHCE, Red Hat Virtualization | libvirt-php bindings | php-virt-control.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] pibvirt php help
Oh, that's interesting. So you can connect fine with the virsh but not
libvirt-php?
Now I may see your point, the issue is in the apache user. You need to
use the apache-key-copy utility (bundled with the php-virt-control) to
generate a key to your apache user account (provided the fact your
apache is running under user apache). This will generate the key (if it
doesn't exist) and transfer it to the destination machine using
ssh-copy-id. The syntax for apache-key-copy is:
apache-key-copy hostname_or_ip
that should be:
apache-key-copy 192.168.102.133
for your case. However it can copy only to root account and not any user
account yet, i.e. you have to connect using
qemu+ssh://root@192.168.102.133/system
and for no other user than root is working yet. You can patch the
apache-key-copy to include the optional user account name and send a
patch to php-virt-control list if you want to.
Michal
On 01/19/2012 04:41 PM, punit gupta wrote:
sir
i am trying to create ssh connention and i am able to connect with
out password from terminal using
virsh -c qemu+ssh://punit@192.168.102.133/system
http://punit@192.168.102.133/system
but this is not working with libvirt-php binding
error :for libvirt-php ssh connection
Libvirt last error: cannot recv data: Resource temporarily unavailable
my php code is
?php
$res1=libvirt_connect(qemu+ssh://punit@192.168.102.133/system
http://punit@192.168.102.133/system,false);
print_r($res1); printf (\n);
if ($res1==false)
{
echo (Libvirt last error: .libvirt_get_last_error());
}
?
--
Punit Gupta
M-Tect (jiit noida)
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
--
Michal Novotny minov...@redhat.com, RHCE, Red Hat
Virtualization | libvirt-php bindings | php-virt-control.org
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt-glib 1/4] Refactor gvir_config_object_delete_child
On Wed, Jan 18, 2012 at 04:50:41PM +0100, Christophe Fergeau wrote: By changing gvir_config_xml_foreach_child to make it robust against current node deletion in the foreach callback, we can use gvir_config_object_foreach_child to implement gvir_config_object_delete_child --- libvirt-gconfig/libvirt-gconfig-helpers.c |5 +++- libvirt-gconfig/libvirt-gconfig-object.c | 32 ++-- 2 files changed, 24 insertions(+), 13 deletions(-) ACK Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt-glib 2/4] Add gvir_config_object_delete_children
On Wed, Jan 18, 2012 at 04:50:42PM +0100, Christophe Fergeau wrote: --- libvirt-gconfig/libvirt-gconfig-object-private.h |1 + libvirt-gconfig/libvirt-gconfig-object.c | 13 + 2 files changed, 14 insertions(+), 0 deletions(-) ACK Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt-glib 3/4] Split gvir_config_object_attach
On Wed, Jan 18, 2012 at 04:50:43PM +0100, Christophe Fergeau wrote: Most of the time we want gvir_config_object_attach to replace existing nodes, but sometimes (for devices subnodes) we want it to append the new node and to keep the existing nodes with the same name. This commit solves this by adding 2 distinct helpers, _attach_add and _attach_replace. This should fix some unexpected behaviour of various _set_ functions which were appending new nodes instead of replacing the existing one. --- libvirt-gconfig/libvirt-gconfig-domain.c | 20 +++- libvirt-gconfig/libvirt-gconfig-object-private.h |6 -- libvirt-gconfig/libvirt-gconfig-object.c | 20 ++-- .../libvirt-gconfig-storage-pool-target.c |4 ++-- libvirt-gconfig/libvirt-gconfig-storage-pool.c |8 .../libvirt-gconfig-storage-vol-target.c |4 ++-- libvirt-gconfig/libvirt-gconfig-storage-vol.c |8 libvirt-gconfig/libvirt-gconfig.sym|1 - 8 files changed, 45 insertions(+), 26 deletions(-) ACK Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt-glib 4/4] Remove 2 unwanted exports
On Wed, Jan 18, 2012 at 04:50:44PM +0100, Christophe Fergeau wrote:
These 2 GVirObject helpers are declared in libvirt-gobject-private.h
and so shouldn't be exported.
---
libvirt-gconfig/libvirt-gconfig.sym |2 --
1 files changed, 0 insertions(+), 2 deletions(-)
diff --git a/libvirt-gconfig/libvirt-gconfig.sym
b/libvirt-gconfig/libvirt-gconfig.sym
index 0fcbd13..efbfafb 100644
--- a/libvirt-gconfig/libvirt-gconfig.sym
+++ b/libvirt-gconfig/libvirt-gconfig.sym
@@ -239,8 +239,6 @@ LIBVIRT_GCONFIG_0.0.4 {
gvir_config_object_get_schema;
gvir_config_object_to_xml;
gvir_config_object_validate;
- gvir_config_object_set_attribute;
- gvir_config_object_set_attribute_with_type;
gvir_config_secret_get_type;
gvir_config_secret_new;
ACK
Daniel
--
|: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC 0/5]: QMP: add balloon-get-memory-stats command
On 01/19/2012 08:56 AM, Luiz Capitulino wrote: Long ago, commit 625a5be added the guest provided memory statistics to the query-balloon command. Unfortunately, it also introduced a severe bug: query-balloon would hang if the guest didn't respond. This, in turn, would also cause a hang in libvirt. Because of that, we decided to disable the guest memory stats feature (commit 11724ff). As we decided to let commands implement ad-hoc async mechanisms until we get a proper way to do it, I decided to try to re-enable that feature. My idea is to have a command and an event. The command gets the process started by sending a request to guest and returns. Later, when the guest makes the memory stats info available, it's sent to the client by means of an QMP event (please, take a look at patch 05/05 for full details). I'm not sure if that approach is good for libvirt though, so it would be very helpful to get their input (Eric, I'm CC'ing you here, but feel free to route this to someone else). [I went ahead and cc'd the libvirt list] Yes, libvirt can live with this approach. And having this in parallel to a qemu-ga verb is nice, since, as it was pointed out, this would allow interaction with guests that have a balloon device but not a guest agent. You may want to read this thread [1], for thoughts on the impact of making another existing blocking command be extended into one that starts an async event and ends when an event is raised; libvirt can expose both a blocking and an asynchronous implementation to the user on top of the qemu model being just asynchronous. [1] https://www.redhat.com/archives/libvir-list/2012-January/msg00562.html Thinking aloud - do we need a means to poll the state of the balloon-stat query? On the one hand, if libvirtd issues the start command, then gets stopped, then the event occurs, then libvirtd is restarted, then libvirt won't know that the event was missed. On the other hand, since this involves guest interaction, libvirt already has to assume that the guest may be malicious and refuse to report stats and/or report invalid stats, so libvirt would already have to be prepared to give up if no event has arrived in a fixed amount of time, and that also means that restarting libvirtd can just ignore any balloon query that was in flight before the restart. So I guess I'm okay with just a start and an event, with no poll of the last-known guest response. But it does mean that qemu has to gracefully handle if libvirt makes two start requests in a row without any intervening events, and conversely that libvirt has to be prepared for an event that happens even when libvirt doesn't remember triggering the start command. Another interesting point is that, there's another way of doing this and it's using qemu-ga instead. That's, qemu-ga could read that information from proc and return it. This is easier simpler, as it doesn't involve guest communication. We also could return a lot more information if needed. The only disadvantage I can see is the dependency on qemu-ga... Most likely, we would want to teach libvirt to use both methods, and give the choice to the user on which approach to use when the guest supports both. -- 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] Hidden symbols
Hello All, When using a systemtap to get a call chain of functions that are executed during the migration of a vm from one host to another I realised that many of the symbols in libvirt are hidden and not visible to systemtap. Is there a way I can make all the symbols visible just for debugging purposes? Many Thanks, Regards, Shradha Shah -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Hidden symbols
On Thu, Jan 19, 2012 at 05:16:48PM +, Shradha Shah wrote: Hello All, When using a systemtap to get a call chain of functions that are executed during the migration of a vm from one host to another I realised that many of the symbols in libvirt are hidden and not visible to systemtap. Is there a way I can make all the symbols visible just for debugging purposes? You shouldn't need todo anything special. If you're building from GIT then your build should already have '-g' flag set, which is sufficient for systemtap to get any symbols. If you're using a RPM build, then just install the libvirt-debuginfo RPM to get the symbols Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 1/6] threads: check for failure to set thread-local value
We had a memory leak on a very arcane OOM situation (unlikely to ever
hit in practice, but who knows if libvirt.so would ever be linked
into some other program that exhausts all thread-local storage keys?).
I found it by code inspection, while analyzing a valgrind report
generated by Alex Jia.
* src/util/threads.h (virThreadLocalSet): Alter signature.
* src/util/threads-pthread.c (virThreadHelper): Reduce allocation
lifetime.
(virThreadLocalSet): Detect failure.
* src/util/threads-win32.c (virThreadLocalSet): Likewise.
(virCondWait): Fix caller.
* src/util/virterror.c (virLastErrorObject): Likewise.
---
src/util/threads-pthread.c | 14 +++---
src/util/threads-win32.c |9 ++---
src/util/threads.h |2 +-
src/util/virterror.c |5 +++--
4 files changed, 21 insertions(+), 9 deletions(-)
diff --git a/src/util/threads-pthread.c b/src/util/threads-pthread.c
index 5b8fd5b..ea64887 100644
--- a/src/util/threads-pthread.c
+++ b/src/util/threads-pthread.c
@@ -154,8 +154,11 @@ struct virThreadArgs {
static void *virThreadHelper(void *data)
{
struct virThreadArgs *args = data;
-args-func(args-opaque);
+struct virThreadArgs local = *args;
+
+/* Free args early, rather than tying it up during the entire thread. */
VIR_FREE(args);
+local.func(local.opaque);
return NULL;
}
@@ -249,7 +252,12 @@ void *virThreadLocalGet(virThreadLocalPtr l)
return pthread_getspecific(l-key);
}
-void virThreadLocalSet(virThreadLocalPtr l, void *val)
+int virThreadLocalSet(virThreadLocalPtr l, void *val)
{
-pthread_setspecific(l-key, val);
+int err = pthread_setspecific(l-key, val);
+if (err) {
+errno = err;
+return -1;
+}
+return 0;
}
diff --git a/src/util/threads-win32.c b/src/util/threads-win32.c
index 63f699b..1c33826 100644
--- a/src/util/threads-win32.c
+++ b/src/util/threads-win32.c
@@ -155,7 +155,10 @@ int virCondWait(virCondPtr c, virMutexPtr m)
if (!event) {
return -1;
}
-virThreadLocalSet(virCondEvent, event);
+if (virThreadLocalSet(virCondEvent, event) 0) {
+CloseHandle(event);
+return -1;
+}
}
virMutexLock(c-lock);
@@ -376,7 +379,7 @@ void *virThreadLocalGet(virThreadLocalPtr l)
return TlsGetValue(l-key);
}
-void virThreadLocalSet(virThreadLocalPtr l, void *val)
+int virThreadLocalSet(virThreadLocalPtr l, void *val)
{
-TlsSetValue(l-key, val);
+return TlsSetValue(l-key, val) == 0 ? -1 : 0;
}
diff --git a/src/util/threads.h b/src/util/threads.h
index e52f3a9..e5000ea 100644
--- a/src/util/threads.h
+++ b/src/util/threads.h
@@ -104,7 +104,7 @@ typedef void (*virThreadLocalCleanup)(void *);
int virThreadLocalInit(virThreadLocalPtr l,
virThreadLocalCleanup c) ATTRIBUTE_RETURN_CHECK;
void *virThreadLocalGet(virThreadLocalPtr l);
-void virThreadLocalSet(virThreadLocalPtr l, void*);
+int virThreadLocalSet(virThreadLocalPtr l, void*) ATTRIBUTE_RETURN_CHECK;
# ifdef WIN32
# include threads-win32.h
diff --git a/src/util/virterror.c b/src/util/virterror.c
index 380dc56..8205516 100644
--- a/src/util/virterror.c
+++ b/src/util/virterror.c
@@ -267,7 +267,8 @@ virLastErrorObject(void)
if (!err) {
if (VIR_ALLOC(err) 0)
return NULL;
-virThreadLocalSet(virLastErr, err);
+if (virThreadLocalSet(virLastErr, err) 0)
+VIR_FREE(err);
}
return err;
}
@@ -612,7 +613,7 @@ virDispatchError(virConnectPtr conn)
virErrorFunc handler = virErrorHandler;
void *userData = virUserData;
-/* Should never happen, but doesn't hurt to check */
+/* Can only happen on OOM. */
if (!err)
return;
--
1.7.7.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 2/6] build: silence some compiler warnings from gnulib
Gnulib claims that there are some classes of warnings that are worth enabling during development, but where silencing those warnings causes code bloat that is not necessary in an optimized build. The code bloat to silence the warnings is only enabled by -Dlint. Follow the lead of coreutils in setting up -Dlint whenever full warnings are requested. * m4/virt-compile-warnings.m4 (LIBVIRT_COMPILE_WARNINGS): Add -Dlint, and move _FORTIFY_SOURCE to config.h instead of CFLAGS. --- m4/virt-compile-warnings.m4 |7 ++- 1 files changed, 6 insertions(+), 1 deletions(-) diff --git a/m4/virt-compile-warnings.m4 b/m4/virt-compile-warnings.m4 index ba388aa..3a428c3 100644 --- a/m4/virt-compile-warnings.m4 +++ b/m4/virt-compile-warnings.m4 @@ -100,8 +100,13 @@ AC_DEFUN([LIBVIRT_COMPILE_WARNINGS],[ gl_WARN_ADD([-Wframe-larger-than=4096]) dnl gl_WARN_ADD([-Wframe-larger-than=256]) +# Silence certain warnings in gnulib, and use improved glibc headers +AC_DEFINE([lint], [1], + [Define to 1 if the compiler is checking for lint.]) +AC_DEFINE([_FORTIFY_SOURCE], [2], + [enable compile-time and run-time bounds-checking, and some warnings]) + # Extra special flags -gl_WARN_ADD([-Wp,-D_FORTIFY_SOURCE=2]) dnl -fstack-protector stuff passes gl_WARN_ADD with gcc dnl on Mingw32, but fails when actually used case $host in -- 1.7.7.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 3/6] lxc: use live/config helper
Based on qemu changes made in commits ae523427 and 659ded58.
* src/lxc/lxc_driver.c (lxcSetSchedulerParametersFlags)
(lxcGetSchedulerParametersFlags, lxcDomainSetBlkioParameters)
(lxcDomainGetBlkioParameters): Use helpers.
(lxcDomainSetBlkioParameters): Allow setting live and config at
once.
---
src/lxc/lxc_driver.c | 160 +-
1 files changed, 29 insertions(+), 131 deletions(-)
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 3baff19..7d6ac59 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2010-2011 Red Hat, Inc.
+ * Copyright (C) 2010-2012 Red Hat, Inc.
* Copyright IBM Corp. 2008
*
* lxc_driver.c: linux container driver functions
@@ -2749,7 +2749,6 @@ lxcSetSchedulerParametersFlags(virDomainPtr dom,
virDomainObjPtr vm = NULL;
virDomainDefPtr vmdef = NULL;
int ret = -1;
-bool isActive;
int rc;
virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
@@ -2765,22 +2764,11 @@ lxcSetSchedulerParametersFlags(virDomainPtr dom,
goto cleanup;
}
-isActive = virDomainObjIsActive(vm);
-
-if (flags == VIR_DOMAIN_AFFECT_CURRENT) {
-if (isActive)
-flags = VIR_DOMAIN_AFFECT_LIVE;
-else
-flags = VIR_DOMAIN_AFFECT_CONFIG;
-}
+if (virDomainLiveConfigHelperMethod(driver-caps, vm, flags,
+vmdef) 0)
+goto cleanup;
if (flags VIR_DOMAIN_AFFECT_CONFIG) {
-if (!vm-persistent) {
-lxcError(VIR_ERR_OPERATION_INVALID, %s,
- _(cannot change persistent config of a transient
domain));
-goto cleanup;
-}
-
/* Make a copy for updated domain. */
vmdef = virDomainObjCopyPersistentDef(driver-caps, vm);
if (!vmdef)
@@ -2788,12 +2776,6 @@ lxcSetSchedulerParametersFlags(virDomainPtr dom,
}
if (flags VIR_DOMAIN_AFFECT_LIVE) {
-if (!isActive) {
-lxcError(VIR_ERR_OPERATION_INVALID,
- %s, _(domain is not running));
-goto cleanup;
-}
-
if (!lxcCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_CPU)) {
lxcError(VIR_ERR_OPERATION_INVALID,
%s, _(cgroup CPU controller is not mounted));
@@ -2919,12 +2901,12 @@ lxcGetSchedulerParametersFlags(virDomainPtr dom,
lxc_driver_t *driver = dom-conn-privateData;
virCgroupPtr group = NULL;
virDomainObjPtr vm = NULL;
+virDomainDefPtr persistentDef;
unsigned long long shares = 0;
unsigned long long period = 0;
long long quota = 0;
int ret = -1;
int rc;
-bool isActive;
bool cpu_bw_status = false;
int saved_nparams = 0;
@@ -2948,52 +2930,19 @@ lxcGetSchedulerParametersFlags(virDomainPtr dom,
goto cleanup;
}
-isActive = virDomainObjIsActive(vm);
-
-if (flags == VIR_DOMAIN_AFFECT_CURRENT) {
-if (isActive)
-flags = VIR_DOMAIN_AFFECT_LIVE;
-else
-flags = VIR_DOMAIN_AFFECT_CONFIG;
-}
+if (virDomainLiveConfigHelperMethod(driver-caps, vm, flags,
+persistentDef) 0)
+goto cleanup;
if (flags VIR_DOMAIN_AFFECT_CONFIG) {
-if (!vm-persistent) {
-lxcError(VIR_ERR_OPERATION_INVALID, %s,
- _(cannot query persistent config of a transient
domain));
-goto cleanup;
-}
-
-if (isActive) {
-virDomainDefPtr persistentDef;
-
-persistentDef = virDomainObjGetPersistentDef(driver-caps, vm);
-if (!persistentDef) {
-lxcError(VIR_ERR_INTERNAL_ERROR, %s,
- _(can't get persistentDef));
-goto cleanup;
-}
-shares = persistentDef-cputune.shares;
-if (*nparams 1 cpu_bw_status) {
-period = persistentDef-cputune.period;
-quota = persistentDef-cputune.quota;
-}
-} else {
-shares = vm-def-cputune.shares;
-if (*nparams 1 cpu_bw_status) {
-period = vm-def-cputune.period;
-quota = vm-def-cputune.quota;
-}
+shares = persistentDef-cputune.shares;
+if (*nparams 1 cpu_bw_status) {
+period = persistentDef-cputune.period;
+quota = persistentDef-cputune.quota;
}
goto out;
}
-if (!isActive) {
-lxcError(VIR_ERR_OPERATION_INVALID, %s,
- _(domain is not running));
-goto cleanup;
-}
-
if (!lxcCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_CPU)) {
lxcError(VIR_ERR_OPERATION_INVALID,
%s, _(cgroup CPU controller is not mounted));
@@ -3091,7 +3040,6 @@ static int lxcDomainSetBlkioParameters(virDomainPtr dom,
virDomainObjPtr vm = NULL;
virDomainDefPtr persistentDef = NULL;
[libvirt] [PATCHv2 6/6] error: drop old-style error reporting
While we still don't want to enable gcc's new -Wformat-literal
warning, I found a rather easy case where the warning could be
reduced, by getting rid of obsolete error-reporting practices.
This is the last place where we were passing the (unused) net
and conn arguments for constructing an error.
* src/util/virterror_internal.h (virErrorMsg): Delete prototype.
(virReportError): Delete macro.
* src/util/virterror.c (virErrorMsg): Make static.
* src/libvirt_private.syms (virterror_internal.h): Drop export.
* src/util/conf.c (virConfError): Convert to macro.
(virConfErrorHelper): New function, and adjust error calls.
* src/xen/xen_hypervisor.c (virXenErrorFunc): Delete.
(xenHypervisorGetSchedulerType)
(xenHypervisorGetSchedulerParameters)
(xenHypervisorSetSchedulerParameters)
(xenHypervisorDomainBlockStats)
(xenHypervisorDomainInterfaceStats)
(xenHypervisorDomainGetOSType)
(xenHypervisorNodeGetCellsFreeMemory, xenHypervisorGetVcpus):
Update callers.
---
src/libvirt_private.syms |1 -
src/util/conf.c | 22 ++---
src/util/virterror.c |2 +-
src/util/virterror_internal.h |8 --
src/xen/xen_hypervisor.c | 164
5 files changed, 76 insertions(+), 121 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 9a7200b..f8d97d1 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1387,7 +1387,6 @@ virSocketAddrSetPort;
# virterror_internal.h
virDispatchError;
-virErrorMsg;
virRaiseErrorFull;
virReportErrorHelper;
virReportOOMErrorFull;
diff --git a/src/util/conf.c b/src/util/conf.c
index 502dafb..8ad60e0 100644
--- a/src/util/conf.c
+++ b/src/util/conf.c
@@ -89,27 +89,23 @@ struct _virConf {
*
* Handle an error at the xend daemon interface
*/
+#define virConfError(ctxt, error, info) \
+virConfErrorHelper(__FILE__, __FUNCTION__, __LINE__, ctxt, error, info)
static void
-virConfError(virConfParserCtxtPtr ctxt,
- virErrorNumber error, const char *info)
+virConfErrorHelper(const char *file, const char *func, size_t line,
+ virConfParserCtxtPtr ctxt,
+ virErrorNumber error, const char *info)
{
-const char *format;
-
if (error == VIR_ERR_OK)
return;
/* Construct the string 'filename:line: info' if we have that. */
if (ctxt ctxt-filename) {
-virRaiseError(NULL, NULL, VIR_FROM_CONF, error, VIR_ERR_ERROR,
-info, ctxt-filename, NULL,
-ctxt-line, 0,
-%s:%d: %s, ctxt-filename, ctxt-line, info);
+virReportErrorHelper(VIR_FROM_CONF, error, file, func, line,
+ _(%s:%d: %s), ctxt-filename, ctxt-line, info);
} else {
-format = virErrorMsg(error, info);
-virRaiseError(NULL, NULL, VIR_FROM_CONF, error, VIR_ERR_ERROR,
-info, NULL, NULL,
-ctxt ? ctxt-line : 0, 0,
-format, info);
+virReportErrorHelper(VIR_FROM_CONF, error, file, func, line,
+ %s, info);
}
}
diff --git a/src/util/virterror.c b/src/util/virterror.c
index 8205516..ff44a57 100644
--- a/src/util/virterror.c
+++ b/src/util/virterror.c
@@ -756,7 +756,7 @@ virRaiseErrorFull(const char *filename ATTRIBUTE_UNUSED,
*
* Returns the constant string associated to @error
*/
-const char *
+static const char *
virErrorMsg(virErrorNumber error, const char *info)
{
const char *errmsg = NULL;
diff --git a/src/util/virterror_internal.h b/src/util/virterror_internal.h
index d61ea0d..b8cb279 100644
--- a/src/util/virterror_internal.h
+++ b/src/util/virterror_internal.h
@@ -47,14 +47,6 @@ void virRaiseErrorFull(const char *filename,
const char *fmt, ...)
ATTRIBUTE_FMT_PRINTF(12, 13);
-/* Includes 'dom' and 'net' for compatbility, but they're ignored */
-# define virRaiseError(dom, net, domain, code, level, \
- str1, str2, str3, int1, int2, msg, ...) \
-virRaiseErrorFull(__FILE__, __FUNCTION__, __LINE__, \
- domain, code, level, str1, str2, str3, int1, int2, \
- msg, __VA_ARGS__)
-
-const char *virErrorMsg(virErrorNumber error, const char *info);
void virReportErrorHelper(int domcode, int errcode,
const char *filename,
const char *funcname,
diff --git a/src/xen/xen_hypervisor.c b/src/xen/xen_hypervisor.c
index 0c92be8..2bb3466 100644
--- a/src/xen/xen_hypervisor.c
+++ b/src/xen/xen_hypervisor.c
@@ -880,40 +880,6 @@ struct xenUnifiedDriver xenHypervisorDriver = {
__FUNCTION__, __LINE__, __VA_ARGS__)
/**
- * virXenErrorFunc:
- * @error: the error number
- * @func: the function failing
- * @info: extra information string
- * @value: extra information number
- *
- * Handle an error at the xend
[libvirt] [PATCHv2 4/6] util: add new file for virTypedParameter utils
Preparation for another patch that refactors common patterns
into the new file for fewer lines of code overall.
* src/util/util.h (virTypedParameterArrayClear): Move...
* src/util/virtypedparam.h: ...to new file.
(virTypedParameterArrayValidate, virTypedParameterAssign): New
prototypes.
* src/util/util.c (virTypedParameterArrayClear): Likewise.
* src/util/virtypedparam.c: New file.
* po/POTFILES.in: Mark file for translation.
* src/Makefile.am (UTIL_SOURCES): Build it.
* src/libvirt_private.syms (util.h): Split...
(virtypedparam.h): to new section.
(virkeycode.h): Sort.
* daemon/remote.c: Adjust callers.
* tools/virsh.c: Likewise.
---
daemon/remote.c |1 +
po/POTFILES.in |1 +
src/Makefile.am |1 +
src/libvirt_private.syms | 20 +++--
src/util/util.c | 16 +
src/util/util.h |4 +-
src/util/virtypedparam.c | 187 ++
src/util/virtypedparam.h | 37 +
tools/virsh.c|1 +
9 files changed, 243 insertions(+), 25 deletions(-)
create mode 100644 src/util/virtypedparam.c
create mode 100644 src/util/virtypedparam.h
diff --git a/daemon/remote.c b/daemon/remote.c
index a28a754..4db8f91 100644
--- a/daemon/remote.c
+++ b/daemon/remote.c
@@ -44,6 +44,7 @@
#include virnetserverservice.h
#include virnetserver.h
#include virfile.h
+#include virtypedparam.h
#include remote_protocol.h
#include qemu_protocol.h
diff --git a/po/POTFILES.in b/po/POTFILES.in
index 3e8359a..ca1db70 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -135,6 +135,7 @@ src/util/virpidfile.c
src/util/virsocketaddr.c
src/util/virterror.c
src/util/virtime.c
+src/util/virtypedparam.c
src/util/xml.c
src/vbox/vbox_MSCOMGlue.c
src/vbox/vbox_XPCOMCGlue.c
diff --git a/src/Makefile.am b/src/Makefile.am
index 0a1221a..c459f2d 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -87,6 +87,7 @@ UTIL_SOURCES =
\
util/virfile.c util/virfile.h \
util/virnodesuspend.c util/virnodesuspend.h \
util/virpidfile.c util/virpidfile.h \
+ util/virtypedparam.c util/virtypedparam.h \
util/xml.c util/xml.h \
util/virterror.c util/virterror_internal.h \
util/virkeycode.c util/virkeycode.h \
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 0dfd4c1..9a7200b 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1145,7 +1145,6 @@ virStrToLong_ull;
virStrcpy;
virStrncpy;
virTrimSpaces;
-virTypedParameterArrayClear;
virVasprintf;
@@ -1176,6 +1175,13 @@ virFileFdopen;
virFileRewrite;
+# virkeycode.h
+virKeycodeSetTypeFromString;
+virKeycodeSetTypeToString;
+virKeycodeValueFromString;
+virKeycodeValueTranslate;
+
+
# virnetclient.h
virNetClientHasPassFD;
@@ -1390,12 +1396,6 @@ virSetError;
virSetErrorLogPriorityFunc;
virStrerror;
-# virkeycode.h
-virKeycodeSetTypeToString;
-virKeycodeSetTypeFromString;
-virKeycodeValueFromString;
-virKeycodeValueTranslate;
-
# virtime.h
virTimeFieldsNow;
@@ -1410,6 +1410,12 @@ virTimeStringThen;
virTimeStringThenRaw;
+# virtypedparam.h
+virTypedParameterArrayClear;
+virTypedParameterArrayValidate;
+virTypedParameterAssign;
+
+
# xml.h
virXMLChildElementCount;
virXMLParseHelper;
diff --git a/src/util/util.c b/src/util/util.c
index 8663c4d..a60ea21 100644
--- a/src/util/util.c
+++ b/src/util/util.c
@@ -1,7 +1,7 @@
/*
* utils.c: common, generic utility functions
*
- * Copyright (C) 2006-2011 Red Hat, Inc.
+ * Copyright (C) 2006-2012 Red Hat, Inc.
* Copyright (C) 2006 Daniel P. Berrange
* Copyright (C) 2006, 2007 Binary Karma
* Copyright (C) 2006 Shuveb Hussain
@@ -2575,17 +2575,3 @@ or other application using the libvirt API.\n\
return 0;
}
-
-void
-virTypedParameterArrayClear(virTypedParameterPtr params, int nparams)
-{
-int i;
-
-if (!params)
-return;
-
-for (i = 0; i nparams; i++) {
-if (params[i].type == VIR_TYPED_PARAM_STRING)
-VIR_FREE(params[i].value.s);
-}
-}
diff --git a/src/util/util.h b/src/util/util.h
index 977ab6c..bed2315 100644
--- a/src/util/util.h
+++ b/src/util/util.h
@@ -1,7 +1,7 @@
/*
* utils.h: common, generic utility functions
*
- * Copyright (C) 2010-2011 Red Hat, Inc.
+ * Copyright (C) 2010-2012 Red Hat, Inc.
* Copyright (C) 2006, 2007 Binary Karma
* Copyright (C) 2006 Shuveb Hussain
*
@@ -257,6 +257,4 @@ int virEmitXMLWarning(int fd,
const char *name,
const char *cmd) ATTRIBUTE_NONNULL(2)
ATTRIBUTE_NONNULL(3);
-void virTypedParameterArrayClear(virTypedParameterPtr params, int nparams);
-
#endif /* __VIR_UTIL_H__ */
diff --git a/src/util/virtypedparam.c b/src/util/virtypedparam.c
new file mode 100644
index 000..f71aa25
--- /dev/null
+++
Re: [libvirt] [PATCHv2 6/6] error: drop old-style error reporting
On Thu, Jan 19, 2012 at 11:44:46AM -0700, Eric Blake wrote: While we still don't want to enable gcc's new -Wformat-literal warning, I found a rather easy case where the warning could be reduced, by getting rid of obsolete error-reporting practices. This is the last place where we were passing the (unused) net and conn arguments for constructing an error. * src/util/virterror_internal.h (virErrorMsg): Delete prototype. (virReportError): Delete macro. * src/util/virterror.c (virErrorMsg): Make static. * src/libvirt_private.syms (virterror_internal.h): Drop export. * src/util/conf.c (virConfError): Convert to macro. (virConfErrorHelper): New function, and adjust error calls. * src/xen/xen_hypervisor.c (virXenErrorFunc): Delete. (xenHypervisorGetSchedulerType) (xenHypervisorGetSchedulerParameters) (xenHypervisorSetSchedulerParameters) (xenHypervisorDomainBlockStats) (xenHypervisorDomainInterfaceStats) (xenHypervisorDomainGetOSType) (xenHypervisorNodeGetCellsFreeMemory, xenHypervisorGetVcpus): Update callers. --- src/libvirt_private.syms |1 - src/util/conf.c | 22 ++--- src/util/virterror.c |2 +- src/util/virterror_internal.h |8 -- src/xen/xen_hypervisor.c | 164 5 files changed, 76 insertions(+), 121 deletions(-) ACK, I didn't realize we still had code using this. 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] [PATCHv2 1/6] threads: check for failure to set thread-local value
On Thu, Jan 19, 2012 at 11:44:41AM -0700, Eric Blake wrote: We had a memory leak on a very arcane OOM situation (unlikely to ever hit in practice, but who knows if libvirt.so would ever be linked into some other program that exhausts all thread-local storage keys?). I found it by code inspection, while analyzing a valgrind report generated by Alex Jia. * src/util/threads.h (virThreadLocalSet): Alter signature. * src/util/threads-pthread.c (virThreadHelper): Reduce allocation lifetime. (virThreadLocalSet): Detect failure. * src/util/threads-win32.c (virThreadLocalSet): Likewise. (virCondWait): Fix caller. * src/util/virterror.c (virLastErrorObject): Likewise. --- src/util/threads-pthread.c | 14 +++--- src/util/threads-win32.c |9 ++--- src/util/threads.h |2 +- src/util/virterror.c |5 +++-- 4 files changed, 21 insertions(+), 9 deletions(-) ACK Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 0/6] various cleanups
I think I've posted all of these before, but with no reviews yet. Eric Blake (6): threads: check for failure to set thread-local value build: silence some compiler warnings from gnulib lxc: use live/config helper util: add new file for virTypedParameter utils util: use new virTypedParameter helpers error: drop old-style error reporting daemon/remote.c |1 + m4/virt-compile-warnings.m4 |7 +- po/POTFILES.in|1 + src/Makefile.am |1 + src/esx/esx_driver.c | 81 +++--- src/libvirt_private.syms | 21 +- src/libxl/libxl_driver.c | 44 +-- src/lxc/lxc_driver.c | 370 ++- src/qemu/qemu_driver.c| 656 + src/test/test_driver.c| 27 +- src/util/conf.c | 22 +- src/util/threads-pthread.c| 14 +- src/util/threads-win32.c |9 +- src/util/threads.h|2 +- src/util/util.c | 16 +- src/util/util.h |4 +- src/util/virterror.c |7 +- src/util/virterror_internal.h |8 - src/util/virtypedparam.c | 187 src/util/virtypedparam.h | 37 +++ src/xen/xen_hypervisor.c | 215 ++ tools/virsh.c |1 + 22 files changed, 721 insertions(+), 1010 deletions(-) create mode 100644 src/util/virtypedparam.c create mode 100644 src/util/virtypedparam.h -- 1.7.7.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 3/6] lxc: use live/config helper
On Thu, Jan 19, 2012 at 11:44:43AM -0700, Eric Blake wrote: Based on qemu changes made in commits ae523427 and 659ded58. * src/lxc/lxc_driver.c (lxcSetSchedulerParametersFlags) (lxcGetSchedulerParametersFlags, lxcDomainSetBlkioParameters) (lxcDomainGetBlkioParameters): Use helpers. (lxcDomainSetBlkioParameters): Allow setting live and config at once. --- src/lxc/lxc_driver.c | 160 +- 1 files changed, 29 insertions(+), 131 deletions(-) ACK Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 2/6] build: silence some compiler warnings from gnulib
On Thu, Jan 19, 2012 at 11:44:42AM -0700, Eric Blake wrote: Gnulib claims that there are some classes of warnings that are worth enabling during development, but where silencing those warnings causes code bloat that is not necessary in an optimized build. The code bloat to silence the warnings is only enabled by -Dlint. Follow the lead of coreutils in setting up -Dlint whenever full warnings are requested. * m4/virt-compile-warnings.m4 (LIBVIRT_COMPILE_WARNINGS): Add -Dlint, and move _FORTIFY_SOURCE to config.h instead of CFLAGS. --- m4/virt-compile-warnings.m4 |7 ++- 1 files changed, 6 insertions(+), 1 deletions(-) diff --git a/m4/virt-compile-warnings.m4 b/m4/virt-compile-warnings.m4 index ba388aa..3a428c3 100644 --- a/m4/virt-compile-warnings.m4 +++ b/m4/virt-compile-warnings.m4 @@ -100,8 +100,13 @@ AC_DEFUN([LIBVIRT_COMPILE_WARNINGS],[ gl_WARN_ADD([-Wframe-larger-than=4096]) dnl gl_WARN_ADD([-Wframe-larger-than=256]) +# Silence certain warnings in gnulib, and use improved glibc headers +AC_DEFINE([lint], [1], + [Define to 1 if the compiler is checking for lint.]) +AC_DEFINE([_FORTIFY_SOURCE], [2], + [enable compile-time and run-time bounds-checking, and some warnings]) + # Extra special flags -gl_WARN_ADD([-Wp,-D_FORTIFY_SOURCE=2]) dnl -fstack-protector stuff passes gl_WARN_ADD with gcc dnl on Mingw32, but fails when actually used case $host in ACK Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 4/6] util: add new file for virTypedParameter utils
On Thu, Jan 19, 2012 at 11:44:44AM -0700, Eric Blake wrote: Preparation for another patch that refactors common patterns into the new file for fewer lines of code overall. * src/util/util.h (virTypedParameterArrayClear): Move... * src/util/virtypedparam.h: ...to new file. (virTypedParameterArrayValidate, virTypedParameterAssign): New prototypes. * src/util/util.c (virTypedParameterArrayClear): Likewise. * src/util/virtypedparam.c: New file. * po/POTFILES.in: Mark file for translation. * src/Makefile.am (UTIL_SOURCES): Build it. * src/libvirt_private.syms (util.h): Split... (virtypedparam.h): to new section. (virkeycode.h): Sort. * daemon/remote.c: Adjust callers. * tools/virsh.c: Likewise. --- daemon/remote.c |1 + po/POTFILES.in |1 + src/Makefile.am |1 + src/libvirt_private.syms | 20 +++-- src/util/util.c | 16 + src/util/util.h |4 +- src/util/virtypedparam.c | 187 ++ src/util/virtypedparam.h | 37 + tools/virsh.c|1 + 9 files changed, 243 insertions(+), 25 deletions(-) create mode 100644 src/util/virtypedparam.c create mode 100644 src/util/virtypedparam.h ACK + +VIR_ENUM_DECL(virTypedParameter) +VIR_ENUM_IMPL(virTypedParameter, VIR_TYPED_PARAM_STRING + 1, I'm slowly coming to the view that we should juust add _LAST to every single one of our public enums. But perhaps have them disabled by default, unless the app developer does #define LIBVIRT_ENUM_SENTINALS before including libvirt.h, so they make a concious decision to use a enum value known to change. 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] [PATCHv2 5/6] util: use new virTypedParameter helpers
On Thu, Jan 19, 2012 at 11:44:45AM -0700, Eric Blake wrote:
Reusing common code makes things smaller; it also buys us some
additional safety, such as now rejecting duplicate parameters
during a set operation.
* src/qemu/qemu_driver.c (qemuDomainSetBlkioParameters)
(qemuDomainSetMemoryParameters, qemuDomainSetNumaParameters)
(qemuSetSchedulerParametersFlags)
(qemuDomainSetInterfaceParameters, qemuDomainSetBlockIoTune)
(qemuDomainGetBlkioParameters, qemuDomainGetMemoryParameters)
(qemuDomainGetNumaParameters, qemuGetSchedulerParametersFlags)
(qemuDomainBlockStatsFlags, qemuDomainGetInterfaceParameters)
(qemuDomainGetBlockIoTune): Use new helpers.
* src/esx/esx_driver.c (esxDomainSetSchedulerParametersFlags)
(esxDomainSetMemoryParameters)
(esxDomainGetSchedulerParametersFlags)
(esxDomainGetMemoryParameters): Likewise.
* src/libxl/libxl_driver.c
(libxlDomainSetSchedulerParametersFlags)
(libxlDomainGetSchedulerParametersFlags): Likewise.
* src/lxc/lxc_driver.c (lxcDomainSetMemoryParameters)
(lxcSetSchedulerParametersFlags, lxcDomainSetBlkioParameters)
(lxcDomainGetMemoryParameters, lxcGetSchedulerParametersFlags)
(lxcDomainGetBlkioParameters): Likewise.
* src/test/test_driver.c (testDomainSetSchedulerParamsFlags)
(testDomainGetSchedulerParamsFlags): Likewise.
* src/xen/xen_hypervisor.c (xenHypervisorSetSchedulerParameters)
(xenHypervisorGetSchedulerParameters): Likewise.
---
src/esx/esx_driver.c | 81 +++---
src/libxl/libxl_driver.c | 44 +---
src/lxc/lxc_driver.c | 218 +---
src/qemu/qemu_driver.c | 656
++
src/test/test_driver.c | 27 +--
src/xen/xen_hypervisor.c | 51 ++--
6 files changed, 350 insertions(+), 727 deletions(-)
@@ -2968,42 +2922,25 @@ lxcGetSchedulerParametersFlags(virDomainPtr dom,
if (cpu_bw_status) {
if (*nparams saved_nparams) {
-params[1].value.ul = period;
-params[1].type = VIR_TYPED_PARAM_ULLONG;
-if (virStrcpyStatic(params[1].field,
-VIR_DOMAIN_SCHEDULER_VCPU_PERIOD) == NULL) {
-lxcError(VIR_ERR_INTERNAL_ERROR,
- _(Field name '%s' too long),
- VIR_DOMAIN_SCHEDULER_VCPU_PERIOD);
+if (virTypedParameterAssign(params[1],
+VIR_DOMAIN_SCHEDULER_VCPU_PERIOD,
+VIR_TYPED_PARAM_ULLONG, shares) 0)
s/shares/period/
@@ -6201,21 +6169,11 @@ static int qemuDomainGetBlkioParameters(virDomainPtr
dom,
}
param-value.s = virBufferContentAndReset(buf);
}
-if (!param-value.s) {
-param-value.s = strdup();
-if (!param-value.s) {
-virReportOOMError();
-goto cleanup;
-}
-}
-param-type = VIR_TYPED_PARAM_STRING;
-if (virStrcpyStatic(param-field,
-VIR_DOMAIN_BLKIO_DEVICE_WEIGHT) == NULL)
{
-qemuReportError(VIR_ERR_INTERNAL_ERROR,
-_(Field name '%s' too long),
-VIR_DOMAIN_BLKIO_DEVICE_WEIGHT);
+if (virTypedParameterAssign(param,
+VIR_DOMAIN_BLKIO_DEVICE_WEIGHT,
+VIR_TYPED_PARAM_STRING,
+param-value.s) 0)
goto cleanup;
Is virTypedParameterAssign happy getting a NULL for the string value ?
Previously we would have set for the parameter rather than NULL
ACK with those 2 points resolved
Regards,
Daniel
--
|: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 4/6] util: add new file for virTypedParameter utils
On 01/19/2012 12:01 PM, Daniel P. Berrange wrote: + +VIR_ENUM_DECL(virTypedParameter) +VIR_ENUM_IMPL(virTypedParameter, VIR_TYPED_PARAM_STRING + 1, I'm slowly coming to the view that we should juust add _LAST to every single one of our public enums. But perhaps have them disabled by default, unless the app developer does #define LIBVIRT_ENUM_SENTINALS before including libvirt.h, so they make a concious decision to use a enum value known to change. Sounds like an interesting idea, but I'll save it for a followup. Would it be acceptable to protect existing _LAST values when adding all the new ones, or must we keep those unconditionally defined to avoid breaking compilation of anyone that was using them? -- 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] [PATCHv2 5/6] util: use new virTypedParameter helpers
On 01/19/2012 12:21 PM, Daniel P. Berrange wrote:
On Thu, Jan 19, 2012 at 11:44:45AM -0700, Eric Blake wrote:
Reusing common code makes things smaller; it also buys us some
additional safety, such as now rejecting duplicate parameters
during a set operation.
@@ -2968,42 +2922,25 @@ lxcGetSchedulerParametersFlags(virDomainPtr dom,
if (cpu_bw_status) {
if (*nparams saved_nparams) {
-params[1].value.ul = period;
-params[1].type = VIR_TYPED_PARAM_ULLONG;
-if (virStrcpyStatic(params[1].field,
-VIR_DOMAIN_SCHEDULER_VCPU_PERIOD) == NULL) {
-lxcError(VIR_ERR_INTERNAL_ERROR,
- _(Field name '%s' too long),
- VIR_DOMAIN_SCHEDULER_VCPU_PERIOD);
+if (virTypedParameterAssign(params[1],
+VIR_DOMAIN_SCHEDULER_VCPU_PERIOD,
+VIR_TYPED_PARAM_ULLONG, shares) 0)
s/shares/period/
Thanks for catching that (hard to find minor mistakes like this in a big
repetitive patch). Fixed.
@@ -6201,21 +6169,11 @@ static int qemuDomainGetBlkioParameters(virDomainPtr
dom,
}
param-value.s = virBufferContentAndReset(buf);
}
-if (!param-value.s) {
-param-value.s = strdup();
-if (!param-value.s) {
-virReportOOMError();
-goto cleanup;
-}
-}
-param-type = VIR_TYPED_PARAM_STRING;
-if (virStrcpyStatic(param-field,
-VIR_DOMAIN_BLKIO_DEVICE_WEIGHT) ==
NULL) {
-qemuReportError(VIR_ERR_INTERNAL_ERROR,
-_(Field name '%s' too long),
-VIR_DOMAIN_BLKIO_DEVICE_WEIGHT);
+if (virTypedParameterAssign(param,
+VIR_DOMAIN_BLKIO_DEVICE_WEIGHT,
+VIR_TYPED_PARAM_STRING,
+param-value.s) 0)
goto cleanup;
Is virTypedParameterAssign happy getting a NULL for the string value ?
Previously we would have set for the parameter rather than NULL
Yes, I specifically documented and implemented it that way:
/* Assign name, type, and the appropriately typed arg to param; in the
* case of a string, the caller is assumed to have malloc'd a string,
* or can pass NULL to have this function malloc an empty string.
* Return 0 on success, -1 after an error message on failure. */
as it made life simpler in the callers to pass NULL rather than worrying
about OOM checks.
ACK with those 2 points resolved
Thanks for the speedy review; series pushed.
--
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] [PATCHv2] network: Avoid memory leaks on networkBuildDnsmasqArgv
On 01/18/2012 03:04 AM, a...@redhat.com wrote:
From: Alex Jia a...@redhat.com
Detected by valgrind. Leaks introduced in commit 973af236.
* src/network/bridge_driver.c: fix memory leaks on failure and successful
path.
* How to reproduce?
% make -C tests check TESTS=networkxml2argvtest
% cd tests valgrind -v --leak-check=full ./networkxml2argvtest
src/network/bridge_driver.c |9 ++---
1 files changed, 6 insertions(+), 3 deletions(-)
Needs a v3.
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 5d0d528..5bd5a50 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -459,6 +459,9 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network,
int r, ret = -1;
int nbleases = 0;
int ii;
+char *recordPort = NULL;
+char *recordPriority = NULL;
+char *recordWeight = NULL;
virNetworkIpDefPtr tmpipdef;
Moving the declaration here, and a cleanup at the end, will only free
_one_ instance of allocation.
/*
@@ -530,9 +533,6 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network,
for (i = 0; i dns-nsrvrecords; i++) {
char *record = NULL;
-char *recordPort = NULL;
-char *recordPriority = NULL;
-char *recordWeight = NULL;
But these values were allocated in a for loop, so if there is more than
one nsrvrecords, then you are leaking on each iteration of the loop.
if (dns-srvrecords[i].service dns-srvrecords[i].protocol) {
if (dns-srvrecords[i].port) {
@@ -671,6 +671,9 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network,
ret = 0;
cleanup:
+VIR_FREE(recordPort);
+VIR_FREE(recordWeight);
+VIR_FREE(recordPriority);
You need to also copy these three lines into the end of the for loop.
--
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] BlockJob: Support sync/async virDomainBlockJobAbort
On Wed, Jan 18, 2012 at 12:59:29PM -0500, Federico Simoncelli wrote: - Original Message - From: Adam Litke a...@us.ibm.com To: Dave Allan dal...@redhat.com Cc: libvir-list@redhat.com Sent: Friday, January 13, 2012 9:51:23 PM Subject: [libvirt] [PATCH] BlockJob: Support sync/async virDomainBlockJobAbort Qemu has changed the semantics of the block_job_cancel API. Originally, the operation was synchronous (ie. upon command completion, the operation was guaranteed to be completely stopped). With the new semantics, a block_job_cancel merely requests that the operation be cancelled and an event is triggered once the cancellation request has been honored. To adopt the new semantics while preserving compatibility I propose the following updates to the virDomainBlockJob API: A new block job event type VIR_DOMAIN_BLOCK_JOB_CANCELLED will be recognized by libvirt. Regardless of the flags used with virDomainBlockJobAbort, this event will be raised whenever it is received from qemu. This event indicates that a block job has been successfully cancelled. [...] Without the VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC flag, libvirt will internally poll using qemu's query-block-jobs API and will not return until the operation has been completed. Why do you raise the event VIR_DOMAIN_BLOCK_JOB_CANCELLED also in the case where libvirt takes care of it internally? Is there a specific use case for this? We just raise the events we receive from qemu and qemu will always generate these. IMO, there is no harm in always raising the events. If a user doesn't care about them, then they don't have to register for notifications. API users are advised that this operation is unbounded and further interaction with the domain during this period may block. Do you have an example of what operation may block? Is this the common behavior for all the other async tasks that libvirt is managing internally? Any operation that needs to issue a monitor command would block. This is a large list. That being said, the existing implementation already has this potential issue with the ASYNC flag being the workaround. I cannot answer definitively as to whether all libvirt async tasks can block. I suspect the answer depends on whether the operation relies on a monitor command that can block in qemu. As far as I know the only other command like this (coincidentally it is being improved in qemu by Luiz) is virDomainMemoryStats(). -- Adam Litke a...@us.ibm.com IBM Linux Technology Center -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 0/5] RFC: grant KVM guests retain arbitrary capabilities
On Thu, Jan 19, 2012 at 01:32:08PM -0700, Eric Blake wrote: On 01/18/2012 12:38 AM, Taku Izumi wrote: I am now wondering if we should do this in a different way. ie if there is some XML configuration parameter for the disk that indicates the need for rawio, then libvirt could automatically ensures that we add CAP_SYS_RAWIO when starting QEMU. I see. You say if a guest has the following XML configuration, CAP_SYS_RAWIO capability is automatically added to it, right? disk type='block' device='lun' Yes, that actually seems reasonable, especially since device='lun' is brand new, and so far, is really the only reason that we'd need CAP_SYS_RAWIO. Does device='lun' really require CAP_SYS_RAWIO ? I hadn't seen that mentioned before now 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] [PATCHv2 4/6] util: add new file for virTypedParameter utils
On Thu, Jan 19, 2012 at 01:16:01PM -0700, Eric Blake wrote: On 01/19/2012 12:01 PM, Daniel P. Berrange wrote: + +VIR_ENUM_DECL(virTypedParameter) +VIR_ENUM_IMPL(virTypedParameter, VIR_TYPED_PARAM_STRING + 1, I'm slowly coming to the view that we should juust add _LAST to every single one of our public enums. But perhaps have them disabled by default, unless the app developer does #define LIBVIRT_ENUM_SENTINALS before including libvirt.h, so they make a concious decision to use a enum value known to change. Sounds like an interesting idea, but I'll save it for a followup. Sure, no problem. Would it be acceptable to protect existing _LAST values when adding all the new ones, or must we keep those unconditionally defined to avoid breaking compilation of anyone that was using them? I think we could get away with protecting existing ones too, since apps can easily just add the #define, even when building with older libvirt and it'll be harmless Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 0/5] RFC: grant KVM guests retain arbitrary capabilities
On 01/19/2012 02:10 PM, Daniel P. Berrange wrote: On Thu, Jan 19, 2012 at 01:32:08PM -0700, Eric Blake wrote: On 01/18/2012 12:38 AM, Taku Izumi wrote: I am now wondering if we should do this in a different way. ie if there is some XML configuration parameter for the disk that indicates the need for rawio, then libvirt could automatically ensures that we add CAP_SYS_RAWIO when starting QEMU. I see. You say if a guest has the following XML configuration, CAP_SYS_RAWIO capability is automatically added to it, right? disk type='block' device='lun' Yes, that actually seems reasonable, especially since device='lun' is brand new, and so far, is really the only reason that we'd need CAP_SYS_RAWIO. Does device='lun' really require CAP_SYS_RAWIO ? I hadn't seen that mentioned before now My understanding (and hopefully Paolo corrects me if I'm wrong) is that some, but not all, SG_IO ioctl usage patterns require CAP_SYS_RAWIO; using device='lun' is what allows any SG_IO ioctl to pass through in the first place, but then there is further validation based on the capabilities. So maybe this calls for an additional xml attribute, only needed for device='lun', that controls whether the cap is also desired when accessing the pass-through block device: disk type='block' device='lun' rawio='enabled' rather than always blindly granting CAP_SYS_RAWIO merely because of the presence of device='lun'. -- 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 1/4] QEMU guest agent support
On 01/17/2012 04:44 AM, Michal Privoznik wrote:
There is now a standard QEMU guest agent that can be installed
and given a virtio serial channel
channel type='unix'
source mode='bind' path='/var/lib/libvirt/qemu/f16x86_64.agent'/
Do we really want to be documenting a path in the libvirt-controlled
hierarchy?
Or is this something that we should be automatically creating on the
user's behalf if it is apparent that qemu supports it, rather than
requiring the user to modify their XML to add it? At which point, it
would _not_ be part of 'virsh dumpxml' output, but would only be visible
in the /var/run/libvirt/qemu/dom.xml runtime state (similar to how
domstatus/monitor is the chardev element automatically created for
the monitor)?
target type='virtio' name='org.qemu.guest_agent.0'/
/channel
The protocol that runs over the guest agent is JSON based and
very similar to the JSON monitor. We can't use exactly the same
code because there are some odd differences in the way messages
and errors are structured. The qemu_agent.c file is based on
a combination and simplification of qemu_monitor.c and
qemu_monitor_json.c
* src/qemu/qemu_agent.c, src/qemu/qemu_agent.h: Support for
talking to the agent for shutdown
* src/qemu/qemu_domain.c, src/qemu/qemu_domain.h: Add thread
helpers for talking to the agent
* src/qemu/qemu_process.c: Connect to agent whenever starting
a guest
* src/qemu/qemu_monitor_json.c: Make variable static
+struct _qemuAgentMessage {
+char *txBuffer;
+int txOffset;
+int txLength;
+
+/* Used by the text monitor reply / error */
+char *rxBuffer;
+int rxLength;
This comment is misleading, since we are a JSON monitor and not a text
monitor.
+/* Used by the JSON monitor to hold reply / error */
+void *rxObject;
+
+/* True if rxBuffer / rxObject are ready, or a
+ * fatal error occurred on the monitor channel
+ */
+bool finished;
+};
+
+
+#if DEBUG_RAW_IO
+# include c-ctype.h
+static char * qemuAgentEscapeNonPrintable(const char *text)
Formatting - I'd do this in two lines:
static char *
qemuAgentEscapeNonPrintable(const char *text)
+{
+int i;
+virBuffer buf = VIR_BUFFER_INITIALIZER;
+for (i = 0 ; text[i] != '\0' ; i++) {
+if (c_isprint(text[i]) ||
+text[i] == '\n' ||
+(text[i] == '\r' text[i+1] == '\n'))
+virBufferAsprintf(buf,%c, text[i]);
+else
+virBufferAsprintf(buf, 0x%02x, text[i]);
That gives ambiguous output, and isn't very efficient. I'd do it more like:
if (text[i] == '\\')
virBufferAddLit(buf, );
else if (c_isprint(text[i]) || text[i] == '\n' ||
(text[i] == '\r' text[i+1] == '\n'))
virBufferAddChar(buf, text[i]);
else
virBufferAsprintf(buf, \\x%02x, text[i]);
+static int
+qemuAgentOpenUnix(const char *monitor, pid_t cpid, bool *inProgress)
+{
+struct sockaddr_un addr;
+int monfd;
+int timeout = 3; /* In seconds */
+int ret, i = 0;
+
+*inProgress = false;
+
+if ((monfd = socket(AF_UNIX, SOCK_STREAM, 0)) 0) {
+virReportSystemError(errno,
+ %s, _(failed to create socket));
+return -1;
+}
+
+if (virSetNonBlock(monfd) 0) {
+qemuReportError(VIR_ERR_INTERNAL_ERROR,
+%s, _(Unable to put monitor into non-blocking
mode));
+goto error;
+}
+
+if (virSetCloseExec(monfd) 0) {
+qemuReportError(VIR_ERR_INTERNAL_ERROR,
+%s, _(Unable to set monitor close-on-exec flag));
+goto error;
+}
You know, if I ever got around to fixing gnulib to guarantee things for
older platforms, we could use socket(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC
| SOCK_NONBLOCK, 0) and shave off a few syscalls. But for now, your
code is the best we can do.
+static int
+qemuAgentOpenPty(const char *monitor)
+{
+int monfd;
+
+if ((monfd = open(monitor, O_RDWR)) 0) {
+qemuReportError(VIR_ERR_INTERNAL_ERROR,
+_(Unable to open monitor path %s), monitor);
+return -1;
+}
+
+if (virSetNonBlock(monfd) 0) {
+qemuReportError(VIR_ERR_INTERNAL_ERROR,
+%s, _(Unable to put monitor into non-blocking
mode));
+goto error;
+}
Why not open(monitor, O_RDWR | O_NONBLOCK), and shave off a syscall here?
+
+if (virSetCloseExec(monfd) 0) {
+qemuReportError(VIR_ERR_INTERNAL_ERROR,
+%s, _(Unable to set monitor close-on-exec flag));
+goto error;
+}
Alas, I also need to find time to make gnulib guarantee open(O_CLOEXEC).
+static int qemuAgentIOProcessData(qemuAgentPtr mon,
+ const char *data,
+ size_t len,
+ qemuAgentMessagePtr msg)
+{
+int used = 0;
Re: [libvirt] [PATCH 2/4] Add new virDomainShutdownFlags API
On 01/17/2012 04:44 AM, Michal Privoznik wrote:
Add a new API virDomainShutdownFlags and define:
VIR_DOMAIN_SHUTDOWN_DEFAULT= 0,
VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN = (1 0),
VIR_DOMAIN_SHUTDOWN_GUEST_AGENT= (1 1),
Also define some flags for the reboot API
VIR_DOMAIN_REBOOT_DEFAULT= 0,
VIR_DOMAIN_REBOOT_ACPI_POWER_BTN = (1 0),
VIR_DOMAIN_REBOOT_GUEST_AGENT= (1 1),
Although these two APIs currently have the same flags, using
separate enums allows them to expand separately in the future.
Fair enough.
Add stub impls of the new API for all existing drivers
---
I might have split this into several patches, but I think doing it in
one go is okay since the stubs are trivial.
include/libvirt/libvirt.h.in | 15 +
src/driver.h |5 +++
src/libvirt.c| 65
+-
src/libvirt_public.syms |4 ++
That is, I might have done part 1 (the public API),
src/remote/remote_driver.c |1 +
src/remote/remote_protocol.x |8 -
src/remote_protocol-structs |5 +++
part 2 (the RPC),
src/esx/esx_driver.c | 11 ++-
src/libxl/libxl_driver.c | 12 +++-
src/openvz/openvz_driver.c |1 +
src/test/test_driver.c | 11 ++-
src/uml/uml_driver.c | 11 ++-
src/vbox/vbox_tmpl.c | 11 ++-
src/vmware/vmware_driver.c |1 +
src/xen/xen_driver.c | 14 -
src/xenapi/xenapi_driver.c | 12 +++-
and part 3 (the stubs).
+++ b/include/libvirt/libvirt.h.in
@@ -1148,7 +1148,22 @@ virDomainPtrvirDomainLookupByUUID
(virConnectPtr conn,
virDomainPtrvirDomainLookupByUUIDString (virConnectPtr conn,
const char *uuid);
+typedef enum {
+VIR_DOMAIN_SHUTDOWN_DEFAULT= 0,
+VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN = (1 0),
+VIR_DOMAIN_SHUTDOWN_GUEST_AGENT= (1 1),
+} virDomainShutdownFlagValues;
No documentation comments? Even a /** doc */ prior to the typedef that
mentions virDomainShutdownFlagValues might be helpful to the doc
generation process.
+++ b/src/libvirt.c
@@ -3106,14 +3106,77 @@ error:
}
/**
+ * virDomainShutdownFlags:
+ * @domain: a domain object
+ * @flags: bitwise-OR of virDomainShutdownFlagValues
+ *
+ * Shutdown a domain, the domain object is still usable thereafter but
+ * the domain OS is being stopped. Note that the guest OS may ignore the
+ * request. For guests that react to a shutdown request, the differences
+ * from virDomainDestroy() are that the guest's disk storage will be in a
+ * stable state rather than having the (virtual) power cord pulled, and
+ * this command returns as soon as the shutdown request is issued rather
+ * than blocking until the guest is no longer running.
+ *
+ * If the domain is transient and has any snapshot metadata (see
+ * virDomainSnapshotNum()), then that metadata will automatically
+ * be deleted when the domain quits.
+ *
+ * If @flags is set to zero, then the hypervisor will chose the
+ * method of shutdown it considers best. To have greater control
+ * pass one of the virDomainShutdownFlagValues.
Maybe mention the existing flag names:
Use VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN to trigger a shutdown through an
ACPI interrupt, and VIR_DOMAIN_SHUTDOWN_GUEST_AGENT to trigger a
shutdown through a guest agent call (this requires that the domain have
a channel device appropriately wired to a guest agent).
Is it an error if multiple flags are requested at once?
+/**
* virDomainReboot:
* @domain: a domain object
- * @flags: extra flags; not used yet, so callers should always pass 0
+ * @flags: bitwise-OR of virDomainRebootFlagValues
*
* Reboot a domain, the domain object is still usable there after but
* the domain OS is being stopped for a restart.
* Note that the guest OS may ignore the request.
*
+ * If @flags is set to zero, then the hypervisor will chose the
+ * method of shutdown it considers best. To have greater control
+ * pass one of the virDomainRebootFlagValues.
Again, mention the possible flag values, and that an agent request
requires additional support from the XML.
+++ b/src/openvz/openvz_driver.c
@@ -1693,6 +1693,7 @@ static virDriver openvzDriver = {
.domainSuspend = openvzDomainSuspend, /* 0.8.3 */
.domainResume = openvzDomainResume, /* 0.8.3 */
.domainShutdown = openvzDomainShutdown, /* 0.3.1 */
+.domainShutdownFlags = openvzDomainShutdownFlags, /* 0.9.10 */
Hmm. Are we setting ourselves up for issues down the road by sharing
shutdown and destroy implementations for openvz? But it's a
pre-existing problem, not made worse by the patch, and okay as long as
we reject all flag values.
+++ b/src/vmware/vmware_driver.c
@@ -980,6 +980,7 @@ static virDriver vmwareDriver = {
.domainSuspend =
Re: [libvirt] [PATCH 3/4] Wire up QEMU agent to reboot/shutdown APIs
On 01/17/2012 04:44 AM, Michal Privoznik wrote:
This makes use of the QEMU guest agent to implement the
virDomainShutdownFlags and virDomainReboot APIs. With
no flags specified, it will prefer to use the agent, but
fallback to ACPI. Explicit choice can be made by using
a suitable flag
* src/qemu/qemu_driver.c: Wire up use of agent
---
src/qemu/qemu_driver.c | 107 ++-
1 files changed, 86 insertions(+), 21 deletions(-)
@@ -1526,6 +1530,26 @@ static int qemuDomainShutdown(virDomainPtr dom) {
goto cleanup;
}
+priv = vm-privateData;
+
+if ((flags VIR_DOMAIN_SHUTDOWN_GUEST_AGENT) ||
+(!(flags VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN)
+ priv-agent))
+useAgent = true;
Should we reject things if the user passes both flags? Or if not,
+
+if (useAgent) {
+if (priv-agentError) {
+qemuReportError(VIR_ERR_INTERNAL_ERROR, %s,
+_(QEMU guest agent is not available due to an
error));
+goto endjob;
+}
+if (!priv-agent) {
+qemuReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, %s,
+_(QEMU guest agent is not configured));
+goto endjob;
+}
+}
if the user passes both flags, but the agent had an error or is not
present, do we silently fall back to acpi?
@@ -1575,22 +1610,54 @@ static int qemuDomainReboot(virDomainPtr dom,
unsigned int flags) {
priv = vm-privateData;
-if (qemuCapsGet(priv-qemuCaps, QEMU_CAPS_MONITOR_JSON)) {
-if (!qemuCapsGet(priv-qemuCaps, QEMU_CAPS_NO_SHUTDOWN)) {
+if ((flags VIR_DOMAIN_SHUTDOWN_GUEST_AGENT) ||
+(!(flags VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN)
+ priv-agent))
+useAgent = true;
+
+if (useAgent) {
+if (priv-agentError) {
+qemuReportError(VIR_ERR_INTERNAL_ERROR, %s,
+_(QEMU guest agent is not available due to an
error));
+goto cleanup;
+}
+if (!priv-agent) {
+qemuReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, %s,
+_(QEMU guest agent is not configured));
+goto cleanup;
+}
Same questions.
@@ -11853,6 +11917,7 @@ static virDriver qemuDriver = {
.domainSuspend = qemudDomainSuspend, /* 0.2.0 */
.domainResume = qemudDomainResume, /* 0.2.0 */
.domainShutdown = qemuDomainShutdown, /* 0.2.0 */
+.domainShutdownFlags = qemuDomainShutdownFlags, /* 0.9.7 */
0.9.10
--
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/4] Allow choice of shutdown method via virsh
On 01/17/2012 04:44 AM, Michal Privoznik wrote: Extend the 'shutdown' and 'reboot' methods so that they both accept a new argument --mode acpi|agent * tools/virsh.c: New args for shutdown/reboot * tools/virsh.pod: Document new args --- tools/virsh.c | 47 +-- tools/virsh.pod | 24 2 files changed, 61 insertions(+), 10 deletions(-) @@ -1796,9 +1804,9 @@ Edit the XML configuration file for a storage pool. This is equivalent to: - virsh pool-dumpxml pool pool.xml - vi pool.xml (or make changes with your other text editor) - virsh pool-define pool.xml +virsh pool-dumpxml pool pool.xml +vi pool.xml (or make changes with your other text editor) +virsh pool-define pool.xml except that it does some error checking. @@ -1853,9 +1861,9 @@ pre-existing volume. BExample - virsh vol-dumpxml --pool storagepool1 appvolume1 newvolume.xml - vi newvolume.xml (or make changes with your other text editor) - virsh vol-create differentstoragepool newvolume.xml +virsh vol-dumpxml --pool storagepool1 appvolume1 newvolume.xml +vi newvolume.xml (or make changes with your other text editor) +virsh vol-create differentstoragepool newvolume.xml These two whitespace-changing hunks look spurious. ACK to the rest of the patch. -- 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] BlockJob: Support sync/async virDomainBlockJobAbort
On 01/13/2012 01:51 PM, Adam Litke wrote:
Qemu has changed the semantics of the block_job_cancel API. Originally, the
operation was synchronous (ie. upon command completion, the operation was
guaranteed to be completely stopped). With the new semantics, a
block_job_cancel merely requests that the operation be cancelled and an
event
is triggered once the cancellation request has been honored.
How can we tell the two implementations of qemu apart? That is, how is
libvirt supposed to know whether it is calling the old (blocking)
variant, or the new async variant, in order to know whether it should
wait for an event? On the other hand, I think we can make things work:
call cancel, then call job status
if the job is complete, then we are done, whether or not we have old or
new command semantics (and if we have new command semantics, be prepared
to ignore an event from qemu)
if the job is not complete, the we have the new semantics, and we know
to expect the event
coupled with:
if the user passed 0 for the flag (they want blocking), then make the
command wait for completion (either by job-status query or by getting
the event), and no event is passed to the user
if the user passed ASYNC for the flag, then they want an event; if we
detect the async variant of the command, then we hook up our qemu event
handler to turn around and issue a user event. Conversely, if job-query
says the command is complete, then we generate our own event without
waiting for a qemu event.
That means that we _don't_ blindly pass the qemu event on to the user;
rather, on reception of a qemu event, we determine if we have an
incomplete cancel in flight, and only then do we pass it on to the user;
otherwise, we assume that the qemu event came after we already saw the
job-info query saying things were complete, and therefore, we had
already generated the user's event and can ignore the qemu event.
It would help things if the new qemu command were spelled with a
different name than the old blocking version.
To adopt the new semantics while preserving compatibility I propose the
following updates to the virDomainBlockJob API:
A new block job event type VIR_DOMAIN_BLOCK_JOB_CANCELLED will be recognized
by
libvirt. Regardless of the flags used with virDomainBlockJobAbort, this event
will be raised whenever it is received from qemu. This event indicates that a
block job has been successfully cancelled.
I'm not sure it should be raised unconditionally on receipt of a qemu
event, but only in the case of a virDomainBlockJobAbort(ASYNC) where we
detect that cancel is in flight when we return control to the user.
A new extension flag VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC will be added to the
virDomainBlockJobAbort API. When enabled, this function will operate
asynchronously (ie, it can return before the job has actually been cancelled).
When the API is used in this mode, it is the responsibility of the caller to
wait for a VIR_DOMAIN_BLOCK_JOB_CANCELLED event or poll via the
virDomainGetBlockJobInfo API to check the cancellation status.
Agreed.
Without the VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC flag, libvirt will internally
poll
using qemu's query-block-jobs API and will not return until the operation
has
been completed. API users are advised that this operation is unbounded and
further interaction with the domain during this period may block.
Agreed.
This patch implements the new event type, the API flag, and the polling. The
main outstanding issue is whether we should bound the amount of time we will
wait for cancellation and return an error.
I'm almost wondering if we should break this patch into a series of
smaller patches, but I'll go ahead and review it intact.
Comments on this proposal?
Signed-off-by: Adam Litke a...@us.ibm.com
Cc: Stefan Hajnoczi stefa...@gmail.com
Cc: Eric Blake ebl...@redhat.com
---
include/libvirt/libvirt.h.in | 10 ++
src/libvirt.c|9 -
src/qemu/qemu_driver.c | 24 +---
src/qemu/qemu_monitor.c | 24
src/qemu/qemu_monitor.h |2 ++
src/qemu/qemu_monitor_json.c | 36 +---
6 files changed, 90 insertions(+), 15 deletions(-)
Your patch adds trailing whitespace :( 'make syntax-check' would
diagnose this, and other issues.
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index e436f3c..fc7f028 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -1776,6 +1776,15 @@ typedef enum {
VIR_DOMAIN_BLOCK_JOB_TYPE_PULL = 1,
} virDomainBlockJobType;
+/**
+ * virDomainBlockJobAbortFlags:
+ *
+ * VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC: Request only, do not wait for completion
+ */
+typedef enum {
+VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC = 1,
I'd write this as (1 0), to make it easier to add new flags in the
future as (1 1) and so forth.
+++ b/src/libvirt.c
Re: [libvirt] [PATCH] BlockJob: Support sync/async virDomainBlockJobAbort
On 01/19/2012 02:00 PM, Adam Litke wrote: Without the VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC flag, libvirt will internally poll using qemu's query-block-jobs API and will not return until the operation has been completed. Why do you raise the event VIR_DOMAIN_BLOCK_JOB_CANCELLED also in the case where libvirt takes care of it internally? Is there a specific use case for this? We just raise the events we receive from qemu and qemu will always generate these. IMO, there is no harm in always raising the events. If a user doesn't care about them, then they don't have to register for notifications. I guess I reviewed under the assumption that you'd only raise the libvirt event if the user is expecting it; but I guess I can see your point of always raising the libvirt event when the qemu event happens. This works fine for the new semantics of the qemu command. The problem is the when the user has the old qemu that only had the blocking semantics, but requests ASYNC operation, at which point, they still expect the libvirt event. Which means that libvirt must raise the event by itself; but now if the user upgrades to new qemu, and we hit the race where libvirt can't tell whether the command completed because it was the old blocking version or whether it was async but completed before we queried, then converting the qemu event to a libvirt event _and_ having libvirt raise the event itself would result in double events to the user. Hence, I still think that being smarter and only converting qemu event to libvirt event when there is a pending async cancel, and discarding it otherwise, is the way to go. API users are advised that this operation is unbounded and further interaction with the domain during this period may block. Do you have an example of what operation may block? Is this the common behavior for all the other async tasks that libvirt is managing internally? Any operation that needs to issue a monitor command would block. This is a large list. That being said, the existing implementation already has this potential issue with the ASYNC flag being the workaround. I cannot answer definitively as to whether all libvirt async tasks can block. I suspect the answer depends on whether the operation relies on a monitor command that can block in qemu. As far as I know the only other command like this (coincidentally it is being improved in qemu by Luiz) is virDomainMemoryStats(). Jiri Denemark worked on some code to classify qemu monitor commands into categories, so that when an async monitor command is in effect, other safe commands can be done in the meantime between times that the async job regains control to issue another monitor command to poll its status. This would involve grabbing an async job around the time where we wait for job cancellation to complete, and dropping the lock each time we sleep until the next poll. Waiting for migration is an example of setting up an async job. But I'm okay if we save that for future improvements (and get Jirka's help), rather than trying to complicate this initial implementation with the additional semantics of using an async monitor job. -- 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] [test-API][PATCH 2/2] Fix problem of a logger instance
---
utils/Python/format.py |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/utils/Python/format.py b/utils/Python/format.py
index 5de8eb0..9c119dd 100644
--- a/utils/Python/format.py
+++ b/utils/Python/format.py
@@ -33,7 +33,7 @@ class Format(object):
def print_string(self, msg, env_logger):
Only print a simple string
-env_logger(msg)
+env_logger.info(msg)
self.write_log('\n%s' %msg)
def print_start(self, msg, env_logger):
--
1.7.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [test-API][PATCH 1/2] Fix compatibility problem on python 2.4
* switch rpartition with rsplit * adjust the style --- generator.py | 15 +++ 1 files changed, 11 insertions(+), 4 deletions(-) diff --git a/generator.py b/generator.py index 9a2ed06..6108963 100644 --- a/generator.py +++ b/generator.py @@ -124,10 +124,14 @@ class FuncGen(object): for i in range(loop_number): case_ref_name = self.cases_ref_names[i] -pkg_casename = case_ref_name.rpartition(:)[0] -funcname = case_ref_name.rpartition(:)[-1] +pkg_casename = case_ref_name.rsplit(:, 1)[0] +funcname = case_ref_name.rsplit(:, 1)[-1] + +if _clean not in funcname: +cleanoper = 0 +else: +cleanoper = 1 -cleanoper = 0 if _clean not in funcname else 1 if not cleanoper: self.fmt.print_start(pkg_casename, env_logger) @@ -182,7 +186,10 @@ class FuncGen(object): if not cleanoper: self.fmt.print_end(pkg_casename, ret, env_logger) else: -self.fmt.print_string(21* + Done if clean_ret 1 else 21* + Fail, env_logger) +if clean_ret 1: +self.fmt.print_string(21* + Done, env_logger) +else: +self.fmt.print_string(21* + Fail, env_logger) end_time = time.strftime(%Y-%m-%d %H:%M:%S) -- 1.7.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 0/5] RFC: grant KVM guests retain arbitrary capabilities
On Thu, 19 Jan 2012 14:48:41 -0700 Eric Blake ebl...@redhat.com wrote: On 01/19/2012 02:10 PM, Daniel P. Berrange wrote: On Thu, Jan 19, 2012 at 01:32:08PM -0700, Eric Blake wrote: On 01/18/2012 12:38 AM, Taku Izumi wrote: I am now wondering if we should do this in a different way. ie if there is some XML configuration parameter for the disk that indicates the need for rawio, then libvirt could automatically ensures that we add CAP_SYS_RAWIO when starting QEMU. I see. You say if a guest has the following XML configuration, CAP_SYS_RAWIO capability is automatically added to it, right? disk type='block' device='lun' Yes, that actually seems reasonable, especially since device='lun' is brand new, and so far, is really the only reason that we'd need CAP_SYS_RAWIO. Does device='lun' really require CAP_SYS_RAWIO ? I hadn't seen that mentioned before now My understanding (and hopefully Paolo corrects me if I'm wrong) is that some, but not all, SG_IO ioctl usage patterns require CAP_SYS_RAWIO; using device='lun' is what allows any SG_IO ioctl to pass through in the first place, but then there is further validation based on the capabilities. So maybe this calls for an additional xml attribute, only needed for device='lun', that controls whether the cap is also desired when accessing the pass-through block device: disk type='block' device='lun' rawio='enabled' rather than always blindly granting CAP_SYS_RAWIO merely because of the presence of device='lun'. OK. I'll try to implement like this way. -- Taku Izumi izumi.t...@jp.fujitsu.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/2] virsh: extend domif-{get, set}link command to accept target name as a parameter
Other virsh domifXXX commands can accept target name
as a parameter to specify interface. From viewpoint of
consistency, virsh domif-getlink command should accept
target name as a parameter. This patch achieves this.
Signed-off-by: Taku Izumi izumi.t...@jp.fujitsu.com
---
tools/virsh.c | 25 ++---
tools/virsh.pod |3 ++-
2 files changed, 20 insertions(+), 8 deletions(-)
Index: libvirt/tools/virsh.c
===
--- libvirt.orig/tools/virsh.c
+++ libvirt/tools/virsh.c
@@ -1509,7 +1509,10 @@ cmdDomIfGetLink (vshControl *ctl, const
const char *iface = NULL;
int flags = 0;
char *state = NULL;
-char *mac = NULL;
+char *value = NULL;
+unsigned char macaddr[VIR_MAC_BUFLEN];
+const char *element;
+const char *attribute;
bool ret = false;
int i;
char *desc;
@@ -1552,27 +1555,35 @@ cmdDomIfGetLink (vshControl *ctl, const
goto cleanup;
}
+if (virParseMacAddr(iface, macaddr) == 0) {
+element = mac;
+attribute = address;
+} else {
+element = target;
+attribute = dev;
+}
+
/* find interface with matching mac addr */
for (i = 0; i obj-nodesetval-nodeNr; i++) {
cur = obj-nodesetval-nodeTab[i]-children;
while (cur) {
if (cur-type == XML_ELEMENT_NODE
-xmlStrEqual(cur-name, BAD_CAST mac)) {
+xmlStrEqual(cur-name, BAD_CAST element)) {
-mac = virXMLPropString(cur, address);
+value = virXMLPropString(cur, attribute);
-if (STRCASEEQ(mac, iface)){
-VIR_FREE(mac);
+if (STRCASEEQ(value, iface)){
+VIR_FREE(value);
goto hit;
}
-VIR_FREE(mac);
+VIR_FREE(value);
}
cur = cur-next;
}
}
-vshError(ctl, _(Interface with address '%s' not found.), iface);
+vshError(ctl, _(Interface (%s: %s) not found.), element, iface);
goto cleanup;
hit:
Index: libvirt/tools/virsh.pod
===
--- libvirt.orig/tools/virsh.pod
+++ libvirt/tools/virsh.pod
@@ -475,10 +475,11 @@ Modify link state of the domain's virtua
state are up and down. If --persistent is specified, only the persistent
configuration of the domain is modified.
-=item Bdomif-getlink Idomain Iinterface-MAC I--persistent
+=item Bdomif-getlink Idomain Iinterface-device I--persistent
Query link state of the domain's virtual interface. If --persistent
is specified, query the persistent configuration.
+Iinterface-device can be the interface's target name or the MAC address.
=item Bdomiftune Idomain Iinterface-device
[[I--config] [I--live] | [I--current]]
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/2] virsh: extend domif-{get, set}link command to accept target name as a parameter
Other virsh domifXXX commands can accept target name
as a parameter to specify interface. From viewpoint of
consistency, virsh domif-setlink command should accept
target name as a parameter. This patch achieves this.
Signd-off-by: Taku Izumi izumi.t...@jp.fujitsu.com
---
tools/virsh.c | 25 ++---
tools/virsh.pod |3 ++-
2 files changed, 20 insertions(+), 8 deletions(-)
Index: libvirt/tools/virsh.c
===
--- libvirt.orig/tools/virsh.c
+++ libvirt/tools/virsh.c
@@ -1344,8 +1344,11 @@ cmdDomIfSetLink (vshControl *ctl, const
virDomainPtr dom;
const char *iface;
const char *state;
-const char *mac;
+const char *value;
const char *desc;
+unsigned char macaddr[VIR_MAC_BUFLEN];
+const char *element;
+const char *attribute;
bool persistent;
bool ret = false;
unsigned int flags = 0;
@@ -1405,26 +1408,34 @@ cmdDomIfSetLink (vshControl *ctl, const
goto cleanup;
}
+if (virParseMacAddr(iface, macaddr) == 0) {
+element = mac;
+attribute = address;
+} else {
+element = target;
+attribute = dev;
+}
+
/* find interface with matching mac addr */
for (i = 0; i obj-nodesetval-nodeNr; i++) {
cur = obj-nodesetval-nodeTab[i]-children;
while (cur) {
if (cur-type == XML_ELEMENT_NODE
-xmlStrEqual(cur-name, BAD_CAST mac)) {
-mac = virXMLPropString(cur, address);
+xmlStrEqual(cur-name, BAD_CAST element)) {
+value = virXMLPropString(cur, attribute);
-if (STRCASEEQ(mac, iface)) {
-VIR_FREE(mac);
+if (STRCASEEQ(value, iface)) {
+VIR_FREE(value);
goto hit;
}
-VIR_FREE(mac);
+VIR_FREE(value);
}
cur = cur-next;
}
}
-vshError(ctl, _(interface with address '%s' not found), iface);
+vshError(ctl, _(interface (%s: %s) not found), element, iface);
goto cleanup;
hit:
Index: libvirt/tools/virsh.pod
===
--- libvirt.orig/tools/virsh.pod
+++ libvirt/tools/virsh.pod
@@ -469,11 +469,12 @@ BExplanation of fields (fields appear
Get network interface stats for a running domain.
-=item Bdomif-setlink Idomain Iinterface-MAC Istate I--persistent
+=item Bdomif-setlink Idomain Iinterface-device Istate I--persistent
Modify link state of the domain's virtual interface. Possible values for
state are up and down. If --persistent is specified, only the persistent
configuration of the domain is modified.
+Iinterface-device can be the interface's target name or the MAC address.
=item Bdomif-getlink Idomain Iinterface-device I--persistent
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
