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

[Cole Robinson]

On 4/11/19 12:33 PM, Daniel P. Berrangé wrote:
> 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.
> 

Okay I'll send a v2 with the NULL bits

Thanks,
Cole