[netcf-devel] [PATCH] support multiple IPv4 addresses in redhat driver

Eric Blake eblake at redhat.com
Fri Mar 27 22:01:40 UTC 2015


On 03/26/2015 12:42 PM, Laine Stump wrote:
> netcf has always supported multiple IPv6 addresses, but only a single
> IPv4 address. This patch remedies that, at least for the redhat driver
> (also used for Fedora and CentOS).
> 
> I can't claim that the xsl transforms in redhat-(get|put).xsl are the
> optimum code, but they do get the job done (no mean feat, since this
> is the first time I've ever made anything beyond a cosmetic change to
> an xsl script).

And I'm no xsl transform expert either, so take my review with a grain
of salt.


> +++ b/data/xml/redhat-get.xsl
> @@ -192,10 +192,18 @@
>        </xsl:when>
>        <xsl:when test="ip">
>          <node label="BOOTPROTO" value="none"/>
> -        <node label="IPADDR" value="{ip/@address}"/>
> -        <xsl:if test="ip/@prefix">
> -          <node label="NETMASK" value="{ipcalc:netmask(ip/@prefix)}"/>
> -        </xsl:if>
> +        <xsl:for-each select="ip">
> +          <xsl:if test="position() = 1">
> +            <xsl:call-template name="ipv4-address">
> +              <xsl:with-param name="index"/>
> +            </xsl:call-template>
> +          </xsl:if>
> +          <xsl:if test="position() > 1 and position() &lt; 100">

I think you have an off-by-one:

1 vs. 2-99...


> +++ b/data/xml/redhat-put.xsl

> +          <xsl:for-each select="node[substring(@label, 1, 6) = 'IPADDR']">
> +            <xsl:variable name="index" select="substring(@label, 7, 3)"/>
> +            <xsl:if test="number($index) &gt; 0 and number($index) &lt; 100">

compared to 0 vs. 1-99.

That is, if position() is 1-based, you want <= 100, or < 101, as your
terminating condition (operating on IPADDR[position()-1] for each
iteration of the for-each).

If you fix or explain the difference, then the rest of the patch looks
okay for an ACK.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 604 bytes
Desc: OpenPGP digital signature
URL: <https://lists.fedorahosted.org/pipermail/netcf-devel/attachments/20150327/bb9e9606/attachment.sig>


More information about the netcf-devel mailing list