[prev in list] [next in list] [prev in thread] [next in thread]
List: netcf-devel
Subject: Re: [netcf-devel] [PATCH 1/2] allow reordering of elements in interface xml
From: Eric Blake <eblake () redhat ! com>
Date: 2014-08-22 2:25:44
Message-ID: 53F6AA28.3090801 () redhat ! com
[Download RAW message or body]
[Attachment #2 (multipart/signed)]
On 08/21/2014 04:27 PM, Laine Stump wrote:
> The interface xml schema was written with strict rules about the
> ordering of the elements. This was never intentional, but just due to
> omission of <interleave> in the appropriate places. This patch just
> adds in <interleave> wherever there is more than one element, and
> re-indents everything else appropriately.
>
> NB: this change was earlier made to the libvirt interface.rng (in
> upstream commit a341fc73, which was in libvirt-1.2.6. Also included in
> libvirt-1.2.6 was commit 0b33d7c9, which re-orders the emited
> interface XML; it turns out that a libvirt that contains the latter
> commit (i.e. it re-orders the XML), when paired with a netcf that
> doesn't have this current patch (allowing the re-ordering) will log
> and error and fail any attempt to define a new host interface. Because
> of this, once this patch is in a netcf release, we may want to
> consider putting
>
> Requires: netcf >= 0.2.6
>
> in the libvirt rpm for libvirt 1.2.6 and beyond.
Bummer. Would it help to put <!-- reminder comments --> in the rng
files that are common between the two projects, so we don't forget to
sync up the definitions when touching them? Or even automate the
syncing of the file where one project (which? netcf or libvirt) is
master, and the other copies in the latest as part of the release process?
> ---
> data/xml/interface.rng | 310 ++++++++++++++++++++++++++-----------------------
> 1 file changed, 166 insertions(+), 144 deletions(-)
>
ACK; easier when you review the diff -w view:
> diff --git c/data/xml/interface.rng w/data/xml/interface.rng
> index 845d871..f22910f 100644
> --- c/data/xml/interface.rng
> +++ w/data/xml/interface.rng
> @@ -27,6 +27,7 @@
> Ethernet adapter
> -->
> <define name="basic-ethernet-content">
> + <interleave>
> <attribute name="type">
> <value>ethernet</value>
> </attribute>
> @@ -51,21 +52,26 @@
> <empty/>
> </optional>
> <!-- FIXME: Allow (some) ethtool options -->
> + </interleave>
> </define>
>
> <!-- Ethernet adapter without IP addressing, e.g. for a bridge -->
> <define name="bare-ethernet-interface">
> <element name="interface">
> + <interleave>
> <ref name="basic-ethernet-content"/>
> + </interleave>
> </element>
An interleave matters only when there are two or more <element>
children; it makes no difference on attributes (which are always
interleavable without markup). On the other hand, it never hurts to
have an <interleave> around zero or one elements (the grammar just
ignores them when canonicalizing into the reduced form that actually
validates a document), and maintenance-wise, it's easier to have too
many on the grounds that you might at an element later. For
justification, observe what happens with this pseudo-rng subset:
<define name='def'>
<!-- spot one -->
<element name='a'/>
<element name='b'/>
</define>
<element name='outer'>
<!-- spot two -->
<ref name='def'/>
<element name='c'/>
</element>
Without interleave, you have to have <outer><a/><b/><c/></outer>.
With interleave in spot one only, you can swap a and b, but c still has
to come last.
With interleave in spot two only, you can have c first or last, but a
and b have to be in strict order.
Only with interleave in both spot one and two can you have a, b, and c
in all 6 possible orders.
Therefore, I agree with liberally using <interleave>, even in places
where it is not technically required. So no need to modify your patch.
> </define>
>
> <define name="ethernet-interface">
> <element name="interface">
> + <interleave>
> <ref name="startmode"/>
> <ref name="basic-ethernet-content"/>
> <ref name="mtu"/>
> <ref name="interface-addressing"/>
> + </interleave>
> </element>
> </define>
>
> @@ -93,18 +99,22 @@
>
> <define name="bare-vlan-interface">
> <element name="interface">
> + <interleave>
> <ref name="vlan-interface-common"/>
> <ref name="vlan-device"/>
> + </interleave>
> </element>
> </define>
>
> <define name="vlan-interface">
> <element name="interface">
> + <interleave>
> <ref name="vlan-interface-common"/>
> <ref name="startmode"/>
> <ref name="mtu"/>
> <ref name="interface-addressing"/>
> <ref name="vlan-device"/>
> + </interleave>
> </element>
> </define>
>
> @@ -113,6 +123,7 @@
> -->
> <define name="bridge-interface">
> <element name="interface">
> + <interleave>
> <attribute name="type">
> <value>bridge</value>
> </attribute>
> @@ -138,6 +149,7 @@
> </choice>
> </zeroOrMore>
> </element>
> + </interleave>
> </element>
> </define>
> <!-- Jim Fehlig would like support for other bridge attributes, in
> @@ -188,6 +200,7 @@
> xmit_hash_policy (since 2.6.3/3.2.2)
> -->
>
> + <interleave>
> <optional>
> <choice>
> <element name="miimon">
> @@ -232,23 +245,28 @@
> <!-- The slave interfaces -->
> <ref name="bare-ethernet-interface"/>
> </oneOrMore>
> + </interleave>
> </element>
> </define>
>
> <define name="bare-bond-interface">
> <element name="interface">
> + <interleave>
> <ref name="bond-interface-common"/>
> <ref name="bond-element"/>
> + </interleave>
> </element>
> </define>
>
> <define name="bond-interface">
> <element name="interface">
> + <interleave>
> <ref name="bond-interface-common"/>
> <ref name="startmode"/>
> <ref name="mtu"/>
> <ref name="interface-addressing"/>
> <ref name="bond-element"/>
> + </interleave>
> </element>
> </define>
>
> @@ -310,6 +328,7 @@
> <attribute name="family">
> <value>ipv4</value>
> </attribute>
> + <interleave>
> <choice>
> <ref name="dhcp-element"/>
> <group>
> @@ -326,6 +345,7 @@
> </optional>
> </group>
> </choice>
> + </interleave>
> </element>
> </define>
>
> @@ -334,6 +354,7 @@
> <attribute name="family">
> <value>ipv6</value>
> </attribute>
> + <interleave>
> <optional>
> <element name="autoconf"><empty/></element>
> </optional>
> @@ -353,6 +374,7 @@
> <attribute name="gateway"><ref name="ipv6-addr"/></attribute>
> </element>
> </optional>
> + </interleave>
> </element>
> </define>
>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
["signature.asc" (application/pgp-signature)]
[Attachment #6 (text/plain)]
_______________________________________________
netcf-devel mailing list
netcf-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/netcf-devel
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic