Code review comment for lp:~tr3buchet/nova/multi_nic

Impressive work, Trey. I don't see any major problems, just some style/cleanup stuff. I would really like to see massive merge props like this split up (Launchpad truncates at 5000 lines), but I can understand if there may not have been a logical break point.

I noticed several of the docstrings you added could use some reformatting. Would you mind adding capitalization/punctuation where necessary and following sphinx syntax for parameters? This doesn't apply to bin/nova-manage (I think).

619: This new method name implies returning a single fixed_ip, but it still returns a list. Would you mind changing it back? Same goes for the 'fixed_ip_get_by_virtual_interface.' I feel like that name should be pluralized since it also returns a list.

1646-1648: This change doesn't seem related to this merge prop. I personally prefer the code you replaced.
4279-4284: This change is also unnecessary.

2106: Can you make this inherit from NovaException and change the name to something more descriptive? Maybe "VirtualInterfaceCreationFailed" or something. I know it's long, but it describes the error better than just "VirtualInterface."

2129: Could you make NoFixedIpsDefinedForHost inherit from NoFixedIpsDefined? If there are any other exceptions like this that you added I would appreciate it if you would define the correct inheritance. It can be really handy.

It also seems that NoFloatingIpsDefined, NoFloatingIpsDefinedForHost, and NoFloatingIpsDefinedForInstance aren't used anymore. Should we delete those?

3947: Aren't we supposed to put "Copyright 2011 OpenStack LLC." on all of the copyright notices, with any other contributing companies listed under it? I may just not know the correct policy here.

As for all of the skipped tests: I would prefer to see them fixed, but if it is going to be a lot of work, I am okay leaving them for now. I think we may want to file a bug or something so we don't forget about it.

I think we may want to follow up with another merge prop to make the OSAPI display this new information correctly. Right now we have hard-coded public/private networks. We should really be using the new network labels. Again, this isn't something I expect in this MP, just something I don't want us to forget about.