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?


      }
-    return -EINVAL;
+    return false;
  }



--
Best regards,
Vladimir

Reply via email to