Libguestfs - Mar 2011 - [PATCH] Don't drop outgoing message when daemon cancels (RHBZ#576879).

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://et.redhat.com/~rjones/virt-df/
-------------- next part --------------
>From c7368ce167d6dbfd3e69ba208301c5af3f17a8a1 Mon Sep 17 00:00:00 2001
From: Richard Jones <rjones at redhat.com>
Date: Fri, 18 Mar 2011 16:18:37 +0000
Subject: [PATCH] proto: Don't drop outgoing message when daemon cancels
(RHBZ#576879).

This is a (potential) fix for the long standing protocol bug
which causes loss of synchronization when a FileIn action
fails very early on the daemon side.  The canonical example
would be the 'upload' action failing immediately if no filesystem
is mounted.

What's supposed to happen is this:

  (1) library sends
  request message              (2) daemon processes request
  first chunk of data          and sees that it will fail,
                               sends cancellation
                               (3) discards chunks of data
  (4) library sees daemon
  cancellation and stops
  sending chunks

It was going wrong in step (1), in guestfs___send_to_daemon.
In some (timing related) circumstances, send_to_daemon could
receive the cancellation before sending the first chunk, at
which point it would exit, *discarding the first chunk*.
This causes the daemon to fail in step (3) since it reads the
next request as if it was a chunk, thus losing synchronization.
(The protocol specifies that you always have to send at least
one chunk if there is a FileIn or FileOut parameter).

The patch changes guestfs___send_to_daemon so that if it detects
cancellation, it sends the remaining data in its output buffer
instead of discarding it.  (This also fixes another edge case
to do with sending partial data although I don't think we
ever saw that in practice).
---
 src/proto.c |   15 +++++++++++++--
 1 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/src/proto.c b/src/proto.c
index 1be7e05..2ca24c2 100644
--- a/src/proto.c
+++ b/src/proto.c
@@ -449,8 +449,19 @@ guestfs___send_to_daemon (guestfs_h *g, const void *v_buf,
size_t n)
     }
     if (FD_ISSET (g->sock, &rset2)) {
       r = check_for_daemon_cancellation_or_eof (g, g->sock);
-      if (r < 0)
-        return r;
+      if (r == -1)
+	return r;
+      if (r == -2) {
+	/* Daemon sent cancel message.  But to maintain
+	 * synchronization we must write out the remainder of the
+	 * write buffer before we return (RHBZ#576879).
+	 */
+	if (xwrite (g->sock, buf, n) == -1) {
+	  perrorf (g, "write");
+	  return -1;
+	}
+	return -2; /* cancelled */
+      }
     }
     if (FD_ISSET (g->sock, &wset2)) {
       r = write (g->sock, buf, n);
-- 
1.7.2.3

While this patch is fine, two further patches are required in order to
completely fix the protocol synchronization problems.  See 2 x follow
ups to this message.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
New in Fedora 11: Fedora Windows cross-compiler. Compile Windows
programs, test, and build Windows installers. Over 70 libraries supprt'd
http://fedoraproject.org/wiki/MinGW http://www.annexia.org/fedora_mingw