[ovs-discuss] [NIC-20 11/11] xenserver: Rename network devices to match MAC addresses of physical PIFs.

[Justin Pettit]

On Aug 5, 2009, at 3:37 PM, Ben Pfaff wrote:

> +def get_netdev_mac(device):
> +    try:
> +        return read_first_line_of_file("/sys/class/net/%s/address"  
> % device)
> +    except:
> +        # Probably no such device.
> +        return None
> +
...
> +def get_netdev_by_mac(mac):
> +    maybe = None
> +    for device in os.listdir("/sys/class/net"):
> +        if mac.lower() == get_netdev_mac(device).lower():

If get_netdev_mac() is handed a device that doesn't exist, it returns  
None, which doesn't have a "lower" member function.  The chances of  
this occurring should be low, since you're iterating through the "/sys/ 
class/net", but it seems like there's a small race if someone destroys  
the device outside of this script (which I agree is unlikely).  An  
easy way to get around this is by having get_netdev_mac() return "" on  
error; Python treats empty strings like a False for purposes of "if"  
checks.

> +        found_mac = get_netdev_mac(device).lower()

Same idea here.

> +        if mac.lower() == found_mac:

Do we know that "mac" from get_pif_record()['MAC'] is always a string?

> +#        if not move_pif_aside(pifrec['host'], device, found_mac,
> +#                              already_renamed):
> +# Attempt to rename 'device' on 'host', which has MAC 'mac'
> +def move_pif_aside(host, device, mac, already_renamed):

This excerpt of commented code sitting above the function description  
looked a little odd to me.

Otherwise, all the rest of the changes associated with this set of  
commits seemed good.

--Justin