[libvirt] ignore vs. error when inappropriate or unrecognized attributes/elements are present in XML

[Daniel P. Berrange]

On Tue, Aug 16, 2011 at 04:44:42AM -0400, Laine Stump wrote:
> This is related to: https://bugzilla.redhat.com/show_bug.cgi?id=638633#c14
> 
> I had started to reply to it in the comments of the bug, but my
> reply became too long, and expanded into an issue wider than that
> single bug, so I figured I'd better discuss it here instead.

[snip]

> Actually, I can see now there are several different classes of this
> problem. Here are the first few that come to mind:
> 
> 1) an attribute/element is completely unknown/unexpected in all
> cases (e.g. "frozzle='fib'" anywhere, or more insidious, something
> that *looks* correct, but isn't, e.g. "<script
> name='/path/to/script'/>"*)

RNG schema validation is the only sane way to catch this

> 2) an attribute/element is useful/expected only when some other
> attribute is set to a particular value (usually one called "type",
> but could be something else), for example keymap='blah' is only
> expected in a <graphics> element when type='spice' or type='vnc'.

We should always catch these when parsing, since this is done
via our enumeration helpers.

> 3) an attribute/element is useful/expected only for certain
> combinations of the value of some other attribute and which driver
> is using the element, e.g. the subject of this bug - script='blah'
> is only expected when type='bridge' and it's used by the Xen driver,
> or type='ethernet' and it's used by the qemu driver.

IMHO this is just another case of 1) really.

> So what are the rules of engagement for these various cases? When do
> we ignore, when do we log an error during parsing, and when do we
> log an error in the code that's using the parsed data?

I think we should add a flag to 'virDomainDefine' and virDomainCreateXML

  VIR_DOMAIN_VALIDATE_XML

and when that is set, run the user specified XML through the
RNG schema validator. Virsh could be extended to have a --validate
flag too.

We'd add an explicit error code  VIR_ERROR_XML_VALIDATION to let
apps catch schema failures.

This would fix a major annoyance with 'virsh edit' where you make
XML changes and they get lost because you typod.  ie virsh edit
sets the validate flag. If it gets a failure it should ask the user
whether they want to abandon the edit, force the edit (ie define without
the validate flag), or re-launch the editor to correct the mistake.

If we did this we'd get much more use of the RNG schemas and so
find mistakes in them sooner

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|