[libvirt] [PATCH] Don't sleep in poll() if there is existing SASL decoded data

[Daniel Veillard]

On Tue, Feb 01, 2011 at 04:20:16PM +0000, Daniel P. Berrange wrote:
> On Tue, Feb 01, 2011 at 09:18:45AM -0700, Eric Blake wrote:
> > On 02/01/2011 09:09 AM, Daniel P. Berrange wrote:
> > > In the SASL codepath we typically read far more data off the
> > > wire than we immediately need. When using a connection from a
> > > single thread this isn't a problem, since only our reply will
> > > be pending (or an event we can handle directly). When using a
> > > connection from multiple threads though, we may read the data
> > > from replies from other threads. If those replies occur after
> > > our own reply, they'll not be processed. The other thread will
> > > then go into poll() and wait for its reply which has already
> > > been received and decoded. The solution is to set poll() timeout
> > > to 0 if there is pending SASL data.
> > > 
> > > * src/remote/remote_driver.c: Don't sleep in poll() if SASL
> > >   data exists
> > > ---
> > >  src/remote/remote_driver.c |   16 +++++++++++++++-
> > >  1 files changed, 15 insertions(+), 1 deletions(-)
> > 
> > ACK.
> > 
> > Will your refactoring to make things use socket wrappers that do SASL
> > transparently under the hood be complete any time soon?  But this patch
> > is good in the meantime.
> 
> Yeah that code is pending soon...it shares this same flaw
> though and this is urgent to fix because it makes the
> client lockup.

  ACtually that patch broke when compiling without SASL because
priv->saslDecoded is not defined in that case. I'm pushing a trivial
fix patch for this,

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel at veillard.com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/
-------------- next part --------------
commit 3028f51c383cc36ab4e87f2c87569d55de5672ff
Author: Daniel Veillard <veillard at redhat.com>
Date:   Thu Feb 3 22:13:24 2011 +0800

    Fix compilation when building without sasl
    
    Use of saslDecoded field need to be guarded by #if HAVE_SASL/endif
    * src/remote/remote_driver.c: fix remoteIOEventLoop accordingly

diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index 347b844..8bae697 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -10186,8 +10186,10 @@ remoteIOEventLoop(virConnectPtr conn,
          * don't want to sleep in the poll(), just
          * check if any other FDs are also ready
          */
+#if HAVE_SASL
         if (priv->saslDecoded)
             timeout = 0;
+#endif
 
         fds[0].events = fds[0].revents = 0;
         fds[1].events = fds[1].revents = 0;
@@ -10236,8 +10238,10 @@ remoteIOEventLoop(virConnectPtr conn,
         /* If we have existing SASL decoded data, pretend
          * the socket became readable so we consume it
          */
+#if HAVE_SASL
         if (priv->saslDecoded)
             fds[0].revents |= POLLIN;
+#endif
 
         if (fds[1].revents) {
             ssize_t s;