[libvirt] [PATCH v3] python: add virDomainGetCPUStats python binding API
dom.getCPUStats(True, 0)
[{'cpu_time': 24699446159L, 'system_time': 1087000L, 'user_time':
95000L}]
dom.getCPUStats(False, 0)
[{'cpu_time': 8535292289L}, {'cpu_time': 1005395355L}, {'cpu_time':
9351766377L}, {'cpu_time': 5813545649L}]
*generator.py Add a new naming rule
*libvirt-override-api.xml The API function description
*libvirt-override.c Implement it.
---
python/generator.py |5 +-
python/libvirt-override-api.xml | 13
python/libvirt-override.c | 147 +++
3 files changed, 164 insertions(+), 1 deletions(-)
diff --git a/python/generator.py b/python/generator.py
index 98072f0..4f95cc9 100755
--- a/python/generator.py
+++ b/python/generator.py
@@ -423,7 +423,7 @@ skip_impl = (
'virDomainGetBlockIoTune',
'virDomainSetInterfaceParameters',
'virDomainGetInterfaceParameters',
-'virDomainGetCPUStats', # not implemented now.
+'virDomainGetCPUStats',
'virDomainGetDiskErrors',
)
@@ -966,6 +966,9 @@ def nameFixup(name, classe, type, file):
elif name[0:19] == virStorageVolLookup:
func = name[3:]
func = string.lower(func[0:1]) + func[1:]
+elif name[0:20] == virDomainGetCPUStats:
+func = name[9:]
+func = string.lower(func[0:1]) + func[1:]
elif name[0:12] == virDomainGet:
func = name[12:]
func = string.lower(func[0:1]) + func[1:]
diff --git a/python/libvirt-override-api.xml b/python/libvirt-override-api.xml
index ab8f33a..26d9902 100644
--- a/python/libvirt-override-api.xml
+++ b/python/libvirt-override-api.xml
@@ -149,6 +149,19 @@
arg name='path' type='char *' info='the path for the block device'/
arg name='flags' type='int' info='flags (unused; pass 0)'/
/function
+function name='virDomainGetCPUStats' file='python'
+ infoExtracts CPU statistics for a running domain. On success it will
+ return a list of data of dictionary type. If boolean total is False, the
+ first element of the list refers to CPU0 on the host, second element is
+ CPU1, and so on. The format of data struct is as follows:
+ [{cpu_time:xxx}, {cpu_time:xxx}, ...]
+ If it is True, it returns total domain CPU statistics in the format of
+ [{cpu_time:xxx, user_time:xxx, system_time:xxx}]/info
+ return type='str *' info='returns a list of dictionary in case of
success, None in case of error'/
+ arg name='domain' type='virDomainPtr' info='pointer to domain object'/
+ arg name='total' type='bool' info='on true, return total domain CPU
statistics, false return per-cpu info'/
+ arg name='flags' type='int' info='flags (unused; pass 0)'/
+/function
function name='virDomainInterfaceStats' file='python'
infoExtracts interface device statistics for a domain/info
return type='virDomainInterfaceStats' info='a tuple of statistics'/
diff --git a/python/libvirt-override.c b/python/libvirt-override.c
index 792cfa3..5920456 100644
--- a/python/libvirt-override.c
+++ b/python/libvirt-override.c
@@ -378,6 +378,152 @@ cleanup:
}
static PyObject *
+libvirt_virDomainGetCPUStats(PyObject *self ATTRIBUTE_UNUSED, PyObject *args)
+{
+virDomainPtr domain;
+PyObject *pyobj_domain, *totalbool;
+PyObject *cpu, *total;
+PyObject *ret = NULL;
+PyObject *error = NULL;
+int ncpus = -1, start_cpu = 0;
+int sumparams = 0, nparams = -1;
+int i, i_retval;
+unsigned int flags, totalflag;
+virTypedParameterPtr params = NULL, cpuparams;
+
+if (!PyArg_ParseTuple(args, (char *)OOi:virDomainGetCPUStats,
+ pyobj_domain, totalbool, flags))
+return NULL;
+domain = (virDomainPtr) PyvirDomain_Get(pyobj_domain);
+
+if (!PyBool_Check(totalbool)) {
+PyErr_Format(PyExc_TypeError,
+The \total\ attribute must be bool);
+return NULL;
+} else {
+/* Hack - Python's definition of Py_True breaks strict
+ * aliasing rules, so can't directly compare
+ */
+PyObject *hacktrue = PyBool_FromLong(1);
+totalflag = hacktrue == totalbool ? 1 : 0;
+Py_DECREF(hacktrue);
+}
+
+if ((ret = PyList_New(0)) == NULL)
+return NULL;
+
+if (!totalflag) {
+LIBVIRT_BEGIN_ALLOW_THREADS;
+ncpus = virDomainGetCPUStats(domain, NULL, 0, 0, 0, flags);
+LIBVIRT_END_ALLOW_THREADS;
+
+if (ncpus 0) {
+error = VIR_PY_NONE;
+goto error;
+}
+
+LIBVIRT_BEGIN_ALLOW_THREADS;
+nparams = virDomainGetCPUStats(domain, NULL, 0, 0, 1, flags);
+LIBVIRT_END_ALLOW_THREADS;
+
+if (nparams 0) {
+error = VIR_PY_NONE;
+goto error;
+}
+
+sumparams = nparams * MIN(ncpus, 128);
+
+if (VIR_ALLOC_N(params, sumparams) 0) {
+error = PyErr_NoMemory();
+goto error;
+}
+
+while (ncpus) {
+int
Re: [libvirt] [PATCH v2] python: add virDomainGetCPUStats python binding API
On 03/17/2012 01:42 AM, Eric Blake wrote:
On 03/16/2012 11:26 AM, Eric Blake wrote:
+
+if (!nparams) {
+if ((cpu = PyDict_New()) == NULL) {
+error = NULL;
Initialize error to NULL at the front, and you won't have to set it here.
+goto failed;
+}
+
+if (PyList_Append(ret, cpu) 0) {
+Py_DECREF(cpu);
+error = NULL;
+goto failed;
+}
+
+Py_DECREF(cpu);
+return ret;
So why are we populating the list with a single empty dictionary?
Shouldn't we instead be populating it with one empty dictionary per cpu?
That is, this code returns [{}], but if ncpus is 4, it should return
[{},{},{},{}].
In fact, instead of returning early here, you can just say:
if (nparams) {
// get parameters, via for loop over...
+}
+sumparams = nparams * ncpus;
+
+if (VIR_ALLOC_N(params, sumparams) 0) {
+error = PyErr_NoMemory();
+goto failed;
+}
+
+LIBVIRT_BEGIN_ALLOW_THREADS;
+i_retval = virDomainGetCPUStats(domain, params, nparams, 0, ncpus,
flags);
This will fail if ncpus 128. You have to do this in a for loop,
grabbing only 128 cpus per iteration.
+LIBVIRT_END_ALLOW_THREADS;
+
+if (i_retval 0) {
+error = VIR_PY_NONE;
+goto cleanup;
+}
} else {
i_retval = 0;
}
+
+for (i = 0; i ncpus; i++) {
+if (params[i * nparams].type == 0)
+continue;
Technically, this is only possible if you called virDomainGetCPUStats
with ncpus larger than what the hypervisor already told you to use. But
I guess it doesn't hurt to leave these two lines in.
then drop these two lines after all (since in the i_retval==0 case,
params might be NULL and this would be a NULL deref),
+
+cpuparams =params[i * nparams];
This is just an address computation, so it doesn't matter if params is NULL.
+if ((cpu = getPyVirTypedParameter(cpuparams, nparams)) == NULL) {
Here, you want to iterate over the number of returned parameters, not
the number of array slots you passed in. s/nparams/i_retval/
and this will properly create your empty dictionary, since
getPyVirTypedParameter does the right thing if i_retval is 0. That way,
your PyList_Append() code can be reused to populate the correct return
value for ncpus regardless of whether nparams was 0.
Thanks for these good suggested points, the v3 has been sent out
based on them.
Guannan Ren
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fixed NULL pointer check
At 03/18/2012 08:29 AM, Martin Kletzander Wrote:
This patch fixes a NULL pointer check that was causing SegFault on
some specific configurations.
---
src/util/conf.c |5 -
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/src/util/conf.c b/src/util/conf.c
index 8ad60e0..e76362c 100644
--- a/src/util/conf.c
+++ b/src/util/conf.c
@@ -1,7 +1,7 @@
/**
* conf.c: parser for a subset of the Python encoded Xen configuration files
*
- * Copyright (C) 2006-2011 Red Hat, Inc.
+ * Copyright (C) 2006-2012 Red Hat, Inc.
*
* See COPYING.LIB for the License of this software
*
@@ -836,6 +836,9 @@ virConfGetValue(virConfPtr conf, const char *setting)
{
virConfEntryPtr cur;
+if (conf == NULL)
+return(NULL);
Please use 'return NULL;' instead of 'return(NULL);'
'return(NULL);' is old style, and we donot use it now and later.
+
cur = conf-entries;
while (cur != NULL) {
if ((cur-name != NULL)
ACK with the nit fixed.
Wen Congyang
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fixed NULL pointer check
On 03/19/2012 08:43 AM, Wen Congyang wrote:
At 03/18/2012 08:29 AM, Martin Kletzander Wrote:
This patch fixes a NULL pointer check that was causing SegFault on
some specific configurations.
---
src/util/conf.c |5 -
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/src/util/conf.c b/src/util/conf.c
index 8ad60e0..e76362c 100644
--- a/src/util/conf.c
+++ b/src/util/conf.c
@@ -1,7 +1,7 @@
/**
* conf.c: parser for a subset of the Python encoded Xen configuration files
*
- * Copyright (C) 2006-2011 Red Hat, Inc.
+ * Copyright (C) 2006-2012 Red Hat, Inc.
*
* See COPYING.LIB for the License of this software
*
@@ -836,6 +836,9 @@ virConfGetValue(virConfPtr conf, const char *setting)
{
virConfEntryPtr cur;
+if (conf == NULL)
+return(NULL);
Please use 'return NULL;' instead of 'return(NULL);'
'return(NULL);' is old style, and we donot use it now and later.
It seems weird to me too, it's just that it's almost everywhere in this
file so that's why I wanted to keep the same look.
+
cur = conf-entries;
while (cur != NULL) {
if ((cur-name != NULL)
ACK with the nit fixed.
I'll send a second version, I don't have push permissions.
Wen Congyang
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2] Fixed NULL pointer check
This patch fixes a NULL pointer check that was causing SegFault on
some specific configurations.
---
src/util/conf.c |5 -
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/src/util/conf.c b/src/util/conf.c
index 8ad60e0..3370337 100644
--- a/src/util/conf.c
+++ b/src/util/conf.c
@@ -1,7 +1,7 @@
/**
* conf.c: parser for a subset of the Python encoded Xen configuration files
*
- * Copyright (C) 2006-2011 Red Hat, Inc.
+ * Copyright (C) 2006-2012 Red Hat, Inc.
*
* See COPYING.LIB for the License of this software
*
@@ -836,6 +836,9 @@ virConfGetValue(virConfPtr conf, const char *setting)
{
virConfEntryPtr cur;
+if (conf == NULL)
+return NULL;
+
cur = conf-entries;
while (cur != NULL) {
if ((cur-name != NULL)
--
1.7.3.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] Fixed NULL pointer check
On 19.03.2012 08:55, Martin Kletzander wrote:
This patch fixes a NULL pointer check that was causing SegFault on
some specific configurations.
---
src/util/conf.c |5 -
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/src/util/conf.c b/src/util/conf.c
index 8ad60e0..3370337 100644
--- a/src/util/conf.c
+++ b/src/util/conf.c
@@ -1,7 +1,7 @@
/**
* conf.c: parser for a subset of the Python encoded Xen configuration files
*
- * Copyright (C) 2006-2011 Red Hat, Inc.
+ * Copyright (C) 2006-2012 Red Hat, Inc.
*
* See COPYING.LIB for the License of this software
*
@@ -836,6 +836,9 @@ virConfGetValue(virConfPtr conf, const char *setting)
{
virConfEntryPtr cur;
+if (conf == NULL)
+return NULL;
+
cur = conf-entries;
while (cur != NULL) {
if ((cur-name != NULL)
Looking good, but I think we should revert 59d0c9801c1ab then.
In addition - maybe we can set ATTRIBUTE_RETURN_CHECK to this function.
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] Fixed NULL pointer check
On 03/19/2012 10:38 AM, Michal Privoznik wrote:
On 19.03.2012 08:55, Martin Kletzander wrote:
This patch fixes a NULL pointer check that was causing SegFault on
some specific configurations.
---
src/util/conf.c |5 -
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/src/util/conf.c b/src/util/conf.c
index 8ad60e0..3370337 100644
--- a/src/util/conf.c
+++ b/src/util/conf.c
@@ -836,6 +836,9 @@ virConfGetValue(virConfPtr conf, const char *setting)
{
virConfEntryPtr cur;
+if (conf == NULL)
+return NULL;
+
cur = conf-entries;
while (cur != NULL) {
if ((cur-name != NULL)
Looking good, but I think we should revert 59d0c9801c1ab then.
In addition - maybe we can set ATTRIBUTE_RETURN_CHECK to this function.
Yes, with this change the check added in that commit is redundant.
Peter
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] Fixed NULL pointer check
At 03/19/2012 05:38 PM, Michal Privoznik Wrote:
On 19.03.2012 08:55, Martin Kletzander wrote:
This patch fixes a NULL pointer check that was causing SegFault on
some specific configurations.
---
src/util/conf.c |5 -
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/src/util/conf.c b/src/util/conf.c
index 8ad60e0..3370337 100644
--- a/src/util/conf.c
+++ b/src/util/conf.c
@@ -1,7 +1,7 @@
/**
* conf.c: parser for a subset of the Python encoded Xen configuration files
*
- * Copyright (C) 2006-2011 Red Hat, Inc.
+ * Copyright (C) 2006-2012 Red Hat, Inc.
*
* See COPYING.LIB for the License of this software
*
@@ -836,6 +836,9 @@ virConfGetValue(virConfPtr conf, const char *setting)
{
virConfEntryPtr cur;
+if (conf == NULL)
+return NULL;
+
cur = conf-entries;
while (cur != NULL) {
if ((cur-name != NULL)
Looking good, but I think we should revert 59d0c9801c1ab then.
I agree with reverting this commit. It does not fix all space.
In addition - maybe we can set ATTRIBUTE_RETURN_CHECK to this function.
Agree with it.
Thanks
Wen Congyang
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] snapshot: make quiesce a bit safer
On 16.03.2012 21:49, Eric Blake wrote: If a guest is paused, we were silently ignoring the quiesce flag, which results in unclean snapshots, contrary to the intent of the flag. Since we can't quiesce without guest agent support, we should instead fail if the guest is not running. Meanwhile, if we attempt a quiesce command, but the guest agent doesn't respond, and we time out, we may have left the command pending on the guest's queue, and when the guest resumes parsing commands, it will freeze even though our command is no longer around to issue a thaw. To be safe, we must _always_ pair every quiesce call with a counterpart thaw, even if the quiesce call failed due to a timeout, so that if a guest wakes up and starts processing a command backlog, it will not get stuck in a frozen state. * src/qemu/qemu_driver.c (qemuDomainSnapshotCreateDiskActive): Always issue thaw after a quiesce, even if quiesce failed. (qemuDomainSnapshotFSThaw): Add a parameter. --- See also: https://bugzilla.redhat.com/show_bug.cgi?id=804210 https://www.redhat.com/archives/libvir-list/2012-March/msg00708.html src/qemu/qemu_driver.c | 51 +-- 1 files changed, 36 insertions(+), 15 deletions(-) ACK this and the follow up patch as well. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3] Fixed NULL pointer check
This patch fixes a NULL pointer check that was causing SegFault on
some specific configurations. It also reverts commit 59d0c9801c1ab
that was checking for this value in one place.
---
v3:
- added revert of 59d0c9801c1ab that's not needed anymore
- (comment) ATTRIBUTE_RETURN_CHECK not added as it is not used with other
functions defined in this file and the commit could be confusing modifying
all of them together. However, there should be one patch to fix all these
at once if possible
v2:
- removed parenthesis around return value
src/libvirt.c |3 +--
src/util/conf.c |5 -
2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/src/libvirt.c b/src/libvirt.c
index 7f8d42c..99b263e 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -1085,8 +1085,7 @@ virConnectOpenResolveURIAlias(virConfPtr conf,
*uri = NULL;
-if (conf
-(value = virConfGetValue(conf, uri_aliases)))
+if ((value = virConfGetValue(conf, uri_aliases)))
ret = virConnectOpenFindURIAliasMatch(value, alias, uri);
else
ret = 0;
diff --git a/src/util/conf.c b/src/util/conf.c
index 8ad60e0..3370337 100644
--- a/src/util/conf.c
+++ b/src/util/conf.c
@@ -1,7 +1,7 @@
/**
* conf.c: parser for a subset of the Python encoded Xen configuration files
*
- * Copyright (C) 2006-2011 Red Hat, Inc.
+ * Copyright (C) 2006-2012 Red Hat, Inc.
*
* See COPYING.LIB for the License of this software
*
@@ -836,6 +836,9 @@ virConfGetValue(virConfPtr conf, const char *setting)
{
virConfEntryPtr cur;
+if (conf == NULL)
+return NULL;
+
cur = conf-entries;
while (cur != NULL) {
if ((cur-name != NULL)
--
1.7.3.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Add migration APIs for libxl driver
On Fri, Mar 09, 2012 at 06:55:55PM +0800, Chunyan Liu wrote:
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index d5fa64a..5dc29a0 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
+static int doParseURI(const char *uri, char **p_hostname, int *p_port)
+{
+char *p, *hostname;
+int port_nr = 0;
+
+if (uri == NULL)
+return -1;
+
+/* URI passed is a string hostname[:port] */
+if ((p = strrchr(uri, ':')) != NULL) { /* hostname:port */
+int n;
+
+if (virStrToLong_i(p+1, NULL, 10, port_nr) 0) {
+libxlError(VIR_ERR_INVALID_ARG,
+_(Invalid port number));
+return -1;
+}
+
+/* Get the hostname. */
+n = p - uri; /* n = Length of hostname in bytes. */
+if (n = 0) {
+libxlError(VIR_ERR_INVALID_ARG,
+ _(Hostname must be specified in the URI));
+return -1;
+}
+
+if (virAsprintf(hostname, %s, uri) 0) {
+virReportOOMError();
+return -1;
+}
+
+hostname[n] = '\0';
+}
+else {/* hostname (or IP address) */
+if (virAsprintf(hostname, %s, uri) 0) {
+virReportOOMError();
+return -1;
+}
+}
+*p_hostname = hostname;
+*p_port = port_nr;
+return 0;
+}
Lets not re-invent URI parsing code with wierd special cases for
base hostnames. Please just use virURIParse, since there is no
compatibility issue here.
+static void doMigrateReceive(void *opaque)
+{
+migrate_receive_args *data = opaque;
+virConnectPtr conn = data-conn;
+int sockfd = data-sockfd;
+virDomainObjPtr vm = data-vm;
+libxlDriverPrivatePtr driver = conn-privateData;
+int recv_fd;
+struct sockaddr_in new_addr;
+socklen_t socklen = sizeof(new_addr);
+int len;
+
+do {
+recv_fd = accept(sockfd, (struct sockaddr *)new_addr, socklen);
+} while(recv_fd 0 errno == EINTR);
[snip]
+sockfd = socket(AF_INET, SOCK_STREAM, 0);
+if (sockfd == -1) {
+libxlError(VIR_ERR_OPERATION_FAILED,
+ _(Failed to create socket for incoming migration));
+goto cleanup;
+}
+
+memset(addr, 0, sizeof(addr));
+addr.sin_family = AF_INET;
+addr.sin_port = htons(port);
+addr.sin_addr.s_addr = htonl(INADDR_ANY);
+
+if (bind(sockfd, (struct sockaddr *)addr, sizeof(addr)) 0) {
+libxlError(VIR_ERR_OPERATION_FAILED,
+ _(Fail to bind port for incoming migration));
+goto cleanup;
+}
+
+if (listen(sockfd, MAXCONN_NUM) 0){
+libxlError(VIR_ERR_OPERATION_FAILED,
+ _(Fail to listen to incoming migration));
+goto cleanup;
+}
+
+if (VIR_ALLOC(args) 0) {
+virReportOOMError();
+goto cleanup;
+}
[snip]
+memset(hints, 0, sizeof(hints));
+hints.ai_family = AF_INET;
+hints.ai_socktype = SOCK_STREAM;
+if (getaddrinfo(hostname, servname, hints, res) || res == NULL) {
+libxlError(VIR_ERR_OPERATION_FAILED,
+ _(IP address lookup failed));
+goto cleanup;
+}
+
+if (connect(sockfd, res-ai_addr, res-ai_addrlen) 0) {
+libxlError(VIR_ERR_OPERATION_FAILED,
+ _(Connect error));
+goto cleanup;
+}
I know the QEMU code doesn't follow what I'm about to say, but I would
prefer it if all this socket code was re-written to use the internal
virNetSocketPtr APIs. In particular this would result in correct
usage of getaddrinfo() which is lacking here.
diff --git a/src/libxl/libxl_driver.h b/src/libxl/libxl_driver.h
index 4632d33..ad8df1f 100644
--- a/src/libxl/libxl_driver.h
+++ b/src/libxl/libxl_driver.h
@@ -21,9 +21,25 @@
/*---*/
#ifndef LIBXL_DRIVER_H
-# define LIBXL_DRIVER_H
+#define LIBXL_DRIVER_H
-# include config.h
+#include config.h
+
+#define LIBXL_MIGRATION_FLAGS \
+(VIR_MIGRATE_LIVE | \
+ VIR_MIGRATE_UNDEFINE_SOURCE | \
+ VIR_MIGRATE_PAUSED)
+
+#define MAXCONN_NUM 10
+#define LIBXL_MIGRATION_MIN_PORT 49512
+#define LIBXL_MIGRATION_NUM_PORTS 64
+#define LIBXL_MIGRATION_MAX_PORT\
+(LIBXL_MIGRATION_MIN_PORT + LIBXL_MIGRATION_NUM_PORTS)
+
+static const char migrate_receiver_banner[]=
+xl migration receiver ready, send binary domain data;
+static const char migrate_receiver_ready[]=
+domain received, ready to unpause;
It is better if these were in the .c file, or if they need to be
shared across multiple driver files, use the libxl_conf.h
Regards,
Daniel
--
|: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o-
Re: [libvirt] heisenbug in command.c
On Fri, Mar 16, 2012 at 04:42:42PM -0500, Serge Hallyn wrote:
On 03/16/2012 03:52 PM, Jim Paris wrote:
Serge Hallyn wrote:
On 03/16/2012 11:50 AM, Eric Blake wrote:
On 03/16/2012 10:36 AM, Serge Hallyn wrote:
Hi,
It seems I've run into quite the heisenbug, reported at
https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/922628
It manifests itself as virPidWait returning status=4 for iptables (which
should never exit with status=4).
Maybe iptables isn't documented as exiting with $? of 4, but that's what
is happening. The libvirt code in question is quite clear that it
grabbed an accurate exit status from the child process.
Well, yes. I figured that either (1) iptables actually got -EINTR
from the kernel and passed that along as its exit code, or (2)
something went wrong with memory being overwritten in libvirt,
however unlikely. Stranger things have happened. If (1), I was
wondering if it was being ignored on purpose.
Why do you bring up EINTR at all? Just because EINTR is 4?
Yup.
That
seems very much unrelated.
I didn't think so, but looks like you're right :)
This is from iptables:
enum xtables_exittype {
OTHER_PROBLEM = 1,
PARAMETER_PROBLEM,
VERSION_PROBLEM,
RESOURCE_PROBLEM,
XTF_ONLY_ONCE,
XTF_NO_INVERT,
XTF_BAD_VALUE,
XTF_ONE_ACTION,
};
So it looks like iptables is returning RESOURCE_PROBLEM (which could
explain why it's intermittent).
That makes a lot more sense then, yes. When scripting a bunch of
parallel iptables rules adds (followed by waits), I do once in
awhile get
iptables: Resource temporarily unavailable.
(and sometimes a -EINVAL message)
though 'wait' still says status was 0. I've never gotten 4. The
next run then succeeds.
This still however sounds like src/util/iptables.c might ought to
re-run the command if it gets 4.
Or do we need some serialization of commands at a higher level perhaps ?
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] Fixed NULL pointer check
On Mon, Mar 19, 2012 at 08:51:24AM +0100, Martin Kletzander wrote:
On 03/19/2012 08:43 AM, Wen Congyang wrote:
At 03/18/2012 08:29 AM, Martin Kletzander Wrote:
This patch fixes a NULL pointer check that was causing SegFault on
some specific configurations.
---
src/util/conf.c |5 -
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/src/util/conf.c b/src/util/conf.c
index 8ad60e0..e76362c 100644
--- a/src/util/conf.c
+++ b/src/util/conf.c
@@ -1,7 +1,7 @@
/**
* conf.c: parser for a subset of the Python encoded Xen configuration
files
*
- * Copyright (C) 2006-2011 Red Hat, Inc.
+ * Copyright (C) 2006-2012 Red Hat, Inc.
*
* See COPYING.LIB for the License of this software
*
@@ -836,6 +836,9 @@ virConfGetValue(virConfPtr conf, const char *setting)
{
virConfEntryPtr cur;
+if (conf == NULL)
+return(NULL);
Please use 'return NULL;' instead of 'return(NULL);'
'return(NULL);' is old style, and we donot use it now and later.
It seems weird to me too, it's just that it's almost everywhere in this
file so that's why I wanted to keep the same look.
That is a historical remant. Do feel free to send another followup patch to
cleanup all the cases of 'return(NULL)' as well.
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] Fixed NULL pointer check
On Mon, Mar 19, 2012 at 11:05:30AM +0100, Martin Kletzander wrote: This patch fixes a NULL pointer check that was causing SegFault on some specific configurations. It also reverts commit 59d0c9801c1ab that was checking for this value in one place. --- v3: - added revert of 59d0c9801c1ab that's not needed anymore - (comment) ATTRIBUTE_RETURN_CHECK not added as it is not used with other functions defined in this file and the commit could be confusing modifying all of them together. However, there should be one patch to fix all these at once if possible v2: - removed parenthesis around return value src/libvirt.c |3 +-- src/util/conf.c |5 - 2 files changed, 5 insertions(+), 3 deletions(-) ACK Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3] Fixed NULL pointer check
On 19.03.2012 11:05, Martin Kletzander wrote: This patch fixes a NULL pointer check that was causing SegFault on some specific configurations. It also reverts commit 59d0c9801c1ab that was checking for this value in one place. --- v3: - added revert of 59d0c9801c1ab that's not needed anymore - (comment) ATTRIBUTE_RETURN_CHECK not added as it is not used with other functions defined in this file and the commit could be confusing modifying all of them together. However, there should be one patch to fix all these at once if possible v2: - removed parenthesis around return value src/libvirt.c |3 +-- src/util/conf.c |5 - 2 files changed, 5 insertions(+), 3 deletions(-) I've tweaked the patch subject (prepended with 'virConfGetValue: ' for easier git-log) and pushed. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/3] snapshot: add qemu capability for 'transaction' command
On 03/16/2012 11:05 PM, Eric Blake wrote: We need a capability bit to gracefully error out if some of the additions in future patches can't be implemented by the running qemu. * src/qemu/qemu_capabilities.h (QEMU_CAPS_TRANSACTION): New cap. * src/qemu/qemu_capabilities.c (qemuCaps): Name it. * src/qemu/qemu_monitor_json.c (qemuMonitorJSONCheckCommands): Set it. --- ACK, Peter -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/3] snapshot: add atomic create flag
On 03/16/2012 11:05 PM, Eric Blake wrote: Right now, it is appallingly easy to cause qemu disk snapshots to alter a domain then fail; for example, by requesting a two-disk snapshot where the second disk name resides on read-only storage. In this failure scenario, libvirt reports failure, but modifies the live domain XML in-place to record that the first disk snapshot was taken; and places a difficult burden on the management app to grab the XML and reparse it to see which disks, if any, were altered by the partial snapshot. This patch adds a new flag where implementations can request that the hypervisor make snapshots atomically; either no changes to XML occur, or all disks were altered as a group. If you request the flag, you either get outright failure up front, or you take advantage of hypervisor abilities to make an atomic snapshot. Of course, drivers should prefer the atomic means even without the flag explicitly requested. There's no way to make snapshots 100% bulletproof - even if the hypervisor does it perfectly atomic, we could run out of memory during the followup tasks of updating our in-memory XML, and report a failure. However, these sorts of catastrophic failures are rare and unlikely, and it is still nicer to know that either all snapshots happened or none of them, as that is an easier state to recover from. * include/libvirt/libvirt.h.in (VIR_DOMAIN_SNAPSHOT_CREATE_ATOMIC): New flag. * src/libvirt.c (virDomainSnapshotCreateXML): Document it. * tools/virsh.c (cmdSnapshotCreate, cmdSnapshotCreateAs): Expose it. * tools/virsh.pod (snapshot-create, snapshot-create-as): Document it. --- ... + * that it is still possible to fail after disks have changed, but only + * in the much rarer cases of running out of memory or disk space). (sometimes it's not that rare (but then you've also got some other problems) :D) ACK, Peter -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 3/3] snapshot: rudimentary qemu support for atomic disk snapshot
On 03/17/2012 05:33 PM, Eric Blake wrote:
Taking an external snapshot of just one disk is atomic, without having
to pause and resume the VM. This also paves the way for later patches
to interact with the new qemu 'transaction' monitor command.
The various scenarios when requesting atomic are:
online, 1 disk, old qemu - safe, allowed by this patch
online, more than 1 disk, old qemu - failure, this patch
offline snapshot - safe, once a future patch implements offline disk snapshot
online, 1 or more disks, new qemu - safe, once future patch uses transaction
Taking an online system checkpoint snapshot is atomic, since it is
done via a single 'savevm' monitor command.
Taking an offline system checkpoint snapshot currently uses multiple
qemu-img invocations with no provision for cleanup of partial failure,
so for now we mark it unsupported.
* src/qemu/qemu_driver.c (qemuDomainSnapshotCreateXML): Support
new flag for single-disk setups.
(qemuDomainSnapshotDiskPrepare): Check for atomic here.
(qemuDomainSnapshotCreateDiskActive): Skip pausing the VM when
atomic supported.
(qemuDomainSnapshotIsAllowed): Use bool instead of int.
---
Patches 1/3 and 2/3 unchanged.
v2: consider interactions of atomic with non-disk-snapshots
src/qemu/qemu_driver.c | 56 +--
1 files changed, 39 insertions(+), 17 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 85f8cf7..9e62738 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -10279,6 +10295,12 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
vm, snap, flags) 0)
goto cleanup;
} else if (!virDomainObjIsActive(vm)) {
+if (flags VIR_DOMAIN_SNAPSHOT_CREATE_ATOMIC) {
+qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+_(atomic snapshots of inactive domains not
+ implemented yet));
+goto cleanup;
I wonder if we shouldn't add a error code for unimplemented
functionality to differentiate it from unsupported configurations.
(Well, but using a configuration that uses unimplemented functionality
makes it an unsupported configuration basically, so I don't really mind)
+}
if (qemuDomainSnapshotCreateInactive(driver, vm, snap) 0)
goto cleanup;
} else {
ACK,
Peter.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Cpu mapping cleanup
Using inheritance, this patch cleans up the cpu_map.xml file and also sorts all CPU features according to the feature and registry values. Model features are sorted the same way as foeatures in the specification. Also few models that are related were organized together and parts of the XML are marked with comments --- src/cpu/cpu_map.xml | 455 +- 1 files changed, 82 insertions(+), 373 deletions(-) diff --git a/src/cpu/cpu_map.xml b/src/cpu/cpu_map.xml index 6af9c40..9a89e2e 100644 --- a/src/cpu/cpu_map.xml +++ b/src/cpu/cpu_map.xml @@ -268,6 +268,7 @@ feature name='pse'/ /model +!-- Intel-based QEMU generic CPU models -- model name='pentium' model name='486'/ feature name='de'/ @@ -302,482 +303,190 @@ feature name='pse'/ feature name='tsc'/ feature name='msr'/ + feature name='pae'/ feature name='mce'/ feature name='cx8'/ + feature name='apic'/ + feature name='sep'/ feature name='pge'/ feature name='cmov'/ feature name='pat'/ - feature name='fxsr'/ feature name='mmx'/ - feature name='sse'/ - feature name='sse2'/ - feature name='pae'/ - feature name='sep'/ - feature name='apic'/ -/model - -model name='qemu32' - model name='pentiumpro'/ - feature name='pni'/ -/model - -model name='cpu64-rhel5' - feature name='apic'/ - feature name='clflush'/ - feature name='cmov'/ - feature name='cx8'/ - feature name='de'/ - feature name='fpu'/ feature name='fxsr'/ - feature name='lm'/ - feature name='mca'/ - feature name='mce'/ - feature name='mmx'/ - feature name='msr'/ - feature name='mtrr'/ - feature name='nx'/ - feature name='pae'/ - feature name='pat'/ - feature name='pge'/ - feature name='pse'/ - feature name='pse36'/ - feature name='sep'/ feature name='sse'/ feature name='sse2'/ - feature name='pni'/ - feature name='syscall'/ - feature name='tsc'/ -/model - -model name='cpu64-rhel6' - feature name='apic'/ - feature name='clflush'/ - feature name='cmov'/ - feature name='cx16'/ - feature name='cx8'/ - feature name='de'/ - feature name='fpu'/ - feature name='fxsr'/ - feature name='lahf_lm'/ - feature name='lm'/ - feature name='mca'/ - feature name='mce'/ - feature name='mmx'/ - feature name='msr'/ - feature name='mtrr'/ - feature name='nx'/ - feature name='pae'/ - feature name='pat'/ - feature name='pge'/ - feature name='pse'/ - feature name='pse36'/ - feature name='sep'/ - feature name='sse'/ - feature name='sse2'/ - feature name='pni'/ - feature name='syscall'/ - feature name='tsc'/ -/model - -model name='kvm32' - model name='pentiumpro'/ - feature name='mtrr'/ - feature name='clflush'/ - feature name='mca'/ - feature name='pse36'/ - feature name='pni'/ /model model name='coreduo' model name='pentiumpro'/ + vendor name='Intel'/ feature name='vme'/ feature name='mtrr'/ - feature name='clflush'/ feature name='mca'/ + feature name='clflush'/ feature name='pni'/ feature name='monitor'/ feature name='nx'/ /model -model name='qemu64' - model name='pentiumpro'/ - feature name='mtrr'/ - feature name='clflush'/ - feature name='mca'/ - feature name='pse36'/ - feature name='pni'/ - feature name='cx16'/ - feature name='lm'/ - feature name='syscall'/ - feature name='nx'/ - feature name='svm'/ - !-- These are supported only by TCG. KVM supports them only if the - host does. So we leave them out: - - feature name='popcnt'/ - feature name='lahf_lm'/ - feature name='sse4a'/ - feature name='abm'/ - -- +model name='n270' + model name='coreduo'/ + feature name='ssse3'/ /model -model name='kvm64' - model name='pentiumpro'/ - feature name='mtrr'/ - feature name='clflush'/ - feature name='mca'/ +model name='core2duo' + model name='n270'/ feature name='pse36'/ - feature name='pni'/ - feature name='cx16'/ - feature name='lm'/ feature name='syscall'/ - feature name='nx'/ + feature name='lm'/ /model -model name='core2duo' +!-- Generic QEMU CPU models -- +model name='qemu32' model name='pentiumpro'/ - feature name='mtrr'/ - feature name='clflush'/ - feature name='mca'/ - feature name='vme'/ - feature name='pse36'/ feature name='pni'/ - feature name='monitor'/ - feature name='ssse3'/ - feature name='lm'/ - feature name='syscall'/ - feature name='nx'/
Re: [libvirt] [PATCH] Fixed NULL pointer check
On 03/19/2012 11:32 AM, Daniel P. Berrange wrote:
On Mon, Mar 19, 2012 at 08:51:24AM +0100, Martin Kletzander wrote:
On 03/19/2012 08:43 AM, Wen Congyang wrote:
At 03/18/2012 08:29 AM, Martin Kletzander Wrote:
This patch fixes a NULL pointer check that was causing SegFault on
some specific configurations.
---
src/util/conf.c |5 -
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/src/util/conf.c b/src/util/conf.c
index 8ad60e0..e76362c 100644
--- a/src/util/conf.c
+++ b/src/util/conf.c
@@ -1,7 +1,7 @@
/**
* conf.c: parser for a subset of the Python encoded Xen configuration
files
*
- * Copyright (C) 2006-2011 Red Hat, Inc.
+ * Copyright (C) 2006-2012 Red Hat, Inc.
*
* See COPYING.LIB for the License of this software
*
@@ -836,6 +836,9 @@ virConfGetValue(virConfPtr conf, const char *setting)
{
virConfEntryPtr cur;
+if (conf == NULL)
+return(NULL);
Please use 'return NULL;' instead of 'return(NULL);'
'return(NULL);' is old style, and we donot use it now and later.
It seems weird to me too, it's just that it's almost everywhere in this
file so that's why I wanted to keep the same look.
That is a historical remant. Do feel free to send another followup patch to
cleanup all the cases of 'return(NULL)' as well.
Daniel
Did you mean in that file or globally? Because I just tried the first
thing that came to my mind and look at the output:
libvirt $ find . -name '*.[ch]' -type f -exec grep -nH -e \
'return(.*);' {} + | wc -l
852
Not that I wouldn't do it, it just seem as a pretty big change. On the
other hand, I don't see the point in changing that in only one file.
Martin
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fixed NULL pointer check
On Mon, Mar 19, 2012 at 01:38:47PM +0100, Martin Kletzander wrote:
On 03/19/2012 11:32 AM, Daniel P. Berrange wrote:
On Mon, Mar 19, 2012 at 08:51:24AM +0100, Martin Kletzander wrote:
On 03/19/2012 08:43 AM, Wen Congyang wrote:
At 03/18/2012 08:29 AM, Martin Kletzander Wrote:
This patch fixes a NULL pointer check that was causing SegFault on
some specific configurations.
---
src/util/conf.c |5 -
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/src/util/conf.c b/src/util/conf.c
index 8ad60e0..e76362c 100644
--- a/src/util/conf.c
+++ b/src/util/conf.c
@@ -1,7 +1,7 @@
/**
* conf.c: parser for a subset of the Python encoded Xen configuration
files
*
- * Copyright (C) 2006-2011 Red Hat, Inc.
+ * Copyright (C) 2006-2012 Red Hat, Inc.
*
* See COPYING.LIB for the License of this software
*
@@ -836,6 +836,9 @@ virConfGetValue(virConfPtr conf, const char *setting)
{
virConfEntryPtr cur;
+if (conf == NULL)
+return(NULL);
Please use 'return NULL;' instead of 'return(NULL);'
'return(NULL);' is old style, and we donot use it now and later.
It seems weird to me too, it's just that it's almost everywhere in this
file so that's why I wanted to keep the same look.
That is a historical remant. Do feel free to send another followup patch to
cleanup all the cases of 'return(NULL)' as well.
Daniel
Did you mean in that file or globally? Because I just tried the first
thing that came to my mind and look at the output:
libvirt $ find . -name '*.[ch]' -type f -exec grep -nH -e \
'return(.*);' {} + | wc -l
852
Not that I wouldn't do it, it just seem as a pretty big change. On the
other hand, I don't see the point in changing that in only one file.
IMHO, we should do this to make our code consistent, and ideally add a
syntax-check rule to prevent them coming back. Anyone disagree ?
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/3] snapshot: make offline qemu snapshots atomic
On 03/17/2012 05:33 PM, Eric Blake wrote:
Offline internal snapshots can be rolled back with just a little
bit of refactoring, meaning that we are now automatically atomic.
* src/qemu/qemu_domain.c (qemuDomainSnapshotForEachQcow2): Move
guts...
(qemuDomainSnapshotForEachQcow2Raw): ...to new helper, to allow
rollbacks.
* src/qemu/qemu_driver.c (qemuDomainSnapshotCreateXML): Offline
snapshots are now atomic.
---
You rollback changes to disks if other disks are not snapshotable, but later
on,
when the actual qemu-img command is run and fails the rollback is not
performed.
I's suggest squashing in:
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index a845480..8dde3c9 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1512,70 +1512,75 @@ qemuDomainSnapshotForEachQcow2Raw(struct qemud_driver
*driver,
for (i = 0; i ndisks; i++) {
/* FIXME: we also need to handle LVM here */
if (def-disks[i]-device == VIR_DOMAIN_DISK_DEVICE_DISK) {
if (!def-disks[i]-driverType ||
STRNEQ(def-disks[i]-driverType, qcow2)) {
if (try_all) {
/* Continue on even in the face of error, since other
* disks in this VM may have the same snapshot name.
*/
VIR_WARN(skipping snapshot action on %s,
def-disks[i]-dst);
skipped = true;
continue;
} else if (STREQ(op, -c) i) {
/* We must roll back partial creation by deleting
* all earlier snapshots. */
qemuDomainSnapshotForEachQcow2Raw(driver, def, name,
-d, false, i);
}
qemuReportError(VIR_ERR_OPERATION_INVALID,
_(Disk device '%s' does not support
snapshotting),
def-disks[i]-dst);
return -1;
}
qemuimgarg[4] = def-disks[i]-src;
if (virRun(qemuimgarg, NULL) 0) {
if (try_all) {
VIR_WARN(skipping snapshot action on %s,
def-disks[i]-dst);
skipped = true;
continue;
+} else if (STREQ(op, -c) i) {
+/* We must roll back partial creation by deleting
+ * all earlier snapshots. */
+qemuDomainSnapshotForEachQcow2Raw(driver, def, name,
+ -d, false, i);
}
return -1;
}
}
}
return skipped ? 1 : 0;
}
If you don't add this change the result is following:
Try to make a snapshot of a domain that has one of images missing:
virsh # snapshot-create-as Bare2 test1 test --atomic
error: internal error Child process (/usr/bin/qemu-img snapshot -c test1
/var/lib/libvirt/images/d2.qcow2) status unexpected: exit status 1
But the first image is still modified:
# qemu-img snapshot -l d.qcow2
Snapshot list:
IDTAG VM SIZEDATE VM CLOCK
1 test1 0 2012-03-19 14:26:50 00:00:00.000
Otherwise looks good. ACK with that suggested change.
Peter
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fixed NULL pointer check
On 03/19/2012 06:43 AM, Daniel P. Berrange wrote:
That is a historical remant. Do feel free to send another followup patch to
cleanup all the cases of 'return(NULL)' as well.
Daniel
Did you mean in that file or globally? Because I just tried the first
thing that came to my mind and look at the output:
libvirt $ find . -name '*.[ch]' -type f -exec grep -nH -e \
'return(.*);' {} + | wc -l
852
Yes, it is a rather big cleanup, which is all the more reason why we'd
want to ensure that 'make syntax-check' prevents it from recurring.
Not that I wouldn't do it, it just seem as a pretty big change. On the
other hand, I don't see the point in changing that in only one file.
IMHO, we should do this to make our code consistent, and ideally add a
syntax-check rule to prevent them coming back. Anyone disagree ?
I'm in favor of such a cleanup. There are still a few places where
return needs parenthesis; for example, I'm fond of the style:
return (big_long_conditional ?
option_1 :
option_2);
since the open '(' lets the rest of the code indent nicely when using
default emacs indentation. But it's still pretty easy to recognize the
difference between complex returns and the real offenders. I think the
number of false positives and false negatives is pretty near zero if you
boil it down to detecting uses where there are no spaces between the '('
and ')'. Thus, for cfg.mk, I suggest a pattern something like:
sc_prohibit_return_as_function:
@prohibit='\return *([^ ]*)' \
halt='avoid extra () with return statements' \
$(_sc_search_regexp)
--
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] Fixed NULL pointer check
On 03/19/2012 04:43 PM, Eric Blake wrote:
On 03/19/2012 06:43 AM, Daniel P. Berrange wrote:
[...]
since the open '(' lets the rest of the code indent nicely when using
default emacs indentation. But it's still pretty easy to recognize the
difference between complex returns and the real offenders. I think the
number of false positives and false negatives is pretty near zero if you
boil it down to detecting uses where there are no spaces between the '('
and ')'. Thus, for cfg.mk, I suggest a pattern something like:
sc_prohibit_return_as_function:
@prohibit='\return *([^ ]*)' \
halt='avoid extra () with return statements' \
$(_sc_search_regexp)
I agree with the first part. There are some places that should be kept
as is. However, the '\return *([^ ]*)' would generate some false
positives, for example 'return (buf[0] 0) | (buf[1] 8)' and few
others I've found.
Don't worry though, I've created a regexp that matches just what's
needed, composing the patch right now.
Martin
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fixed NULL pointer check
On Mon, Mar 19, 2012 at 09:43:26AM -0600, Eric Blake wrote:
On 03/19/2012 06:43 AM, Daniel P. Berrange wrote:
That is a historical remant. Do feel free to send another followup patch
to
cleanup all the cases of 'return(NULL)' as well.
Daniel
Did you mean in that file or globally? Because I just tried the first
thing that came to my mind and look at the output:
libvirt $ find . -name '*.[ch]' -type f -exec grep -nH -e \
'return(.*);' {} + | wc -l
852
Yes, it is a rather big cleanup, which is all the more reason why we'd
want to ensure that 'make syntax-check' prevents it from recurring.
Not that I wouldn't do it, it just seem as a pretty big change. On the
other hand, I don't see the point in changing that in only one file.
IMHO, we should do this to make our code consistent, and ideally add a
syntax-check rule to prevent them coming back. Anyone disagree ?
I'm in favor of such a cleanup. There are still a few places where
return needs parenthesis; for example, I'm fond of the style:
return (big_long_conditional ?
option_1 :
option_2);
Yes, that usage scenario is fine.
since the open '(' lets the rest of the code indent nicely when using
default emacs indentation. But it's still pretty easy to recognize the
difference between complex returns and the real offenders. I think the
number of false positives and false negatives is pretty near zero if you
boil it down to detecting uses where there are no spaces between the '('
and ')'. Thus, for cfg.mk, I suggest a pattern something like:
sc_prohibit_return_as_function:
@prohibit='\return *([^ ]*)' \
halt='avoid extra () with return statements' \
$(_sc_search_regexp)
Looks good.
Daniel
--
|: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv3] python: Avoid memory leaks on libvirt_virNodeGetMemoryStats
On 17.02.2012 09:15, a...@redhat.com wrote: From: Alex Jia a...@redhat.com Detected by valgrind. Leaks are introduced in commit 17c7795. * python/libvirt-override.c (libvirt_virNodeGetMemoryStats): fix memory leaks and improve codes return value. For details, please see the following link: RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=770944 Signed-off-by: Alex Jia a...@redhat.com --- python/libvirt-override.c | 46 ++-- 1 files changed, 31 insertions(+), 15 deletions(-) ACK Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/5] qemu: Avoid dangling migration-in job on shutoff domains
Destination daemon should not rely on the client or source daemon
(depending on the type of migration) to call Finish when migration
fails, because the client may crash before it can do so. The domain
prepared for incoming migration is set to be destroyed (and migration
job cleaned up) when connection with the client closes but this is not
enough. If the associated qemu process crashes after Prepare step and
the domain is cleaned up before the connection gets closed, autodestroy
is not called for the domain and migration jobs remains set. In case the
domain is defined on destination host (i.e., it is not completely
removed once destroyed) we keep the job set for ever. To fix this, we
register a cleanup callback which is responsible to clean migration-in
job when a domain dies anywhere between Prepare and Finish steps. Note
that we can't blindly clean any job when spotting EOF on monitor since
normally an API is running at that time.
---
src/qemu/qemu_domain.c|2 --
src/qemu/qemu_domain.h|2 ++
src/qemu/qemu_migration.c | 22 ++
3 files changed, 24 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index a9469cf..41ffd6a 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -51,7 +51,6 @@
(VIR_DOMAIN_XML_SECURE |\
VIR_DOMAIN_XML_UPDATE_CPU)
-VIR_ENUM_DECL(qemuDomainJob)
VIR_ENUM_IMPL(qemuDomainJob, QEMU_JOB_LAST,
none,
query,
@@ -64,7 +63,6 @@ VIR_ENUM_IMPL(qemuDomainJob, QEMU_JOB_LAST,
async nested,
);
-VIR_ENUM_DECL(qemuDomainAsyncJob)
VIR_ENUM_IMPL(qemuDomainAsyncJob, QEMU_ASYNC_JOB_LAST,
none,
migration out,
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index af83c0e..d79ff1d 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -64,6 +64,7 @@ enum qemuDomainJob {
QEMU_JOB_LAST
};
+VIR_ENUM_DECL(qemuDomainJob)
/* Async job consists of a series of jobs that may change state. Independent
* jobs that do not change state (and possibly others if explicitly allowed by
@@ -78,6 +79,7 @@ enum qemuDomainAsyncJob {
QEMU_ASYNC_JOB_LAST
};
+VIR_ENUM_DECL(qemuDomainAsyncJob)
struct qemuDomainJobObj {
virCond cond; /* Use to coordinate jobs */
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 81b2d5b..4eb3bf4 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -1107,6 +1107,23 @@ cleanup:
/* Prepare is the first step, and it runs on the destination host.
*/
+static void
+qemuMigrationPrepareCleanup(struct qemud_driver *driver,
+virDomainObjPtr vm)
+{
+qemuDomainObjPrivatePtr priv = vm-privateData;
+
+VIR_DEBUG(driver=%p, vm=%s, job=%s, asyncJob=%s,
+ driver,
+ vm-def-name,
+ qemuDomainJobTypeToString(priv-job.active),
+ qemuDomainAsyncJobTypeToString(priv-job.asyncJob));
+
+if (!qemuMigrationJobIsActive(vm, QEMU_ASYNC_JOB_MIGRATION_IN))
+return;
+qemuDomainObjDiscardAsyncJob(driver, vm);
+}
+
static int
qemuMigrationPrepareAny(struct qemud_driver *driver,
virConnectPtr dconn,
@@ -1264,6 +1281,9 @@ qemuMigrationPrepareAny(struct qemud_driver *driver,
VIR_WARN(Unable to encode migration cookie);
}
+if (qemuDomainCleanupAdd(vm, qemuMigrationPrepareCleanup) 0)
+goto endjob;
+
virDomainAuditStart(vm, migrated, true);
event = virDomainEventNewFromObj(vm,
VIR_DOMAIN_EVENT_STARTED,
@@ -2703,6 +2723,8 @@ qemuMigrationFinish(struct qemud_driver *driver,
v3proto ? QEMU_MIGRATION_PHASE_FINISH3
: QEMU_MIGRATION_PHASE_FINISH2);
+qemuDomainCleanupRemove(vm, qemuMigrationPrepareCleanup);
+
if (flags VIR_MIGRATE_PERSIST_DEST)
cookie_flags |= QEMU_MIGRATION_COOKIE_PERSISTENT;
--
1.7.8.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/5] qemu: Make autodestroy utilize connection close callbacks
---
src/qemu/qemu_conf.h|5 --
src/qemu/qemu_driver.c |5 --
src/qemu/qemu_process.c | 107 ++
3 files changed, 24 insertions(+), 93 deletions(-)
diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
index a22ce0c..3306014 100644
--- a/src/qemu/qemu_conf.h
+++ b/src/qemu/qemu_conf.h
@@ -141,11 +141,6 @@ struct qemud_driver {
virLockManagerPluginPtr lockManager;
-/* Mapping of 'char *uuidstr' - virConnectPtr
- * of guests which will be automatically killed
- * when the virConnectPtr is closed*/
-virHashTablePtr autodestroy;
-
/* Mapping of 'char *uuidstr' - qemuDriverCloseDefPtr of domains
* which want a specific cleanup to be done when a connection is
* closed. Such cleanup may be to automatically destroy the
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index ce82535..3a5ef09 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -659,9 +659,6 @@ qemudStartup(int privileged) {
qemu_driver-hugepage_path = mempath;
}
-if (qemuProcessAutoDestroyInit(qemu_driver) 0)
-goto error;
-
if (qemuDriverCloseCallbackInit(qemu_driver) 0)
goto error;
@@ -803,7 +800,6 @@ qemudShutdown(void) {
virSysinfoDefFree(qemu_driver-hostsysinfo);
-qemuProcessAutoDestroyShutdown(qemu_driver);
qemuDriverCloseCallbackShutdown(qemu_driver);
VIR_FREE(qemu_driver-configDir);
@@ -925,7 +921,6 @@ static int qemudClose(virConnectPtr conn) {
qemuDriverLock(driver);
virDomainEventStateDeregisterConn(conn,
driver-domainEventState);
-qemuProcessAutoDestroyRun(driver, conn);
qemuDriverCloseCallbackRunAll(driver, conn);
qemuDriverUnlock(driver);
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 1945864..8915a9a 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -4120,124 +4120,65 @@ cleanup:
}
-int qemuProcessAutoDestroyInit(struct qemud_driver *driver)
+static virDomainObjPtr
+qemuProcessAutoDestroy(struct qemud_driver *driver,
+ virDomainObjPtr dom,
+ virConnectPtr conn)
{
-if (!(driver-autodestroy = virHashCreate(5, NULL)))
-return -1;
-
-return 0;
-}
-
-struct qemuProcessAutoDestroyData {
-struct qemud_driver *driver;
-virConnectPtr conn;
-};
-
-static void qemuProcessAutoDestroyDom(void *payload,
- const void *name,
- void *opaque)
-{
-struct qemuProcessAutoDestroyData *data = opaque;
-virConnectPtr conn = payload;
-const char *uuidstr = name;
-unsigned char uuid[VIR_UUID_BUFLEN];
-virDomainObjPtr dom;
-qemuDomainObjPrivatePtr priv;
+qemuDomainObjPrivatePtr priv = dom-privateData;
virDomainEventPtr event = NULL;
-VIR_DEBUG(conn=%p uuidstr=%s thisconn=%p, conn, uuidstr, data-conn);
+VIR_DEBUG(vm=%s, conn=%p, dom-def-name, conn);
-if (data-conn != conn)
-return;
-
-if (virUUIDParse(uuidstr, uuid) 0) {
-VIR_WARN(Failed to parse %s, uuidstr);
-return;
-}
-
-if (!(dom = virDomainFindByUUID(data-driver-domains,
-uuid))) {
-VIR_DEBUG(No domain object to kill);
-return;
-}
-
-priv = dom-privateData;
if (priv-job.asyncJob) {
VIR_DEBUG(vm=%s has long-term job active, cancelling,
dom-def-name);
-qemuDomainObjDiscardAsyncJob(data-driver, dom);
+qemuDomainObjDiscardAsyncJob(driver, dom);
}
-if (qemuDomainObjBeginJobWithDriver(data-driver, dom,
+if (qemuDomainObjBeginJobWithDriver(driver, dom,
QEMU_JOB_DESTROY) 0)
goto cleanup;
VIR_DEBUG(Killing domain);
-qemuProcessStop(data-driver, dom, 1, VIR_DOMAIN_SHUTOFF_DESTROYED);
+qemuProcessStop(driver, dom, 1, VIR_DOMAIN_SHUTOFF_DESTROYED);
virDomainAuditStop(dom, destroyed);
event = virDomainEventNewFromObj(dom,
VIR_DOMAIN_EVENT_STOPPED,
VIR_DOMAIN_EVENT_STOPPED_DESTROYED);
-if (qemuDomainObjEndJob(data-driver, dom) == 0)
+
+if (qemuDomainObjEndJob(driver, dom) == 0)
dom = NULL;
if (dom !dom-persistent)
-qemuDomainRemoveInactive(data-driver, dom);
-
-cleanup:
-if (dom)
-virDomainObjUnlock(dom);
+qemuDomainRemoveInactive(driver, dom);
if (event)
-qemuDomainEventQueue(data-driver, event);
-virHashRemoveEntry(data-driver-autodestroy, uuidstr);
-}
-
-/*
- * Precondition: driver is locked
- */
-void qemuProcessAutoDestroyRun(struct qemud_driver *driver, virConnectPtr conn)
-{
-struct qemuProcessAutoDestroyData data = {
-driver, conn
-};
-VIR_DEBUG(conn=%p, conn);
-virHashForEach(driver-autodestroy,
[libvirt] [PATCH 1/5] qemu: Add support for domain cleanup callbacks
Add support for registering cleanup callbacks to be run when a domain
transitions to shutoff state.
---
src/qemu/qemu_domain.c | 73 +++
src/qemu/qemu_domain.h | 15 +
src/qemu/qemu_process.c |2 +
3 files changed, 90 insertions(+), 0 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 625c595..a9469cf 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -232,6 +232,7 @@ static void qemuDomainObjPrivateFree(void *data)
VIR_ERROR(_(Unexpected QEMU agent still active during domain
deletion));
qemuAgentClose(priv-agent);
}
+VIR_FREE(priv-cleanupCallbacks);
VIR_FREE(priv);
}
@@ -1769,3 +1770,75 @@ qemuDomainCheckDiskPresence(struct qemud_driver *driver,
cleanup:
return ret;
}
+
+/*
+ * The vm must be locked when any of the following cleanup functions is
+ * called.
+ */
+int
+qemuDomainCleanupAdd(virDomainObjPtr vm,
+ qemuDomainCleanupCallback cb)
+{
+qemuDomainObjPrivatePtr priv = vm-privateData;
+int i;
+
+VIR_DEBUG(vm=%s, cb=%p, vm-def-name, cb);
+
+for (i = 0; i priv-ncleanupCallbacks; i++) {
+if (priv-cleanupCallbacks[i] == cb)
+return 0;
+}
+
+if (VIR_RESIZE_N(priv-cleanupCallbacks,
+ priv-ncleanupCallbacks_max,
+ priv-ncleanupCallbacks, 1) 0) {
+virReportOOMError();
+return -1;
+}
+
+priv-cleanupCallbacks[priv-ncleanupCallbacks++] = cb;
+return 0;
+}
+
+void
+qemuDomainCleanupRemove(virDomainObjPtr vm,
+qemuDomainCleanupCallback cb)
+{
+qemuDomainObjPrivatePtr priv = vm-privateData;
+int i;
+
+VIR_DEBUG(vm=%s, cb=%p, vm-def-name, cb);
+
+for (i = 0; i priv-ncleanupCallbacks; i++) {
+if (priv-cleanupCallbacks[i] == cb) {
+memmove(priv-cleanupCallbacks + i,
+priv-cleanupCallbacks + i + 1,
+priv-ncleanupCallbacks - i - 1);
+priv-ncleanupCallbacks--;
+}
+}
+
+VIR_SHRINK_N(priv-cleanupCallbacks,
+ priv-ncleanupCallbacks_max,
+ priv-ncleanupCallbacks_max - priv-ncleanupCallbacks);
+}
+
+void
+qemuDomainCleanupRun(struct qemud_driver *driver,
+ virDomainObjPtr vm)
+{
+qemuDomainObjPrivatePtr priv = vm-privateData;
+int i;
+
+VIR_DEBUG(driver=%p, vm=%s, driver, vm-def-name);
+
+/* run cleanup callbacks in reverse order */
+for (i = priv-ncleanupCallbacks - 1; i = 0; i--) {
+if (priv-cleanupCallbacks[i])
+priv-cleanupCallbacks[i](driver, vm);
+}
+
+VIR_FREE(priv-cleanupCallbacks);
+priv-ncleanupCallbacks = 0;
+priv-ncleanupCallbacks_max = 0;
+}
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index f8e943f..af83c0e 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -94,6 +94,9 @@ struct qemuDomainJobObj {
typedef struct _qemuDomainPCIAddressSet qemuDomainPCIAddressSet;
typedef qemuDomainPCIAddressSet *qemuDomainPCIAddressSetPtr;
+typedef void (*qemuDomainCleanupCallback)(struct qemud_driver *driver,
+ virDomainObjPtr vm);
+
typedef struct _qemuDomainObjPrivate qemuDomainObjPrivate;
typedef qemuDomainObjPrivate *qemuDomainObjPrivatePtr;
struct _qemuDomainObjPrivate {
@@ -130,6 +133,10 @@ struct _qemuDomainObjPrivate {
char *origname;
virConsolesPtr cons;
+
+qemuDomainCleanupCallback *cleanupCallbacks;
+size_t ncleanupCallbacks;
+size_t ncleanupCallbacks_max;
};
struct qemuDomainWatchdogEvent
@@ -307,4 +314,12 @@ bool qemuDomainJobAllowed(qemuDomainObjPrivatePtr priv,
int qemuDomainCheckDiskPresence(struct qemud_driver *driver,
virDomainObjPtr vm,
bool start_with_state);
+
+int qemuDomainCleanupAdd(virDomainObjPtr vm,
+ qemuDomainCleanupCallback cb);
+void qemuDomainCleanupRemove(virDomainObjPtr vm,
+ qemuDomainCleanupCallback cb);
+void qemuDomainCleanupRun(struct qemud_driver *driver,
+ virDomainObjPtr vm);
+
#endif /* __QEMU_DOMAIN_H__ */
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 0af3751..1945864 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3803,6 +3803,8 @@ void qemuProcessStop(struct qemud_driver *driver,
/* shut it off for sure */
ignore_value(qemuProcessKill(driver, vm, VIR_QEMU_PROCESS_KILL_FORCE));
+qemuDomainCleanupRun(driver, vm);
+
/* Stop autodestroy in case guest is restarted */
qemuProcessAutoDestroyRemove(driver, vm);
--
1.7.8.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/5] Make migration more robust when client dies
Libvirt daemon was not completely ready to lose a connection to the client (may even be source libvirtd) which is controling the migration. We might end up with dangling migration jobs on both source and destination daemons. More details about the scenarios this might happen can be found in patches 2/5 and 5/5 which fix the issue using infrastructure set up by the other patches in this series. Jiri Denemark (5): qemu: Add support for domain cleanup callbacks qemu: Avoid dangling migration-in job on shutoff domains qemu: Add connection close callbacks qemu: Make autodestroy utilize connection close callbacks qemu: Avoid dangling migration-out job when client dies src/qemu/qemu_conf.c | 172 + src/qemu/qemu_conf.h | 30 +++- src/qemu/qemu_domain.c| 75 +++- src/qemu/qemu_domain.h| 17 + src/qemu/qemu_driver.c| 10 ++- src/qemu/qemu_migration.c | 88 +++ src/qemu/qemu_migration.h |4 + src/qemu/qemu_process.c | 109 +++-- 8 files changed, 413 insertions(+), 92 deletions(-) -- 1.7.8.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 5/5] qemu: Avoid dangling migration-out job when client dies
When a client which started non-p2p migration dies in a bad time, the
source libvirtd never clears the migration job and almost nothing can be
done with the domain without restarting the daemon. This patch makes use
of connection close callbacks and ensures that migration job is properly
discarded when the client disconnects.
---
src/qemu/qemu_driver.c|4 +++
src/qemu/qemu_migration.c | 66 +
src/qemu/qemu_migration.h |4 +++
3 files changed, 74 insertions(+), 0 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 3a5ef09..c2622d3 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -8838,6 +8838,9 @@ qemuDomainMigrateBegin3(virDomainPtr domain,
* This prevents any other APIs being invoked while migration is taking
* place.
*/
+if (qemuDriverCloseCallbackSet(driver, vm, domain-conn,
+ qemuMigrationCleanup) 0)
+goto endjob;
if (qemuMigrationJobContinue(vm) == 0) {
vm = NULL;
qemuReportError(VIR_ERR_OPERATION_FAILED,
@@ -9069,6 +9072,7 @@ qemuDomainMigrateConfirm3(virDomainPtr domain,
phase = QEMU_MIGRATION_PHASE_CONFIRM3;
qemuMigrationJobStartPhase(driver, vm, phase);
+qemuDriverCloseCallbackUnset(driver, vm, qemuMigrationCleanup);
ret = qemuMigrationConfirm(driver, domain-conn, vm,
cookiein, cookieinlen,
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 4eb3bf4..e02ba86 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -1037,6 +1037,67 @@ qemuDomainMigrateGraphicsRelocate(struct qemud_driver
*driver,
}
+/* This is called for outgoing non-p2p migrations when a connection to the
+ * client which initiated the migration was closed and we are waiting for it
+ * to follow up with the next phase, that is, in between
+ * virDomainMigrateBegin3 and qemuDomainMigratePerform3 or
+ * qemuDomainMigratePerform3 and qemuDomainMigrateConfirm3.
+ */
+virDomainObjPtr
+qemuMigrationCleanup(struct qemud_driver *driver,
+ virDomainObjPtr vm,
+ virConnectPtr conn)
+{
+qemuDomainObjPrivatePtr priv = vm-privateData;
+
+VIR_DEBUG(vm=%s, conn=%p, asyncJob=%s, phase=%s,
+ vm-def-name, conn,
+ qemuDomainAsyncJobTypeToString(priv-job.asyncJob),
+ qemuDomainAsyncJobPhaseToString(priv-job.asyncJob,
+ priv-job.phase));
+
+if (!qemuMigrationJobIsActive(vm, QEMU_ASYNC_JOB_MIGRATION_OUT))
+goto cleanup;
+
+VIR_DEBUG(The connection which started outgoing migration of domain %s
+ was closed; cancelling the migration,
+ vm-def-name);
+
+switch ((enum qemuMigrationJobPhase) priv-job.phase) {
+case QEMU_MIGRATION_PHASE_BEGIN3:
+/* just forget we were about to migrate */
+qemuDomainObjDiscardAsyncJob(driver, vm);
+break;
+
+case QEMU_MIGRATION_PHASE_PERFORM3_DONE:
+VIR_WARN(Migration of domain %s finished but we don't know if the
+ domain was successfully started on destination or not,
+ vm-def-name);
+/* clear the job and let higher levels decide what to do */
+qemuDomainObjDiscardAsyncJob(driver, vm);
+break;
+
+case QEMU_MIGRATION_PHASE_PERFORM3:
+/* cannot be seen without an active migration API; unreachable */
+case QEMU_MIGRATION_PHASE_CONFIRM3:
+case QEMU_MIGRATION_PHASE_CONFIRM3_CANCELLED:
+/* all done; unreachable */
+case QEMU_MIGRATION_PHASE_PREPARE:
+case QEMU_MIGRATION_PHASE_FINISH2:
+case QEMU_MIGRATION_PHASE_FINISH3:
+/* incoming migration; unreachable */
+case QEMU_MIGRATION_PHASE_PERFORM2:
+/* single phase outgoing migration; unreachable */
+case QEMU_MIGRATION_PHASE_NONE:
+case QEMU_MIGRATION_PHASE_LAST:
+/* unreachable */
+;
+}
+
+cleanup:
+return vm;
+}
+
/* The caller is supposed to lock the vm and start a migration job. */
char *qemuMigrationBegin(struct qemud_driver *driver,
virDomainObjPtr vm,
@@ -2547,6 +2608,7 @@ qemuMigrationPerformPhase(struct qemud_driver *driver,
}
qemuMigrationJobStartPhase(driver, vm, QEMU_MIGRATION_PHASE_PERFORM3);
+qemuDriverCloseCallbackUnset(driver, vm, qemuMigrationCleanup);
resume = virDomainObjGetState(vm, NULL) == VIR_DOMAIN_RUNNING;
ret = doNativeMigrate(driver, vm, uri, cookiein, cookieinlen,
@@ -2577,6 +2639,10 @@ qemuMigrationPerformPhase(struct qemud_driver *driver,
qemuMigrationJobSetPhase(driver, vm, QEMU_MIGRATION_PHASE_PERFORM3_DONE);
+if (qemuDriverCloseCallbackSet(driver, vm, conn,
+ qemuMigrationCleanup) 0)
+goto endjob;
+
endjob:
if (ret 0)
refs =
[libvirt] [PATCH 3/5] qemu: Add connection close callbacks
Add support for registering arbitrary callback to be called for a domain
when a connection gets closed.
---
src/qemu/qemu_conf.c | 172
src/qemu/qemu_conf.h | 27
src/qemu/qemu_driver.c |5 ++
3 files changed, 204 insertions(+), 0 deletions(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index a709cbf..88a04bc 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -56,6 +56,11 @@
#define VIR_FROM_THIS VIR_FROM_QEMU
+struct _qemuDriverCloseDef {
+virConnectPtr conn;
+qemuDriverCloseCallback cb;
+};
+
void qemuDriverLock(struct qemud_driver *driver)
{
virMutexLock(driver-lock);
@@ -490,3 +495,170 @@ int qemudLoadDriverConfig(struct qemud_driver *driver,
virConfFree (conf);
return 0;
}
+
+static void
+qemuDriverCloseCallbackFree(void *payload,
+const void *name ATTRIBUTE_UNUSED)
+{
+VIR_FREE(payload);
+}
+
+int
+qemuDriverCloseCallbackInit(struct qemud_driver *driver)
+{
+driver-closeCallbacks = virHashCreate(5, qemuDriverCloseCallbackFree);
+if (!driver-closeCallbacks)
+return -1;
+
+return 0;
+}
+
+void
+qemuDriverCloseCallbackShutdown(struct qemud_driver *driver)
+{
+virHashFree(driver-closeCallbacks);
+}
+
+int
+qemuDriverCloseCallbackSet(struct qemud_driver *driver,
+ virDomainObjPtr vm,
+ virConnectPtr conn,
+ qemuDriverCloseCallback cb)
+{
+char uuidstr[VIR_UUID_STRING_BUFLEN];
+qemuDriverCloseDefPtr closeDef;
+
+virUUIDFormat(vm-def-uuid, uuidstr);
+VIR_DEBUG(vm=%s, uuid=%s, conn=%p, cb=%p,
+ vm-def-name, uuidstr, conn, cb);
+
+closeDef = virHashLookup(driver-closeCallbacks, uuidstr);
+if (closeDef) {
+if (closeDef-conn != conn) {
+qemuReportError(VIR_ERR_INTERNAL_ERROR,
+_(Close callback for domain %s already registered
+ with another connection %p),
+vm-def-name, closeDef-conn);
+return -1;
+}
+if (closeDef-cb closeDef-cb != cb) {
+qemuReportError(VIR_ERR_INTERNAL_ERROR,
+_(Another close callback is already defined for
+ domain %s), vm-def-name);
+return -1;
+}
+
+closeDef-cb = cb;
+} else {
+if (VIR_ALLOC(closeDef) 0) {
+virReportOOMError();
+return -1;
+}
+
+closeDef-conn = conn;
+closeDef-cb = cb;
+if (virHashAddEntry(driver-closeCallbacks, uuidstr, closeDef) 0) {
+VIR_FREE(closeDef);
+return -1;
+}
+}
+return 0;
+}
+
+int
+qemuDriverCloseCallbackUnset(struct qemud_driver *driver,
+ virDomainObjPtr vm,
+ qemuDriverCloseCallback cb)
+{
+char uuidstr[VIR_UUID_STRING_BUFLEN];
+qemuDriverCloseDefPtr closeDef;
+
+virUUIDFormat(vm-def-uuid, uuidstr);
+VIR_DEBUG(vm=%s, uuid=%s, cb=%p,
+ vm-def-name, uuidstr, cb);
+
+closeDef = virHashLookup(driver-closeCallbacks, uuidstr);
+if (!closeDef)
+return -1;
+
+if (closeDef-cb closeDef-cb != cb) {
+qemuReportError(VIR_ERR_INTERNAL_ERROR,
+_(Trying to remove mismatching close callback for
+ domain %s), vm-def-name);
+return -1;
+}
+
+return virHashRemoveEntry(driver-closeCallbacks, uuidstr);
+}
+
+qemuDriverCloseCallback
+qemuDriverCloseCallbackGet(struct qemud_driver *driver,
+ virDomainObjPtr vm,
+ virConnectPtr conn)
+{
+char uuidstr[VIR_UUID_STRING_BUFLEN];
+qemuDriverCloseDefPtr closeDef;
+qemuDriverCloseCallback cb = NULL;
+
+virUUIDFormat(vm-def-uuid, uuidstr);
+VIR_DEBUG(vm=%s, uuid=%s, conn=%p,
+ vm-def-name, uuidstr, conn);
+
+closeDef = virHashLookup(driver-closeCallbacks, uuidstr);
+if (closeDef (!conn || closeDef-conn == conn))
+cb = closeDef-cb;
+
+VIR_DEBUG(cb=%p, cb);
+return cb;
+}
+
+struct qemuDriverCloseCallbackData {
+struct qemud_driver *driver;
+virConnectPtr conn;
+};
+
+static void
+qemuDriverCloseCallbackRun(void *payload,
+ const void *name,
+ void *opaque)
+{
+struct qemuDriverCloseCallbackData *data = opaque;
+qemuDriverCloseDefPtr closeDef = payload;
+const char *uuidstr = name;
+unsigned char uuid[VIR_UUID_BUFLEN];
+virDomainObjPtr dom;
+
+VIR_DEBUG(conn=%p, thisconn=%p, uuid=%s, cb=%p,
+ closeDef-conn, data-conn, uuidstr, closeDef-cb);
+
+if (data-conn != closeDef-conn || !closeDef-cb)
+return;
+
+if (virUUIDParse(uuidstr, uuid) 0) {
+VIR_WARN(Failed to parse %s, uuidstr);
+return;
+}
+
+
Re: [libvirt] [PATCHv3] python: Avoid memory leaks on libvirt_virNodeGetMemoryStats
On 02/17/2012 01:15 AM, a...@redhat.com wrote:
From: Alex Jia a...@redhat.com
Detected by valgrind. Leaks are introduced in commit 17c7795.
* python/libvirt-override.c (libvirt_virNodeGetMemoryStats): fix memory leaks
and improve codes return value.
For details, please see the following link:
RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=770944
Signed-off-by: Alex Jia a...@redhat.com
---
python/libvirt-override.c | 46 ++--
1 files changed, 31 insertions(+), 15 deletions(-)
LIBVIRT_BEGIN_ALLOW_THREADS;
c_retval = virNodeGetMemoryStats(conn, cellNum, NULL, nparams, flags);
LIBVIRT_END_ALLOW_THREADS;
if (c_retval 0)
-return VIR_PY_NONE;
+return ret;
Wrong. At this point, ret is still NULL, but we really do want to
return VIR_PY_NONE (we have a valid libvirt error, and not a python
exception).
if (nparams) {
if (VIR_ALLOC_N(stats, nparams) 0)
-return VIR_PY_NONE;
+return PyErr_NoMemory();
Correct (here we have a python exception, not a libvirt error).
LIBVIRT_BEGIN_ALLOW_THREADS;
c_retval = virNodeGetMemoryStats(conn, cellNum, stats, nparams,
flags);
LIBVIRT_END_ALLOW_THREADS;
-if (c_retval 0) {
-VIR_FREE(stats);
-return VIR_PY_NONE;
-}
-}
-if (!(ret = PyDict_New())) {
-VIR_FREE(stats);
-return VIR_PY_NONE;
+if (c_retval 0)
+goto error;
Wrong - here's another place where we want to return VIR_PY_NONE.
}
+
+if (!(ret = PyDict_New()))
+goto error;
Good - here we have a python exception, and ret is still NULL.
+
for (i = 0; i nparams; i++) {
-PyDict_SetItem(ret,
- libvirt_constcharPtrWrap(stats[i].field),
- libvirt_ulonglongWrap(stats[i].value));
+key = libvirt_constcharPtrWrap(stats[i].field);
+val = libvirt_ulonglongWrap(stats[i].value);
+
+if (!key || !val)
+goto error;
Hmm. Here, ret is allocated, so we mistakenly end up returning a python
object when we really want to return NULL to cover the python error we
just encountered.
+
+if (PyDict_SetItem(ret, key, val) 0) {
+Py_DECREF(ret);
+goto error;
Even worse - we are allowing ret to be garbage collected, but still
returning its address. If PyDict_SetItem fails, you want to return
NULL, not the just-allocated dictionary object.
+}
+
+Py_DECREF(key);
+Py_DECREF(val);
}
VIR_FREE(stats);
return ret;
+
+error:
+VIR_FREE(stats);
+Py_XDECREF(key);
+Py_XDECREF(val);
+return ret;
I think we still need a v4 to fix things properly.
--
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] Cpu mapping cleanup
On 03/19/2012 06:29 AM, Martin Kletzander wrote: Using inheritance, this patch cleans up the cpu_map.xml file and also sorts all CPU features according to the feature and registry values. Model features are sorted the same way as foeatures in the s/foeatures/features/ specification. Also few models that are related were organized together and parts of the XML are marked with comments --- src/cpu/cpu_map.xml | 455 +- 1 files changed, 82 insertions(+), 373 deletions(-) The inheritance is definitely an improvement for reducing lines of code. The patch was a bit hard to read (splitting things into three patches: reordering options, reordering CPU models into groups, then taking advantage of inheritance; rather than doing all three actions in one patch, might have been easier), but seems correct after the few minutes I've spent on it, and isn't worth the time to respin the series just to split the patch. ACK. -- Eric Blake ebl...@redhat.com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] snapshot: make quiesce a bit safer
On 03/19/2012 04:04 AM, Michal Privoznik wrote: On 16.03.2012 21:49, Eric Blake wrote: If a guest is paused, we were silently ignoring the quiesce flag, which results in unclean snapshots, contrary to the intent of the flag. Since we can't quiesce without guest agent support, we should instead fail if the guest is not running. Meanwhile, if we attempt a quiesce command, but the guest agent doesn't respond, and we time out, we may have left the command pending on the guest's queue, and when the guest resumes parsing commands, it will freeze even though our command is no longer around to issue a thaw. To be safe, we must _always_ pair every quiesce call with a counterpart thaw, even if the quiesce call failed due to a timeout, so that if a guest wakes up and starts processing a command backlog, it will not get stuck in a frozen state. * src/qemu/qemu_driver.c (qemuDomainSnapshotCreateDiskActive): Always issue thaw after a quiesce, even if quiesce failed. (qemuDomainSnapshotFSThaw): Add a parameter. --- See also: https://bugzilla.redhat.com/show_bug.cgi?id=804210 https://www.redhat.com/archives/libvir-list/2012-March/msg00708.html src/qemu/qemu_driver.c | 51 +-- 1 files changed, 36 insertions(+), 15 deletions(-) ACK this and the follow up patch as well. Thanks; 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
[libvirt] [PATCH] conf: return immediately on error in dhcp host element
If and error was encountered parsing a dhcp host entry mac address or
name, parsing would continue and log a less descriptive error that
might make it more difficult to notice the true nature of the problem.
This patch returns immediately on logging the first error.
---
src/conf/network_conf.c |2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index 0333141..0fa58f8 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -430,6 +430,7 @@ virNetworkDHCPRangeDefParseXML(const char *networkName,
_(Cannot parse MAC address '%s' in
network '%s'),
mac, networkName);
VIR_FREE(mac);
+return -1;
}
name = virXMLPropString(cur, name);
if ((name != NULL) (!c_isalpha(name[0]))) {
@@ -437,6 +438,7 @@ virNetworkDHCPRangeDefParseXML(const char *networkName,
_(Cannot use name address '%s' in
network '%s'),
name, networkName);
VIR_FREE(name);
+return -1;
}
/*
* You need at least one MAC address or one host name
--
1.7.7.6
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] conf: forbid use of multicast mac addresses
A few times libvirt users manually setting mac addresses have
complained of networking failure that ends up being due to a multicast
mac address being used for a guest interface. This patch prevents that
by loggin an error and failing if a multicast mac address is
encountered in each of the three following cases:
1) domain xml interface mac address.
2) network xml bridge mac address.
3) network xml dhcp/host mac address.
There are several other places where a mac address cna be input that
aren't controlled in this manner because failure to do so has no
consequences (e.g., if the address will be used to search through
existing interfaces for a match).
The RNG has been updated to add multiMacAddr and uniMacAddr along with
the existing macAddr, and macAddr was switched to uniMacAddr where
appropriate.
---
docs/schemas/basictypes.rng | 17 -
docs/schemas/domaincommon.rng |6 +++---
docs/schemas/network.rng |4 ++--
src/conf/domain_conf.c|8 +++-
src/conf/network_conf.c | 33 -
src/libvirt_private.syms |2 ++
src/util/virmacaddr.c | 13 +
src/util/virmacaddr.h |3 ++-
8 files changed, 69 insertions(+), 17 deletions(-)
diff --git a/docs/schemas/basictypes.rng b/docs/schemas/basictypes.rng
index 7d7911d..9dbda4a 100644
--- a/docs/schemas/basictypes.rng
+++ b/docs/schemas/basictypes.rng
@@ -55,9 +55,24 @@
/define
!-- a 6 byte MAC address in ASCII-hex format, eg 12:34:56:78:9A:BC --
+ !-- The lowest bit of the 1st byte is the multicast bit. a --
+ !-- uniMacAddr requires that bit to be 0, and a multiMacAddr --
+ !-- requires it to be 1. Plain macAddr will accept either. --
+ !-- Currently there is no use of multiMacAddr in libvirt, it --
+ !-- is included here for documentation/comparison purposes. --
+ define name=uniMacAddr
+data type=string
+ param
name=pattern[a-fA-F0-9][02468aAcCeE](:[a-fA-F0-9]{2}){5}/param
+/data
+ /define
+ define name=multiMacAddr
+data type=string
+ param
name=pattern[a-fA-F0-9][13579bBdDfF](:[a-fA-F0-9]{2}){5}/param
+/data
+ /define
define name=macAddr
data type=string
- param name=pattern([a-fA-F0-9]{2}:){5}[a-fA-F0-9]{2}/param
+ param name=pattern[a-fA-F0-9]{2}(:[a-fA-F0-9]{2}){5}/param
/data
/define
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 5b3e5fa..f629691 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -1391,7 +1391,7 @@
optional
element name=mac
attribute name=address
- ref name=macAddr/
+ ref name=uniMacAddr/
/attribute
empty/
/element
@@ -1417,7 +1417,7 @@
optional
element name=mac
attribute name=address
- ref name=macAddr/
+ ref name=uniMacAddr/
/attribute
empty/
/element
@@ -1498,7 +1498,7 @@
optional
element name=mac
attribute name=address
-ref name=macAddr/
+ref name=uniMacAddr/
/attribute
empty/
/element
diff --git a/docs/schemas/network.rng b/docs/schemas/network.rng
index 6e1002f..2ae879e 100644
--- a/docs/schemas/network.rng
+++ b/docs/schemas/network.rng
@@ -57,7 +57,7 @@
!-- mac element --
optional
element name=mac
-attribute name=addressref name=macAddr//attribute
+attribute name=addressref name=uniMacAddr//attribute
empty/
/element
/optional
@@ -218,7 +218,7 @@
/zeroOrMore
zeroOrMore
element name=host
-attribute name=macref name=macAddr//attribute
+attribute name=macref name=uniMacAddr//attribute
attribute name=nametext//attribute
attribute name=ipref name=ipv4Addr//attribute
/element
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index e6d0f4b..d5def1c 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -4416,11 +4416,17 @@ virDomainNetDefParseXML(virCapsPtr caps,
if (macaddr) {
if (virMacAddrParse((const char *)macaddr, def-mac) 0) {
-virDomainReportError(VIR_ERR_INTERNAL_ERROR,
+virDomainReportError(VIR_ERR_XML_ERROR,
_(unable to parse mac address '%s'),
(const char *)macaddr);
goto error;
}
+if (virMacAddrIsMulticast(def-mac)) {
+virDomainReportError(VIR_ERR_XML_ERROR,
+ _(expected unicast mac address, found
multicast '%s'),
+ (const
Re: [libvirt] [PATCH v2] python: add virDomainGetCPUStats python binding API
On 03/18/2012 04:41 AM, Guannan Ren wrote:
+if (totalbool == Py_False) {
Per other code in libvirt-override.c, you can't compare totalbool (type
PyObject) with Py_False, at least not on all compilers. You need
something like this instead:
/* Hack - Python's definition of Py_True breaks strict
* aliasing rules, so can't directly compare
*/
if (PyBool_Check(value)) {
PyObject *hacktrue = PyBool_FromLong(1);
temp-value.b = hacktrue == value ? 1 : 0;
Py_DECREF(hacktrue);
Yes, it did report warning in compiling as follows due to the case
from PyIntObject* to PyObject*
warning :dereferencing type-punned pointer might break
strict-aliasing rules [-Wstrict-aliasing]
And that would trip up a -Werror compilation, so I'm glad to see you
changed it in v3.
GCC command line to reproduce the error:
gcc -Wstrict-aliasing=1 -O2 cpythonexample.c
Actually PyObject_IsTrue() is a more light-weight approach to do
the checking instead of
creating a intermediate PyObject * for the compare.
Is PyObject_IsTrue() available in the version of python present on RHEL
5? If so, I'd be in favor of a followup cleanup patch that removes all
our hacks in favor of the python glue code that does the same thing.
And even if not, we should write a decent wrapper in our own
typewrappers.c, so that the rest of our code doesn't have to look so
ugly with so much copy-and-paste.
--
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] conf: return immediately on error in dhcp host element
On 03/19/2012 11:32 AM, Laine Stump wrote:
If and error was encountered parsing a dhcp host entry mac address or
name, parsing would continue and log a less descriptive error that
might make it more difficult to notice the true nature of the problem.
This patch returns immediately on logging the first error.
---
src/conf/network_conf.c |2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index 0333141..0fa58f8 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -430,6 +430,7 @@ virNetworkDHCPRangeDefParseXML(const char *networkName,
_(Cannot parse MAC address '%s' in
network '%s'),
mac, networkName);
VIR_FREE(mac);
+return -1;
}
name = virXMLPropString(cur, name);
if ((name != NULL) (!c_isalpha(name[0]))) {
@@ -437,6 +438,7 @@ virNetworkDHCPRangeDefParseXML(const char *networkName,
_(Cannot use name address '%s' in
network '%s'),
name, networkName);
VIR_FREE(name);
+return -1;
Memory leak - you just leaked mac. Needs a v2.
--
Eric Blake ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] python: make virDomainSetSchedulerParameters accpet integer argument
When sending a Python integer as an argument to
PyLong_AsUnsignedLong or PyLong_AsUnsignedLongLong,
the following error occurs
SystemError: Objects/longobject.c:980:
bad argument to internal function
This error comes from the fact that
PyLong_AsUnsignedLong and PyLong_AsUnsignedLongLong,
unlike PyLong_AsLong or PyLong_AsLongLong, does not check
to see if the number is a long or integer, but only a long.
The regression is introduced by
9c8466daac19379c41be39ec8f18db36c9573c02
dom.setSchedulerParameters({'cpu_shares': 1024})
0
dom.schedulerParameters()
{'cpu_shares': 1024L}
dom.setSchedulerParameters({'cpu_shares': 1024L})
0
dom.schedulerParameters()
{'cpu_shares': 1024L}
---
python/libvirt-override.c |9 -
1 files changed, 8 insertions(+), 1 deletions(-)
diff --git a/python/libvirt-override.c b/python/libvirt-override.c
index 792cfa3..aec8f5a 100644
--- a/python/libvirt-override.c
+++ b/python/libvirt-override.c
@@ -233,7 +233,14 @@ setPyVirTypedParameter(PyObject *info,
break;
case VIR_TYPED_PARAM_ULLONG:
{
-unsigned long long ullong_val = PyLong_AsUnsignedLongLong(value);
+unsigned long long ullong_val;
+if (PyInt_Check(value))
+ullong_val = (unsigned long long)PyInt_AsLong(value);
+else if (PyLong_Check(value))
+ullong_val = PyLong_AsUnsignedLongLong(value);
+else
+PyErr_SetString(PyExc_TypeError, an integer is required);
+
if ((ullong_val == -1) PyErr_Occurred())
goto cleanup;
temp-value.ul = ullong_val;
--
1.7.7.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] python: make virDomainSetSchedulerParameters accpet integer argument
On 03/19/2012 11:45 AM, Guannan Ren wrote:
When sending a Python integer as an argument to
PyLong_AsUnsignedLong or PyLong_AsUnsignedLongLong,
the following error occurs
SystemError: Objects/longobject.c:980:
bad argument to internal function
+++ b/python/libvirt-override.c
@@ -233,7 +233,14 @@ setPyVirTypedParameter(PyObject *info,
break;
case VIR_TYPED_PARAM_ULLONG:
{
-unsigned long long ullong_val = PyLong_AsUnsignedLongLong(value);
+unsigned long long ullong_val;
Pre-initialize this to -1; otherwise...
+if (PyInt_Check(value))
+ullong_val = (unsigned long long)PyInt_AsLong(value);
Cast is unnecessary.
+else if (PyLong_Check(value))
+ullong_val = PyLong_AsUnsignedLongLong(value);
+else
+PyErr_SetString(PyExc_TypeError, an integer is required);
+
if ((ullong_val == -1) PyErr_Occurred())
...if I don't pass in PyInt or PyLong, then this error detection will be
based on uninitialized memory.
Why are you doing this for just VIR_TYPED_PARAM_ULLONG? I argue that we
should be doing it for all of the integral conversions.
Needs a v2.
--
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] python: make virDomainSetSchedulerParameters accpet integer argument
On 03/19/2012 12:03 PM, Eric Blake wrote:
Why are you doing this for just VIR_TYPED_PARAM_ULLONG? I argue that we
should be doing it for all of the integral conversions.
In fact, I'd argue that we want new helper functions in typewrappers.c,
as counterparts to our libvirt_intWrap() and friends. Perhaps:
/* Return 0 if obj could be unwrapped into the corresponding C type, -1
with python exception set otherwise. */
int libvirt_intUnwrap(PyObject *obj, int *val);
int libvirt_uintUnwrap(PyObject *obj, unsigned int *val);
int libvirt_longlongUnwrap(PyObject *obj, long long *val);
int libvirt_ulonglongUnwrap(PyObject *obj, unsigned long long *val);
Then the code in question simplifies to:
case VIR_TYPED_PARAM_ULLONG:
{
unsigned long long ullong_val;
if (libvirt_ulonglongUnwrap(value, ullong_val) 0)
goto cleanup;
temp-value.ul = ullong_val;
--
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] Cpu mapping cleanup
On 03/19/2012 06:10 PM, Eric Blake wrote: On 03/19/2012 06:29 AM, Martin Kletzander wrote: Using inheritance, this patch cleans up the cpu_map.xml file and also sorts all CPU features according to the feature and registry values. Model features are sorted the same way as foeatures in the s/foeatures/features/ specification. Also few models that are related were organized together and parts of the XML are marked with comments --- src/cpu/cpu_map.xml | 455 +- 1 files changed, 82 insertions(+), 373 deletions(-) The inheritance is definitely an improvement for reducing lines of code. The patch was a bit hard to read (splitting things into three patches: reordering options, reordering CPU models into groups, then taking advantage of inheritance; rather than doing all three actions in one patch, might have been easier), but seems correct after the few minutes I've spent on it, and isn't worth the time to respin the series just to split the patch. I was thinking how to split the patch and this variant didn't cross my mind. Definitely next time =) ACK. Could you also push it, please (I don't have permissions for that)? Thanks -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2] conf: return immediately on error in dhcp host element
If an error was encountered parsing a dhcp host entry mac address or
name, parsing would continue and log a less descriptive error that
might make it more difficult to notice the true nature of the problem.
This patch returns immediately on logging the first error.
---
src/conf/network_conf.c |3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index 0333141..4341f11 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -430,13 +430,16 @@ virNetworkDHCPRangeDefParseXML(const char *networkName,
_(Cannot parse MAC address '%s' in
network '%s'),
mac, networkName);
VIR_FREE(mac);
+return -1;
}
name = virXMLPropString(cur, name);
if ((name != NULL) (!c_isalpha(name[0]))) {
virNetworkReportError(VIR_ERR_INTERNAL_ERROR,
_(Cannot use name address '%s' in
network '%s'),
name, networkName);
+VIR_FREE(mac);
VIR_FREE(name);
+return -1;
}
/*
* You need at least one MAC address or one host name
--
1.7.7.6
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2] conf: return immediately on error in dhcp host element
On 03/19/2012 12:46 PM, Laine Stump wrote:
If an error was encountered parsing a dhcp host entry mac address or
name, parsing would continue and log a less descriptive error that
might make it more difficult to notice the true nature of the problem.
This patch returns immediately on logging the first error.
---
src/conf/network_conf.c |3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index 0333141..4341f11 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -430,13 +430,16 @@ virNetworkDHCPRangeDefParseXML(const char *networkName,
_(Cannot parse MAC address '%s' in
network '%s'),
mac, networkName);
VIR_FREE(mac);
+return -1;
}
name = virXMLPropString(cur, name);
if ((name != NULL) (!c_isalpha(name[0]))) {
virNetworkReportError(VIR_ERR_INTERNAL_ERROR,
_(Cannot use name address '%s' in
network '%s'),
name, networkName);
+VIR_FREE(mac);
VIR_FREE(name);
+return -1;
ACK. I was debating if it would have been any cleaner to have a
cleanup: label, but then you start having to do major refactoring to
take advantage of it, at which point this patch would no longer be quite
so small.
--
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 v3] python: add virDomainGetCPUStats python binding API
On 03/19/2012 12:27 AM, Guannan Ren wrote:
dom.getCPUStats(True, 0)
[{'cpu_time': 24699446159L, 'system_time': 1087000L, 'user_time':
95000L}]
dom.getCPUStats(False, 0)
[{'cpu_time': 8535292289L}, {'cpu_time': 1005395355L}, {'cpu_time':
9351766377L}, {'cpu_time': 5813545649L}]
*generator.py Add a new naming rule
*libvirt-override-api.xml The API function description
*libvirt-override.c Implement it.
---
python/generator.py |5 +-
python/libvirt-override-api.xml | 13
python/libvirt-override.c | 147
+++
3 files changed, 164 insertions(+), 1 deletions(-)
ACK once you fix one bug:
+
+for (i = 0; i queried_ncpus; i++) {
+cpuparams = params[i * i_retval];
s/i_retval/nparams/
Your testing worked because you happened to have a situation where
i_retval==nparams, but the API allows for i_retval nparams (that is,
the number of filled entries within a stride can be less than the number
of slots reserved by the stride).
--
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 3/3] snapshot: rudimentary qemu support for atomic disk snapshot
On 03/19/2012 06:20 AM, Peter Krempa wrote:
On 03/17/2012 05:33 PM, Eric Blake wrote:
Taking an external snapshot of just one disk is atomic, without having
to pause and resume the VM. This also paves the way for later patches
to interact with the new qemu 'transaction' monitor command.
+++ b/src/qemu/qemu_driver.c
@@ -10279,6 +10295,12 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
vm, snap, flags) 0)
goto cleanup;
} else if (!virDomainObjIsActive(vm)) {
+if (flags VIR_DOMAIN_SNAPSHOT_CREATE_ATOMIC) {
+qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+_(atomic snapshots of inactive domains
not
+ implemented yet));
+goto cleanup;
I wonder if we shouldn't add a error code for unimplemented
functionality to differentiate it from unsupported configurations.
(Well, but using a configuration that uses unimplemented functionality
makes it an unsupported configuration basically, so I don't really mind)
This exact same error message is removed in 4/3, so I don't think it
quite matters _what_ we put here. I guess I could rebase the series to
swap 3/4 and 4/4, to avoid the churn.
+}
if (qemuDomainSnapshotCreateInactive(driver, vm, snap) 0)
goto cleanup;
} else {
ACK,
Thanks for the reviews. I may wait another day or two before actually
pushing, since I'm still in the middle of testing other snapshot-related
patches, just to make sure my tests don't turn up any other needed
last-minute changes to the ones I've posted so far.
--
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 4/3] snapshot: make offline qemu snapshots atomic
On 03/19/2012 07:40 AM, Peter Krempa wrote:
On 03/17/2012 05:33 PM, Eric Blake wrote:
Offline internal snapshots can be rolled back with just a little
bit of refactoring, meaning that we are now automatically atomic.
* src/qemu/qemu_domain.c (qemuDomainSnapshotForEachQcow2): Move
guts...
(qemuDomainSnapshotForEachQcow2Raw): ...to new helper, to allow
rollbacks.
* src/qemu/qemu_driver.c (qemuDomainSnapshotCreateXML): Offline
snapshots are now atomic.
---
You rollback changes to disks if other disks are not snapshotable, but later
on,
when the actual qemu-img command is run and fails the rollback is not
performed.
Good catch.
I's suggest squashing in:
if (virRun(qemuimgarg, NULL) 0) {
if (try_all) {
VIR_WARN(skipping snapshot action on %s,
def-disks[i]-dst);
skipped = true;
continue;
+} else if (STREQ(op, -c) i) {
+/* We must roll back partial creation by deleting
+ * all earlier snapshots. */
+qemuDomainSnapshotForEachQcow2Raw(driver, def, name,
+ -d, false, i);
}
Yep, that looks right. Thanks for the test case.
Otherwise looks good. ACK with that suggested change.
Same story as for 3/3 - I'll wait to push this until I've run a few more
tests for the rest of my pending series, in case I find any more
last-minute issues.
--
Eric Blake ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Minor docs fix
End tag for host element was missing in example configuration --- docs/formatnetwork.html.in |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/docs/formatnetwork.html.in b/docs/formatnetwork.html.in index 907c046..7e8e991 100644 --- a/docs/formatnetwork.html.in +++ b/docs/formatnetwork.html.in @@ -363,6 +363,7 @@ lt;host ip='192.168.122.2'gt; lt;hostnamegt;myhostlt;/hostnamegt; lt;hostnamegt;myhostaliaslt;/hostnamegt; + lt;/hostgt; lt;/dnsgt; lt;ip address=192.168.122.1 netmask=255.255.255.0gt; lt;dhcpgt; -- 1.7.3.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2] conf: return immediately on error in dhcp host element
On 03/19/2012 03:49 PM, Eric Blake wrote:
On 03/19/2012 12:46 PM, Laine Stump wrote:
If an error was encountered parsing a dhcp host entry mac address or
name, parsing would continue and log a less descriptive error that
might make it more difficult to notice the true nature of the problem.
This patch returns immediately on logging the first error.
---
src/conf/network_conf.c |3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index 0333141..4341f11 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -430,13 +430,16 @@ virNetworkDHCPRangeDefParseXML(const char *networkName,
_(Cannot parse MAC address '%s' in
network '%s'),
mac, networkName);
VIR_FREE(mac);
+return -1;
}
name = virXMLPropString(cur, name);
if ((name != NULL) (!c_isalpha(name[0]))) {
virNetworkReportError(VIR_ERR_INTERNAL_ERROR,
_(Cannot use name address '%s' in
network '%s'),
name, networkName);
+VIR_FREE(mac);
VIR_FREE(name);
+return -1;
ACK. I was debating if it would have been any cleaner to have a
cleanup: label, but then you start having to do major refactoring to
take advantage of it, at which point this patch would no longer be quite
so small.
Yeah, I go through the same thought process every time I see cleanup
code like this. It's so much more difficult to introduce leaks when
there is a single exit from the function and everything is always
cleaned up.
Pushed. Thanks!
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Minor docs fix
On 03/19/2012 05:52 PM, Martin Kletzander wrote: End tag for host element was missing in example configuration --- docs/formatnetwork.html.in |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/docs/formatnetwork.html.in b/docs/formatnetwork.html.in index 907c046..7e8e991 100644 --- a/docs/formatnetwork.html.in +++ b/docs/formatnetwork.html.in @@ -363,6 +363,7 @@ lt;host ip='192.168.122.2'gt; lt;hostnamegt;myhostlt;/hostnamegt; lt;hostnamegt;myhostaliaslt;/hostnamegt; + lt;/hostgt; lt;/dnsgt; lt;ip address=192.168.122.1 netmask=255.255.255.0gt; lt;dhcpgt; ACK and pushed. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] conf: forbid use of multicast mac addresses
On 03/19/2012 01:46 PM, Eric Blake wrote:
On 03/19/2012 11:32 AM, Laine Stump wrote:
A few times libvirt users manually setting mac addresses have
complained of networking failure that ends up being due to a multicast
mac address being used for a guest interface. This patch prevents that
by loggin an error and failing if a multicast mac address is
s/loggin/logging/
encountered in each of the three following cases:
1) domain xml interface mac address.
2) network xml bridge mac address.
3) network xml dhcp/host mac address.
There are several other places where a mac address cna be input that
s/cna/can/
aren't controlled in this manner because failure to do so has no
consequences (e.g., if the address will be used to search through
existing interfaces for a match).
The RNG has been updated to add multiMacAddr and uniMacAddr along with
the existing macAddr, and macAddr was switched to uniMacAddr where
appropriate.
---
docs/schemas/basictypes.rng | 17 -
docs/schemas/domaincommon.rng |6 +++---
docs/schemas/network.rng |4 ++--
src/conf/domain_conf.c|8 +++-
src/conf/network_conf.c | 33 -
src/libvirt_private.syms |2 ++
src/util/virmacaddr.c | 13 +
src/util/virmacaddr.h |3 ++-
Is there any way to add a test of an expected error, where we prove that
a multicast mac fails to validate? But since the existing tests use
unicast addrs, and those still validate, you've at least tested for no
regressions on sane usage.
+ !-- The lowest bit of the 1st byte is the multicast bit. a --
+ !-- uniMacAddr requires that bit to be 0, and a multiMacAddr --
+ !-- requires it to be 1. Plain macAddr will accept either. --
+ !-- Currently there is no use of multiMacAddr in libvirt, it --
+ !-- is included here for documentation/comparison purposes. --
+ define name=uniMacAddr
+data type=string
+ param
name=pattern[a-fA-F0-9][02468aAcCeE](:[a-fA-F0-9]{2}){5}/param
+/data
+ /define
+ define name=multiMacAddr
+data type=string
+ param
name=pattern[a-fA-F0-9][13579bBdDfF](:[a-fA-F0-9]{2}){5}/param
+/data
+ /define
define name=macAddr
data type=string
- param name=pattern([a-fA-F0-9]{2}:){5}[a-fA-F0-9]{2}/param
+ param name=pattern[a-fA-F0-9]{2}(:[a-fA-F0-9]{2}){5}/param
Changed to match the earlier patterns. Nice patterns.
ACK. If you do figure out how to add a negative test for expected
validation failure, I'm okay if you submit it as a separate followup patch.
Pushed (I agree that negative tests are in general a weak spot of the
xml2argv and xml2xml tests, just can't afford the distraction to deal
with it now).
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/11 v2] Add supports for three new QMP events
ping. On 03/14/2012 11:26 PM, Osier Yang wrote: v1 ~ v2: * Two more patches, [5/11] to prohibit tray='open' for block type disk. And [9/11] to introduce a new domain state 'pmsuspended' * Definition (including name) for DEVICE_TRAY_MOVED event is changed into: typedef void (*virConnectDomainEventTrayChangeCallback)(virConnectPtr conn, virDomainPtr dom, const char *devAlias, int reason, void *opaque); * Definition for both SUSPEND and WAKEUP event are changed, one more parameter 'int reason' for future extension. * While SUSPEND event is emitted, the running domain will be transfered into the new domain state pmsuspended, instead of paused. And while WAKEUP is emitted, it will transfer the domain state to running only if the domain state is pmsuspended. This patch series adds support for 3 new QMP events: WAKEUP, SUSPEND, and DEVICE_TRAY_MOVED, and related changes on domain's conf and status. [1/11] Add support for tray moved event [2/11] ~ [6/11]: New attribute tray is added to disk target, it indicates the tray status of removable disk, i.e. CDROM and Floppy disks, its value could be either of open or closed, defaults to closed, and a removable disk with tray == open won't have the source when domain is started. The value of tray will be updated while tray moved event is emitted from guest. tray status 'open' is prohibited for block type disk. Prior to these patches, if the user ejected the medium of removable disk from guest side, and then do migration or save/restoring, the guest will still starts the medium source ,and thus the medium will still exists in guest, which is strange. These patches fix it. [7/11] + [11/11]: Add support for wakeup event, and update the domain status to running if the domain is in pmsuspended state, while the wakeup event is emitted. [9/11] Introduce new domain state pmsuspended, which indicates the domain has been suspended by guest power management, e.g, entered into s3 state. [8/11] + [10/11]: Add support for suspend event, and update the domain status to pmsuspended if the domain was running, while the suspend event is emitted. Osier Yang(11) Add support for event tray moved of removable disks docs: Add documentation for new attribute tray of disk target conf: Parse and for the tray attribute qemu: Do not start with source for removable disks if tray is open qemu: Prohibit setting tray status as open for block type disk qemu: Update tray status while tray moved event is emitted Add support for the wakeup event Add support for the suspend event New domain state pmsuspended qemu: Update domain state to pmsuspended while suspend event occurs qemu: Update domain status to running while wakeup event is emitted daemon/remote.c| 79 ++ docs/formatdomain.html.in | 13 ++- docs/schemas/domaincommon.rng |8 + examples/domain-events/events-c/event-test.c | 66 +- examples/domain-events/events-python/event-test.py | 12 ++ include/libvirt/libvirt.h.in | 98 - python/libvirt-override-virConnect.py | 27 python/libvirt-override.c | 150 src/conf/domain_conf.c | 71 -- src/conf/domain_conf.h |9 ++ src/conf/domain_event.c| 115 +++ src/conf/domain_event.h| 10 ++ src/libvirt_private.syms |6 + src/qemu/qemu_command.c| 31 - src/qemu/qemu_monitor.c| 33 + src/qemu/qemu_monitor.h| 16 ++- src/qemu/qemu_monitor_json.c | 45 ++ src/qemu/qemu_process.c| 121 src/remote/remote_driver.c | 95 src/remote/remote_protocol.x | 19 +++- src/remote_protocol-structs| 14 ++ tests/qemuxml2argvdata/qemuxml2argv-boot-cdrom.xml |2 +- .../qemuxml2argv-boot-complex-bootindex.xml|6 +- .../qemuxml2argvdata/qemuxml2argv-boot-complex.xml |6 +- .../qemuxml2argvdata/qemuxml2argv-boot-floppy.xml |2 +- ...uxml2argv-boot-menu-disable-drive-bootindex.xml |2 +- .../qemuxml2argv-boot-menu-disable-drive.xml |2 +- .../qemuxml2argv-boot-menu-disable.xml |2 +- .../qemuxml2argv-boot-menu-enable.xml |2 +-
[libvirt] [PATCH v4] python: add virDomainGetCPUStats python binding API
dom.getCPUStats(True, 0)
[{'cpu_time': 24699446159L, 'system_time': 1087000L, 'user_time':
95000L}]
dom.getCPUStats(False, 0)
[{'cpu_time': 8535292289L}, {'cpu_time': 1005395355L}, {'cpu_time':
9351766377L}, {'cpu_time': 5813545649L}]
*generator.py Add a new naming rule
*libvirt-override-api.xml The API function description
*libvirt-override.c Implement it.
---
python/generator.py |5 +-
python/libvirt-override-api.xml | 13
python/libvirt-override.c | 147 +++
3 files changed, 164 insertions(+), 1 deletions(-)
diff --git a/python/generator.py b/python/generator.py
index 98072f0..4f95cc9 100755
--- a/python/generator.py
+++ b/python/generator.py
@@ -423,7 +423,7 @@ skip_impl = (
'virDomainGetBlockIoTune',
'virDomainSetInterfaceParameters',
'virDomainGetInterfaceParameters',
-'virDomainGetCPUStats', # not implemented now.
+'virDomainGetCPUStats',
'virDomainGetDiskErrors',
)
@@ -966,6 +966,9 @@ def nameFixup(name, classe, type, file):
elif name[0:19] == virStorageVolLookup:
func = name[3:]
func = string.lower(func[0:1]) + func[1:]
+elif name[0:20] == virDomainGetCPUStats:
+func = name[9:]
+func = string.lower(func[0:1]) + func[1:]
elif name[0:12] == virDomainGet:
func = name[12:]
func = string.lower(func[0:1]) + func[1:]
diff --git a/python/libvirt-override-api.xml b/python/libvirt-override-api.xml
index ab8f33a..26d9902 100644
--- a/python/libvirt-override-api.xml
+++ b/python/libvirt-override-api.xml
@@ -149,6 +149,19 @@
arg name='path' type='char *' info='the path for the block device'/
arg name='flags' type='int' info='flags (unused; pass 0)'/
/function
+function name='virDomainGetCPUStats' file='python'
+ infoExtracts CPU statistics for a running domain. On success it will
+ return a list of data of dictionary type. If boolean total is False, the
+ first element of the list refers to CPU0 on the host, second element is
+ CPU1, and so on. The format of data struct is as follows:
+ [{cpu_time:xxx}, {cpu_time:xxx}, ...]
+ If it is True, it returns total domain CPU statistics in the format of
+ [{cpu_time:xxx, user_time:xxx, system_time:xxx}]/info
+ return type='str *' info='returns a list of dictionary in case of
success, None in case of error'/
+ arg name='domain' type='virDomainPtr' info='pointer to domain object'/
+ arg name='total' type='bool' info='on true, return total domain CPU
statistics, false return per-cpu info'/
+ arg name='flags' type='int' info='flags (unused; pass 0)'/
+/function
function name='virDomainInterfaceStats' file='python'
infoExtracts interface device statistics for a domain/info
return type='virDomainInterfaceStats' info='a tuple of statistics'/
diff --git a/python/libvirt-override.c b/python/libvirt-override.c
index 792cfa3..5920456 100644
--- a/python/libvirt-override.c
+++ b/python/libvirt-override.c
@@ -378,6 +378,152 @@ cleanup:
}
static PyObject *
+libvirt_virDomainGetCPUStats(PyObject *self ATTRIBUTE_UNUSED, PyObject *args)
+{
+virDomainPtr domain;
+PyObject *pyobj_domain, *totalbool;
+PyObject *cpu, *total;
+PyObject *ret = NULL;
+PyObject *error = NULL;
+int ncpus = -1, start_cpu = 0;
+int sumparams = 0, nparams = -1;
+int i, i_retval;
+unsigned int flags, totalflag;
+virTypedParameterPtr params = NULL, cpuparams;
+
+if (!PyArg_ParseTuple(args, (char *)OOi:virDomainGetCPUStats,
+ pyobj_domain, totalbool, flags))
+return NULL;
+domain = (virDomainPtr) PyvirDomain_Get(pyobj_domain);
+
+if (!PyBool_Check(totalbool)) {
+PyErr_Format(PyExc_TypeError,
+The \total\ attribute must be bool);
+return NULL;
+} else {
+/* Hack - Python's definition of Py_True breaks strict
+ * aliasing rules, so can't directly compare
+ */
+PyObject *hacktrue = PyBool_FromLong(1);
+totalflag = hacktrue == totalbool ? 1 : 0;
+Py_DECREF(hacktrue);
+}
+
+if ((ret = PyList_New(0)) == NULL)
+return NULL;
+
+if (!totalflag) {
+LIBVIRT_BEGIN_ALLOW_THREADS;
+ncpus = virDomainGetCPUStats(domain, NULL, 0, 0, 0, flags);
+LIBVIRT_END_ALLOW_THREADS;
+
+if (ncpus 0) {
+error = VIR_PY_NONE;
+goto error;
+}
+
+LIBVIRT_BEGIN_ALLOW_THREADS;
+nparams = virDomainGetCPUStats(domain, NULL, 0, 0, 1, flags);
+LIBVIRT_END_ALLOW_THREADS;
+
+if (nparams 0) {
+error = VIR_PY_NONE;
+goto error;
+}
+
+sumparams = nparams * MIN(ncpus, 128);
+
+if (VIR_ALLOC_N(params, sumparams) 0) {
+error = PyErr_NoMemory();
+goto error;
+}
+
+while (ncpus) {
+int
[libvirt] sysctl variables, FreeBSD
Is there a method that can be used to remove the sysctl tuning for FreeBSD in the pipeline? Here are the current patches I have, but would like to remove: http://www.freebsd.org/cgi/cvsweb.cgi/ports/devel/libvirt/files/ Thanks! Jason -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
