]> review.fuel-infra Code Review - openstack-build/neutron-build.git/commitdiff
Refactor initialize() of sriov mech driver
authorJakub Libosvar <libosvar@redhat.com>
Tue, 5 May 2015 12:32:21 +0000 (14:32 +0200)
committerJakub Libosvar <libosvar@redhat.com>
Thu, 14 May 2015 15:56:16 +0000 (17:56 +0200)
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

neutron/plugins/ml2/drivers/mech_sriov/mech_driver.py
neutron/tests/unit/plugins/ml2/drivers/mech_sriov/test_mech_sriov_nic_switch.py

index d70edf22fd1cb7d09c99a3bdf0a12deffaf7b7b5..56aa810a5d6d37f5f9c3113916988b44d2fe30d0 100644 (file)
@@ -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)
index 5a866c7c4b702b83925a316aa831d1de0c22ee41..8a94920cf0fcb9a11fab73fdffb28998efddf28c 100644 (file)
@@ -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)