[libvirt] [PATCH v2 1/5] Internal refactory of data structures

[Daniel P. Berrange]

On Wed, Jul 18, 2012 at 11:10:10AM -0600, Eric Blake wrote:
> On 07/18/2012 10:31 AM, Michal Privoznik wrote:
> > On 18.07.2012 03:28, Marcelo Cerri wrote:
> >> This patch updates the structures that store information about each
> >> domain and each hypervisor to support multiple security labels and
> >> drivers. It also updates all the remaining code to use the new fields.
> >> ---
> 
> > 
> > We must update XML schema as well as we are going to allow more
> > <model> and <doi> elements under <secmodel>. And maybe we want to add a test case. But that can be a follow up patch.
> 
> Absolutely add a testcase if you are enhancing the xml parser to accept
> new tags.
> 
> 
> > I think needs to be squashed in:
> > 
> > diff --git a/docs/schemas/capability.rng b/docs/schemas/capability.rng
> > index 06ff685..be9d295 100644
> > --- a/docs/schemas/capability.rng
> > +++ b/docs/schemas/capability.rng
> 
> If you're going to touch docs/schemas, then it would also be nice to
> touch the counterpart docs/*.html.in.  Unfortunately, I think our
> capability.rng is currently underdocumented in the html.
> 
> > @@ -52,12 +52,14 @@
> >  
> >    <define name='secmodel'>
> >      <element name='secmodel'>
> > -      <element name='model'>
> > -        <text/>
> > -      </element>
> > -      <element name='doi'>
> > -        <text/>
> > -      </element>
> > +      <oneOrMore>
> > +        <element name='model'>
> > +          <text/>
> > +        </element>
> > +        <element name='doi'>
> > +          <text/>
> > +        </element>
> > +      </oneOrMore>
> >      </element>
> 
> Hmm, this says that:
> 
> <secmodel>
>   <model>...</model>
>   <doi>...</doi>
>   <model>...</model>
>   <doi>...</doi>
> </secmodel>
> 
> will validate, but:
> 
> <secmodel>
>   <doi>...</doi>
>   <model>...</model>
>   <model>...</model>
>   <doi>...</doi>
> </secmodel>
> 
> will not.  I think that's somewhat good (since the parameters are
> positional, and we insist that they come in pairs, then the pairs must
> be properly interleaved), but did the C code enforce that?
> 
> > 
> > But don't we rather want multiple <secmodel> elements than multiple <doi> and <model> inside one <secmodel>?
> 
> Indeed - that makes the XML layout problem MUCH easier.  If we add an
> <interleave> around the 'model' and 'doi' sub-elements, then you could
> write:
> 
> <secmodel>
>   <doi>...</doi>
>   <model>...</model>
> </secmodel>
> <secmodel>
>   <model>...</model>
>   <doi>...</doi>
> </secmodel>
> 
> and have an unambiguous 2-model design, without quite so much attention
> being placed on the exactly-one 'doi' and 'model' subelement per 'secmodel'.

Agreed, multiple <secmodel> elements is preferrable.

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