[libvirt] python module set-up ignores virInitialize failure

[Daniel P. Berrange]

On Tue, May 18, 2010 at 01:56:23PM +0200, Jim Meyering wrote:
> Daniel P. Berrange wrote:
> 
> > On Tue, May 18, 2010 at 11:43:30AM +0100, Daniel P. Berrange wrote:
> >> On Tue, May 18, 2010 at 12:32:13PM +0200, Jim Meyering wrote:
> >> > I've just fixed code in a test that ignored virInitialize failure.
> >> > Looking at all uses, I saw one other: in python/libvirt-override.c,
> >> > where the initialization function ignores virInitialize failure:
> >> >
> >> >   void
> >> >   #ifndef __CYGWIN__
> >> >   initlibvirtmod
> >> >   #else
> >> >   initcygvirtmod
> >> >   #endif
> >> >     (void)
> >> >   {
> >> >       static int initialized = 0;
> >> >
> >> >       if (initialized != 0)
> >> >           return;
> >> >
> >> >       virInitialize();
> >> >
> >> >       /* initialize the python extension module */
> >> >       Py_InitModule((char *)
> >> >   #ifndef __CYGWIN__
> >> >                     "libvirtmod"
> >> >   #else
> >> >                     "cygvirtmod"
> >> >   #endif
> >> >                     , libvirtMethods);
> >> >
> >> >       initialized = 1;
> >> >   }
> >> >
> >> > Unfortunately, this function is public, so we can't change its signature.
> >>
> >> More specifically, the signature is defined by Python's loadable
> >> module interface so we're not at liberty to redeclare that.
> >>
> >> > Any suggestions?
> >>
> >> abort()
> >
> > Actually I've got another idea. Make the Py_InitModule() call conditional
> > on virInitialize() suceeding. That way if virInitialize() fails, none of
> > the libvirt APIs will get bound to the python layer, preventing their
> > use
> 
> Either works for me:
> 
> From 7dbf938ab10657a94702cd766afa336fc68d8c80 Mon Sep 17 00:00:00 2001
> From: Jim Meyering <meyering at redhat.com>
> Date: Tue, 18 May 2010 13:46:27 +0200
> Subject: [PATCH] python: don't ignore virInitialize failure in module initialization
> 
> * python/libvirt-override.c (initlibvirtmod): Upon virInitialize
> failure, skip the Py_InitModule call.
> ---
>  python/libvirt-override.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/python/libvirt-override.c b/python/libvirt-override.c
> index b97445b..c9721f7 100644
> --- a/python/libvirt-override.c
> +++ b/python/libvirt-override.c
> @@ -3534,25 +3534,26 @@ void
>  #ifndef __CYGWIN__
>  initlibvirtmod
>  #else
>  initcygvirtmod
>  #endif
>    (void)
>  {
>      static int initialized = 0;
> 
>      if (initialized != 0)
>          return;
> 
> -    virInitialize();
> +    if (virInitialize() < 0)
> +        return;
> 
>      /* initialize the python extension module */
>      Py_InitModule((char *)
>  #ifndef __CYGWIN__
>                    "libvirtmod"
>  #else
>                    "cygvirtmod"
>  #endif
>                    , libvirtMethods);
> 
>      initialized = 1;
>  }

ACK


Daniel
-- 
|: Red Hat, Engineering, London    -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org        -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|