omaciel / fauxfactory

Generates random data for your tests.
Other
37 stars 26 forks source link

Added a method gen_vm_mac() to generate valid mac for QEMU/KVM virtual machines #78

Closed sghai closed 9 years ago

sghai commented 9 years ago

For discovery feature, I need a valid mac that I can use for VM provisioning. However the current gen_mac() doesn't generate a valid mac for VM's on QEMU/KVM. The mac address must start with sequence: 54:52:00 otherwise VM creation fails with error:

ERROR XML error: expected unicast mac address, found multicast '63:8e:b3:53:77:b1'

So I added a new method to generate vm mac. gen_vm_mac(). We can update existing one too if that's the best option ?

omaciel commented 9 years ago

Yeah. I think we should add a flag to the existing method and avoid adding a new one. Also, can I get tests? 😊

omaciel commented 9 years ago

How about

@@ -574,10 +574,12 @@ def gen_ipaddr(ip3=False, ipv6=False, prefix=()):
     return _make_unicode(ipaddr)

-def gen_mac(delimiter=":"):
+def gen_mac(delimiter=":", vm=False):
     """Generates a random MAC address.

     :param str delimeter: Valid MAC delimeter (e.g ':', '-').
+    :param bool vm: If the MAC address is for a VM (eg. QEMU/KVM),
+    then it should start with '54:52:00'.
     :returns: A random MAC address.
     :rtype: str

@@ -589,9 +591,15 @@ def gen_mac(delimiter=":"):
     chars = ['a', 'b', 'c', 'd', 'e', 'f',
              '0', '1', '2', '3', '4', '5', '6', '7', '8', '9']

+    # If we need a MAC for a VM, then only generate the last 3 values.
+    mac_range = 3 if vm else 6
+
     mac = delimiter.join(
         chars[random.randrange(0, len(chars), 1)]+chars[random.randrange(
-            0, len(chars), 1)] for x in range(6))
+            0, len(chars), 1)] for x in range(mac_range))
+
+    if vm:
+        mac = u'54:52:00:{0}'.format(mac)

     return _make_unicode(mac)
sthirugn commented 9 years ago

@omaciel your suggestion looks great, but is 54:52:00 a universal standard? if not, I dont think it is a good idea to hard code that value here.

omaciel commented 9 years ago

I honestly don't know... I could change the method to perhaps take an optional start and end parameter which would allow one to pass a string containing what the final MAC should have... will await on @sghai

Ichimonji10 commented 9 years ago

We could do something like:

def gen_mac(delimiter=':', type=None):

Where type could have a value of None, or it could be a keyword such as "multicast" or "unicast". If type is None, then a completely random MAC address is generated. Otherwise, the specified type of MAC address is generated. We can flesh out the valid values for the type argument over time.

omaciel commented 9 years ago

Reading this...

omaciel commented 9 years ago

@sghai do you think that the issue here is that you need a unicast MAC address but we're generating values that include multicast as well? If so, would it be a matter of allowing the user to optionally select unicast or multicast when calling the method?

See this for an example.

omaciel commented 9 years ago

@sghai together with @elyezer the following PR has been submitted. I would like to ask you to review it and perhaps test creating a MAC with multicast=False for your specific test.

sghai commented 9 years ago

Closing this. Referenced PR looks good and already merged.