Re: [Qemu-trivial] [PATCH] sheepdog: Fix error message if failed to load vmstate

[Fam Zheng]

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 <address@hidden>
> > ---
> >  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