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 <[email protected]> > > > > Reviewed-by: Peter Xu <[email protected]> > > > > 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
