Re: [Qemu-devel] [PATCH] sheepdog: fix confused return values

[Liu Yuan <namei.unix@gmail.com>]

On Mon, Feb 16, 2015 at 12:56:04PM +0100, Kevin Wolf wrote:
> Am 13.02.2015 um 04:45 hat Liu Yuan geschrieben:
> > From: Liu Yuan <liuyuan@cmss.chinamobile.com>
> > 
> > These functions mix up -1 and -errno in return values and would might cause
> > trouble error handling in the call chain.
> > 
> > This patch let them return -errno and add some comments.
> > 
> > Reported-by: Markus Armbruster <armbru@redhat.com>
> > Signed-off-by: Liu Yuan <liuyuan@cmss.chinamobile.com>
> > ---
> >  block/sheepdog.c | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> > 
> > diff --git a/block/sheepdog.c b/block/sheepdog.c
> > index be3176f..c28658c 100644
> > --- a/block/sheepdog.c
> > +++ b/block/sheepdog.c
> > @@ -527,6 +527,7 @@ static SheepdogAIOCB *sd_aio_setup(BlockDriverState *bs, QEMUIOVector *qiov,
> >      return acb;
> >  }
> >  
> > +/* Return -EIO in case of error, file descriptor on success */
> >  static int connect_to_sdog(BDRVSheepdogState *s, Error **errp)
> >  {
> >      int fd;
> > @@ -546,11 +547,14 @@ static int connect_to_sdog(BDRVSheepdogState *s, Error **errp)
> >  
> >      if (fd >= 0) {
> >          qemu_set_nonblock(fd);
> > +    } else {
> > +        fd = -EIO;
> >      }
> >  
> >      return fd;
> >  }
> >  
> > +/* Return 0 on success and -errno in case of error */
> >  static coroutine_fn int send_co_req(int sockfd, SheepdogReq *hdr, void *data,
> >                                      unsigned int *wlen)
> >  {
> > @@ -559,11 +563,13 @@ static coroutine_fn int send_co_req(int sockfd, SheepdogReq *hdr, void *data,
> >      ret = qemu_co_send(sockfd, hdr, sizeof(*hdr));
> >      if (ret != sizeof(*hdr)) {
> >          error_report("failed to send a req, %s", strerror(errno));
> > +        ret = -errno;
> 
> qemu_co_sendv_recvv() uses socket_error() internally to access the
> return code. This is defined as errno on POSIX, but as WSAGetLastError()
> on Windows.
> 
> You should probably either use socket_error() here, or change
> qemu_co_sendv_recvv() to return a negative error code instead of -1.
> 
> >          return ret;
> >      }
> >  
> >      ret = qemu_co_send(sockfd, data, *wlen);
> >      if (ret != *wlen) {
> > +        ret = -errno;
> >          error_report("failed to send a req, %s", strerror(errno));
> >      }
> 
> The same here.
> 

Hi, Kevin, thanks for your tips. I'll post v2 against your comments.

Yuan