On 25/02/2015 19:08, Max Reitz wrote:
> Just returning -EINVAL for everything is bad. -EIO is often better, and
> sometimes there is an even more fitting value.
Propagating the return value from write_sync is uglier, but it is even
better in terms of returned value.
Paolo
> Signed-off-by: Max Reitz <mre...@redhat.com>
> ---
> nbd.c | 49 +++++++++++++++++++++++++------------------------
> 1 file changed, 25 insertions(+), 24 deletions(-)
>
> diff --git a/nbd.c b/nbd.c
> index b4776ce..d2eb1c5 100644
> --- a/nbd.c
> +++ b/nbd.c
> @@ -238,22 +238,22 @@ static int nbd_send_rep(int csock, uint32_t type,
> uint32_t opt)
> magic = cpu_to_be64(NBD_REP_MAGIC);
> if (write_sync(csock, &magic, sizeof(magic)) != sizeof(magic)) {
> LOG("write failed (rep magic)");
> - return -EINVAL;
> + return -EIO;
> }
> opt = cpu_to_be32(opt);
> if (write_sync(csock, &opt, sizeof(opt)) != sizeof(opt)) {
> LOG("write failed (rep opt)");
> - return -EINVAL;
> + return -EIO;
> }
> type = cpu_to_be32(type);
> if (write_sync(csock, &type, sizeof(type)) != sizeof(type)) {
> LOG("write failed (rep type)");
> - return -EINVAL;
> + return -EIO;
> }
> len = cpu_to_be32(0);
> if (write_sync(csock, &len, sizeof(len)) != sizeof(len)) {
> LOG("write failed (rep data length)");
> - return -EINVAL;
> + return -EIO;
> }
> return 0;
> }
> @@ -267,31 +267,31 @@ static int nbd_send_rep_list(int csock, NBDExport *exp)
> magic = cpu_to_be64(NBD_REP_MAGIC);
> if (write_sync(csock, &magic, sizeof(magic)) != sizeof(magic)) {
> LOG("write failed (magic)");
> - return -EINVAL;
> + return -EIO;
> }
> opt = cpu_to_be32(NBD_OPT_LIST);
> if (write_sync(csock, &opt, sizeof(opt)) != sizeof(opt)) {
> LOG("write failed (opt)");
> - return -EINVAL;
> + return -EIO;
> }
> type = cpu_to_be32(NBD_REP_SERVER);
> if (write_sync(csock, &type, sizeof(type)) != sizeof(type)) {
> LOG("write failed (reply type)");
> - return -EINVAL;
> + return -EIO;
> }
> len = cpu_to_be32(name_len + sizeof(len));
> if (write_sync(csock, &len, sizeof(len)) != sizeof(len)) {
> LOG("write failed (length)");
> - return -EINVAL;
> + return -EIO;
> }
> len = cpu_to_be32(name_len);
> if (write_sync(csock, &len, sizeof(len)) != sizeof(len)) {
> LOG("write failed (length)");
> - return -EINVAL;
> + return -EIO;
> }
> if (write_sync(csock, exp->name, name_len) != name_len) {
> LOG("write failed (buffer)");
> - return -EINVAL;
> + return -EIO;
> }
> return 0;
> }
> @@ -309,7 +309,7 @@ static int nbd_handle_list(NBDClient *client, uint32_t
> length)
> /* For each export, send a NBD_REP_SERVER reply. */
> QTAILQ_FOREACH(exp, &exports, next) {
> if (nbd_send_rep_list(csock, exp)) {
> - return -EINVAL;
> + return -EIO;
> }
> }
> /* Finish with a NBD_REP_ACK. */
> @@ -331,6 +331,7 @@ static int nbd_handle_export_name(NBDClient *client,
> uint32_t length)
> }
> if (read_sync(csock, name, length) != length) {
> LOG("read failed");
> + rc = -EIO;
> goto fail;
> }
> name[length] = '\0';
> @@ -376,22 +377,22 @@ static int nbd_receive_options(NBDClient *client)
>
> if (read_sync(csock, &magic, sizeof(magic)) != sizeof(magic)) {
> LOG("read failed");
> - return -EINVAL;
> + return -EIO;
> }
> TRACE("Checking opts magic");
> if (magic != be64_to_cpu(NBD_OPTS_MAGIC)) {
> LOG("Bad magic received");
> - return -EINVAL;
> + return -EIO;
> }
>
> if (read_sync(csock, &tmp, sizeof(tmp)) != sizeof(tmp)) {
> LOG("read failed");
> - return -EINVAL;
> + return -EIO;
> }
>
> if (read_sync(csock, &length, sizeof(length)) != sizeof(length)) {
> LOG("read failed");
> - return -EINVAL;
> + return -EIO;
> }
> length = be32_to_cpu(length);
>
> @@ -404,7 +405,7 @@ static int nbd_receive_options(NBDClient *client)
> break;
>
> case NBD_OPT_ABORT:
> - return -EINVAL;
> + return -ECONNABORTED;
>
> case NBD_OPT_EXPORT_NAME:
> return nbd_handle_export_name(client, length);
> @@ -446,7 +447,7 @@ static int nbd_send_negotiate(NBDClient *client)
> */
>
> qemu_set_block(csock);
> - rc = -EINVAL;
> + rc = -EIO;
>
> TRACE("Beginning negotiation.");
> memset(buf, 0, sizeof(buf));
> @@ -503,7 +504,7 @@ int nbd_receive_negotiate(int csock, const char *name,
> uint32_t *flags,
>
> TRACE("Receiving negotiation.");
>
> - rc = -EINVAL;
> + rc = -EIO;
>
> if (read_sync(csock, buf, 8) != 8) {
> error_setg(errp, "Failed to read data");
> @@ -752,7 +753,7 @@ ssize_t nbd_send_request(int csock, struct nbd_request
> *request)
>
> if (ret != sizeof(buf)) {
> LOG("writing to socket failed");
> - return -EINVAL;
> + return -EIO;
> }
> return 0;
> }
> @@ -770,7 +771,7 @@ static ssize_t nbd_receive_request(int csock, struct
> nbd_request *request)
>
> if (ret != sizeof(buf)) {
> LOG("read failed");
> - return -EINVAL;
> + return -EIO;
> }
>
> /* Request
> @@ -793,7 +794,7 @@ static ssize_t nbd_receive_request(int csock, struct
> nbd_request *request)
>
> if (magic != NBD_REQUEST_MAGIC) {
> LOG("invalid magic (got 0x%x)", magic);
> - return -EINVAL;
> + return -EIO;
> }
> return 0;
> }
> @@ -811,7 +812,7 @@ ssize_t nbd_receive_reply(int csock, struct nbd_reply
> *reply)
>
> if (ret != sizeof(buf)) {
> LOG("read failed");
> - return -EINVAL;
> + return -EIO;
> }
>
> /* Reply
> @@ -830,7 +831,7 @@ ssize_t nbd_receive_reply(int csock, struct nbd_reply
> *reply)
>
> if (magic != NBD_REPLY_MAGIC) {
> LOG("invalid magic (got 0x%x)", magic);
> - return -EINVAL;
> + return -EIO;
> }
> return 0;
> }
> @@ -858,7 +859,7 @@ static ssize_t nbd_send_reply(int csock, struct nbd_reply
> *reply)
>
> if (ret != sizeof(buf)) {
> LOG("writing to socket failed");
> - return -EINVAL;
> + return -EIO;
> }
> return 0;
> }
>
