Re: [libvirt] [PATCH 0/8] Add XML validation to the APIs

On Wed, Nov 19, 2014 at 09:45:39AM +0000, Daniel P. Berrange wrote: > On Wed, Nov 19, 2014 at 08:23:22AM +0100, Martin Kletzander wrote: > > On Tue, Nov 18, 2014 at 05:59:47PM +0000, Daniel P. Berrange wrote: > > >This proof of concept patch extends the virDomainDefineXML > > >and virDomainCreateXML APIs so that they can validate > > >the user supplied XML document against the RNG schemas. > > > > > >The virsh command will enable validation by default, it > > >must be turned off with --skip-validation if desired. > > > > > >This series is not complete > > > > > >- The network, interface, storage pool, etc APIs are > > > not wired up to support validation. > > >- Only the QEMU virt driver is wired up to validate > > >- The virsh edit command is not wired up to validate > > > > > >It is enough to demonstrate it working with 'virsh define' > > >and the QEMU driver though. > > > > > >The biggest problem I see is the really awful error > > >messages we get back from libxml2 when validation > > >fails :-( They are essentially useless :-( > > > > > > > This is one of the things why I'm not convinced this work is worth > > it. It may be nice if we tell the user their XML is invalid instead > > of silently losing information. But error message similar to "invalid > > element in interleave" doesn't help much when you are adding 100-line > > XML. There are some better validators, but requiring those would be > > too cumbersome. > > At least when using 'virsh edit' you would know what element you > just changed / added. So if you got get a generic 'validation failed' > error you have a pretty good idea of where in teh document you made > the mistake. So I think it'd be useful in that scenario. The error > reporting is more of a problem for the apps where they're passing in > a big XML document to define the guest and basically anything could > be wrong. So, it seems not all of the error messages are so awful. It does a bad job of reporting unknown elements, but if you have an unknown attribute it does better: "Invalid attribute foo for element name" If you give an invalid value for an attribute which is an enum it is semi-usefull "Element domain failed to validate attributes" So this does seem somewhat more useful to have in libvirt