Re: [PATCH v2 16/16] migration/vmstate-types: move to new migration APIs

[Peter Xu <peterx@redhat.com>]

On Wed, Mar 04, 2026 at 08:38:04PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> On 03.03.26 19:56, Peter Xu wrote:
> > On Sat, Feb 21, 2026 at 12:02:14AM +0300, Vladimir Sementsov-Ogievskiy wrote:
> > > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> > 
> > Reviewed-by: Peter Xu <peterx@redhat.com>
> > 
> > Likewise.. one trivial thing to mention below..
> > 
> > [...]
> > 
> > >   /* 32 bit int. See that the received value is the same than the one
> > >      in the field */
> > > -static int get_int32_equal(QEMUFile *f, void *pv, size_t size,
> > > -                           const VMStateField *field)
> > > +static bool load_int32_equal(QEMUFile *f, void *pv, size_t size,
> > > +                             const VMStateField *field, Error **errp)
> > >   {
> > > +    ERRP_GUARD();
> > >       int32_t *v = pv;
> > >       int32_t v2;
> > >       qemu_get_sbe32s(f, &v2);
> > >       if (*v == v2) {
> > > -        return 0;
> > > +        return true;
> > >       }
> > > -    error_report("%" PRIx32 " != %" PRIx32, *v, v2);
> > > +
> > > +    error_setg(errp, "%" PRIx32 " != %" PRIx32, *v, v2);
> > >       if (field->err_hint) {
> > > -        error_printf("%s\n", field->err_hint);
> > > +        error_append_hint(errp, "%s\n", field->err_hint);
> > 
> > According to doc for error_append_hint():
> > 
> > /*
> >   * Append a printf-style human-readable explanation to an existing error.
> >   * If the error is later reported to a human user with
> >   * error_report_err() or warn_report_err(), the hints will be shown,
> >   * too.  If it's reported via QMP, the hints will be ignored.
> >   * Intended use is adding helpful hints on the human user interface,
> >   * e.g. a list of valid values.  It's not for clarifying a confusing
> >   * error message.
> >   * @errp may be NULL, but not &error_fatal or &error_abort.
> >   * Trivially the case if you call it only after error_setg() or
> >   * error_propagate().
> >   * May be called multiple times.  The resulting hint should end with a
> >   * newline.
> >   */
> > 
> > It may be ignored if the errp will be finally persisted in MigrationState.
> > 
> > Not a huge deal because I believe we also do error_report_err()
> > somewhere.. but if we want, I think we could use error_append() to make the
> > hint available to both.
> > 
> > This comment applies to all 5 similar occurances in this patch.
> 
> Agree, will do. That was intuitive choice "err_hint" -> "append_hint".
> 
> Actually, I'm unsure, how much is good to omit hints for QMP interface.
> Of course, QMP is for machines. But provided errors are for humans anyway,
> machines never parse them (OK, sometimes they do, but that's not a good
> practice).
> 
> I always wandered, why we try to hide information when reporting errors:
> don't report filename, line number.. Don't report these hints. In production
> solutions final user will never see any errors from QEMU, so these error
> messages are only for engineers. Why we have to grep error messages in
> code, instead of get exact filename and line number?

I confess I don't know a solid answer.

My expectation on whatever that will be visible via QMP is, there is always
chance an user will read the message as-is.

Maybe reading file name and line numbers are too much for most users,
especially if that'll be a default behavior for all errors.  So it makes
some sense to me to not include those at least in most errors.

For this specific one, I think it's slightly different: here we already
report something as hard to understand to the user with an "A != B"
message..  hence maybe it'll always be good to prepend with "what is put
into the equation", until we will change that to a more human-friendly
error..

-- 
Peter Xu