Libguestfs - Aug 2009 - [PATCH libguestfs] daemon: diagnose socket write failure

On principle, we shouldn't ignore write failure:
>From 56317a61bc22e935dc750cf669a164bacc12cf12 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering at redhat.com>
Date: Thu, 20 Aug 2009 12:29:46 +0200
Subject: [PATCH libguestfs] daemon: diagnose socket write failure

* daemon/proto.c (send_chunk): Don't ignore socket-write error.
---
 daemon/proto.c |    8 +++++---
 1 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/daemon/proto.c b/daemon/proto.c
index 9b33902..c2dd50c 100644
--- a/daemon/proto.c
+++ b/daemon/proto.c
@@ -495,8 +495,10 @@ send_chunk (const guestfs_chunk *chunk)
   xdr_uint32_t (&xdr, &len);
   xdr_destroy (&xdr);

-  (void) xwrite (sock, lenbuf, 4);
-  (void) xwrite (sock, buf, len);
+  int err = (xwrite (sock, lenbuf, 4) == 0
+             && xwrite (sock, buf, len) == 0 ? 0 : -1);
+  if (err)
+    fprintf (stderr, "send_chunk: write failed\n");

-  return 0;
+  return err;
 }
--
1.6.4.378.g88f2f

On Thu, Aug 20, 2009 at 12:42:47PM +0200, Jim Meyering
wrote:
> On principle, we shouldn't ignore write failure:
[...]
> -  (void) xwrite (sock, lenbuf, 4);
> -  (void) xwrite (sock, buf, len);
What's happening in the original code is that if the daemon can't
write its reply message / reply chunk back over the guestfwd socket to
the host (library), then it just ignores the error.

The code at the  moment is wrong - I can't think  of any reason why we
should ignore this error.

If the daemon cannot contact the host, then things have gone badly
wrong, so the daemon should exit, which causes an appliance kernel
panic, which causes qemu to exit, and the library notices this and
informs the caller through either an error result or a callback.

So, the code below is better because it checks the return value, but:
> +  int err = (xwrite (sock, lenbuf, 4) == 0
> +             && xwrite (sock, buf, len) == 0 ? 0 : -1);
> +  if (err)
> +    fprintf (stderr, "send_chunk: write failed\n");
I think in this case it should exit(1).

If you look at the earlier calls to xwrite in daemon/proto.c:reply()
you'll see that those exit.  It's the same situation here.

Rich.

-- 
Richard Jones, Emerging Technologies, Red Hat  http://et.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