[libvirt] [PATCH] conf: forbid use of multicast mac addresses

[Eric Blake]

On 03/19/2012 11:32 AM, Laine Stump wrote:
> A few times libvirt users manually setting mac addresses have
> complained of networking failure that ends up being due to a multicast
> mac address being used for a guest interface. This patch prevents that
> by loggin an error and failing if a multicast mac address is

s/loggin/logging/

> encountered in each of the three following cases:
> 
> 1) domain xml <interface> mac address.
> 2) network xml bridge mac address.
> 3) network xml dhcp/host mac address.
> 
> There are several other places where a mac address cna be input that

s/cna/can/

> aren't controlled in this manner because failure to do so has no
> consequences (e.g., if the address will be used to search through
> existing interfaces for a match).
> 
> The RNG has been updated to add multiMacAddr and uniMacAddr along with
> the existing macAddr, and macAddr was switched to uniMacAddr where
> appropriate.
> ---
>  docs/schemas/basictypes.rng   |   17 ++++++++++++++++-
>  docs/schemas/domaincommon.rng |    6 +++---
>  docs/schemas/network.rng      |    4 ++--
>  src/conf/domain_conf.c        |    8 +++++++-
>  src/conf/network_conf.c       |   33 ++++++++++++++++++++++++---------
>  src/libvirt_private.syms      |    2 ++
>  src/util/virmacaddr.c         |   13 +++++++++++++
>  src/util/virmacaddr.h         |    3 ++-

Is there any way to add a test of an expected error, where we prove that
a multicast mac fails to validate?  But since the existing tests use
unicast addrs, and those still validate, you've at least tested for no
regressions on sane usage.

> +  <!-- The lowest bit of the 1st byte is the "multicast" bit. a         -->
> +  <!-- uniMacAddr requires that bit to be 0, and a multiMacAddr         -->
> +  <!-- requires it to be 1. Plain macAddr will accept either.           -->
> +  <!-- Currently there is no use of multiMacAddr in libvirt, it         -->
> +  <!-- is included here for documentation/comparison purposes.          -->
> +  <define name="uniMacAddr">
> +    <data type="string">
> +      <param name="pattern">[a-fA-F0-9][02468aAcCeE](:[a-fA-F0-9]{2}){5}</param>
> +    </data>
> +  </define>
> +  <define name="multiMacAddr">
> +    <data type="string">
> +      <param name="pattern">[a-fA-F0-9][13579bBdDfF](:[a-fA-F0-9]{2}){5}</param>
> +    </data>
> +  </define>
>    <define name="macAddr">
>      <data type="string">
> -      <param name="pattern">([a-fA-F0-9]{2}:){5}[a-fA-F0-9]{2}</param>
> +      <param name="pattern">[a-fA-F0-9]{2}(:[a-fA-F0-9]{2}){5}</param>

Changed to match the earlier patterns.  Nice patterns.

ACK.  If you do figure out how to add a negative test for expected
validation failure, I'm okay if you submit it as a separate followup patch.

-- 
Eric Blake   eblake at 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: 620 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20120319/9552b3f6/attachment-0001.sig>