Merge lp:~openstack-gd/nova/libvirt-multinic-nova into lp:~hudson-openstack/nova/trunk

[Ilya Alekseyev]

libvirt driver multi_nic support. In this phase libvirt can work with and without multi_nic support, as in multi_nic support for xenapi: https://code.launchpad.net/~tr3buchet/nova/xs_multi_nic/+merge/53458

[Sandy Walsh (sandy-walsh)]

149 if not network_info:

180 if _m is unused, you can just use _

182 if network_ref['injected']:
if 'injected' in network_ref: might be safer (no KeyError if missing)

or better
if not 'injected' in network_ref:
    continue

and pull the indent in the for rest

183 do we need to make the context inside the loop? can this be done outside?

186-188 can be replaced with ra_server = network_ref.get('ra_server', "fd00::"

The 'eth##' will be incremented each time ... even if injected, is that what you wanted?

253 if not ...

263 commented out code?

264 seems to overwrite 262?

351 353 commented out code?

354 if ra_servers:

398 should be like 416

450, 459, 482 use append()?

review: Needs Fixing

[Ilya Alekseyev (ilyaalekseyev)]

Sandy, thank you very much for good review. Please look at my only comment below.

> The 'eth##' will be incremented each time ... even if injected, is that what
> you wanted?
>
yes

[lp:~openstack-gd/nova/libvirt-multinic-nova]

821. By Eldar Nugaev on 2011-03-23

migration gateway_v6 to network_info

822. By Eldar Nugaev on 2011-03-23

small fix

[Nachi Ueno (nati-ueno)]

Unit test fails, would you please check?

Failure
    runTest ERROR

======================================================================
ERROR: test_403 (nova.tests.api.openstack.test_limits.WsgiLimiterProxyTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/nova/libvirt-multinic-nova/nova/tests/api/openstack/test_limits.py", line 578, in test_403
    delay, error = self.proxy.check_for_delay("GET", "/delayed")
  File "/home/nova/libvirt-multinic-nova/nova/api/openstack/limits.py", line 351, in check_for_delay
    conn.request("POST", "/", body, headers)
  File "/home/nova/libvirt-multinic-nova/nova/tests/api/openstack/test_limits.py", line 510, in request
    resp = str(req.get_response(self.app))
  File "/usr/lib/pymodules/python2.6/webob/request.py", line 918, in get_response
    application, catch_exc_info=False)
  File "/usr/lib/pymodules/python2.6/webob/request.py", line 886, in call_application
    app_iter = application(self.environ, start_response)
  File "/usr/lib/pymodules/python2.6/webob/dec.py", line 159, in __call__
    return resp(environ, start_response)
  File "/usr/lib/pymodules/python2.6/webob/exc.py", line 248, in __call__
    return self.generate_response(environ, start_response)
  File "/usr/lib/pymodules/python2.6/webob/exc.py", line 239, in generate_response
    content_type=content_type
  File "/usr/lib/pymodules/python2.6/webob/response.py", line 94, in __init__
    "You cannot set the body to a unicode value without a charset")
TypeError: You cannot set the body to a unicode value without a charset

======================================================================
ERROR: test_response_to_delays (nova.tests.api.openstack.test_limits.WsgiLimiterTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/nova/libvirt-multinic-nova/nova/tests/api/openstack/test_limits.py", line 455, in test_response_to_delays
    delay = self._request("GET", "/delayed")
  File "/home/nova/libvirt-multinic-nova/nova/tests/api/openstack/test_limits.py", line 426, in _request
    response = request.get_response(self.app)
  File "/usr/lib/pymodules/python2.6/webob/request.py", line 918, in get_response
    application, catch_exc_info=False)
  File "/usr/lib/pymodules/python2.6/webob/request.py", line 886, in call_application
    app_iter = application(self.environ, start_response)
  File "/usr/lib/pymodules/python2.6/webob/dec.py", line 159, in __call__
    return resp(environ, start_response)
  File "/usr/lib/pymodules/python2.6/webob/exc.py", line 248, in __call__
    return self.generate_response(environ, start_response)
  File "/usr/lib/pymodules/python2.6/webob/exc.py", line 239, in generate_response
    content_type=content_type
  File "/usr/lib/pymodules/python2.6/webob/response.py", line 94, in __init__
    "You cannot set the body to a unicode value without a charset")
TypeError: You cannot set the body to a unicode value without a charset

======================================================================
ERROR: test_response_to_d...

[Nachi Ueno (nati-ueno)]

I found the versions of some libraries causes the unit test fails.
Please ignore above comment.

[Andrey Brindeyev (abrindeyev)]

> I found the versions of some libraries causes the unit test fails.

Nachi,

Thanks for letting us know!

Could you please document here which libraries (and their versions) caused problems on your side?

Thanks in advance, Andrey.

[Nachi Ueno (nati-ueno)]

Hi Andrey,

We need following package when we use nova.sh.
sudo apt-get install python-webob
sudo apt-get install python-novaclient
# Sorry again,this is not by your branch.

Unit test was OK.
But I can not run instances.

When I run instance, I got a error message.
2011-03-24 07:01:07,634 ERROR nova.exception [-] Uncaught exception
(nova.exception): TRACE: Traceback (most recent call last):
(nova.exception): TRACE: File "/home/nova/libvirt-multinic-nova/nova/exception.py", line 120, in _wrap
(nova.exception): TRACE: return f(*args, **kw)
(nova.exception): TRACE: File "/home/nova/libvirt-multinic-nova/nova/virt/libvirt_conn.py", line 466, in spawn
(nova.exception): TRACE: xml = self.to_xml(instance, network_info)
(nova.exception): TRACE: File "/home/nova/libvirt-multinic-nova/nova/virt/libvirt_conn.py", line 811, in to_xml
(nova.exception): TRACE: network_info = _get_network_info(instance)
(nova.exception): TRACE: File "/home/nova/libvirt-multinic-nova/nova/virt/libvirt_conn.py", line 193, in _get_network_info
(nova.exception): TRACE: 'ip6s': [ip6_dict(ip) for ip in network_ips]}
(nova.exception): TRACE: File "/home/nova/libvirt-multinic-nova/nova/virt/libvirt_conn.py", line 178, in ip6_dict
(nova.exception): TRACE: "ip": utils.to_global_ipv6(prefix, mac),
(nova.exception): TRACE: File "/home/nova/libvirt-multinic-nova/nova/utils.py", line 316, in to_global_ipv6
(nova.exception): TRACE: maskIP = netaddr.IPNetwork(prefix).ip
(nova.exception): TRACE: File "/usr/lib/pymodules/python2.6/netaddr/ip/__init__.py", line 688, in __init__
(nova.exception): TRACE: prefix, suffix = addr.split('/')
(nova.exception): TRACE: AttributeError: 'NoneType' object has no attribute 'split'
(nova.exception): TRACE:
2011-03-24 07:01:07,645 ERROR nova.compute.manager [MAD68M-0SV70DSREX1ZM admin admin] Instance '1' failed to spawn. Is virtualization enabled in the BIOS?
(nova.compute.manager): TRACE: Traceback (most recent call last):
(nova.compute.manager): TRACE: File "/home/nova/libvirt-multinic-nova/nova/compute/manager.py", line 225, in run_instance
(nova.compute.manager): TRACE: self.driver.spawn(instance_ref)
(nova.compute.manager): TRACE: File "/home/nova/libvirt-multinic-nova/nova/exception.py", line 126, in _wrap
(nova.compute.manager): TRACE: raise Error(str(e))
(nova.compute.manager): TRACE: Error: 'NoneType' object has no attribute 'split'
(nova.compute.manager): TRACE:
libvir: QEMU error : Domain not found: no domain with matching name 'instance-00000001'

I found that _get_network_info should check FLAGS.use_ipv6.

156 def _get_network_info(instance):
 157 #TODO(ilyaalekseyev) If we will keep this function
 158 # we should cache network_info
 159 admin_context = context.get_admin_context()
 160 ip_addresses = db.fixed_ip_get_all_by_instance(admin_context,
 161 instance['id'])
 162
 163 networks = db.network_get_all_by_instance(admin_context,
 164 instance['id'])
 165 network_info = []
 166
 167 def i...

review: Needs Fixing

[Ilya Alekseyev (ilyaalekseyev)]

Nachi, thank you much for review.
Fixing.

[lp:~openstack-gd/nova/libvirt-multinic-nova]

823. By Ilya Alekseyev on 2011-03-24

couple of bugs fixed

824. By Eldar Nugaev on 2011-03-24

pep8 clearing

825. By Ilya Alekseyev on 2011-03-24

trunk merged. conflicts resolved

[Matt Dietz (cerberus)]

Nachi: could you use http://paste.openstack.org/ in the future for large tracebacks or test failure lists? It would make things a lot easier to read overall

[Ilya Alekseyev (ilyaalekseyev)]

Hi Nachi!

We checked that VMs is starting and available via ipv4.
Thank you very much for you help.

[Matt Dietz (cerberus)]

I'd like to see a lot more unit tests covering this functionality. The only addition I see is you updated one test to check for a mac address. Considering the substantial change this represents, going forward untested is pretty scary. Do you think you could throw a few tests in there?

Nit-picks / Style junk:

42 + return (mac64_addr ^ netaddr.IPAddress('::0200:0:0:0') | maskIP).\
43 + format()

I'd like to see format() indented at least 8 spaces for readability's sake.

146 + #TODO(ilyaalekseyev) If we will keep this function
147 + # we should cache network_info

You should be consistent with your spacing. I'd move TODO over one space

review: Needs Fixing

[lp:~openstack-gd/nova/libvirt-multinic-nova]

826. By Ilya Alekseyev on 2011-03-24

style and spacing fixed

[Eldar Nugaev (reldan)]

Hi Matt.
Thank you too much for your review.

We're synchronizing our changes with Trey Morris, he's working on add multi-nic support for xen https://code.launchpad.net/~tr3buchet/nova/xs_multi_nic/+merge/53458. So we did our changes by the same way and at this moment it is not changing functionality. Existed unittest already covered our changes. But we agree with you. Let us merge our changes to the trunk before FF and create additional tests as a bug fix.

Next blueprint https://blueprints.launchpad.net/nova/+spec/nova-multi-nic contains changes in db model.

> I'd like to see a lot more unit tests covering this functionality. The only
> addition I see is you updated one test to check for a mac address. Considering
> the substantial change this represents, going forward untested is pretty
> scary. Do you think you could throw a few tests in there?
>
> Nit-picks / Style junk:
>
> 42 + return (mac64_addr ^ netaddr.IPAddress('::0200:0:0:0') |
> maskIP).\
> 43 + format()
>
> I'd like to see format() indented at least 8 spaces for readability's sake.
>
> 146 + #TODO(ilyaalekseyev) If we will keep this function
> 147 + # we should cache network_info
>
> You should be consistent with your spacing. I'd move TODO over one space

[Eldar Nugaev (reldan)]

Bug for additional tests - https://bugs.launchpad.net/nova/+bug/742053

[Matt Dietz (cerberus)]

Alright Eldar, that works for me.

review: Approve

[Paul Voccio (pvo)]

ok

review: Approve

[Trey Morris (tr3buchet)]

looks good to me. You'll want to add 'rxtx_cap' and 'broadcast' to network info in _get_network_info() as in vmops.

review: Needs Fixing

[Trey Morris (tr3buchet)]

or it's already merged.. holy hell.

[Ilya Alekseyev (ilyaalekseyev)]

Thank you Trey.
I've created new bug, we will fix it.
https://bugs.launchpad.net/nova/+bug/742128

[Salvatore Orlando (salvatore-orlando)]

Hi Ilya,

I have updated xenapi code for doing network injection, using interfaces.template.
I've seen that you populate the template in the following way:

net = str(Template(ifc_template, searchList=[{'interfaces': nets}]))

I tried to do the same thing, but Cheetah was complaining that it could not find 'use_ipv6'. I looked at the template and found out that use_ipv6 is a first-level variable:

# This file describes the network interfaces available on your system
# and how to activate them. For more information, see interfaces(5).

# The loopback network interface
auto lo
iface lo inet loopback

#for $ifc in $interfaces
auto ${ifc.name}
iface ${ifc.name} inet static
        address ${ifc.address}
        netmask ${ifc.netmask}
        broadcast ${ifc.broadcast}
        gateway ${ifc.gateway}
        dns-nameservers ${ifc.dns}

--> #if $use_ipv6 <--
iface ${ifc.name} inet6 static
    address ${ifc.address_v6}
    netmask ${ifc.netmask_v6}
    gateway ${ifc.gateway_v6}
#end if

#end for

I therefore updated my code in the following way (and it worked):

        net = str(template(template_data,
                        searchList=[{'interfaces': interfaces_info,
                                     'use_ipv6': FLAGS.use_ipv6}]))

What am I missing? Do you have any hint about why I neeeded to pass use_ipv6 in this way?

Thanks,
Salvatore

[Ilya Alekseyev (ilyaalekseyev)]

Hi Salvatore!

I think you did it in right way.
I kept use_ipv6 as first level variable bacause it depends on global flag.

I'll fix my code, asap. Thank you.

=== modified file 'nova/tests/test_virt.py'
--- nova/tests/test_virt.py	2011-03-24 09:52:41 +0000
+++ nova/tests/test_virt.py	2011-03-24 20:02:31 +0000
@@ -796,7 +796,8 @@
 
         instance_ref = db.instance_create(self.context,
                                           {'user_id': 'fake',
-                                          'project_id': 'fake'})
+                                          'project_id': 'fake',
+                                          'mac_address': '00:A0:C9:14:C8:29'})
         inst_id = instance_ref['id']
 
         ip = '10.11.12.13'
@@ -813,7 +814,8 @@
                                             'instance_id': instance_ref['id']})
 
         def _ensure_all_called():
