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 :|