[libvirt PATCH 0/3] cpu: Make unknown XML elements fail CPU comparison

[Tim Wiederhake]

On Wed, 2020-09-16 at 14:48 +0100, Daniel P. Berrangé wrote:
> On Wed, Sep 16, 2020 at 03:33:53PM +0200, Tim Wiederhake wrote:
> > We currently ignore unknown elements in the CPU XML description,
> > e.g. with vi=
> > rsh
> > cpu-compare and hypervisor-cpu-compare. This makes '<cpu><faeture
> > name=3D"...=
> > "/></cpu>'
> > (note the typo in "faeture") semantically identic to '<cpu/>'. No
> > error is re=
> > ported.
> 
> This is an intentional implementation choice in libvirt. The XML
> documents
> that we're parsing are way to large and complex for us to justify
> adding
> code to report errors for every invalid attribute or child element
> name.
>
> We provide an RNG schema, however, which declares the valid set of
> attrs
> and thus by running RNG schema validation, a client can get a error
> report
> of invalid info.
> 
> We enable this validation by default in "virsh edit" and a few other
> similar commands.
> 
> There are certainly libvirt APIs though which accept XML and don't
> have
> a flag defined to enable RNG schema validation. I presume the CPU
> APIs
> are one such case.  We should add support for validation to all such
> APIs accepting XML documents, and wire it up in virsh.
> 
> Regards,
> Daniel

Hi Daniel, all,

thanks for the input!

My first attempt at the problem I described did in fact employ RNG
schema validation on the xml documents describing the CPU. The problem
though with that approach is that `virsh [hypervisor-]cpu-compare` can
consume quite different xml documents. The tests add more possible
document types, e.g. multicpu, which force the schema for a validation
performed in `virCPUDefParseXML` to basically import all other schemas
(which produced some naming conflicts that I was unable to solve) and,
secondly, perform a lot of unneccessary validation.

I created a schema that would perform only a check for "unknown"
elements and attributes in the cpu element and ignore everything else.
I attached this for your entertainment. There you can see what I meant
with "different xml documents".

Copying / moving the node into a new document just for validation seems
not like an elegant solution (if possible at all) and I am unsure about
potential performance impact.

The best solution in my opinion would be to not perform XPath
evaluation in the parser, but instead use some stream parser and error
out if the parser encounteres an element with an unknown name.

The next best solution in my opinion is the one you reviewed. I agree
with all comments made that this is far from perfect, but at least it
would trigger visible test fails when / if new elements and attributes
are added without that list being expanded.

I will further explore whether it is possible to do node-only
validation without creating an include-everything schema, but I would
be grateful for any hint or suggestion in that direction. We should
have at least some error reporting in the case I described initially.

Thanks,
Tim
-------------- next part --------------
A non-text attachment was scrubbed...
Name: only_cpu.rng
Type: application/xml
Size: 4035 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20200917/afcb86fe/attachment-0001.wsdl>