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

Martin Kletzander mkletzan at redhat.com
Mon Nov 24 16:44:25 UTC 2014


On Thu, Nov 20, 2014 at 01:43:29PM +0000, Daniel P. Berrange wrote:
>On Thu, Nov 20, 2014 at 12:02:12PM +0100, Martin Kletzander wrote:
>> On Wed, Nov 19, 2014 at 12:51:22PM +0000, Daniel P. Berrange wrote:
>> >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
>> >
>>
>> As I said, I'm not against this, I agree that it will still be useful.
>>
>> If you meant this as an RFC, then I misunderstood that and I should've
>> just wrote that as an initial PoC it's fine with me :)
>>
>> Do you want me to finish the review?
>
>Actually if you want to review patches 4, 5, 6, 7 that would be useful.
>Those are general refactoring of the way we handle flags with the XML
>parsers/formatters. The 7th patch was awful to create and will be a
>rebase nightmare if we leave it too long.
>

ACK to 4-7, just read that 7/8 before pushing.  Unfortunately it will
need a bigger rebase, still :(

Martin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20141124/72f52357/attachment-0001.sig>


More information about the libvir-list mailing list