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

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

813. By Eldar Nugaev on 2011-03-18

merge with trunk

814. By Eldar Nugaev on 2011-03-21

merge with trunk

815. By Eldar Nugaev on 2011-03-22

merge with trunk

[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

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

816. By Ilya Alekseyev on 2011-03-23

review comments fixed

817. By Ilya Alekseyev on 2011-03-23

trunk merged

818. By Ilya Alekseyev on 2011-03-23

pep8 fixed

819. By Ilya Alekseyev on 2011-03-23

xml template fixed

820. By Ilya Alekseyev on 2011-03-23

one more minor fix

[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

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

826. By Ilya Alekseyev on 2011-03-24

style and spacing fixed

Unmerged revisions

=== modified file 'nova/virt/interfaces.template'
--- nova/virt/interfaces.template	2011-03-02 01:12:47 +0000
+++ nova/virt/interfaces.template	2011-03-23 19:11:11 +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-23 19:11:11 +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-23 05:29:32 +0000
+++ nova/virt/libvirt_conn.py	2011-03-23 19:11:11 +0000
@@ -153,6 +153,49 @@
         return int(net.version())
 
 
+def _get_network_info(instance):
+    #TODO(ilyaalekseyev) 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],
+            'ip6s': [ip6_dict(ip) for ip in network_ips]}
+
+        network_info.append((network, mapping))
+    return network_info
+
+
 class LibvirtConnection(object):
 
     def __init__(self, read_only):
@@ -416,16 +459,18 @@
         #             the normal xml file, we can just call reboot here
         self.reboot(instance)
 
+    # 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)
@@ -582,7 +627,11 @@
         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)
+
         # syntactic nicety
         def basepath(fname='', suffix=suffix):
             return os.path.join(FLAGS.instances_path,
@@ -658,28 +707,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
@@ -701,20 +757,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, instance_id, 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:
@@ -739,6 +786,41 @@
                               (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):
+        admin_context = context.get_admin_context()
+
+        # TODO(termie): cache?
+        LOG.debug(_('instance %s: starting toXML method'), instance['name'])
+
+        #TODO(ilyaalekseyev) 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(instance['id'],
+                                              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:
@@ -750,17 +832,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"
@@ -773,7 +849,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):
@@ -1274,7 +1349,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."""
@@ -1308,7 +1383,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
@@ -1322,6 +1397,11 @@
                                              instance['id'])
         return network['gateway_v6']
 
+    def _all_gateway_v6_for_instance(self, instance):
+        networks = db.network_get_all_by_instance(context.get_admin_context(),
+                                                  instance['id'])
+        return [network['gateway_v6'] for network in networks]
+
 
 class NWFilterFirewall(FirewallDriver):
     """
@@ -1413,7 +1493,7 @@
                               </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')
 
@@ -1518,7 +1598,7 @@
         # 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
@@ -1536,8 +1616,8 @@
                                              'nova-base-ipv6',
                                              'nova-allow-dhcp-server']
         if FLAGS.use_ipv6:
-            gateway_v6 = self._gateway_v6_for_instance(instance)
-            if gateway_v6:
+            gateways_v6 = self._all_gateway_v6_for_instance(instance)
+            if gateways_v6:
                 instance_secgroup_filter_children += ['nova-allow-ra-server']
 
         ctxt = context.get_admin_context()
@@ -1623,9 +1703,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"""
@@ -1639,29 +1721,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)
@@ -1677,7 +1770,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 = []
@@ -1691,28 +1786,35 @@
         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 = self._all_gateway_v6_for_instance(instance)
+            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'])