From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47273) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YVeia-0000ip-05 for qemu-devel@nongnu.org; Wed, 11 Mar 2015 07:22:29 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YVeiU-0006Cd-70 for qemu-devel@nongnu.org; Wed, 11 Mar 2015 07:22:27 -0400 Sender: Paolo Bonzini Message-ID: <55002568.2070201@redhat.com> Date: Wed, 11 Mar 2015 12:22:16 +0100 From: Paolo Bonzini MIME-Version: 1.0 References: <1424887718-10800-1-git-send-email-mreitz@redhat.com> <1424887718-10800-6-git-send-email-mreitz@redhat.com> In-Reply-To: <1424887718-10800-6-git-send-email-mreitz@redhat.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 05/25] nbd: Avoid generic -EINVAL List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Max Reitz , qemu-block@nongnu.org Cc: Kevin Wolf , qemu-devel@nongnu.org, Stefan Hajnoczi 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 > --- > 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; > } >