[libvirt] PATCH: Fix remote driver RPC recv handling

[Daniel P. Berrange]

I noticed a odd error message randomly appearing from virsh after all 
commands had been run

  # virsh dominfo VirtTest
  Id:             -
  Name:           VirtTest
  UUID:           82038f2a-1344-aaf7-1a85-2a7250be2076
  OS Type:        hvm
  State:          shut off
  CPU(s):         3
  Max memory:     256000 kB
  Used memory:    256000 kB
  Autostart:      disable

  libvir: Remote error : server closed connection

It turns out that inthe for(;;) loop where I'm procesing incoming data,
it was forgetting to break out of the loop when a completed RPC call
had been received. Most of the time this wasn't  problem, because there'd
rarely be more data following, so it'd get EAGAIN next iteration & quit
the loop. When shutting down though, you'd occassionally see the SIGHUP
from read() before you get an EAGAIN, causing this error to be raised
even though the RPC call had been successfully received.

In addition, if there was a 2nd RPC reply already pending, we'd read and
loose some of its data. Though I never saw this happen, it is a definite
theoretical possibility, particularly over a TCP link with bad latency
or fragementation or bursty traffic.

Daniel

diff -r e582072116a3 src/remote_internal.c
--- a/src/remote_internal.c	Mon Jan 26 16:21:42 2009 +0000
+++ b/src/remote_internal.c	Mon Jan 26 17:02:15 2009 +0000
@@ -6135,12 +6135,27 @@ processCallRecv(virConnectPtr conn, stru
         if (priv->bufferOffset == priv->bufferLength) {
             if (priv->bufferOffset == 4) {
                 ret = processCallRecvLen(conn, priv, in_open);
+                if (ret < 0)
+                    return -1;
+
+                /*
+                 * We'll carry on around the loop to immediately
+                 * process the message body, because it has probably
+                 * already arrived. Worst case, we'll get EAGAIN on
+                 * next iteration.
+                 */
             } else {
                 ret = processCallRecvMsg(conn, priv, in_open);
                 priv->bufferOffset = priv->bufferLength = 0;
-            }
-            if (ret < 0)
-                return -1;
+                /*
+                 * We've completed one call, so return even
+                 * though there might still be more data on
+                 * the wire. We need to actually let the caller
+                 * deal with this arrived message to keep good
+                 * response, and also to correctly handle EOF.
+                 */
+                return ret;
+            }
         }
     }
 }


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