[libvirt] [PATCH 3/4] util: Add 'label' field to VIR_ENUM_IMPL

[Daniel P. Berrangé]

On Thu, Apr 11, 2019 at 12:27:56PM -0400, Cole Robinson wrote:
> On 4/11/19 6:57 AM, Daniel P. Berrangé wrote:
> > On Mon, Apr 08, 2019 at 11:48:18AM -0400, Cole Robinson wrote:
> >> This allows us to raise error messages from virEnum*String functions.
> >>
> >> FromString failure will report this error for value 'zzzz'
> >>
> >>   invalid argument: Unknown 'domain type' value 'zzzz'
> >>
> >> ToString failure will report this error for unknown type=83
> >>
> >>   internal error: Unknown 'domain type' internal value '83'
> >>
> >> However we disable the error reporting for now. It should only be
> >> enabled when we decide to begin dropping duplicate error reporting.
> > 
> > Why don't we *enable* error reporting now, but only if the label
> > string is non-NULL. Then just change your second patch to pass
> > NULL everywhere.  We can then incrementally replace the NULL with
> > a real message at the same time as we update each caller to drop
> > the existing error reporting.
> > 
> > This avoids any point in time having double reporting.
> > 
> 
> Yes but this hits the case I mentioned in the cover letter: until the
> code base is converted it will be easier for new code to neglect to
> raise virReportError when the enum hasn't been converted yet, and IMO
> reporting no error is much worse than double reporting.
> 
> If you still think it's the way to go I'll rework it. But then maybe we
> need to think more about the strategy and timing of how we plan to
> convert the code, do we shoot for doing it in a single release window?

Given that the conversion work is essentially just a case of deleting
100's of calls to virReportError, it should be largely mechanical.
So its reasonable to do it all in one release cycle.

Assuming we get it done in a single cycle, I'm not concerned if we
have a few temporary mistakes where we don't report any error.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|