Hi Matt, 
thanks for reviewing this branch

> I merged with trunk and got a conflict in network/manager.py
> 
> <<<<<<< TREE
> flags.DEFINE_string('vlan_interface', 'eth0',
>                     'network device for vlans')
> flags.DEFINE_integer('num_networks', 1000, 'Number of networks to support')
> =======
> flags.DEFINE_integer('num_networks', 1, 'Number of networks to support')
> >>>>>>> MERGE-SOURCE
> 

I guess the conflict has arisen from a recent merge (the one for solving a bug in nova-manage). I will merge my branch with trunk, and push again...

> Also had a unit-test error in one of the test files, so the rest of the XenAPI
> tests aren't currently running. Forget a bzr add?

...and when pushing, this time I will not forget to add fake_utils.py!

> 
> ======================================================================
> ERROR: Failure: ImportError (cannot import name fake_utils)
> ----------------------------------------------------------------------
> Traceback (most recent call last):
>   File "/usr/local/lib/python2.6/dist-packages/nose/loader.py", line 390, in
> loadTestsFromName
>     addr.filename, addr.module)
>   File "/usr/local/lib/python2.6/dist-packages/nose/importer.py", line 39, in
> importFromPath
>     return self.importFromDir(dir_path, fqname)
>   File "/usr/local/lib/python2.6/dist-packages/nose/importer.py", line 86, in
> importFromDir
>     mod = load_module(part_fqname, fh, filename, desc)
>   File "/root/code/nova/xenapi-vlan-network-
> manager/nova/tests/test_xenapi.py", line 41, in <module>
>     from nova.tests import fake_utils
> ImportError: cannot import name fake_utils
> 
> ----------------------------------------------------------------------
> 
> 36) Shouldn't this be the citrix copyright?
> 

Yes, it should. Shame on me...

> 623-632) this seems really odd to me. What's an example of how we would
> actually fail into the exception block?
> 

I agree this try/except block is quite weird. 
The main reason is that find_network_with_bridge expects to find a network on the Xen backend whose attribute 'bridge' matches the 'bridge' column  in the nova db or the FLAGS.flat_network_bridge flag. 
However, while this is true for flat networking (in which we tipically set the flag to xenbr0 or something like that), this does not work anymore with VLAN networks. In XenAPI names for bridges associated with VLAN networks are auto-generated. For instance, for VLAN 111 it will not be possible to call the bridge br111, but a name like xapiXX (where XX is an integer) will be generated. 
To solve this problem, when we create a VLAN network in XenAPI, we set its 'name' attribute to the value of the 'bridge' column in the name db. We then lookup for the appropriate network by looking at the 'name' attribute instead of the 'bridge' one. 

The try/except block first tries to identify the network by bridge, and then, if it fails, tries with the 'name' attribute. I agree however that this does not look good at all. I will therefore shift the burden of the bridge/name lookup into NetworkHelper, implementing a method that looks for both properties and then returns the first network for which either the 'bridge' or the 'name' attribute match. 


> 
> Tiny nits:
> 
> 116     +                                  "Expected %(vlan_num)d") %
> locals())
> 
> You might want a space before Expected or it will run up against the period
> from the previous sentence

Right. I accidentaly removed it while doing pep8 fixes.

Thanks, 
Salvatore