From: Jakub Libosvar <libosvar@redhat.com>
Date: Tue, 5 May 2015 12:32:21 +0000 (+0200)
Subject: Refactor initialize() of sriov mech driver
X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=750ae6979d920007dc87701cb69db82d72f99fd7;p=openstack-build%2Fneutron-build.git

Refactor initialize() of sriov mech driver

This patch rewrites checking correctness of supported_pci_vendor_devs
config value from C-style to Python-style. Patch also adds some tests
for wrong values passed.

Change-Id: I90855d665ab8d42c4dd26b91d2e8b63feef122f4
---

diff --git a/neutron/plugins/ml2/drivers/mech_sriov/mech_driver.py b/neutron/plugins/ml2/drivers/mech_sriov/mech_driver.py
index d70edf22f..56aa810a5 100644
--- a/neutron/plugins/ml2/drivers/mech_sriov/mech_driver.py
+++ b/neutron/plugins/ml2/drivers/mech_sriov/mech_driver.py
@@ -83,8 +83,8 @@ class SriovNicSwitchMechanismDriver(api.MechanismDriver):
 
     def initialize(self):
         try:
-            self.pci_vendor_info = self._parse_pci_vendor_config(
-                        cfg.CONF.ml2_sriov.supported_pci_vendor_devs)
+            self.pci_vendor_info = cfg.CONF.ml2_sriov.supported_pci_vendor_devs
+            self._check_pci_vendor_config(self.pci_vendor_info)
             self.agent_required = cfg.CONF.ml2_sriov.agent_required
         except ValueError:
             LOG.exception(_LE("Failed to parse supported PCI vendor devices"))
@@ -175,20 +175,14 @@ class SriovNicSwitchMechanismDriver(api.MechanismDriver):
         vif_details[portbindings.VIF_DETAILS_VLAN] = str(vlan_id)
         return vif_details
 
-    def _parse_pci_vendor_config(self, pci_vendor_list):
-        parsed_list = []
-        for elem in pci_vendor_list:
-            elem = elem.strip()
-            if not elem:
-                continue
-            split_result = elem.split(':')
-            if len(split_result) != 2:
-                raise ValueError(_("Invalid pci_vendor_info: '%s'") % elem)
-            vendor_id = split_result[0].strip()
-            if not vendor_id:
-                raise ValueError(_("Missing vendor_id in: '%s'") % elem)
-            product_id = split_result[1].strip()
-            if not product_id:
-                raise ValueError(_("Missing product_id in: '%s'") % elem)
-            parsed_list.append(elem)
-        return parsed_list
+    @staticmethod
+    def _check_pci_vendor_config(pci_vendor_list):
+        for pci_vendor_info in pci_vendor_list:
+            try:
+                vendor_id, product_id = [
+                    item.strip() for item in pci_vendor_info.split(':')
+                    if item.strip()]
+            except ValueError:
+                raise ValueError(_('Incorrect pci_vendor_info: "%s", should be'
+                                   ' pair vendor_id:product_id') %
+                                 pci_vendor_info)
diff --git a/neutron/tests/unit/plugins/ml2/drivers/mech_sriov/test_mech_sriov_nic_switch.py b/neutron/tests/unit/plugins/ml2/drivers/mech_sriov/test_mech_sriov_nic_switch.py
index 5a866c7c4..8a94920cf 100644
--- a/neutron/tests/unit/plugins/ml2/drivers/mech_sriov/test_mech_sriov_nic_switch.py
+++ b/neutron/tests/unit/plugins/ml2/drivers/mech_sriov/test_mech_sriov_nic_switch.py
@@ -248,5 +248,21 @@ class SriovSwitchMechConfigTestCase(SriovNicSwitchMechanismBaseTestCase):
                          self.driver.pci_vendor_info)
 
     def test_pci_vendor_config_wrong_entry(self):
-        self._set_config('wrong_entry')
+        self._set_config(['wrong_entry'])
+        self.assertRaises(cfg.Error, self.driver.initialize)
+
+    def test_initialize_missing_product_id(self):
+        self._set_config(['vendor_id:'])
+        self.assertRaises(cfg.Error, self.driver.initialize)
+
+    def test_initialize_missing_vendor_id(self):
+        self._set_config([':product_id'])
+        self.assertRaises(cfg.Error, self.driver.initialize)
+
+    def test_initialize_multiple_colons(self):
+        self._set_config(['foo:bar:baz'])
+        self.assertRaises(cfg.Error, self.driver.initialize)
+
+    def test_initialize_empty_string(self):
+        self._set_config([''])
         self.assertRaises(cfg.Error, self.driver.initialize)