[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 05/25] nbd: Avoid generic -EINVAL

From: Paolo Bonzini
Subject: Re: [Qemu-devel] [PATCH 05/25] nbd: Avoid generic -EINVAL
Date: Mon, 16 Mar 2015 15:49:57 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.5.0

On 16/03/2015 15:48, Max Reitz wrote:
> On 2015-03-16 at 10:42, Paolo Bonzini wrote:
>> On 16/03/2015 14:51, Max Reitz wrote:
>>>> Propagating the return value from write_sync is uglier, but it is even
>>>> better in terms of returned value.
>>> We can only return -errno values, but write_sync() may do partial writes
>>> so it may return non-negative values which still indicate an error. So
>>> we'd have to check whether the return value is negative, if it is,
>>> return that, if it isn't but if it's still below what we wanted to
>>> write, return a fixed error (such as -EIO). I'd rather just return -EIO
>>> and be done with it, but if you really want me to, I can of course do it
>>> differently.
>> nbd_wr_sync doesn't do that, it always returns negative errno for a
>> partial
>> error:
> qemu_send() might do a partial send, returning a value smaller than len.
> nbd_wr_sync() will try iterating until everything has been sent, but if
> send() returns 0, the loop is aborted and a value smaller than len may
> be returned.

Indeed, send() will never return 0.  It will return either EPIPE or
EWOULDBLOCK.  recv() on the other hand can return 0 or EWOULDBLOCK.


> Maybe send() never returns 0, in which case nbd_wr_sync() will actually
> return either len or -errno, but this isn't clear from the structure of
> nbd_wr_sync(). If you really want to pass the value returned from
> nbd_wr_sync(), I'd rather restructure that function so that it always
> returns either len or -errno.
> Max
>>          if (len < 0) {
>>              err = socket_error();
>>              /* recoverable error */
>>              if (err == EINTR || (offset > 0 && (err == EAGAIN || err
>> == EWOULDBLOCK))) {
>>                  continue;
>>              }
>>              /* unrecoverable error */
>>              return -err;
>>          }
>> The precise error can be useful to distinguish a network error from
>> something
>> else.  I'm just in doubt about partial reads; those can return a
>> positive error,
>> in which case you can return ESHUTDOWN (in read_sync).
>> Paolo

reply via email to

[Prev in Thread] Current Thread [Next in Thread]