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