* Halil Pasic (pa...@linux.vnet.ibm.com) wrote: > In some cases a failing VMSTATE_*_EQUAL does not mean we detected a bug > (it's actually the best we can do). Especially in these cases a verbose > error message is required. > > Let's introduce infrastructure for specifying a error hint to be used if > equal check fails. > > Signed-off-by: Halil Pasic <pa...@linux.vnet.ibm.com> > --- > Macros come in part 2. Once we are happy with the macros > this two patches should be squashed into one. > --- > include/migration/vmstate.h | 1 + > migration/vmstate-types.c | 36 +++++++++++++++++++++++++++++++----- > 2 files changed, 32 insertions(+), 5 deletions(-) > > diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h > index 66895623da..d90d9b12ca 100644 > --- a/include/migration/vmstate.h > +++ b/include/migration/vmstate.h > @@ -200,6 +200,7 @@ typedef enum { > > struct VMStateField { > const char *name; > + const char *err_hint; > size_t offset; > size_t size; > size_t start; > diff --git a/migration/vmstate-types.c b/migration/vmstate-types.c > index 7287c6baa6..84d0545a38 100644 > --- a/migration/vmstate-types.c > +++ b/migration/vmstate-types.c > @@ -19,6 +19,7 @@ > #include "qemu/error-report.h" > #include "qemu/queue.h" > #include "trace.h" > +#include "qapi/error.h" > > /* bool */ > > @@ -118,6 +119,7 @@ const VMStateInfo vmstate_info_int32 = { > static int get_int32_equal(QEMUFile *f, void *pv, size_t size, > VMStateField *field) > { > + Error *err = NULL; > int32_t *v = pv; > int32_t v2; > qemu_get_sbe32s(f, &v2); > @@ -125,7 +127,11 @@ static int get_int32_equal(QEMUFile *f, void *pv, size_t > size, > if (*v == v2) { > return 0; > } > - error_report("%" PRIx32 " != %" PRIx32, *v, v2); > + error_setg(&err, "%" PRIx32 " != %" PRIx32, *v, v2); > + if (field->err_hint) { > + error_append_hint(&err, "%s\n", field->err_hint); > + } > + error_report_err(err);
I'm a bit worried as to whether the error_append_hint data gets printed out by error_report_err if we're being driven by a QMP monitor. error_report_err uses error_printf_unless_qmp Since this code doesn't really handle Error *'s back up, and always prints it's errors into stderr, I'd prefer if you just used error_report again for the hint, something like: if (field->err_hint) { error_report("%" PRIx32 " != %" PRIx32 "(%s)", *v, v2, field->err_hint); } else { error_report("%" PRIx32 " != %" PRIx32, *v, v2); } Dave > return -EINVAL; > } > > @@ -259,6 +265,7 @@ const VMStateInfo vmstate_info_uint32 = { > static int get_uint32_equal(QEMUFile *f, void *pv, size_t size, > VMStateField *field) > { > + Error *err = NULL; > uint32_t *v = pv; > uint32_t v2; > qemu_get_be32s(f, &v2); > @@ -266,7 +273,11 @@ static int get_uint32_equal(QEMUFile *f, void *pv, > size_t size, > if (*v == v2) { > return 0; > } > - error_report("%" PRIx32 " != %" PRIx32, *v, v2); > + error_setg(&err, "%" PRIx32 " != %" PRIx32, *v, v2); > + if (field->err_hint) { > + error_append_hint(&err, "%s\n", field->err_hint); > + } > + error_report_err(err); > return -EINVAL; > } > > @@ -333,6 +344,7 @@ const VMStateInfo vmstate_info_nullptr = { > static int get_uint64_equal(QEMUFile *f, void *pv, size_t size, > VMStateField *field) > { > + Error *err = NULL; > uint64_t *v = pv; > uint64_t v2; > qemu_get_be64s(f, &v2); > @@ -340,7 +352,11 @@ static int get_uint64_equal(QEMUFile *f, void *pv, > size_t size, > if (*v == v2) { > return 0; > } > - error_report("%" PRIx64 " != %" PRIx64, *v, v2); > + error_setg(&err, "%" PRIx64 " != %" PRIx64, *v, v2); > + if (field->err_hint) { > + error_append_hint(&err, "%s\n", field->err_hint); > + } > + error_report_err(err); > return -EINVAL; > } > > @@ -356,6 +372,7 @@ const VMStateInfo vmstate_info_uint64_equal = { > static int get_uint8_equal(QEMUFile *f, void *pv, size_t size, > VMStateField *field) > { > + Error *err = NULL; > uint8_t *v = pv; > uint8_t v2; > qemu_get_8s(f, &v2); > @@ -363,7 +380,11 @@ static int get_uint8_equal(QEMUFile *f, void *pv, size_t > size, > if (*v == v2) { > return 0; > } > - error_report("%x != %x", *v, v2); > + error_setg(&err, "%x != %x", *v, v2); > + if (field->err_hint) { > + error_append_hint(&err, "%s\n", field->err_hint); > + } > + error_report_err(err); > return -EINVAL; > } > > @@ -379,6 +400,7 @@ const VMStateInfo vmstate_info_uint8_equal = { > static int get_uint16_equal(QEMUFile *f, void *pv, size_t size, > VMStateField *field) > { > + Error *err = NULL; > uint16_t *v = pv; > uint16_t v2; > qemu_get_be16s(f, &v2); > @@ -386,7 +408,11 @@ static int get_uint16_equal(QEMUFile *f, void *pv, > size_t size, > if (*v == v2) { > return 0; > } > - error_report("%x != %x", *v, v2); > + error_setg(&err, "%x != %x", *v, v2); > + if (field->err_hint) { > + error_append_hint(&err, "%s\n", field->err_hint); > + } > + error_report_err(err); > return -EINVAL; > } > > -- > 2.11.2 > -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK