Re: [libvirt] [PATCH v10 08/19] backup: Parse and output checkpoint XML

On Wed, Jul 24, 2019 at 17:11:19 +0100, Daniel Berrange wrote: > On Wed, Jul 24, 2019 at 12:55:58AM -0500, Eric Blake wrote: > > Add a new file checkpoint_conf.c that performs the translation to and > > from new XML describing a checkpoint. The code shares a common base > > class with snapshots, since a checkpoint similarly represents the > > domain state at a moment in time. Add some basic testing of round trip > > XML handling through the new code. > > > > Of note - this code intentionally differs from snapshots in that XML > > schema validation is unconditional, rather than based on a public API > > flag. > > I'm in two minds as to whether this is really a good idea. > > I always have a strong tendancy to prefer consistency across the APIs > we expose, which would point to an opt-in flag for validation. If we > did later find we need a way to skip validation, then we end up having > to add a NO_VALIDATION API flag, which makes inconsitency even more > glaring to app devs > > I understand that this can help catch mistakes in apps, but I'm not > sold on it as a default, when it is easy for apps to opt-in if they > want to. I've specifically requested this behaviour as I don't think that there's value in not validating the XML. The XML parser is tied to what we parse anyways and the checks done when parsing are _WAY_ simpler if we know that all the values conform to the schema.

Also if we'd ever want to do a generated parser, we will need to strictly adhere to the schema first. Consistency is great as long as it does not hinder progress. And in this case I think that validating everything right away is progress.