[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 1/2 v3] Python: Refactoring virTypedParameter conversion for NUMA tuning APIs

On 02/03/2012 01:48 AM, Eric Blake wrote:
On 01/28/2012 07:53 AM, Guannan Ren wrote:
  python/Makefile.am              |    4 +-
  python/libvirt-override-api.xml |   13 ++
  python/libvirt-override.c       |  314 +++++++++++++++++++++++++++++++++++++++
  3 files changed, 330 insertions(+), 1 deletions(-)

diff --git a/python/Makefile.am b/python/Makefile.am
index 3068eee..4302fa5 100644
--- a/python/Makefile.am
+++ b/python/Makefile.am
@@ -8,6 +8,8 @@ SUBDIRS= . tests
  	-I$(top_srcdir)/include \
+        -I$(top_srcdir)/src \
+        -I$(top_srcdir)/gnulib/lib \
Hmm, you converted TAB to space.

  	-I$(top_builddir)/include \
Also, since gnulib has some directly-supplied headers (srcdir) and some
generated headers (builddir), we really should be using both locations
so as not to break VPATH.

  	-I$(top_builddir)/$(subdir) \
@@ -42,7 +44,7 @@ all-local: libvirt.py libvirt_qemu.py

  pyexec_LTLIBRARIES = libvirtmod.la libvirtmod_qemu.la

-libvirtmod_la_SOURCES = libvirt-override.c typewrappers.c
+libvirtmod_la_SOURCES = libvirt-override.c typewrappers.c ../src/util/virtypedparam.c
I'm not sure I like this.  Rather than pulling in just one or two source
files, we should probably instead figure out how to directly link
against the libvirt_util library and have all of the functions
available.  This would also make it possible to use VIR_FREE and friends
(at which point, we should disable the syntax-check exceptions currently
in effect on the python files).

I think I will do a preliminary patch, which does _just_ the makefile
work to pull in the use of libvirt_util, then we can rebase this patch
on top of that one.  I know Alex Jia was also complaining about the
inability to use normal libvirt conventions, because the Makefile wasn't
yet set up for it, so this will be a good move overall.

+<function name='virDomainSetNumaParameters' file='python'>
+<info>Change the NUMA tunables</info>
+<return type='int' info='-1 in case of error, 0 in case of success.'/>
+<arg name='domain' type='virDomainPtr' info='pointer to domain object'/>
+<arg name='params' type='virTypedParameterPtr' info='pointer to numa tunable objects'/>
Is th is type correct, or can it be any python dictionary type that maps
valid numa tunable parameter names to values?

+<arg name='flags'  type='int' info='an OR&apos;ed set of virDomainModificationImpact'/>
+<function name='virDomainGetNumaParameters' file='python'>
+<info>Get the NUMA parameters</info>
+<return type='int' info='returns a dictionary of params in case of success, -1 in case of error'/>
The return type should be a python object - a dictionary on success,
PyNone on failure where libvirt populated an error message, or NULL on a
python exception.

+++ b/python/libvirt-override.c
@@ -21,6 +21,7 @@
  #include "libvirt/virterror.h"
  #include "typewrappers.h"
  #include "libvirt.h"
+#include "util/virtypedparam.h"
Hmm, the rest of our code sets up INCLUDES so that we can use just
"virtypedparam.h" instead of "util/virtypedparam.h"; another thing for
me to do in pulling out the infrastructure into a preliminary patch.

  #ifndef __CYGWIN__
  extern void initlibvirtmod(void);
@@ -61,6 +62,208 @@ static char *py_str(PyObject *obj)
      return PyString_AsString(str);

+/* Two helper functions to help the conversions between C to Python
+ * for the virTypedParameter used in the following APIs. */
+static PyObject *
+getPyVirTypedParameter(virTypedParameterPtr params, int nparams)
+    PyObject *info;
+    PyObject *key, *val;
+    PyObject *ret = NULL;
+    int i;
+    if (!params)
+        return ret;
If we return NULL, we should ensure that there is a valid python
exception object ready for the caller to access.  I'm thinking it might
be better to mark this function with ATTRIBUTE_NONNULL(1) to avoid
worrying about whether the caller has properly generated a python
exception before passing us NULL.

     Hi Eric

         I saw your comments about the nonnull attribute usage as follows
         I am not clear about it is still helpful to use it here?

     Guannan Ren

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]