5:41 a.m.
this small patch set:
* move common logic of all libvirt_virDomainPin* functions to new helper
in util module.
* add check for pycpumap length.
Changes since v1:
- add new helper in util module.
Konstantin Neumoin (2):
minor clean-up for libvirt_virDomainPin*
add check for pycpumap length
libvirt-override.c | 131 +++++------------------------------------------------
libvirt-utils.c | 63 ++++++++++++++++++++++++++
libvirt-utils.h | 5 ++
3 files changed, 79 insertions(+), 120 deletions(-)
--
2.5.5
5:41 a.m.
New subject: [libvirt] [PATCH v2 python 1/2] minor clean-up for libvirt_virDomainPin*
All libvirt_virDomainPin* functions do the same thing for convert
pycpumap to cpumap, so this patch moves all common logic to new
helper - virPyCpuMapToChar.
Signed-off-by: Konstantin Neumoin <kneumoin(a)virtuozzo.com>
---
libvirt-override.c | 131 +++++------------------------------------------------
libvirt-utils.c | 60 ++++++++++++++++++++++++
libvirt-utils.h | 5 ++
3 files changed, 76 insertions(+), 120 deletions(-)
diff --git a/libvirt-override.c b/libvirt-override.c
index fa3e2ca..ba0d87c 100644
--- a/libvirt-override.c
+++ b/libvirt-override.c
@@ -1302,8 +1302,7 @@ libvirt_virDomainPinVcpu(PyObject *self ATTRIBUTE_UNUSED,
PyObject *pyobj_domain, *pycpumap;
PyObject *ret = NULL;
unsigned char *cpumap;
- int cpumaplen, vcpu, tuple_size, cpunum;
- size_t i;
+ int cpumaplen, vcpu, cpunum;
int i_retval;
if (!PyArg_ParseTuple(args, (char *)"OiO:virDomainPinVcpu",
@@ -1314,34 +1313,8 @@ libvirt_virDomainPinVcpu(PyObject *self ATTRIBUTE_UNUSED,
if ((cpunum = getPyNodeCPUCount(virDomainGetConnect(domain))) < 0)
return VIR_PY_INT_FAIL;
- if (PyTuple_Check(pycpumap)) {
- tuple_size = PyTuple_Size(pycpumap);
- if (tuple_size == -1)
- return ret;
- } else {
- PyErr_SetString(PyExc_TypeError, "Unexpected type, tuple is
required");
- return ret;
- }
-
- cpumaplen = VIR_CPU_MAPLEN(cpunum);
- if (VIR_ALLOC_N(cpumap, cpumaplen) < 0)
- return PyErr_NoMemory();
-
- for (i = 0; i < tuple_size; i++) {
- PyObject *flag = PyTuple_GetItem(pycpumap, i);
- bool b;
-
- if (!flag || libvirt_boolUnwrap(flag, &b) < 0)
- goto cleanup;
-
- if (b)
- VIR_USE_CPU(cpumap, i);
- else
- VIR_UNUSE_CPU(cpumap, i);
- }
-
- for (; i < cpunum; i++)
- VIR_UNUSE_CPU(cpumap, i);
+ if (virPyCpuMapToChar(cpunum, pycpumap, &cpumap, &cpumaplen) < 0)
+ return NULL;
LIBVIRT_BEGIN_ALLOW_THREADS;
i_retval = virDomainPinVcpu(domain, vcpu, cpumap, cpumaplen);
@@ -1366,8 +1339,7 @@ libvirt_virDomainPinVcpuFlags(PyObject *self ATTRIBUTE_UNUSED,
PyObject *pyobj_domain, *pycpumap;
PyObject *ret = NULL;
unsigned char *cpumap;
- int cpumaplen, vcpu, tuple_size, cpunum;
- size_t i;
+ int cpumaplen, vcpu, cpunum;
unsigned int flags;
int i_retval;
@@ -1379,34 +1351,8 @@ libvirt_virDomainPinVcpuFlags(PyObject *self ATTRIBUTE_UNUSED,
if ((cpunum = getPyNodeCPUCount(virDomainGetConnect(domain))) < 0)
return VIR_PY_INT_FAIL;
- if (PyTuple_Check(pycpumap)) {
- tuple_size = PyTuple_Size(pycpumap);
- if (tuple_size == -1)
- return ret;
- } else {
- PyErr_SetString(PyExc_TypeError, "Unexpected type, tuple is
required");
- return ret;
- }
-
- cpumaplen = VIR_CPU_MAPLEN(cpunum);
- if (VIR_ALLOC_N(cpumap, cpumaplen) < 0)
- return PyErr_NoMemory();
-
- for (i = 0; i < tuple_size; i++) {
- PyObject *flag = PyTuple_GetItem(pycpumap, i);
- bool b;
-
- if (!flag || libvirt_boolUnwrap(flag, &b) < 0)
- goto cleanup;
-
- if (b)
- VIR_USE_CPU(cpumap, i);
- else
- VIR_UNUSE_CPU(cpumap, i);
- }
-
- for (; i < cpunum; i++)
- VIR_UNUSE_CPU(cpumap, i);
+ if (virPyCpuMapToChar(cpunum, pycpumap, &cpumap, &cpumaplen) < 0)
+ return NULL;
LIBVIRT_BEGIN_ALLOW_THREADS;
i_retval = virDomainPinVcpuFlags(domain, vcpu, cpumap, cpumaplen, flags);
@@ -1505,8 +1451,7 @@ libvirt_virDomainPinEmulator(PyObject *self ATTRIBUTE_UNUSED,
virDomainPtr domain;
PyObject *pyobj_domain, *pycpumap;
unsigned char *cpumap = NULL;
- int cpumaplen, tuple_size, cpunum;
- size_t i;
+ int cpumaplen, cpunum;
int i_retval;
unsigned int flags;
@@ -1519,37 +1464,9 @@ libvirt_virDomainPinEmulator(PyObject *self ATTRIBUTE_UNUSED,
if ((cpunum = getPyNodeCPUCount(virDomainGetConnect(domain))) < 0)
return VIR_PY_INT_FAIL;
- cpumaplen = VIR_CPU_MAPLEN(cpunum);
-
- if (!PyTuple_Check(pycpumap)) {
- PyErr_SetString(PyExc_TypeError, "Unexpected type, tuple is
required");
- return NULL;
- }
-
- if ((tuple_size = PyTuple_Size(pycpumap)) == -1)
+ if (virPyCpuMapToChar(cpunum, pycpumap, &cpumap, &cpumaplen) < 0)
return NULL;
- if (VIR_ALLOC_N(cpumap, cpumaplen) < 0)
- return PyErr_NoMemory();
-
- for (i = 0; i < tuple_size; i++) {
- PyObject *flag = PyTuple_GetItem(pycpumap, i);
- bool b;
-
- if (!flag || libvirt_boolUnwrap(flag, &b) < 0) {
- VIR_FREE(cpumap);
- return NULL;
- }
-
- if (b)
- VIR_USE_CPU(cpumap, i);
- else
- VIR_UNUSE_CPU(cpumap, i);
- }
-
- for (; i < cpunum; i++)
- VIR_UNUSE_CPU(cpumap, i);
-
LIBVIRT_BEGIN_ALLOW_THREADS;
i_retval = virDomainPinEmulator(domain, cpumap, cpumaplen, flags);
LIBVIRT_END_ALLOW_THREADS;
@@ -1713,8 +1630,7 @@ libvirt_virDomainPinIOThread(PyObject *self ATTRIBUTE_UNUSED,
PyObject *pyobj_domain, *pycpumap;
PyObject *ret = NULL;
unsigned char *cpumap;
- int cpumaplen, iothread_val, tuple_size, cpunum;
- size_t i;
+ int cpumaplen, iothread_val, cpunum;
unsigned int flags;
int i_retval;
@@ -1726,33 +1642,8 @@ libvirt_virDomainPinIOThread(PyObject *self ATTRIBUTE_UNUSED,
if ((cpunum = getPyNodeCPUCount(virDomainGetConnect(domain))) < 0)
return VIR_PY_INT_FAIL;
- if (PyTuple_Check(pycpumap)) {
- if ((tuple_size = PyTuple_Size(pycpumap)) == -1)
- return ret;
- } else {
- PyErr_SetString(PyExc_TypeError, "Unexpected type, tuple is
required");
- return ret;
- }
-
- cpumaplen = VIR_CPU_MAPLEN(cpunum);
- if (VIR_ALLOC_N(cpumap, cpumaplen) < 0)
- return PyErr_NoMemory();
-
- for (i = 0; i < tuple_size; i++) {
- PyObject *flag = PyTuple_GetItem(pycpumap, i);
- bool b;
-
- if (!flag || libvirt_boolUnwrap(flag, &b) < 0)
- goto cleanup;
-
- if (b)
- VIR_USE_CPU(cpumap, i);
- else
- VIR_UNUSE_CPU(cpumap, i);
- }
-
- for (; i < cpunum; i++)
- VIR_UNUSE_CPU(cpumap, i);
+ if (virPyCpuMapToChar(cpunum, pycpumap, &cpumap, &cpumaplen) < 0)
+ return NULL;
LIBVIRT_BEGIN_ALLOW_THREADS;
i_retval = virDomainPinIOThread(domain, iothread_val,
diff --git a/libvirt-utils.c b/libvirt-utils.c
index 2bf7519..aaf4bea 100644
--- a/libvirt-utils.c
+++ b/libvirt-utils.c
@@ -586,3 +586,63 @@ virPyDictToTypedParams(PyObject *dict,
return ret;
}
#endif /* LIBVIR_CHECK_VERSION(1, 1, 0) */
+
+
+/* virPyCpuMapToChar
+ * @cpunum: the number of cpus
+ * @pycpumap: source Py cpu map
+ * @cpumapptr: destination cpu map
+ * @cpumaplen: destination cpu map length
+ *
+ * Helper function to convert a pycpumap to char*
+ *
+ * return -1 on error.
+ */
+int
+virPyCpuMapToChar(int cpunum,
+ PyObject *pycpumap,
+ unsigned char **cpumapptr,
+ int *cpumaplen)
+{
+ int tuple_size;
+ size_t i;
+ int i_retval = -1;
+ *cpumapptr = NULL;
+
+ if (!PyTuple_Check(pycpumap)) {
+ PyErr_SetString(PyExc_TypeError, "Unexpected type, tuple is
required");
+ goto exit;
+ }
+
+ *cpumaplen = VIR_CPU_MAPLEN(cpunum);
+
+ if ((tuple_size = PyTuple_Size(pycpumap)) == -1)
+ goto exit;
+
+ if (VIR_ALLOC_N(*cpumapptr, *cpumaplen) < 0) {
+ PyErr_NoMemory();
+ goto exit;
+ }
+
+ for (i = 0; i < tuple_size; i++) {
+ PyObject *flag = PyTuple_GetItem(pycpumap, i);
+ bool b;
+
+ if (!flag || libvirt_boolUnwrap(flag, &b) < 0) {
+ VIR_FREE(*cpumapptr);
+ goto exit;
+ }
+
+ if (b)
+ VIR_USE_CPU(*cpumapptr, i);
+ else
+ VIR_UNUSE_CPU(*cpumapptr, i);
+ }
+
+ for (; i < cpunum; i++)
+ VIR_UNUSE_CPU(*cpumapptr, i);
+
+ i_retval = 0;
+exit:
+ return i_retval;
+}
diff --git a/libvirt-utils.h b/libvirt-utils.h
index f74654c..34d40b7 100644
--- a/libvirt-utils.h
+++ b/libvirt-utils.h
@@ -349,4 +349,9 @@ int virPyDictToTypedParams(PyObject *dict,
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);
# endif /* LIBVIR_CHECK_VERSION(1, 1, 0) */
+int virPyCpuMapToChar(int cpunum,
+ PyObject *pycpumap,
+ unsigned char **cpumapptr,
+ int *cpumaplen);
+
#endif /* __LIBVIRT_UTILS_H__ */
--
2.5.5
11:13 p.m.
New subject: [libvirt] [PATCH v2 python 1/2] minor clean-up for libvirt_virDomainPin*
On Fri, Oct 28, 2016 at 13:41:09 +0300, Konstantin Neumoin wrote:
All libvirt_virDomainPin* functions do the same thing for convert
pycpumap to cpumap, so this patch moves all common logic to new
helper - virPyCpuMapToChar.
Signed-off-by: Konstantin Neumoin <kneumoin(a)virtuozzo.com>
---
libvirt-override.c | 131 +++++------------------------------------------------
libvirt-utils.c | 60 ++++++++++++++++++++++++
libvirt-utils.h | 5 ++
3 files changed, 76 insertions(+), 120 deletions(-)
Nice cleanup.
diff --git a/libvirt-override.c b/libvirt-override.c
index fa3e2ca..ba0d87c 100644
--- a/libvirt-override.c
+++ b/libvirt-override.c
The usage here looks good to me.
diff --git a/libvirt-utils.c b/libvirt-utils.c
index 2bf7519..aaf4bea 100644
--- a/libvirt-utils.c
+++ b/libvirt-utils.c
@@ -586,3 +586,63 @@ virPyDictToTypedParams(PyObject *dict,
return ret;
}
#endif /* LIBVIR_CHECK_VERSION(1, 1, 0) */
+
+
+/* virPyCpuMapToChar
virPyCpumapConvert perhaps?
+ * @cpunum: the number of cpus
This is the number of physical cpus of the host we are talking to. It
should be noted.
+ * @pycpumap: source Py cpu map
You should note that this should be a python tuple of bools.
+ * @cpumapptr: destination cpu map
+ * @cpumaplen: destination cpu map length
+ *
+ * Helper function to convert a pycpumap to char*
+ *
+ * return -1 on error.
Success case is not documented. Also the fact that the proper python
error is set in cases when it makes sense is not documented.
+ */
+int
+virPyCpuMapToChar(int cpunum,
+ PyObject *pycpumap,
+ unsigned char **cpumapptr,
+ int *cpumaplen)
+{
+ int tuple_size;
+ size_t i;
+ int i_retval = -1;
We usually call this "ret".
+ *cpumapptr = NULL;
+
+ if (!PyTuple_Check(pycpumap)) {
+ PyErr_SetString(PyExc_TypeError, "Unexpected type, tuple is
required");
+ goto exit;
+ }
+
+ *cpumaplen = VIR_CPU_MAPLEN(cpunum);
+
+ if ((tuple_size = PyTuple_Size(pycpumap)) == -1)
+ goto exit;
+
+ if (VIR_ALLOC_N(*cpumapptr, *cpumaplen) < 0) {
+ PyErr_NoMemory();
+ goto exit;
+ }
+
+ for (i = 0; i < tuple_size; i++) {
+ PyObject *flag = PyTuple_GetItem(pycpumap, i);
+ bool b;
+
+ if (!flag || libvirt_boolUnwrap(flag, &b) < 0) {
+ VIR_FREE(*cpumapptr);
+ goto exit;
+ }
+
+ if (b)
+ VIR_USE_CPU(*cpumapptr, i);
+ else
+ VIR_UNUSE_CPU(*cpumapptr, i);
+ }
+
+ for (; i < cpunum; i++)
+ VIR_UNUSE_CPU(*cpumapptr, i);
+
+ i_retval = 0;
+exit:
We indent labels by one space in libvirt so that git grep -p works well.
Also we have a naming convention for labels passed both on success and
error and call them "cleanup".
Finally the label is not necessary in this case as you are cleaning up
the pointer in the code and thus the label can be avoided completely and
you can return -1 directly. This will also avoid the "ret" or
"i_retval"
variable.
+ return i_retval;
+}
Peter
11:18 p.m.
New subject: [libvirt] [PATCH v2 python 1/2] minor clean-up for libvirt_virDomainPin*
In subject:
"Move cpumap conversion code to a common helper" or something more
descriptive than "minor clean-up"
On Fri, Oct 28, 2016 at 13:41:09 +0300, Konstantin Neumoin wrote:
All libvirt_virDomainPin* functions do the same thing for convert
pycpumap to cpumap, so this patch moves all common logic to new
helper - virPyCpuMapToChar.
Signed-off-by: Konstantin Neumoin <kneumoin(a)virtuozzo.com>
---
Peter
5:41 a.m.
New subject: [libvirt] [PATCH v2 python 2/2] add check for pycpumap length
If we pass large(more than cpunum) cpu mask to any libvirt_virDomainPin*
function, it could leads to crash. So we have to check tuple size in
virPyCpuMapToChar and ignore extra tuple members.
Signed-off-by: Konstantin Neumoin <kneumoin(a)virtuozzo.com>
---
libvirt-utils.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/libvirt-utils.c b/libvirt-utils.c
index aaf4bea..3fc0fdd 100644
--- a/libvirt-utils.c
+++ b/libvirt-utils.c
@@ -589,7 +589,8 @@ virPyDictToTypedParams(PyObject *dict,
/* virPyCpuMapToChar
- * @cpunum: the number of cpus
+ * @cpunum: the number of cpus, only this first elements make sense,
+ * so others will be ignored(filled by zeros).
* @pycpumap: source Py cpu map
* @cpumapptr: destination cpu map
* @cpumaplen: destination cpu map length
@@ -604,7 +605,7 @@ virPyCpuMapToChar(int cpunum,
unsigned char **cpumapptr,
int *cpumaplen)
{
- int tuple_size;
+ int tuple_size, rel_cpumaplen;
size_t i;
int i_retval = -1;
*cpumapptr = NULL;
@@ -624,7 +625,9 @@ virPyCpuMapToChar(int cpunum,
goto exit;
}
- for (i = 0; i < tuple_size; i++) {
+ rel_cpumaplen = MIN(cpunum, tuple_size);
+
+ for (i = 0; i < rel_cpumaplen; i++) {
PyObject *flag = PyTuple_GetItem(pycpumap, i);
bool b;
--
2.5.5
11:17 p.m.
New subject: [libvirt] [PATCH v2 python 2/2] add check for pycpumap length
In subject:
"Don't overrun buffer when converting cpumap" perhaps? That would IMHO
explain the patch a bit more when looking at shortlog.
On Fri, Oct 28, 2016 at 13:41:10 +0300, Konstantin Neumoin wrote:
If we pass large(more than cpunum) cpu mask to any
libvirt_virDomainPin*
function, it could leads to crash. So we have to check tuple size in
virPyCpuMapToChar and ignore extra tuple members.
Signed-off-by: Konstantin Neumoin <kneumoin(a)virtuozzo.com>
---
libvirt-utils.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/libvirt-utils.c b/libvirt-utils.c
index aaf4bea..3fc0fdd 100644
--- a/libvirt-utils.c
+++ b/libvirt-utils.c
@@ -589,7 +589,8 @@ virPyDictToTypedParams(PyObject *dict,
/* virPyCpuMapToChar
- * @cpunum: the number of cpus
+ * @cpunum: the number of cpus, only this first elements make sense,
+ * so others will be ignored(filled by zeros).
So this sentence belongs to the previous patch and I'd put it below into
the text explaining how this variable is treated.
* @pycpumap: source Py cpu map
* @cpumapptr: destination cpu map
* @cpumaplen: destination cpu map length
@@ -604,7 +605,7 @@ virPyCpuMapToChar(int cpunum,
unsigned char **cpumapptr,
int *cpumaplen)
{
- int tuple_size;
+ int tuple_size, rel_cpumaplen;
size_t i;
int i_retval = -1;
*cpumapptr = NULL;
@@ -624,7 +625,9 @@ virPyCpuMapToChar(int cpunum,
goto exit;
}
- for (i = 0; i < tuple_size; i++) {
+ rel_cpumaplen = MIN(cpunum, tuple_size);
+
+ for (i = 0; i < rel_cpumaplen; i++) {
You can avoid the temporary variable by checking both tuple_size and
cpumaplen in the condition.
PyObject *flag = PyTuple_GetItem(pycpumap, i);
bool b;
Not visible in the context is the second for loop that clears the rest
of the bits from the tuple which exceed "cpumap". This is not necessary
any more since you now fill only the first elements.
Peter
2946
days inactive
2950
days old
5 comments
2 participants
participants (2)
-
Konstantin Neumoin -
Peter Krempa