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 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
