[Libguestfs] [PATCH libguestfs] daemon: diagnose socket write failure

[Jim Meyering]

Richard W.M. Jones wrote:

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

Ok, then by that argument, the preceding code that checks for encoding
failure should also exit:

    static int
    send_chunk (const guestfs_chunk *chunk)
    {
      char buf[GUESTFS_MAX_CHUNK_SIZE + 48];
      char lenbuf[4];
      XDR xdr;
      uint32_t len;

      xdrmem_create (&xdr, buf, sizeof buf, XDR_ENCODE);
      if (!xdr_guestfs_chunk (&xdr, (guestfs_chunk *) chunk)) {
        fprintf (stderr, "send_chunk: failed to encode chunk\n");
        xdr_destroy (&xdr);
        return -1;
      }

However, if we do that, we might as well make the function void,
since it will never return an error status.

But then, look at its two callers.

  send_file_write currently tests for send_chunk failure
  send_file_end does not

Easy to adapt.  Just remove the test in send_file_end.

Ok with all that?