[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC PATCH 1/3] vmstate: error hint for failed equal ch
From: |
Halil Pasic |
Subject: |
Re: [Qemu-devel] [RFC PATCH 1/3] vmstate: error hint for failed equal checks |
Date: |
Thu, 8 Jun 2017 13:05:40 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.1.0 |
On 06/07/2017 11:51 AM, Dr. David Alan Gilbert wrote:
> * Halil Pasic (address@hidden) 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 <address@hidden>
>> ---
>> 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
One reason I choose error_report_err is to be consistent about hint
reporting (the other one is that was what Connie suggested). I do
not understand why do we omit hints if QMP, but I figured that's
our policy. So the hint I'm adding must not be printed in QMP
context -- because that's our policy. I was pretty sure what I
want to do is add a hint (and not make a very long 'core' error
message).
Can you (or somebody else) explain why are hints dropped in QMP
context?
Don't misunderstand I'm open towards your proposal, it's just
that:
1) I would like to understand.
2) I would like to get the very same result as produced by
https://lists.nongnu.org/archive/html/qemu-devel/2017-06/msg01472.html
Regards,
Halil
- Re: [Qemu-devel] [RFC PATCH 2/3] vmstate: error hint for failed equal checks part 2, (continued)
- Re: [Qemu-devel] [RFC PATCH 2/3] vmstate: error hint for failed equal checks part 2, Halil Pasic, 2017/06/07
- Re: [Qemu-devel] [RFC PATCH 2/3] vmstate: error hint for failed equal checks part 2, Dr. David Alan Gilbert, 2017/06/07
- Re: [Qemu-devel] [RFC PATCH 2/3] vmstate: error hint for failed equal checks part 2, Halil Pasic, 2017/06/07
- Re: [Qemu-devel] [RFC PATCH 2/3] vmstate: error hint for failed equal checks part 2, Dr. David Alan Gilbert, 2017/06/07
- Re: [Qemu-devel] [RFC PATCH 2/3] vmstate: error hint for failed equal checks part 2, Halil Pasic, 2017/06/07
- Re: [Qemu-devel] [RFC PATCH 2/3] vmstate: error hint for failed equal checks part 2, Dr. David Alan Gilbert, 2017/06/07
Re: [Qemu-devel] [RFC PATCH 2/3] vmstate: error hint for failed equal checks part 2, Juan Quintela, 2017/06/07
[Qemu-devel] [RFC PATCH 1/3] vmstate: error hint for failed equal checks, Halil Pasic, 2017/06/06
Re: [Qemu-devel] [RFC PATCH 1/3] vmstate: error hint for failed equal checks, Eric Blake, 2017/06/29
Re: [Qemu-devel] [RFC PATCH 1/3] vmstate: error hint for failed equal checks, Halil Pasic, 2017/06/30
Re: [Qemu-devel] [RFC PATCH 1/3] vmstate: error hint for failed equal checks, Eric Blake, 2017/06/30
Re: [Qemu-devel] [RFC PATCH 1/3] vmstate: error hint for failed equal checks, Halil Pasic, 2017/06/30
Re: [Qemu-devel] [RFC PATCH 3/3] s390x/css: add hint for devno missmatch, Halil Pasic, 2017/06/06