[libvirt] python module set-up ignores virInitialize failure
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. Any suggestions? For reference, here's the function definition. It shows that there are many ways in which virInitialize can fail, including its many registration functions: /** * virInitialize: * * Initialize the library. It's better to call this routine at startup * in multithreaded applications to avoid potential race when initializing * the library. * * Returns 0 in case of success, -1 in case of error */ int virInitialize(void) { if (initialized) return(0); initialized = 1; if (virThreadInitialize() < 0 || virErrorInitialize() < 0 || virRandomInitialize(time(NULL) ^ getpid())) return -1; gcry_control(GCRYCTL_SET_THREAD_CBS, &virTLSThreadImpl); gcry_check_version(NULL); virLogSetFromEnv(); DEBUG0("register drivers"); #if HAVE_WINSOCK2_H if (winsock_init () == -1) return -1; #endif if (!bindtextdomain(GETTEXT_PACKAGE, LOCALEBASEDIR)) return (-1); /* * Note that the order is important: the first ones have a higher * priority when calling virConnectOpen. */ #ifdef WITH_DRIVER_MODULES /* We don't care if any of these fail, because the whole point * is to allow users to only install modules they want to use. * If they try to open a connection for a module that * is not loaded they'll get a suitable error at that point */ virDriverLoadModule("test"); virDriverLoadModule("xen"); virDriverLoadModule("openvz"); virDriverLoadModule("vbox"); virDriverLoadModule("esx"); virDriverLoadModule("xenapi"); virDriverLoadModule("remote"); #else # ifdef WITH_TEST if (testRegister() == -1) return -1; # endif # ifdef WITH_XEN if (xenRegister () == -1) return -1; # endif # ifdef WITH_OPENVZ if (openvzRegister() == -1) return -1; # endif # ifdef WITH_PHYP if (phypRegister() == -1) return -1; # endif # ifdef WITH_VBOX if (vboxRegister() == -1) return -1; # endif # ifdef WITH_ESX if (esxRegister() == -1) return -1; # endif # ifdef WITH_XENAPI if (xenapiRegister() == -1) return -1; # endif # ifdef WITH_REMOTE if (remoteRegister () == -1) return -1; # endif #endif return(0); }
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() 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 :|
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()
Here you go. Do you think it's worth a diagnostic first? If so, using what function?
From 9225353b89c6ee4ff550d0fee18e759a2d949686 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering@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): Abort upon virInitialize failure. --- 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..b754f5e 100644 --- a/python/libvirt-override.c +++ b/python/libvirt-override.c @@ -3543,7 +3543,8 @@ initcygvirtmod if (initialized != 0) return; - virInitialize(); + if (virInitialize() < 0) + abort(); /* initialize the python extension module */ Py_InitModule((char *) -- 1.7.1.250.g7d1e8
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 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 :|
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@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; } -- 1.7.1.250.g7d1e8
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@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 :|
On 05/18/2010 05:56 AM, Jim Meyering wrote:
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@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.
I like this second approach better. ACK.
{ static int initialized = 0;
While you're touching this file, do you want to remove the extraneous initialization (we might as well rely on .bss for zero-initialized data). -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
participants (3)
-
Daniel P. Berrange -
Eric Blake -
Jim Meyering