-            instance_filter = 'nova-instance-%s' % instance_ref['name']
+            instance_filter = 'nova-instance-%s-%s' % (instance_ref['name'],
+                                                       '00A0C914C829')
             secgroup_filter = 'nova-secgroup-%s' % self.security_group['id']
             for required in [secgroup_filter, 'allow-dhcp-server',
                              'no-arp-spoofing', 'no-ip-spoofing',

=== modified file 'nova/utils.py'
--- nova/utils.py	2011-03-24 18:51:38 +0000
+++ nova/utils.py	2011-03-24 20:02:31 +0000
@@ -311,11 +311,15 @@
 
 
 def to_global_ipv6(prefix, mac):
-    mac64 = netaddr.EUI(mac).eui64().words
-    int_addr = int(''.join(['%02x' % i for i in mac64]), 16)
-    mac64_addr = netaddr.IPAddress(int_addr)
-    maskIP = netaddr.IPNetwork(prefix).ip
-    return (mac64_addr ^ netaddr.IPAddress('::0200:0:0:0') | maskIP).format()
+    try:
+        mac64 = netaddr.EUI(mac).eui64().words
+        int_addr = int(''.join(['%02x' % i for i in mac64]), 16)
+        mac64_addr = netaddr.IPAddress(int_addr)
+        maskIP = netaddr.IPNetwork(prefix).ip
+        return (mac64_addr ^ netaddr.IPAddress('::0200:0:0:0') | maskIP).\
+                                                                    format()
+    except TypeError:
+        raise TypeError(_("Bad mac for to_global_ipv6: %s") % mac)
 
 
 def to_mac(ipv6_address):

=== modified file 'nova/virt/driver.py'
--- nova/virt/driver.py	2011-03-24 02:15:41 +0000
+++ nova/virt/driver.py	2011-03-24 20:02:31 +0000
@@ -61,7 +61,7 @@
         """Return a list of InstanceInfo for all registered VMs"""
         raise NotImplementedError()
 
-    def spawn(self, instance):
+    def spawn(self, instance, network_info=None):
         """Launch a VM for the specified instance"""
         raise NotImplementedError()
 

=== modified file 'nova/virt/interfaces.template'
--- nova/virt/interfaces.template	2011-03-02 01:12:47 +0000
+++ nova/virt/interfaces.template	2011-03-24 20:02:31 +0000
@@ -5,19 +5,20 @@
 auto lo
 iface lo inet loopback
 
-# The primary network interface
-auto eth0
-iface eth0 inet static
-        address ${address}
-        netmask ${netmask}
-        broadcast ${broadcast}
-        gateway ${gateway}
-        dns-nameservers ${dns}
+#for $ifc in $interfaces
+auto ${ifc.name}
+iface ${ifc.name} inet static
+        address ${ifc.address}
+        netmask ${ifc.netmask}
+        broadcast ${ifc.broadcast}
+        gateway ${ifc.gateway}
+        dns-nameservers ${ifc.dns}
 
 #if $use_ipv6
-iface eth0 inet6 static
-    address ${address_v6}
-    netmask ${netmask_v6}
-    gateway ${gateway_v6}
+iface ${ifc.name} inet6 static
+    address ${ifc.address_v6}
+    netmask ${ifc.netmask_v6}
+    gateway ${ifc.gateway_v6}
 #end if
 
+#end for

=== modified file 'nova/virt/libvirt.xml.template'
--- nova/virt/libvirt.xml.template	2011-03-02 01:12:47 +0000
+++ nova/virt/libvirt.xml.template	2011-03-24 20:02:31 +0000
@@ -69,21 +69,24 @@
         </disk>
     #end if
 #end if
+
+#for $nic in $nics
         <interface type='bridge'>
-            <source bridge='${bridge_name}'/>
-            <mac address='${mac_address}'/>
+            <source bridge='${nic.bridge_name}'/>
+            <mac address='${nic.mac_address}'/>
             <!--   <model type='virtio'/>  CANT RUN virtio network right now -->
-            <filterref filter="nova-instance-${name}">
-                <parameter name="IP" value="${ip_address}" />
-                <parameter name="DHCPSERVER" value="${dhcp_server}" />           
-#if $getVar('extra_params', False)
-                ${extra_params}
+            <filterref filter="nova-instance-${name}-${nic.id}">
+                <parameter name="IP" value="${nic.ip_address}" />
+                <parameter name="DHCPSERVER" value="${nic.dhcp_server}" />
+#if $getVar('nic.extra_params', False)
+                ${nic.extra_params}
 #end if
-#if $getVar('gateway_v6', False)
-                <parameter name="RASERVER" value="${gateway_v6}" />
+#if $getVar('nic.gateway_v6', False)
+                <parameter name="RASERVER" value="${nic.gateway_v6}" />
 #end if
             </filterref>
         </interface>
+#end for
 
         <!-- The order is significant here.  File must be defined first -->
         <serial type="file">

=== modified file 'nova/virt/libvirt_conn.py'
--- nova/virt/libvirt_conn.py	2011-03-24 17:53:54 +0000
+++ nova/virt/libvirt_conn.py	2011-03-24 20:02:31 +0000
@@ -153,6 +153,51 @@
         return int(net.version())
 
 
+def _get_network_info(instance):
+    # TODO(adiantum) If we will keep this function
+    # we should cache network_info
+    admin_context = context.get_admin_context()
+
+    ip_addresses = db.fixed_ip_get_all_by_instance(admin_context,
+                                                   instance['id'])
+
+    networks = db.network_get_all_by_instance(admin_context,
+                                              instance['id'])
+    network_info = []
+
+    def ip_dict(ip):
+        return {
+            "ip": ip.address,
+            "netmask": network["netmask"],
+            "enabled": "1"}
+
+    def ip6_dict(ip6):
+        prefix = ip6.network.cidr_v6
+        mac = instance.mac_address
+        return  {
+            "ip": utils.to_global_ipv6(prefix, mac),
+            "netmask": ip6.network.netmask_v6,
+            "gateway": ip6.network.gateway_v6,
+            "enabled": "1"}
+
+    for network in networks:
+        network_ips = [ip for ip in ip_addresses
+                       if ip.network_id == network.id]
+
+        mapping = {
+            'label': network['label'],
+            'gateway': network['gateway'],
+            'mac': instance.mac_address,
+            'dns': [network['dns']],
+            'ips': [ip_dict(ip) for ip in network_ips]}
+
+        if FLAGS.use_ipv6:
+            mapping['ip6s'] = [ip6_dict(ip) for ip in network_ips]
+
+        network_info.append((network, mapping))
+    return network_info
+
+
 class LibvirtConnection(driver.ComputeDriver):
 
     def __init__(self, read_only):
@@ -444,16 +489,18 @@
     def poll_rescued_instances(self, timeout):
         pass
 
+    # NOTE(ilyaalekseyev): Implementation like in multinics
+    # for xenapi(tr3buchet)
     @exception.wrap_exception
-    def spawn(self, instance):
-        xml = self.to_xml(instance)
+    def spawn(self, instance, network_info=None):
+        xml = self.to_xml(instance, network_info)
         db.instance_set_state(context.get_admin_context(),
                               instance['id'],
                               power_state.NOSTATE,
                               'launching')
-        self.firewall_driver.setup_basic_filtering(instance)
-        self.firewall_driver.prepare_instance_filter(instance)
-        self._create_image(instance, xml)
+        self.firewall_driver.setup_basic_filtering(instance, network_info)
+        self.firewall_driver.prepare_instance_filter(instance, network_info)
+        self._create_image(instance, xml, network_info)
         self._conn.createXML(xml, 0)
         LOG.debug(_("instance %s: is running"), instance['name'])
         self.firewall_driver.apply_instance_filter(instance)
@@ -609,7 +656,14 @@
         utils.execute('truncate', target, '-s', "%dG" % local_gb)
         # TODO(vish): should we format disk by default?
 
-    def _create_image(self, inst, libvirt_xml, suffix='', disk_images=None):
+    def _create_image(self, inst, libvirt_xml, suffix='', disk_images=None,
+                        network_info=None):
+        if not network_info:
+            network_info = _get_network_info(inst)
+
+        if not suffix:
+            suffix = ''
+
         # syntactic nicety
         def basepath(fname='', suffix=suffix):
             return os.path.join(FLAGS.instances_path,
@@ -685,28 +739,35 @@
 
         key = str(inst['key_data'])
         net = None
-        network_ref = db.network_get_by_instance(context.get_admin_context(),
-                                                 inst['id'])
-        if network_ref['injected']:
-            admin_context = context.get_admin_context()
-            address = db.instance_get_fixed_address(admin_context, inst['id'])
+
+        nets = []
+        ifc_template = open(FLAGS.injected_network_template).read()
+        ifc_num = -1
+        admin_context = context.get_admin_context()
+        for (network_ref, mapping) in network_info:
+            ifc_num += 1
+
+            if not 'injected' in network_ref:
+                continue
+
+            address = mapping['ips'][0]['ip']
             address_v6 = None
             if FLAGS.use_ipv6:
-                address_v6 = db.instance_get_fixed_address_v6(admin_context,
-                                                              inst['id'])
-
-            interfaces_info = {'address': address,
-                               'netmask': network_ref['netmask'],
-                               'gateway': network_ref['gateway'],
-                               'broadcast': network_ref['broadcast'],
-                               'dns': network_ref['dns'],
-                               'address_v6': address_v6,
-                               'gateway_v6': network_ref['gateway_v6'],
-                               'netmask_v6': network_ref['netmask_v6'],
-                               'use_ipv6': FLAGS.use_ipv6}
-
-            net = str(Template(self.interfaces_xml,
-                                searchList=[interfaces_info]))
+                address_v6 = mapping['ip6s'][0]['ip']
+            net_info = {'name': 'eth%d' % ifc_num,
+                   'address': address,
+                   'netmask': network_ref['netmask'],
+                   'gateway': network_ref['gateway'],
+                   'broadcast': network_ref['broadcast'],
+                   'dns': network_ref['dns'],
+                   'address_v6': address_v6,
+                   'gateway_v6': network_ref['gateway_v6'],
+                   'netmask_v6': network_ref['netmask_v6'],
+                   'use_ipv6': FLAGS.use_ipv6}
+            nets.append(net_info)
+
+        net = str(Template(ifc_template, searchList=[{'interfaces': nets}]))
+
         if key or net:
             inst_name = inst['name']
             img_id = inst.image_id
@@ -728,20 +789,11 @@
         if FLAGS.libvirt_type == 'uml':
             utils.execute('sudo', 'chown', 'root', basepath('disk'))
 
-    def to_xml(self, instance, rescue=False):
-        # TODO(termie): cache?
-        LOG.debug(_('instance %s: starting toXML method'), instance['name'])
-        network = db.network_get_by_instance(context.get_admin_context(),
-                                             instance['id'])
-        # FIXME(vish): stick this in db
-        instance_type = instance['instance_type']
-        # instance_type = test.INSTANCE_TYPES[instance_type]
-        instance_type = instance_types.get_instance_type(instance_type)
-        ip_address = db.instance_get_fixed_address(context.get_admin_context(),
-                                                   instance['id'])
+    def _get_nic_for_xml(self, network, mapping):
         # Assume that the gateway also acts as the dhcp server.
         dhcp_server = network['gateway']
         gateway_v6 = network['gateway_v6']
+        mac_id = mapping['mac'].replace(':', '')
 
         if FLAGS.allow_project_net_traffic:
             if FLAGS.use_ipv6:
@@ -766,6 +818,38 @@
                               (net, mask)
         else:
             extra_params = "\n"
+
+        result = {
+            'id': mac_id,
+            'bridge_name': network['bridge'],
+            'mac_address': mapping['mac'],
+            'ip_address': mapping['ips'][0]['ip'],
+            'dhcp_server': dhcp_server,
+            'extra_params': extra_params,
+        }
+
+        if gateway_v6:
+            result['gateway_v6'] = gateway_v6 + "/128"
+
+        return result
+
+    def to_xml(self, instance, rescue=False, network_info=None):
+        # TODO(termie): cache?
+        LOG.debug(_('instance %s: starting toXML method'), instance['name'])
+
+        # TODO(adiantum) remove network_info creation code
+        # when multinics will be completed
+        if not network_info:
+            network_info = _get_network_info(instance)
+
+        nics = []
+        for (network, mapping) in network_info:
+            nics.append(self._get_nic_for_xml(network,
+                                              mapping))
+        # FIXME(vish): stick this in db
+        instance_type_name = instance['instance_type']
+        instance_type = instance_types.get_instance_type(instance_type_name)
+
         if FLAGS.use_cow_images:
             driver_type = 'qcow2'
         else:
@@ -777,17 +861,11 @@
                                              instance['name']),
                     'memory_kb': instance_type['memory_mb'] * 1024,
                     'vcpus': instance_type['vcpus'],
-                    'bridge_name': network['bridge'],
-                    'mac_address': instance['mac_address'],
-                    'ip_address': ip_address,
-                    'dhcp_server': dhcp_server,
-                    'extra_params': extra_params,
                     'rescue': rescue,
                     'local': instance_type['local_gb'],
-                    'driver_type': driver_type}
+                    'driver_type': driver_type,
+                    'nics': nics}
 
