Re: [PATCH v2 7/7] migration: Provide explicit error message for file shutdowns

[Peter Xu <peterx@redhat.com>]

On Wed, Jul 05, 2023 at 07:05:13PM -0300, Fabiano Rosas wrote:
> Peter Xu <peterx@redhat.com> writes:
> 
> > Provide an explicit reason for qemu_file_shutdown()s, which can be
> > displayed in query-migrate when used.
> >
> 
> Can we consider this to cover the TODO:
> 
>  * TODO: convert to propagate Error objects instead of squashing
>  * to a fixed errno value
> 
> or would that need something fancier?

The TODO seems to say we want to allow qemu_file_shutdown() to report an
Error* when anything wrong happened (e.g. shutdown() failed)?  While this
patch was trying to store a specific error string so when query migration
later it'll show up to the user.  If so, IMHO they're two things.

> 
> > This will make e.g. migrate-pause to display explicit error descriptions,
> > from:
> >
> > "error-desc": "Channel error: Input/output error"
> >
> > To:
> >
> > "error-desc": "Channel is explicitly shutdown by the user"
> >
> > in query-migrate.
> >
> > Signed-off-by: Peter Xu <peterx@redhat.com>
> > ---
> >  migration/qemu-file.c | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/migration/qemu-file.c b/migration/qemu-file.c
> > index 419b4092e7..ff605027de 100644
> > --- a/migration/qemu-file.c
> > +++ b/migration/qemu-file.c
> > @@ -87,7 +87,10 @@ int qemu_file_shutdown(QEMUFile *f)
> >       *      --> guest crash!
> >       */
> >      if (!f->last_error) {
> > -        qemu_file_set_error(f, -EIO);
> > +        Error *err = NULL;
> > +
> > +        error_setg(&err, "Channel is explicitly shutdown by the user");
> 
> It is good that we can grep this message. However, I'm confused about
> who the "user" is meant to be here and how are they implicated in this
> error.

Ah, here the user is who sends the "migrate-pause" command, according to
the example of the commit message.

What I wanted to do is provide a clear message (besides -EIO) when
query-migrate, so we know more on how the migration is paused/stopped/...
Before that it shows that the same as e.g. any form of IO errors happened.

Thanks,

-- 
Peter Xu