From: Richard Jones <rjones@redhat.com>
Date: Fri, 18 Mar 2011 16:18:37 +0000 (+0000)
Subject: proto: Don't drop outgoing message when daemon cancels (RHBZ#576879).
X-Git-Tag: 1.9.12~3
X-Git-Url: http://git.annexia.org/?a=commitdiff_plain;h=c7368ce167d6dbfd3e69ba208301c5af3f17a8a1;p=libguestfs.git

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).
---

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);