-        if gateway_v6:
-            xml_info['gateway_v6'] = gateway_v6 + "/128"
         if not rescue:
             if instance['kernel_id']:
                 xml_info['kernel'] = xml_info['basepath'] + "/kernel"
@@ -800,7 +878,6 @@
         xml = str(Template(self.libvirt_xml, searchList=[xml_info]))
         LOG.debug(_('instance %s: finished toXML method'),
                         instance['name'])
-
         return xml
 
     def get_info(self, instance_name):
@@ -1316,7 +1393,7 @@
 
 
 class FirewallDriver(object):
-    def prepare_instance_filter(self, instance):
+    def prepare_instance_filter(self, instance, network_info=None):
         """Prepare filters for the instance.
 
         At this point, the instance isn't running yet."""
@@ -1350,7 +1427,7 @@
         the security group."""
         raise NotImplementedError()
 
-    def setup_basic_filtering(self, instance):
+    def setup_basic_filtering(self, instance, network_info=None):
         """Create rules to block spoofing and allow dhcp.
 
         This gets called when spawning an instance, before
@@ -1359,11 +1436,6 @@
         """
         raise NotImplementedError()
 
-    def _gateway_v6_for_instance(self, instance):
-        network = db.network_get_by_instance(context.get_admin_context(),
-                                             instance['id'])
-        return network['gateway_v6']
-
 
 class NWFilterFirewall(FirewallDriver):
     """
