Code review comment for lp://qastaging/~midokura/nova/network-refactoring-l2

Hi!

Looks like some excellent cleanup.

Tiny consistency suggestion

For the unfilter_instance() method on the driver, you use a kwarg "network_info":

184 + self.driver.unfilter_instance(instance_ref, network_info=network_info)

But for all the other driver methods, you are passing network_info as a positional param:

132 + self.driver.destroy(instance_ref, network_info)
166 + self.driver.plug_vifs(instance_ref, network_info)

It would be good to keep this consistent.

Also, here:

262 + @property
263 + def _create_bridge(self):
264 + """Indicate whether this manager requires VIF to create a bridge."""
265 + return False

and

271 + @property
272 + def _create_vlan(self):
273 + """Indicate whether this manager requires VIF to create a VLAN tag."""
274 + return False

I would recommend naming those should_create_bridge and should_create_vlan to better indicate that the property is a boolean value and not an action. You could also consider making these simply class-level constants.

948 + LOG.warning("Failed while unplugging vif of instance '%s'" % \
1790 + LOG.warning("Failed while unplugging vif of instance '%s'" % \

Need i18n in added strings above...

Cheers!
jay