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

[Guannan Ren]

On 02/03/2012 01:48 AM, Eric Blake wrote:

On 01/28/2012 07:53 AM, Guannan Ren wrote:

         *virDomainSetNumaParameters
         *virDomainGetNumaParameters
---
  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
  INCLUDES = \
        $(PYTHON_INCLUDES) \
        -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) \
        $(GETTEXT_CPPFLAGS)
@@ -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>
+<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
         http://gcc.gnu.org/bugzilla/show_bug.cgi?id=17308
         I am not clear about it is still helpful to use it here?

     Guannan Ren

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