[Libvir] Make python binding release GIL when using threads
In the virt-manager app I'm using the python bindings to libvirt for all operations. For long running operations such as creating new domains, or saving/restoring domains I fork a background thread to do the call, so that the main UI / status refresh can carry on working. Except this wasn't working. The main thread was being blocked for the entire duration of the background thread. After a lot of nasty debug sessions I discovered that the main thread was being blocked in the GTK event loop attempting to aquire the Python GIL (Global Interpreter Lock). The only way this could be occurring is if the background thread doing the libvirt call held the GIL. And indeed after looking at the source for the libvirt python bindings it appears we never release the GIL when calling into the libvirt C apis, so for the entire duration of virDomainCreateLinux the python interpreter is completely stopped. Clearly this sucks because virDomainCreateLinux can take a very long time. I read up a little on python threading & looked at how PyGTK deals with this & have come up with a patch to make libvirt release the GIL when entering a C call & re-aquire it on completion. Indeed, since PyGTK & GTK are both LGPL, I just copied their C macros straight over. The patch appears to work - the python interpreter is no longer deadlocked when calling virDomainCreateLinux (or any other libvirt calls). Unfortuntely the virtmanager app still deadlocks, because it turns out XenD itself serializes all SEXPR HTTP requests, but once this is fixed the whole thing is fully parallelized & works without blocking the app. Attached the patch - I've not done much python threading before so let me know if i missed anything / messed stuff up. NB, I also hacked up the error handling callback so it re-acquires the lock before calling back into the python. Regards, Dan. -- |=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=| |=- Perl modules: http://search.cpan.org/~danberr/ -=| |=- Projects: http://freshmeat.net/~danielpb/ -=| |=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|
On Fri, Oct 06, 2006 at 01:10:18AM +0100, Daniel P. Berrange wrote:
In the virt-manager app I'm using the python bindings to libvirt for all operations. For long running operations such as creating new domains, or saving/restoring domains I fork a background thread to do the call, so that the main UI / status refresh can carry on working. Except this wasn't working. The main thread was being blocked for the entire duration of the background thread. After a lot of nasty debug sessions I discovered that the main thread was being blocked in the GTK event loop attempting to aquire the Python GIL (Global Interpreter Lock). The only way this could be occurring is if the background thread doing the libvirt call held the GIL.
And indeed after looking at the source for the libvirt python bindings it appears we never release the GIL when calling into the libvirt C apis, so for the entire duration of virDomainCreateLinux the python interpreter is completely stopped. Clearly this sucks because virDomainCreateLinux can take a very long time.
Okay, true. Simply that I never had to deal with something similar in the past so I never looked into this before.
I read up a little on python threading & looked at how PyGTK deals with this & have come up with a patch to make libvirt release the GIL when entering a C call & re-aquire it on completion. Indeed, since PyGTK & GTK are both LGPL, I just copied their C macros straight over.
The patch appears to work - the python interpreter is no longer deadlocked when calling virDomainCreateLinux (or any other libvirt calls). Unfortuntely the virtmanager app still deadlocks, because it turns out XenD itself serializes all SEXPR HTTP requests, but once this is fixed the whole thing is fully parallelized & works without blocking the app.
Okay, if this works fine by me :-)
Attached the patch - I've not done much python threading before so let me know if i missed anything / messed stuff up. NB, I also hacked up
Me either so any comment may only be stylistic.
the error handling callback so it re-acquires the lock before calling back into the python.
Okay make sense, hopefully that's the only time where we go back from C to Python (I can't think of any other)
output.write("libvirt_begin_allow_threads;\n");
425a427
output.write("libvirt_end_allow_threads;\n");
I was wondering if it wasn't nicer to use functions rather than inlined macros. Maybe if kept as macro this should be turned to upper case to keep consistant with existing practice, but it's not a big deal.
diff -r1.3 libvirt_wrap.h 50a51,102
extern int libvirt_threads_enabled;
/* Provide simple macro statement wrappers (adapted from Perl): * LIBVIRT_STMT_START { statements; } LIBVIRT_STMT_END; * can be used as a single statement, as in * if (x) LIBVIRT_STMT_START { ... } LIBVIRT_STMT_END; else ... * * When GCC is compiling C code in non-ANSI mode, it will use the * compiler __extension__ to wrap the statements wihin `({' and '})' braces. * When compiling on platforms where configure has defined * HAVE_DOWHILE_MACROS, statements will be wrapped with `do' and `while (0)'. * For any other platforms (SunOS4 is known to have this issue), wrap the * statements with `if (1)' and `else (void) 0'. */ #if !(defined (LIBVIRT_STMT_START) && defined (LIBVIRT_STMT_END)) # if defined (__GNUC__) && !defined (__STRICT_ANSI__) && !defined (__cplusplus) # define LIBVIRT_STMT_START (void) __extension__ ( # define LIBVIRT_STMT_END ) # else /* !(__GNUC__ && !__STRICT_ANSI__ && !__cplusplus) */ # if defined (HAVE_DOWHILE_MACROS) # define LIBVIRT_STMT_START do # define LIBVIRT_STMT_END while (0) # else /* !HAVE_DOWHILE_MACROS */ # define LIBVIRT_STMT_START if (1) # define LIBVIRT_STMT_END else (void) 0 # endif /* !HAVE_DOWHILE_MACROS */ # endif /* !(__GNUC__ && !__STRICT_ANSI__ && !__cplusplus) */ #endif
#define libvirt_begin_allow_threads \ LIBVIRT_STMT_START { \ PyThreadState *_save = NULL; \ if (PyEval_ThreadsInitialized()) \ _save = PyEval_SaveThread();
#define libvirt_end_allow_threads \ if (PyEval_ThreadsInitialized()) \ PyEval_RestoreThread(_save); \ } LIBVIRT_STMT_END
#define libvirt_ensure_thread_state \ LIBVIRT_STMT_START { \ PyGILState_STATE _save; \ if (PyEval_ThreadsInitialized()) \ _save = PyGILState_Ensure();
#define libvirt_release_thread_state \ if (PyEval_ThreadsInitialized()) \ PyGILState_Release(_save); \ } LIBVIRT_STMT_END
I assume the original Perl code works, and that the rewrite didn't introduced problems :-) Feel free to commit, basically if virt-manager works better now, it's a good incentive to use that code ! Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
On Mon, Oct 09, 2006 at 11:40:21AM -0400, Daniel Veillard wrote:
On Fri, Oct 06, 2006 at 01:10:18AM +0100, Daniel P. Berrange wrote:
In the virt-manager app I'm using the python bindings to libvirt for all operations. For long running operations such as creating new domains, or saving/restoring domains I fork a background thread to do the call, so that the main UI / status refresh can carry on working. Except this wasn't working. The main thread was being blocked for the entire duration of the background thread. After a lot of nasty debug sessions I discovered that the main thread was being blocked in the GTK event loop attempting to aquire the Python GIL (Global Interpreter Lock). The only way this could be occurring is if the background thread doing the libvirt call held the GIL.
And indeed after looking at the source for the libvirt python bindings it appears we never release the GIL when calling into the libvirt C apis, so for the entire duration of virDomainCreateLinux the python interpreter is completely stopped. Clearly this sucks because virDomainCreateLinux can take a very long time.
Okay, true. Simply that I never had to deal with something similar in the past so I never looked into this before.
I've been doing more testing with this patch & virt-manager and it all seems to be operating as desired, so I've committed this patch to CVS.
output.write("libvirt_begin_allow_threads;\n");
425a427
output.write("libvirt_end_allow_threads;\n");
I was wondering if it wasn't nicer to use functions rather than inlined macros. Maybe if kept as macro this should be turned to upper case to keep consistant with existing practice, but it's not a big deal.
I changed the macros into uppercase to match coding standards. They can't actually be written as functions because the BEGIN macro opens a statement block with '{' and declares a local variable within it, that the END macro then uses & closes. Regards, Dan. -- |=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=| |=- Perl modules: http://search.cpan.org/~danberr/ -=| |=- Projects: http://freshmeat.net/~danielpb/ -=| |=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|
participants (2)
-
Daniel P. Berrange -
Daniel Veillard