From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:49554) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YzjLU-0002NZ-T6 for qemu-devel@nongnu.org; Tue, 02 Jun 2015 06:22:57 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YzjLT-0005cs-VW for qemu-devel@nongnu.org; Tue, 02 Jun 2015 06:22:56 -0400 Date: Tue, 2 Jun 2015 18:22:40 +0800 From: Fam Zheng Message-ID: <20150602102240.GC22059@dhcp-14-238.nay.redhat.com> References: <1433237561-10846-1-git-send-email-famz@redhat.com> <20150602094531.GH3765@noname.str.redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150602094531.GH3765@noname.str.redhat.com> Subject: Re: [Qemu-devel] [PATCH] sheepdog: Fix error message if failed to load vmstate List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: qemu-block@nongnu.org, qemu-trivial@nongnu.org, Jeff Cody , qemu-devel@nongnu.org, mitake.hitoshi@lab.ntt.co.jp, namei.unix@gmail.com On Tue, 06/02 11:45, Kevin Wolf wrote: > [ CC to Sheepdog maintainers ] > > Am 02.06.2015 um 11:32 hat Fam Zheng geschrieben: > > Signed-off-by: Fam Zheng > > --- > > block/sheepdog.c | 6 +++++- > > 1 file changed, 5 insertions(+), 1 deletion(-) > > > > diff --git a/block/sheepdog.c b/block/sheepdog.c > > index bd7cbed..a22f838 100644 > > --- a/block/sheepdog.c > > +++ b/block/sheepdog.c > > @@ -2556,7 +2556,11 @@ static int do_load_save_vmstate(BDRVSheepdogState *s, uint8_t *data, > > } > > > > if (ret < 0) { > > - error_report("failed to save vmstate %s", strerror(errno)); > > + if (load) { > > + error_report("failed to load vmstate %s", strerror(errno)); > > + } else { > > + error_report("failed to save vmstate %s", strerror(errno)); > > + } > > goto cleanup; > > } > > Why do we even print this message? We usually don't do this for a failed > request, and much less so if we don't actually add any information that > isn't covered by the return code. qemu_savevm_state() will already set > an error that is even visible in QMP. > > In fact, errno doesn't even contain anything relevant here, or in most > (all?) other places that it's used in the sheepdog block driver. Now I followed the called function and agree that reporting errno is not helpful. Maybe it's better to do a clean up on error reportings in this driver, later. Fam