Re: [Qemu-devel] [RFC PATCH 2/3] vmstate: error hint for failed equal checks part 2

["Dr. David Alan Gilbert" <dgilbert@redhat.com>]

* Halil Pasic (pasic@linux.vnet.ibm.com) wrote:
> 
> 
> On 06/07/2017 02:01 PM, Dr. David Alan Gilbert wrote:
> > * Halil Pasic (pasic@linux.vnet.ibm.com) wrote:
> >>
> >>
> >> On 06/07/2017 01:07 PM, Dr. David Alan Gilbert wrote:
> >>> * Halil Pasic (pasic@linux.vnet.ibm.com) wrote:
> >>>> Verbose error reporting for the _EQUAL family. Modify the standard _EQUAL
> >>>> so the hint states the assertion probably failed due to a bug. Introduce
> >>>> _EQUAL_HINT for specifying a context specific hint.
> >>>>
> >>>> Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com>
> >>>
> >>> I'd prefer not to print 'Bug!?' by default - they already get the
> >>> message telling them something didn't match and the migration fails.
> >>> There are none-bug ways of this happening, e.g. a user starting a VM on
> >>> the source and destination with different configs.
> >>
> >> I admit, my objective with 'Bug!?' was to provoke. My train of thought is
> >> to encourage the programmer to think about and document the circumstances
> >> under which such an assertion is supposed to fail (and against which it
> >> is supposed to guard).
> >>
> >> I do not know how skillful are our users but a 4 != 5 then maybe a name
> >> of a vmstate field is probably quite scary and not very revealing. I doubt
> >> a non qemu developer can use it for something else that reporting a bug.
> >>
> >> Consequently if there are non-bug ways one can use the hint and state them.
> >> Your example with the misconfigured target, by the way, is IMHO also be due
> >> to a bug of the management software IMHO.
> >>
> >> To sum it up: IMHO the message provided by a failing _EQUAL is to ugly
> >> and Qemuspeak to be presented to an user-user in non-bug cases. Agree?
> >> Disagree?
> > 
> > Disagree.
> > 
> > I don't mind giving field names etc; they make it easy for us as
> > developers to track down what's happening, but also sometimes they help
> > endusers work around a prolem or see where the problem is; of course
> > that varies depending on the field name, but some of our names are
> > reasonable (e.g. there's a VMSTATE_INT32_EQUAL on 'queue_size' in
> > vmmouse.c).  They're also pretty good if two end users hit the same
> > problem they can see the same error message in a bug report.
> > 
> > We often have customer-facing support people look at logs before they
> > get as far as us developers; if we have bugs that are 
> > 'if it's a failing BLAH device complaining about the BAR field'
> > then this fixes it, then that helps them find workarounds/fixes quickly
> > even if they don't understand what the BAR field is.
> > 
> 
> You seem to forget, that I'm not proposing omitting this information,
> but extending it with something civilized so one can distinguish between
> an assert failed should have never happened situation an a as good as
> reasonable error handling for an expected error scenario. IMHO the current
> EQUAL looks more like the former (assert) and less like the later (error
> reporting for an expected error scenario). Agree? Dissagree?

Yes, the current EQUAL is very terse; but we can't actually tell from
the use which case it is; it'll all work nicely when people actually add
the correct hint text in useful locations.

> Having a field name is great! That's beyond discussion.
> 
> I see, my 'sum it up' above was a bit unfortunate: it sounds like I'm
> against the inclusion of technical info and not against a lack of non
> technical info. Sorry for that!

No, that's fine.

Dave

> >>
> >>>
> >>> (I also worry we have a lot f macros for each size;
> >>> EQUAL, EQUAL_V, EQUAL_V_HINT but I don't know of a better answer for
> >>> that)
> >>>
> >>
> >> If we are going to drop the default hint ('Bug?!' or whatever) then
> >> I think we could just add an extra NULL hint to each existing  _EQUAL
> >> usage, re-purpose EQUAL, and forget about introducing new _HINT macros.
> >>
> >> What to you think?
> > 
> > Yes, that would be a lot simpler; and there aren't that many
> > VMSTATE*EQUAL* macros in use.
> > 
> 
> I still have not given up on the discussion above. Will do depending
> on the outcome.
> 
> Regards,
> Halil
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK