On Thu, Apr 20, 2017 at 04:30:16PM +0100, Daniel P. Berrange wrote: > On Thu, Apr 20, 2017 at 12:00:03PM +0800, Fam Zheng wrote: > > Signed-off-by: Fam Zheng <f...@redhat.com> > > --- > > block/sheepdog.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/block/sheepdog.c b/block/sheepdog.c > > index fb9203e..7e889ee 100644 > > --- a/block/sheepdog.c > > +++ b/block/sheepdog.c > > @@ -608,6 +608,7 @@ static int connect_to_sdog(BDRVSheepdogState *s, Error > > **errp) > > qemu_set_nonblock(fd); > > } else { > > fd = -EIO; > > + error_setg(errp, "Failed to connect to sheepdog server"); > > } > > This doesn't make much sense to me. The lines just above the > diff context have this: > > fd = socket_connect(s->addr, errp, NULL, NULL); > > socket_connect should have already reported an error on "errp" > in the scenario that 'fd == -1'. So AFAICT the new error_setg is > just throwing away the real detailed error message in favour of > a generic message. > > So I'm puzzelled why we need to change anything - error reporting > should already be working fine. >
Indeed, you are right. (Dequeuing patch) It would also make more sense to check fd after the socket_connect() call and return error then, rather than keep checking fd throughout the rest of the function. -Jeff