<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.0 TRANSITIONAL//EN">
<HTML>
<HEAD>
<META HTTP-EQUIV="Content-Type" CONTENT="text/html; CHARSET=UTF-8">
<META NAME="GENERATOR" CONTENT="GtkHTML/4.6.6">
</HEAD>
<BODY>
Hi Brent,<BR>
<BR>
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:<BR>
<BR>
<PRE>
bad i18n string formatting:
KCHVMIF0010E: The custom MAC address %(value) is already in use on this guest
</PRE>
<BR>
You need to provide a formatting mask (e.g. "%(value)s") when using a variable inside an internationalized string.<BR>
<BR>
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:<BR>
<BR>
<PRE>
[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'
</PRE>
<BR>
The exception above is also raised when we run our test suite (i.e. by typing "sudo make check").<BR>
<BR>
And I have one last comment about the code, which is below:<BR>
<BR>
On Qua, 2014-10-01 at 10:03 -0500, bbaude@redhat.com wrote:
<BLOCKQUOTE TYPE=CITE>
<PRE>
+ 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)
</PRE>
</BLOCKQUOTE>
<BR>
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 <TT>"prependDefer"</TT> would be better placed right after <TT>"vmifaces_create".</TT> Using the code above, however, if the call to <TT>"vmiface_lookup"</TT> somehow fails, the network interface won't be removed by the transaction.
</BODY>
</HTML>