@@ -1455,10 +1527,13 @@
                               </rule>
                             </filter>'''
 
-    def setup_basic_filtering(self, instance):
+    def setup_basic_filtering(self, instance, network_info=None):
         """Set up basic filtering (MAC, IP, and ARP spoofing protection)"""
         logging.info('called setup_basic_filtering in nwfilter')
 
+        if not network_info:
+            network_info = _get_network_info(instance)
+
         if self.handle_security_groups:
             # No point in setting up a filter set that we'll be overriding
             # anyway.
@@ -1467,9 +1542,11 @@
         logging.info('ensuring static filters')
         self._ensure_static_filters()
 
-        instance_filter_name = self._instance_filter_name(instance)
-        self._define_filter(self._filter_container(instance_filter_name,
-                                                   ['nova-base']))
+        for (network, mapping) in network_info:
+            nic_id = mapping['mac'].replace(':', '')
+            instance_filter_name = self._instance_filter_name(instance, nic_id)
+            self._define_filter(self._filter_container(instance_filter_name,
+                                                       ['nova-base']))
 
     def _ensure_static_filters(self):
         if self.static_filters_configured:
@@ -1560,48 +1637,60 @@
         # Nothing to do
         pass
 
-    def prepare_instance_filter(self, instance):
+    def prepare_instance_filter(self, instance, network_info=None):
         """
         Creates an NWFilter for the given instance. In the process,
         it makes sure the filters for the security groups as well as
         the base filter are all in place.
         """
+        if not network_info:
+            network_info = _get_network_info(instance)
         if instance['image_id'] == FLAGS.vpn_image_id:
             base_filter = 'nova-vpn'
         else:
             base_filter = 'nova-base'
 
-        instance_filter_name = self._instance_filter_name(instance)
-        instance_secgroup_filter_name = '%s-secgroup' % (instance_filter_name,)
-        instance_filter_children = [base_filter, instance_secgroup_filter_name]
+        ctxt = context.get_admin_context()
+
+        instance_secgroup_filter_name = \
+            '%s-secgroup' % (self._instance_filter_name(instance))
+            #% (instance_filter_name,)
+
         instance_secgroup_filter_children = ['nova-base-ipv4',
                                              'nova-base-ipv6',
                                              'nova-allow-dhcp-server']
-        if FLAGS.use_ipv6:
-            gateway_v6 = self._gateway_v6_for_instance(instance)
-            if gateway_v6:
-                instance_secgroup_filter_children += ['nova-allow-ra-server']
-
-        ctxt = context.get_admin_context()
-
-        if FLAGS.allow_project_net_traffic:
-            instance_filter_children += ['nova-project']
-            if FLAGS.use_ipv6:
-                instance_filter_children += ['nova-project-v6']
-
-        for security_group in db.security_group_get_by_instance(ctxt,
-                                                               instance['id']):
+
+        for security_group in \
+                db.security_group_get_by_instance(ctxt, instance['id']):
 
             self.refresh_security_group_rules(security_group['id'])
 
             instance_secgroup_filter_children += [('nova-secgroup-%s' %
-                                                         security_group['id'])]
+                                                    security_group['id'])]
 
-        self._define_filter(
+            self._define_filter(
                     self._filter_container(instance_secgroup_filter_name,
                                            instance_secgroup_filter_children))
 
-        self._define_filter(
+        for (network, mapping) in network_info:
+            nic_id = mapping['mac'].replace(':', '')
+            instance_filter_name = self._instance_filter_name(instance, nic_id)
+            instance_filter_children = \
+                [base_filter, instance_secgroup_filter_name]
+
+            if FLAGS.use_ipv6:
+                gateway_v6 = network['gateway_v6']
+
+                if gateway_v6:
+                    instance_secgroup_filter_children += \
+                        ['nova-allow-ra-server']
+
+            if FLAGS.allow_project_net_traffic:
+                instance_filter_children += ['nova-project']
+                if FLAGS.use_ipv6:
+                    instance_filter_children += ['nova-project-v6']
+
+            self._define_filter(
                     self._filter_container(instance_filter_name,
                                            instance_filter_children))
 
@@ -1649,8 +1738,10 @@
             xml += "chain='ipv4'>%s</filter>" % rule_xml
         return xml
 
-    def _instance_filter_name(self, instance):
-        return 'nova-instance-%s' % instance['name']
+    def _instance_filter_name(self, instance, nic_id=None):
+        if not nic_id:
+            return 'nova-instance-%s' % (instance['name'])
+        return 'nova-instance-%s-%s' % (instance['name'], nic_id)
 
 
 class IptablesFirewallDriver(FirewallDriver):
@@ -1665,9 +1756,11 @@
         self.iptables.ipv6['filter'].add_chain('sg-fallback')
         self.iptables.ipv6['filter'].add_rule('sg-fallback', '-j DROP')
 
-    def setup_basic_filtering(self, instance):
+    def setup_basic_filtering(self, instance, network_info=None):
         """Use NWFilter from libvirt for this."""
-        return self.nwfilter.setup_basic_filtering(instance)
+        if not network_info:
+            network_info = _get_network_info(instance)
+        return self.nwfilter.setup_basic_filtering(instance, network_info)
 
     def apply_instance_filter(self, instance):
         """No-op. Everything is done in prepare_instance_filter"""
@@ -1681,29 +1774,40 @@
             LOG.info(_('Attempted to unfilter instance %s which is not '
                      'filtered'), instance['id'])
 
-    def prepare_instance_filter(self, instance):
+    def prepare_instance_filter(self, instance, network_info=None):
+        if not network_info:
+            network_info = _get_network_info(instance)
         self.instances[instance['id']] = instance
-        self.add_filters_for_instance(instance)
+        self.add_filters_for_instance(instance, network_info)
         self.iptables.apply()
 
-    def add_filters_for_instance(self, instance):
+    def add_filters_for_instance(self, instance, network_info=None):
+        if not network_info:
+            network_info = _get_network_info(instance)
         chain_name = self._instance_chain_name(instance)
 
         self.iptables.ipv4['filter'].add_chain(chain_name)
-        ipv4_address = self._ip_for_instance(instance)
-        self.iptables.ipv4['filter'].add_rule('local',
-                                              '-d %s -j $%s' %
-                                              (ipv4_address, chain_name))
+
+        ips_v4 = [ip['ip'] for (_, mapping) in network_info
+                            for ip in mapping['ips']]
+
+        for ipv4_address in ips_v4:
+            self.iptables.ipv4['filter'].add_rule('local',
+                                                  '-d %s -j $%s' %
+                                                  (ipv4_address, chain_name))
 
         if FLAGS.use_ipv6:
             self.iptables.ipv6['filter'].add_chain(chain_name)
-            ipv6_address = self._ip_for_instance_v6(instance)
-            self.iptables.ipv6['filter'].add_rule('local',
-                                                  '-d %s -j $%s' %
-                                                  (ipv6_address,
-                                                   chain_name))
-
-        ipv4_rules, ipv6_rules = self.instance_rules(instance)
+            ips_v6 = [ip['ip'] for (_, mapping) in network_info
+                                 for ip in mapping['ip6s']]
+
+            for ipv6_address in ips_v6:
+                self.iptables.ipv6['filter'].add_rule('local',
+                                                      '-d %s -j $%s' %
+                                                      (ipv6_address,
+                                                       chain_name))
+
+        ipv4_rules, ipv6_rules = self.instance_rules(instance, network_info)
 
         for rule in ipv4_rules:
             self.iptables.ipv4['filter'].add_rule(chain_name, rule)
@@ -1719,7 +1823,9 @@
         if FLAGS.use_ipv6:
             self.iptables.ipv6['filter'].remove_chain(chain_name)
 
-    def instance_rules(self, instance):
+    def instance_rules(self, instance, network_info=None):
+        if not network_info:
+            network_info = _get_network_info(instance)
         ctxt = context.get_admin_context()
 
         ipv4_rules = []
@@ -1733,28 +1839,36 @@
         ipv4_rules += ['-m state --state ESTABLISHED,RELATED -j ACCEPT']
         ipv6_rules += ['-m state --state ESTABLISHED,RELATED -j ACCEPT']
 
-        dhcp_server = self._dhcp_server_for_instance(instance)
-        ipv4_rules += ['-s %s -p udp --sport 67 --dport 68 '
-                       '-j ACCEPT' % (dhcp_server,)]
+        dhcp_servers = [network['gateway'] for (network, _m) in network_info]
+
+        for dhcp_server in dhcp_servers:
+            ipv4_rules.append('-s %s -p udp --sport 67 --dport 68 '
+                              '-j ACCEPT' % (dhcp_server,))
 
         #Allow project network traffic
         if FLAGS.allow_project_net_traffic:
-            cidr = self._project_cidr_for_instance(instance)
-            ipv4_rules += ['-s %s -j ACCEPT' % (cidr,)]
+            cidrs = [network['cidr'] for (network, _m) in network_info]
+            for cidr in cidrs:
+                ipv4_rules.append('-s %s -j ACCEPT' % (cidr,))
 
         # We wrap these in FLAGS.use_ipv6 because they might cause
         # a DB lookup. The other ones are just list operations, so
         # they're not worth the clutter.
         if FLAGS.use_ipv6:
             # Allow RA responses
-            gateway_v6 = self._gateway_v6_for_instance(instance)
-            if gateway_v6:
-                ipv6_rules += ['-s %s/128 -p icmpv6 -j ACCEPT' % (gateway_v6,)]
+            gateways_v6 = [network['gateway_v6'] for (network, _) in
+                           network_info]
+            for gateway_v6 in gateways_v6:
+                ipv6_rules.append(
+                        '-s %s/128 -p icmpv6 -j ACCEPT' % (gateway_v6,))
 
             #Allow project network traffic
             if FLAGS.allow_project_net_traffic:
-                cidrv6 = self._project_cidrv6_for_instance(instance)
-                ipv6_rules += ['-s %s -j ACCEPT' % (cidrv6,)]
+                cidrv6s = [network['cidr_v6'] for (network, _m)
+                          in network_info]
+
+                for cidrv6 in cidrv6s:
+                    ipv6_rules.append('-s %s -j ACCEPT' % (cidrv6,))
 
         security_groups = db.security_group_get_by_instance(ctxt,
                                                             instance['id'])
@@ -1836,31 +1950,3 @@
 
     def _instance_chain_name(self, instance):
         return 'inst-%s' % (instance['id'],)
-
-    def _ip_for_instance(self, instance):
-        return db.instance_get_fixed_address(context.get_admin_context(),
-                                             instance['id'])
-
-    def _ip_for_instance_v6(self, instance):
-        return db.instance_get_fixed_address_v6(context.get_admin_context(),
-                                             instance['id'])
-
-    def _dhcp_server_for_instance(self, instance):
-        network = db.network_get_by_instance(context.get_admin_context(),
-                                             instance['id'])
-        return network['gateway']
-
-    def _gateway_v6_for_instance(self, instance):
-        network = db.network_get_by_instance(context.get_admin_context(),
-                                             instance['id'])
-        return network['gateway_v6']
-
-    def _project_cidr_for_instance(self, instance):
-        network = db.network_get_by_instance(context.get_admin_context(),
-                                             instance['id'])
-        return network['cidr']
-
-    def _project_cidrv6_for_instance(self, instance):
-        network = db.network_get_by_instance(context.get_admin_context(),
-                                             instance['id'])
-        return network['cidr_v6']