Re: [PATCH 4/6] cpus: Improve error messages on memsave, pmemsave write error

[Philippe Mathieu-Daudé]

On 13/5/24 16:45, Markus Armbruster wrote:
> Philippe Mathieu-Daudé <philmd@linaro.org> writes:
>
> > On 13/5/24 16:17, Markus Armbruster wrote:
> > > qmp_memsave() and qmp_pmemsave() report fwrite() error as
> > >       An IO error has occurred
> > > Improve this to
> > >       writing memory to '<filename>' failed
> > > Signed-off-by: Markus Armbruster <armbru@redhat.com>
> > > ---
> > >    system/cpus.c | 6 ++++--
> > >    1 file changed, 4 insertions(+), 2 deletions(-)
> > > diff --git a/system/cpus.c b/system/cpus.c
> > > index 68d161d96b..f8fa78f33d 100644
> > > --- a/system/cpus.c
> > > +++ b/system/cpus.c
> > > @@ -813,7 +813,8 @@ void qmp_memsave(int64_t addr, int64_t size, const char 
> > > *filename,
> > >                goto exit;
> > >            }
> > >            if (fwrite(buf, 1, l, f) != l) {
> > > -            error_setg(errp, QERR_IO_ERROR);
> > > +            error_setg(errp, "writing memory to '%s' failed",
> > > +                       filename);
> > >                goto exit;
> > >            }
> > >            addr += l;
> > > @@ -843,7 +844,8 @@ void qmp_pmemsave(int64_t addr, int64_t size, const char 
> > > *filename,
> > >                l = size;
> > >            cpu_physical_memory_read(addr, buf, l);
> > >            if (fwrite(buf, 1, l, f) != l) {
> > > -            error_setg(errp, QERR_IO_ERROR);
> > > +            error_setg(errp, "writing memory to '%s' failed",
> > > +                       filename);
> >
> > What about including errno with error_setg_errno()?
>
> Sure fwrite() fails with errno reliably set?  The manual page doesn't
> mention it...

Indeed. I can see some uses in the code base:

qemu-io-cmds.c:409:    if (ferror(f)) {
qemu-io-cmds.c-410-        perror(file_name);

qga/commands-posix.c-632-    write_count = fwrite(buf, 1, count, fh);
qga/commands-posix.c:633:    if (ferror(fh)) {
qga/commands-posix.c-634-        error_setg_errno(errp, errno, "failed 
to write to file");

util/qemu-config.c:152:    if (ferror(fp)) {
util/qemu-config.c-153-        loc_pop(&loc);
util/qemu-config.c-154-        error_setg_errno(errp, errno, "Cannot 
read config file");

Regardless,

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>