Hi Brent,
We have a set of commands to check whether the code has the right syntax according to our standards (e.g. Python PEP8, etc.). You can run it by typing "make check-local". I tried it after applying your patch and I got the following error:
bad i18n string formatting:
KCHVMIF0010E: The custom MAC address %(value) is already in use on this guest
You need to provide a formatting mask (e.g. "%(value)s") when using a variable inside an internationalized string.
I also launched Kimchi after applying this patch and I tried to add a few new NICs with custom MAC addresses. I tried "52:54:00:41:65:a1" and it worked fine. However, when I tried "ab:cd:ef:ab:cd:ef", an unexpected exception was raised:
[01/Oct/2014:16:14:47] HTTP Traceback (most recent call last):
File "/usr/lib/python2.7/site-packages/cherrypy/_cprequest.py", line 656, in respond
response.body = self.handler()
File "/usr/lib/python2.7/site-packages/cherrypy/lib/encoding.py", line 188, in __call__
self.body = self.oldhandler(*args, **kwargs)
File "/usr/lib/python2.7/site-packages/cherrypy/_cpdispatch.py", line 34, in __call__
return self.callable(*self.args, **self.kwargs)
File "/home/vianac/LTC/kimchi/src/kimchi/control/base.py", line 310, in index
return self.create(parse_request(), *args)
File "/home/vianac/LTC/kimchi/src/kimchi/control/base.py", line 242, in create
name = create(*args)
File "/home/vianac/LTC/kimchi/src/kimchi/model/vmifaces.py", line 88, in create
validMac(mac)
File "/home/vianac/LTC/kimchi/src/kimchi/model/vmifaces.py", line 61, in validMac
raise InvalidParameter("KCHVMIF0011E", {'custommac': mac})
File "/home/vianac/LTC/kimchi/src/kimchi/exception.py", line 49, in __init__
msg = unicode(msg, 'utf-8') % args
KeyError: u'value'
The exception above is also raised when we run our test suite (i.e. by typing "sudo make check").
And I have one last comment about the code, which is below:
On Qua, 2014-10-01 at 10:03 -0500, bbaude@redhat.com wrote:
+ mac = inst.vmifaces_create('kimchi-ifaces', iface_args)
+ iface = inst.vmiface_lookup('kimchi-ifaces', mac)
+ self.assertEquals("52:54:00:17:cd:e7", iface['mac'])
+ self.assertRaises(InvalidParameter, inst.vmifaces_create,
+ 'kimchi-ifaces', iface_args)
+ rollback.prependDefer(inst.vmiface_delete, 'kimchi-ifaces', mac)
It's a good practice to add a delete rollback action as soon as the referred object is created. That means that the call to "prependDefer" would be better placed right after "vmifaces_create". Using the code above, however, if the call to "vmiface_lookup" somehow fails, the network interface won't be removed by the transaction.