NSB: Fix standalone.model.Libvirt SR-IOV modeling 37/49037/2
authorDino Madarang <dinox.madarang@intel.com>
Wed, 1 Nov 2017 16:10:16 +0000 (09:10 -0700)
committerRoss Brattain <ross.b.brattain@intel.com>
Fri, 15 Dec 2017 06:34:20 +0000 (06:34 +0000)
Fixed standalone.model.Libvirt SR-IOV XML interface modeling, acording
to [1]:
- All PCI attributes now are printed in hexadecimal format.
- The PCI address is now added in the correct section, 'interface'.

network_services.utils.PciAddress was refactored to accept both 'domain:
bus:slot:function' and 'bus:slot:function' format inputs. This class is
used as input in the previous class, Libvirt, to print in XML the PCI
address of a SR-IOV interface.

network_services.utils.PciAddress.parse_address is now deprecated. Instead
the class standard instantiation must be used:

    libvirt_obj = utils.PciAddress(text_with_address)

A deprecation decorator is implemented along with this patch. This
decorator is used for the first time in the previously mentioned function.
This decorator stores every decorated function name and deprecation message
and raises a logging warning message the first time this function is used.

[1] https://goo.gl/so2Mrp

Change-Id: I22e95c488e27d6e2a8fdf6c1a07faab275fa6bba
Signed-off-by: Dino Simeon Madarang <dinox.madarang@intel.com>
Reviewed-by: Alain Jebara <alain.jebara@intel.com>
Reviewed-by: Deepak S <deepak.s@linux.intel.com>
Reviewed-by: Ross Brattain <ross.b.brattain@intel.com>
Reviewed-by: Rodolfo Alonso Hernandez <rodolfo.alonso.hernandez@intel.com>
(cherry picked from commit 6cfec77db6b95af5b31b741d513955ee3dfa3bb2)

tests/unit/benchmark/contexts/standalone/test_model.py
tests/unit/benchmark/contexts/standalone/test_sriov.py
tests/unit/network_services/test_utils.py
yardstick/benchmark/contexts/standalone/model.py
yardstick/benchmark/contexts/standalone/ovs_dpdk.py
yardstick/benchmark/contexts/standalone/sriov.py
yardstick/network_services/utils.py

index 6899a0a..31ec2b7 100644 (file)
 
 # Unittest for yardstick.benchmark.contexts.standalone.model
 
-
 from __future__ import absolute_import
+import copy
 import os
 import unittest
-import errno
 import mock
 
-from yardstick.common import constants as consts
-from yardstick.benchmark.contexts.standalone.model import Libvirt
-from yardstick.benchmark.contexts.standalone.model import StandaloneContextHelper
-from yardstick.benchmark.contexts.standalone import model
-from yardstick.network_services.utils import PciAddress
+from xml.etree import ElementTree
 
+from yardstick.benchmark.contexts.standalone import model
+from yardstick.network_services import utils
+from yardstick.network_services.helpers import cpu
+
+
+XML_SAMPLE = """<?xml version="1.0"?>
+<domain type="kvm">
+    <devices>
+    </devices>
+</domain>
+"""
+
+XML_SAMPLE_INTERFACE = """<?xml version="1.0"?>
+<domain type="kvm">
+    <devices>
+        <interface>
+        </interface>
+    </devices>
+</domain>
+"""
 
 class ModelLibvirtTestCase(unittest.TestCase):
 
+    def setUp(self):
+        self.xml = ElementTree.ElementTree(
+            element=ElementTree.fromstring(XML_SAMPLE))
+        self.pci_address_str = '0001:04:03.2'
+        self.pci_address = utils.PciAddress(self.pci_address_str)
+        self.mac = '00:00:00:00:00:01'
+        self._mock_write_xml = mock.patch.object(ElementTree.ElementTree,
+                                                 'write')
+        self.mock_write_xml = self._mock_write_xml.start()
+
+        self.addCleanup(self._cleanup)
+
+    def _cleanup(self):
+        self._mock_write_xml.stop()
+
     def test_check_if_vm_exists_and_delete(self):
         with mock.patch("yardstick.ssh.SSH") as ssh:
             ssh_mock = mock.Mock(autospec=ssh.SSH)
-            ssh_mock.execute = \
-                mock.Mock(return_value=(0, "a", ""))
+            ssh_mock.execute = mock.Mock(return_value=(0, "a", ""))
             ssh.return_value = ssh_mock
-        result = Libvirt.check_if_vm_exists_and_delete("vm_0", ssh_mock)
-        self.assertIsNone(result)
+        # NOTE(ralonsoh): this test doesn't cover function execution.
+        model.Libvirt.check_if_vm_exists_and_delete("vm_0", ssh_mock)
 
     def test_virsh_create_vm(self):
         with mock.patch("yardstick.ssh.SSH") as ssh:
             ssh_mock = mock.Mock(autospec=ssh.SSH)
-            ssh_mock.execute = \
-                mock.Mock(return_value=(0, "a", ""))
+            ssh_mock.execute = mock.Mock(return_value=(0, "a", ""))
             ssh.return_value = ssh_mock
-        result = Libvirt.virsh_create_vm(ssh_mock, "vm_0")
-        self.assertIsNone(result)
+        # NOTE(ralonsoh): this test doesn't cover function execution.
+        model.Libvirt.virsh_create_vm(ssh_mock, "vm_0")
 
     def test_virsh_destroy_vm(self):
         with mock.patch("yardstick.ssh.SSH") as ssh:
             ssh_mock = mock.Mock(autospec=ssh.SSH)
-            ssh_mock.execute = \
-                mock.Mock(return_value=(0, "a", ""))
+            ssh_mock.execute = mock.Mock(return_value=(0, "a", ""))
             ssh.return_value = ssh_mock
-        result = Libvirt.virsh_destroy_vm("vm_0", ssh_mock)
-        self.assertIsNone(result)
-
-    @mock.patch('yardstick.benchmark.contexts.standalone.model.ET')
-    def test_add_interface_address(self, mock_et):
-        pci_address = PciAddress.parse_address("0000:00:04.0", multi_line=True)
-        result = Libvirt.add_interface_address("<interface/>", pci_address)
-        self.assertIsNotNone(result)
-
-    @mock.patch('yardstick.benchmark.contexts.standalone.model.Libvirt.add_interface_address')
-    @mock.patch('yardstick.benchmark.contexts.standalone.model.ET')
-    def test_add_ovs_interfaces(self, mock_et, mock_add_interface_address):
-        pci_address = PciAddress.parse_address("0000:00:04.0", multi_line=True)
-        result = Libvirt.add_ovs_interface("/usr/local", 0, "0000:00:04.0",
-                                                "00:00:00:00:00:01", "xml")
-        self.assertIsNone(result)
-
-    @mock.patch('yardstick.benchmark.contexts.standalone.model.Libvirt.add_interface_address')
-    @mock.patch('yardstick.benchmark.contexts.standalone.model.ET')
-    def test_add_sriov_interfaces(self, mock_et, mock_add_interface_address):
-        pci_address = PciAddress.parse_address("0000:00:04.0", multi_line=True)
-        result = Libvirt.add_sriov_interfaces("0000:00:05.0", "0000:00:04.0",
-                                              "00:00:00:00:00:01", "xml")
-        self.assertIsNone(result)
+        # NOTE(ralonsoh): this test doesn't cover function execution.
+        model.Libvirt.virsh_destroy_vm("vm_0", ssh_mock)
+
+    def test_add_interface_address(self):
+        xml = ElementTree.ElementTree(
+            element=ElementTree.fromstring(XML_SAMPLE_INTERFACE))
+        interface = xml.find('devices').find('interface')
+        result = model.Libvirt._add_interface_address(interface, self.pci_address)
+        self.assertEqual('pci', result.get('type'))
+        self.assertEqual('0x{}'.format(self.pci_address.domain),
+                         result.get('domain'))
+        self.assertEqual('0x{}'.format(self.pci_address.bus),
+                         result.get('bus'))
+        self.assertEqual('0x{}'.format(self.pci_address.slot),
+                         result.get('slot'))
+        self.assertEqual('0x{}'.format(self.pci_address.function),
+                         result.get('function'))
+
+    def test_add_ovs_interfaces(self):
+        xml_input = mock.Mock()
+        with mock.patch.object(ElementTree, 'parse', return_value=self.xml) \
+                as mock_parse:
+            xml = copy.deepcopy(self.xml)
+            mock_parse.return_value = xml
+            model.Libvirt.add_ovs_interface(
+                '/usr/local', 0, self.pci_address_str, self.mac, xml_input)
+            mock_parse.assert_called_once_with(xml_input)
+            self.mock_write_xml.assert_called_once_with(xml_input)
+            interface = xml.find('devices').find('interface')
+            self.assertEqual('vhostuser', interface.get('type'))
+            mac = interface.find('mac')
+            self.assertEqual(self.mac, mac.get('address'))
+            source = interface.find('source')
+            self.assertEqual('unix', source.get('type'))
+            self.assertEqual('/usr/local/var/run/openvswitch/dpdkvhostuser0',
+                             source.get('path'))
+            self.assertEqual('client', source.get('mode'))
+            _model = interface.find('model')
+            self.assertEqual('virtio', _model.get('type'))
+            driver = interface.find('driver')
+            self.assertEqual('4', driver.get('queues'))
+            host = driver.find('host')
+            self.assertEqual('off', host.get('mrg_rxbuf'))
+            self.assertIsNotNone(interface.find('address'))
+
+    def test_add_sriov_interfaces(self):
+        xml_input = mock.Mock()
+        with mock.patch.object(ElementTree, 'parse', return_value=self.xml) \
+                as mock_parse:
+            xml = copy.deepcopy(self.xml)
+            mock_parse.return_value = xml
+            vf_pci = '0001:05:04.2'
+            model.Libvirt.add_sriov_interfaces(
+                self.pci_address_str, vf_pci, self.mac, xml_input)
+            mock_parse.assert_called_once_with(xml_input)
+            self.mock_write_xml.assert_called_once_with(xml_input)
+            interface = xml.find('devices').find('interface')
+            self.assertEqual('yes', interface.get('managed'))
+            self.assertEqual('hostdev', interface.get('type'))
+            mac = interface.find('mac')
+            self.assertEqual(self.mac, mac.get('address'))
+            source = interface.find('source')
+            self.assertIsNotNone(source.find('address'))
+            self.assertIsNotNone(interface.find('address'))
 
     def test_create_snapshot_qemu(self):
         result = "/var/lib/libvirt/images/0.qcow2"
@@ -88,15 +155,15 @@ class ModelLibvirtTestCase(unittest.TestCase):
             ssh_mock.execute = \
                 mock.Mock(return_value=(0, "a", ""))
             ssh.return_value = ssh_mock
-        image = Libvirt.create_snapshot_qemu(ssh_mock, "0", "ubuntu.img")
+        image = model.Libvirt.create_snapshot_qemu(ssh_mock, "0", "ubuntu.img")
         self.assertEqual(image, result)
 
-    @mock.patch("yardstick.benchmark.contexts.standalone.model.Libvirt.pin_vcpu_for_perf")
-    @mock.patch("yardstick.benchmark.contexts.standalone.model.Libvirt.create_snapshot_qemu")
-    @mock.patch('yardstick.benchmark.contexts.standalone.model.open')
-    @mock.patch('yardstick.benchmark.contexts.standalone.model.write_file')
-    def test_build_vm_xml(self, mock_open, mock_write_file, mock_create_snapshot_qemu,
-                          mock_pin_vcpu_for_perf):
+    @mock.patch.object(model.Libvirt, 'pin_vcpu_for_perf')
+    @mock.patch.object(model.Libvirt, 'create_snapshot_qemu')
+    def test_build_vm_xml(self, mock_create_snapshot_qemu,
+                          *args):
+        # NOTE(ralonsoh): this test doesn't cover function execution. This test
+        # should also check mocked function calls.
         result = [4]
         with mock.patch("yardstick.ssh.SSH") as ssh:
             ssh_mock = mock.Mock(autospec=ssh.SSH)
@@ -105,7 +172,7 @@ class ModelLibvirtTestCase(unittest.TestCase):
             ssh.return_value = ssh_mock
         mock_create_snapshot_qemu.return_value = "0.img"
 
-        status = Libvirt.build_vm_xml(ssh_mock, {}, "test", "vm_0", 0)
+        status = model.Libvirt.build_vm_xml(ssh_mock, {}, "test", "vm_0", 0)
         self.assertEqual(status[0], result[0])
 
     def test_update_interrupts_hugepages_perf(self):
@@ -114,19 +181,21 @@ class ModelLibvirtTestCase(unittest.TestCase):
             ssh_mock.execute = \
                 mock.Mock(return_value=(0, "a", ""))
             ssh.return_value = ssh_mock
-        status = Libvirt.update_interrupts_hugepages_perf(ssh_mock)
-        self.assertIsNone(status)
+        # NOTE(ralonsoh): this test doesn't cover function execution. This test
+        # should also check mocked function calls.
+        model.Libvirt.update_interrupts_hugepages_perf(ssh_mock)
+
+    @mock.patch.object(cpu.CpuSysCores, 'get_core_socket')
+    def test_pin_vcpu_for_perf(self, mock_get_core_socket):
+        mock_get_core_socket.return_value = {
+            'cores_per_socket': 1,
+            'thread_per_core': 1,
+            '0': [1, 2]
+        }
+        # NOTE(ralonsoh): this test doesn't cover function execution. This
+        # function needs more tests.
+        model.Libvirt.pin_vcpu_for_perf(mock.Mock())
 
-    @mock.patch("yardstick.benchmark.contexts.standalone.model.CpuSysCores")
-    @mock.patch("yardstick.benchmark.contexts.standalone.model.Libvirt.update_interrupts_hugepages_perf")
-    def test_pin_vcpu_for_perf(self, mock_update_interrupts_hugepages_perf, mock_CpuSysCores):
-        with mock.patch("yardstick.ssh.SSH") as ssh:
-            ssh_mock = mock.Mock(autospec=ssh.SSH)
-            ssh_mock.execute = \
-                mock.Mock(return_value=(0, "a", ""))
-            ssh.return_value = ssh_mock
-        status = Libvirt.pin_vcpu_for_perf(ssh_mock, "vm_0", 4)
-        self.assertIsNotNone(status)
 
 class StandaloneContextHelperTestCase(unittest.TestCase):
 
@@ -148,7 +217,7 @@ class StandaloneContextHelperTestCase(unittest.TestCase):
     }
 
     def setUp(self):
-        self.helper = StandaloneContextHelper()
+        self.helper = model.StandaloneContextHelper()
 
     def test___init__(self):
         self.assertIsNone(self.helper.file_path)
@@ -159,8 +228,9 @@ class StandaloneContextHelperTestCase(unittest.TestCase):
             ssh_mock.execute = \
                 mock.Mock(return_value=(1, "a", ""))
             ssh.return_value = ssh_mock
-        status = StandaloneContextHelper.install_req_libs(ssh_mock)
-        self.assertIsNone(status)
+        # NOTE(ralonsoh): this test doesn't cover function execution. This test
+        # should also check mocked function calls.
+        model.StandaloneContextHelper.install_req_libs(ssh_mock)
 
     def test_get_kernel_module(self):
         with mock.patch("yardstick.ssh.SSH") as ssh:
@@ -168,10 +238,12 @@ class StandaloneContextHelperTestCase(unittest.TestCase):
             ssh_mock.execute = \
                 mock.Mock(return_value=(1, "i40e", ""))
             ssh.return_value = ssh_mock
-        status = StandaloneContextHelper.get_kernel_module(ssh_mock, "05:00.0", None)
-        self.assertEqual(status, "i40e")
+        # NOTE(ralonsoh): this test doesn't cover function execution. This test
+        # should also check mocked function calls.
+        model.StandaloneContextHelper.get_kernel_module(
+            ssh_mock, "05:00.0", None)
 
-    @mock.patch('yardstick.benchmark.contexts.standalone.model.StandaloneContextHelper.get_kernel_module')
+    @mock.patch.object(model.StandaloneContextHelper, 'get_kernel_module')
     def test_get_nic_details(self, mock_get_kernel_module):
         with mock.patch("yardstick.ssh.SSH") as ssh:
             ssh_mock = mock.Mock(autospec=ssh.SSH)
@@ -179,8 +251,10 @@ class StandaloneContextHelperTestCase(unittest.TestCase):
                 mock.Mock(return_value=(1, "i40e ixgbe", ""))
             ssh.return_value = ssh_mock
         mock_get_kernel_module.return_value = "i40e"
-        status = StandaloneContextHelper.get_nic_details(ssh_mock, self.NETWORKS, "dpdk-devbind.py")
-        self.assertIsNotNone(status)
+        # NOTE(ralonsoh): this test doesn't cover function execution. This test
+        # should also check mocked function calls.
+        model.StandaloneContextHelper.get_nic_details(
+            ssh_mock, self.NETWORKS, 'dpdk-devbind.py')
 
     def test_get_virtual_devices(self):
         pattern = "PCI_SLOT_NAME=0000:05:00.0"
@@ -189,8 +263,10 @@ class StandaloneContextHelperTestCase(unittest.TestCase):
             ssh_mock.execute = \
                     mock.Mock(return_value=(1, pattern, ""))
             ssh.return_value = ssh_mock
-        status = StandaloneContextHelper.get_virtual_devices(ssh_mock, "0000:00:05.0")
-        self.assertIsNotNone(status)
+        # NOTE(ralonsoh): this test doesn't cover function execution. This test
+        # should also check mocked function calls.
+        model.StandaloneContextHelper.get_virtual_devices(
+            ssh_mock, '0000:00:05.0')
 
     def _get_file_abspath(self, filename):
         curr_path = os.path.dirname(os.path.abspath(__file__))
@@ -204,40 +280,48 @@ class StandaloneContextHelperTestCase(unittest.TestCase):
 
     def test_parse_pod_file(self):
         self.helper.file_path = self._get_file_abspath("dummy")
-        self.assertRaises(IOError, self.helper.parse_pod_file, self.helper.file_path)
+        self.assertRaises(IOError, self.helper.parse_pod_file,
+                          self.helper.file_path)
 
         self.helper.file_path = self._get_file_abspath(self.NODE_SAMPLE)
-        self.assertRaises(TypeError, self.helper.parse_pod_file, self.helper.file_path)
+        self.assertRaises(TypeError, self.helper.parse_pod_file,
+                          self.helper.file_path)
 
         self.helper.file_path = self._get_file_abspath(self.NODE_SRIOV_SAMPLE)
         self.assertIsNotNone(self.helper.parse_pod_file(self.helper.file_path))
 
     def test_get_mac_address(self):
-        status = StandaloneContextHelper.get_mac_address()
+        status = model.StandaloneContextHelper.get_mac_address()
         self.assertIsNotNone(status)
 
     @mock.patch('yardstick.ssh.SSH')
-    def test_get_mgmt_ip(self, mock_ssh):
+    def test_get_mgmt_ip(self, *args):
         with mock.patch("yardstick.ssh.SSH") as ssh:
             ssh_mock = mock.Mock(autospec=ssh.SSH)
-            ssh_mock.execute = \
-                    mock.Mock(return_value=(1, "1.2.3.4 00:00:00:00:00:01", ""))
+            ssh_mock.execute = mock.Mock(
+                return_value=(1, "1.2.3.4 00:00:00:00:00:01", ""))
             ssh.return_value = ssh_mock
-        status = StandaloneContextHelper.get_mgmt_ip(ssh_mock, "00:00:00:00:00:01", "1.1.1.1/24", {})
+        # NOTE(ralonsoh): this test doesn't cover function execution. This test
+        # should also check mocked function calls.
+        status = model.StandaloneContextHelper.get_mgmt_ip(
+            ssh_mock, "00:00:00:00:00:01", "1.1.1.1/24", {})
         self.assertIsNotNone(status)
 
     @mock.patch('yardstick.ssh.SSH')
-    def test_get_mgmt_ip_no(self, mock_ssh):
+    def test_get_mgmt_ip_no(self, *args):
         with mock.patch("yardstick.ssh.SSH") as ssh:
             ssh_mock = mock.Mock(autospec=ssh.SSH)
             ssh_mock.execute = \
                     mock.Mock(return_value=(1, "", ""))
             ssh.return_value = ssh_mock
-
+        # NOTE(ralonsoh): this test doesn't cover function execution. This test
+        # should also check mocked function calls.
         model.WAIT_FOR_BOOT = 0
-        status = StandaloneContextHelper.get_mgmt_ip(ssh_mock, "99", "1.1.1.1/24", {})
+        status = model.StandaloneContextHelper.get_mgmt_ip(
+            ssh_mock, "99", "1.1.1.1/24", {})
         self.assertIsNone(status)
 
+
 class ServerTestCase(unittest.TestCase):
 
     NETWORKS = {
@@ -257,6 +341,7 @@ class ServerTestCase(unittest.TestCase):
          'cidr': '152.16.40.10/24',
          'gateway_ip': '152.16.100.20'}
     }
+
     def setUp(self):
         self.server = model.Server()
 
@@ -282,7 +367,8 @@ class ServerTestCase(unittest.TestCase):
                 'xe1': ['public_0'],
             }
         }
-        status = self.server.generate_vnf_instance({}, self.NETWORKS, "1.1.1.1/24", 'vm_0', vnf, '00:00:00:00:00:01')
+        status = self.server.generate_vnf_instance(
+            {}, self.NETWORKS, '1.1.1.1/24', 'vm_0', vnf, '00:00:00:00:00:01')
         self.assertIsNotNone(status)
 
 class OvsDeployTestCase(unittest.TestCase):
@@ -312,14 +398,17 @@ class OvsDeployTestCase(unittest.TestCase):
         self.assertIsNotNone(self.ovs_deploy.connection)
 
     @mock.patch('yardstick.benchmark.contexts.standalone.model.os')
-    def test_prerequisite(self, mock_ssh):
+    def test_prerequisite(self, *args):
+        # NOTE(ralonsoh): this test should check mocked function calls.
         self.ovs_deploy.helper = mock.Mock()
         self.assertIsNone(self.ovs_deploy.prerequisite())
 
     @mock.patch('yardstick.benchmark.contexts.standalone.model.os')
-    def test_prerequisite(self, mock_ssh):
+    def test_prerequisite_2(self, *args):
+        # NOTE(ralonsoh): this test should check mocked function calls. Rename
+        # this test properly.
         self.ovs_deploy.helper = mock.Mock()
-        self.ovs_deploy.connection.execute = \
-                    mock.Mock(return_value=(1, "1.2.3.4 00:00:00:00:00:01", ""))
+        self.ovs_deploy.connection.execute = mock.Mock(
+            return_value=(1, '1.2.3.4 00:00:00:00:00:01', ''))
         self.ovs_deploy.prerequisite = mock.Mock()
         self.assertIsNone(self.ovs_deploy.ovs_deploy())
index 50ae5fe..3ea673a 100644 (file)
 from __future__ import absolute_import
 import os
 import unittest
-import errno
 import mock
 
-from yardstick.common import constants as consts
+from yardstick import ssh
 from yardstick.benchmark.contexts.standalone import sriov
-from yardstick.network_services.utils import PciAddress
 
 
 class SriovContextTestCase(unittest.TestCase):
@@ -66,6 +64,9 @@ class SriovContextTestCase(unittest.TestCase):
     @mock.patch('yardstick.benchmark.contexts.standalone.sriov.Libvirt')
     @mock.patch('yardstick.benchmark.contexts.standalone.model.Server')
     def test___init__(self, mock_helper, mock_libvirt, mock_server):
+        # pylint: disable=unused-argument
+        # NOTE(ralonsoh): this test doesn't cover function execution.
+        # The pylint exception should be removed.
         self.sriov.helper = mock_helper
         self.sriov.vnf_node = mock_server
         self.assertIsNone(self.sriov.file_path)
@@ -75,14 +76,11 @@ class SriovContextTestCase(unittest.TestCase):
         self.sriov.helper.parse_pod_file = mock.Mock(return_value=[{}, {}, {}])
         self.assertIsNone(self.sriov.init(self.ATTRS))
 
-    @mock.patch('yardstick.ssh.SSH')
+    @mock.patch.object(ssh, 'SSH', return_value=(0, "a", ""))
     def test_deploy(self, mock_ssh):
-        with mock.patch("yardstick.ssh.SSH") as ssh:
-            ssh_mock = mock.Mock(autospec=ssh.SSH)
-            ssh_mock.execute = \
-                mock.Mock(return_value=(0, "a", ""))
-            ssh.return_value = ssh_mock
-
+        # pylint: disable=unused-argument
+        # NOTE(ralonsoh): this test doesn't cover function execution.
+        # The pylint exception should be removed.
         self.sriov.vm_deploy = False
         self.assertIsNone(self.sriov.deploy())
 
@@ -94,20 +92,16 @@ class SriovContextTestCase(unittest.TestCase):
         self.sriov.wait_for_vnfs_to_start = mock.Mock(return_value={})
         self.assertIsNone(self.sriov.deploy())
 
-    @mock.patch('yardstick.ssh.SSH')
+    @mock.patch.object(ssh, 'SSH', return_value=(0, "a", ""))
     @mock.patch('yardstick.benchmark.contexts.standalone.sriov.Libvirt')
     def test_undeploy(self, mock_libvirt, mock_ssh):
-        with mock.patch("yardstick.ssh.SSH") as ssh:
-            ssh_mock = mock.Mock(autospec=ssh.SSH)
-            ssh_mock.execute = \
-                mock.Mock(return_value=(0, "a", ""))
-            ssh.return_value = ssh_mock
-
+        # pylint: disable=unused-argument
+        # NOTE(ralonsoh): the pylint exception should be removed.
         self.sriov.vm_deploy = False
         self.assertIsNone(self.sriov.undeploy())
 
         self.sriov.vm_deploy = True
-        self.sriov.connection = ssh_mock
+        self.sriov.connection = mock_ssh
         self.sriov.vm_names = ['vm_0', 'vm_1']
         self.sriov.drivers = ['vm_0', 'vm_1']
         self.assertIsNone(self.sriov.undeploy())
@@ -246,27 +240,27 @@ class SriovContextTestCase(unittest.TestCase):
         self.sriov.drivers = []
         self.sriov.networks = self.NETWORKS
         self.sriov.helper.get_mac_address = mock.Mock(return_value="")
-        self.sriov.get_vf_data = mock.Mock(return_value="")
+        self.sriov._get_vf_data = mock.Mock(return_value="")
         self.assertIsNone(self.sriov.configure_nics_for_sriov())
 
+    @mock.patch.object(ssh, 'SSH', return_value=(0, "a", ""))
     @mock.patch('yardstick.benchmark.contexts.standalone.sriov.Libvirt')
-    def test__enable_interfaces(self, mock_libvirt):
-        with mock.patch("yardstick.ssh.SSH") as ssh:
-            ssh_mock = mock.Mock(autospec=ssh.SSH)
-            ssh_mock.execute = \
-                mock.Mock(return_value=(0, "a", ""))
-            ssh.return_value = ssh_mock
+    def test__enable_interfaces(self, mock_libvirt, mock_ssh):
+        # pylint: disable=unused-argument
+        # NOTE(ralonsoh): the pylint exception should be removed.
         self.sriov.vm_deploy = True
-        self.sriov.connection = ssh_mock
+        self.sriov.connection = mock_ssh
         self.sriov.vm_names = ['vm_0', 'vm_1']
         self.sriov.drivers = []
         self.sriov.networks = self.NETWORKS
-        self.sriov.get_vf_data = mock.Mock(return_value="")
+        self.sriov._get_vf_data = mock.Mock(return_value="")
         self.assertIsNone(self.sriov._enable_interfaces(0, 0, ["private_0"], 'test'))
 
     @mock.patch('yardstick.benchmark.contexts.standalone.model.Server')
     @mock.patch('yardstick.benchmark.contexts.standalone.sriov.Libvirt')
     def test_setup_sriov_context(self, mock_libvirt, mock_server):
+        # pylint: disable=unused-argument
+        # NOTE(ralonsoh): the pylint exception should be removed.
         with mock.patch("yardstick.ssh.SSH") as ssh:
             ssh_mock = mock.Mock(autospec=ssh.SSH)
             ssh_mock.execute = \
@@ -296,7 +290,7 @@ class SriovContextTestCase(unittest.TestCase):
         self.sriov.vnf_node.generate_vnf_instance = mock.Mock(return_value={})
         self.assertIsNotNone(self.sriov.setup_sriov_context())
 
-    def test_get_vf_data(self):
+    def test__get_vf_data(self):
         with mock.patch("yardstick.ssh.SSH") as ssh:
             ssh_mock = mock.Mock(autospec=ssh.SSH)
             ssh_mock.execute = \
@@ -318,5 +312,7 @@ class SriovContextTestCase(unittest.TestCase):
             }
         }
         self.sriov.networks = self.NETWORKS
-        self.sriov.helper.get_virtual_devices = mock.Mock(return_value={"0000:00:01.0": ""})
-        self.assertIsNotNone(self.sriov.get_vf_data("", "0000:00:01.0", "00:00:00:00:00:01", "if0"))
+        self.sriov.helper.get_virtual_devices = mock.Mock(
+            return_value={'0000:00:01.0': ''})
+        self.assertIsNotNone(self.sriov._get_vf_data(
+            '0000:00:01.0', '00:00:00:00:00:01', 'if0'))
index 993adbe..bf98a44 100644 (file)
@@ -15,8 +15,6 @@
 
 # Unittest for yardstick.network_services.utils
 
-from __future__ import absolute_import
-
 import os
 import unittest
 import mock
@@ -62,3 +60,84 @@ class UtilsTestCase(unittest.TestCase):
             ssh.return_value = ssh_mock
             tool_path = utils.provision_tool(ssh_mock, self.DPDK_PATH)
             self.assertEqual(tool_path, self.DPDK_PATH)
+
+
+class PciAddressTestCase(unittest.TestCase):
+
+    PCI_ADDRESS_DBSF = '000A:07:03.2'
+    PCI_ADDRESS_BSF = '06:02.1'
+    PCI_ADDRESS_DBSF_MULTILINE_1 = '0001:08:04.3\nother text\n'
+    PCI_ADDRESS_DBSF_MULTILINE_2 = 'first line\n   0001:08:04.3 \nother text\n'
+    # Will match and return the first address found.
+    PCI_ADDRESS_DBSF_MULTILINE_3 = '  0001:08:04.1  \n  05:03.1 \nother\n'
+    PCI_ADDRESS_BSF_MULTILINE_1 = 'first line\n   08:04.3 \n 0002:05:03.1\n'
+    BAD_INPUT_1 = 'no address found'
+    BAD_INPUT_2 = '001:08:04.1'
+    BAD_INPUT_3 = '08:4.1'
+
+    def test_pciaddress_dbsf(self):
+        pci_address = utils.PciAddress(PciAddressTestCase.PCI_ADDRESS_DBSF)
+        self.assertEqual('000a', pci_address.domain)
+        self.assertEqual('07', pci_address.bus)
+        self.assertEqual('03', pci_address.slot)
+        self.assertEqual('2', pci_address.function)
+
+    def test_pciaddress_bsf(self):
+        pci_address = utils.PciAddress(PciAddressTestCase.PCI_ADDRESS_BSF)
+        self.assertEqual('0000', pci_address.domain)
+        self.assertEqual('06', pci_address.bus)
+        self.assertEqual('02', pci_address.slot)
+        self.assertEqual('1', pci_address.function)
+
+    def test_pciaddress_dbsf_multiline_1(self):
+        pci_address = utils.PciAddress(
+            PciAddressTestCase.PCI_ADDRESS_DBSF_MULTILINE_1)
+        self.assertEqual('0001', pci_address.domain)
+        self.assertEqual('08', pci_address.bus)
+        self.assertEqual('04', pci_address.slot)
+        self.assertEqual('3', pci_address.function)
+
+    def test_pciaddress_dbsf_multiline_2(self):
+        pci_address = utils.PciAddress(
+            PciAddressTestCase.PCI_ADDRESS_DBSF_MULTILINE_2)
+        self.assertEqual('0001', pci_address.domain)
+        self.assertEqual('08', pci_address.bus)
+        self.assertEqual('04', pci_address.slot)
+        self.assertEqual('3', pci_address.function)
+
+    def test_pciaddress_dbsf_multiline_3(self):
+        pci_address = utils.PciAddress(
+            PciAddressTestCase.PCI_ADDRESS_DBSF_MULTILINE_3)
+        self.assertEqual('0001', pci_address.domain)
+        self.assertEqual('08', pci_address.bus)
+        self.assertEqual('04', pci_address.slot)
+        self.assertEqual('1', pci_address.function)
+
+    def test_pciaddress_bsf_multiline_1(self):
+        pci_address = utils.PciAddress(
+            PciAddressTestCase.PCI_ADDRESS_BSF_MULTILINE_1)
+        self.assertEqual('0000', pci_address.domain)
+        self.assertEqual('08', pci_address.bus)
+        self.assertEqual('04', pci_address.slot)
+        self.assertEqual('3', pci_address.function)
+
+    def test_pciaddress_bad_input_no_address(self):
+        with self.assertRaises(ValueError) as exception:
+            utils.PciAddress(PciAddressTestCase.BAD_INPUT_1)
+        self.assertEqual('Invalid PCI address: {}'.format(
+                PciAddressTestCase.BAD_INPUT_1), str(exception.exception))
+
+    def test_pciaddress_bad_input_dbsf_bad_formatted(self):
+        # In this test case, the domain has only 3 characters instead of 4.
+        pci_address = utils.PciAddress(
+            PciAddressTestCase.BAD_INPUT_2)
+        self.assertEqual('0000', pci_address.domain)
+        self.assertEqual('08', pci_address.bus)
+        self.assertEqual('04', pci_address.slot)
+        self.assertEqual('1', pci_address.function)
+
+    def test_pciaddress_bad_input_bsf_bad_formatted(self):
+        with self.assertRaises(ValueError) as exception:
+            utils.PciAddress(PciAddressTestCase.BAD_INPUT_3)
+        self.assertEqual('Invalid PCI address: {}'.format(
+                PciAddressTestCase.BAD_INPUT_3), str(exception.exception))
index 45ae3c1..a8943c3 100644 (file)
@@ -94,33 +94,63 @@ class Libvirt(object):
         cmd_template = "virsh list --name | grep -i %s"
         status = connection.execute(cmd_template % vm_name)[0]
         if status == 0:
-            LOG.info("VM '%s' is already present.. destroying" % vm_name)
+            LOG.info("VM '%s' is already present... destroying", vm_name)
             connection.execute("virsh destroy %s" % vm_name)
 
     @staticmethod
     def virsh_create_vm(connection, cfg):
         err = connection.execute("virsh create %s" % cfg)[0]
-        LOG.info("VM create status: %s" % (err))
+        LOG.info("VM create status: %s", err)
 
     @staticmethod
     def virsh_destroy_vm(vm_name, connection):
         connection.execute("virsh destroy %s" % vm_name)
 
     @staticmethod
-    def add_interface_address(interface, pci_address):
+    def _add_interface_address(interface, pci_address):
+        """Add a PCI 'address' XML node
+
+        <address type='pci' domain='0x0000' bus='0x00' slot='0x08'
+         function='0x0'/>
+
+        Refence: https://software.intel.com/en-us/articles/
+                 configure-sr-iov-network-virtual-functions-in-linux-kvm
+        """
         vm_pci = ET.SubElement(interface, 'address')
         vm_pci.set('type', 'pci')
-        vm_pci.set('domain', '0x%s' % pci_address.domain)
-        vm_pci.set('bus', '0x%s' % pci_address.bus)
-        vm_pci.set('slot', '0x%s' % pci_address.slot)
-        vm_pci.set('function', '0x%s' % pci_address.function)
+        vm_pci.set('domain', '0x{}'.format(pci_address.domain))
+        vm_pci.set('bus', '0x{}'.format(pci_address.bus))
+        vm_pci.set('slot', '0x{}'.format(pci_address.slot))
+        vm_pci.set('function', '0x{}'.format(pci_address.function))
         return vm_pci
 
     @classmethod
     def add_ovs_interface(cls, vpath, port_num, vpci, vports_mac, xml):
-        vhost_path = '{0}/var/run/openvswitch/dpdkvhostuser{1}'
+        """Add a DPDK OVS 'interface' XML node in 'devices' node
+
+        <devices>
+            <interface type='vhostuser'>
+                <mac address='00:00:00:00:00:01'/>
+                <source type='unix' path='/usr/local/var/run/openvswitch/
+                 dpdkvhostuser0' mode='client'/>
+                <model type='virtio'/>
+                <driver queues='4'>
+                    <host mrg_rxbuf='off'/>
+                </driver>
+                <address type='pci' domain='0x0000' bus='0x00' slot='0x03'
+                 function='0x0'/>
+            </interface>
+            ...
+        </devices>
+
+        Reference: http://docs.openvswitch.org/en/latest/topics/dpdk/
+                   vhost-user/
+        """
+
+        vhost_path = ('{0}/var/run/openvswitch/dpdkvhostuser{1}'.
+                      format(vpath, port_num))
         root = ET.parse(xml)
-        pci_address = PciAddress.parse_address(vpci.strip(), multi_line=True)
+        pci_address = PciAddress(vpci.strip())
         device = root.find('devices')
 
         interface = ET.SubElement(device, 'interface')
@@ -130,7 +160,7 @@ class Libvirt(object):
 
         source = ET.SubElement(interface, 'source')
         source.set('type', 'unix')
-        source.set('path', vhost_path.format(vpath, port_num))
+        source.set('path', vhost_path)
         source.set('mode', 'client')
 
         model = ET.SubElement(interface, 'model')
@@ -142,14 +172,35 @@ class Libvirt(object):
         host = ET.SubElement(driver, 'host')
         host.set('mrg_rxbuf', 'off')
 
-        cls.add_interface_address(interface, pci_address)
+        cls._add_interface_address(interface, pci_address)
 
         root.write(xml)
 
     @classmethod
-    def add_sriov_interfaces(cls, vm_pci, vf_pci, vfmac, xml):
+    def add_sriov_interfaces(cls, vm_pci, vf_pci, vf_mac, xml):
+        """Add a SR-IOV 'interface' XML node in 'devices' node
+
+        <devices>
+           <interface type='hostdev' managed='yes'>
+             <source>
+               <address type='pci' domain='0x0000' bus='0x00' slot='0x03'
+                function='0x0'/>
+             </source>
+             <mac address='52:54:00:6d:90:02'>
+             <address type='pci' domain='0x0000' bus='0x02' slot='0x04'
+              function='0x1'/>
+           </interface>
+           ...
+         </devices>
+
+        Reference: https://access.redhat.com/documentation/en-us/
+            red_hat_enterprise_linux/6/html/
+            virtualization_host_configuration_and_guest_installation_guide/
+            sect-virtualization_host_configuration_and_guest_installation_guide
+            -sr_iov-how_sr_iov_libvirt_works
+        """
+
         root = ET.parse(xml)
-        pci_address = PciAddress.parse_address(vf_pci.strip(), multi_line=True)
         device = root.find('devices')
 
         interface = ET.SubElement(device, 'interface')
@@ -157,18 +208,15 @@ class Libvirt(object):
         interface.set('type', 'hostdev')
 
         mac = ET.SubElement(interface, 'mac')
-        mac.set('address', vfmac)
-        source = ET.SubElement(interface, 'source')
+        mac.set('address', vf_mac)
 
-        addr = ET.SubElement(source, "address")
-        addr.set('domain', "0x0")
-        addr.set('bus', "{0}".format(pci_address.bus))
-        addr.set('function', "{0}".format(pci_address.function))
-        addr.set('slot', "0x{0}".format(pci_address.slot))
-        addr.set('type', "pci")
+        source = ET.SubElement(interface, 'source')
+        addr = ET.SubElement(source, 'address')
+        pci_address = PciAddress(vf_pci.strip())
+        cls._add_interface_address(addr, pci_address)
 
-        pci_vm_address = PciAddress.parse_address(vm_pci.strip(), multi_line=True)
-        cls.add_interface_address(interface, pci_vm_address)
+        pci_vm_address = PciAddress(vm_pci.strip())
+        cls._add_interface_address(interface, pci_vm_address)
 
         root.write(xml)
 
@@ -192,7 +240,7 @@ class Libvirt(object):
         vcpu = int(cpu) * int(threads)
         numa_cpus = '0-%s' % (vcpu - 1)
         hw_socket = flavor.get('hw_socket', '0')
-        cpuset = Libvirt.pin_vcpu_for_perf(connection, vm_name, vcpu, hw_socket)
+        cpuset = Libvirt.pin_vcpu_for_perf(connection, hw_socket)
 
         mac = StandaloneContextHelper.get_mac_address(0x00)
         image = cls.create_snapshot_qemu(connection, index,
@@ -216,7 +264,7 @@ class Libvirt(object):
         connection.execute("echo never > /sys/kernel/mm/transparent_hugepage/enabled")
 
     @classmethod
-    def pin_vcpu_for_perf(cls, connection, vm_name, cpu, socket="0"):
+    def pin_vcpu_for_perf(cls, connection, socket='0'):
         threads = ""
         sys_obj = CpuSysCores(connection)
         soc_cpu = sys_obj.get_core_socket()
@@ -244,9 +292,6 @@ class StandaloneContextHelper(object):
             if connection.execute(cmd_template % pkg)[0]:
                 connection.execute("apt-get update")
                 connection.execute("apt-get -y install %s" % pkg)
-        else:
-            # all installed
-            return
 
     @staticmethod
     def get_kernel_module(connection, pci, driver):
@@ -283,7 +328,7 @@ class StandaloneContextHelper(object):
                 'interface': str(interface),
                 'driver': driver
             })
-        LOG.info("{0}".format(networks))
+        LOG.info(networks)
 
         return networks
 
@@ -352,7 +397,7 @@ class StandaloneContextHelper(object):
         while not mgmtip and times:
             connection.execute("fping -c 1 -g %s > /dev/null 2>&1" % cidr)
             out = connection.execute("ip neighbor | grep '%s'" % mac)[1]
-            LOG.info("fping -c 1 -g %s > /dev/null 2>&1" % cidr)
+            LOG.info("fping -c 1 -g %s > /dev/null 2>&1", cidr)
             if out.strip():
                 mgmtip = str(out.split(" ")[0]).strip()
                 client = ssh.SSH.from_node(node, overrides={"ip": mgmtip})
index fcb7bb6..a6c35de 100644 (file)
@@ -118,7 +118,7 @@ class OvsDpdkContext(Context):
             self.connection.execute(cmd)
         bind_cmd = "{dpdk_nic_bind} --force -b {driver} {port}"
         phy_driver = "vfio-pci"
-        for key, port in self.networks.items():
+        for _, port in self.networks.items():
             vpci = port.get("phy_port")
             self.connection.execute(bind_cmd.format(dpdk_nic_bind=self.dpdk_nic_bind,
                                                     driver=phy_driver, port=vpci))
@@ -170,7 +170,7 @@ class OvsDpdkContext(Context):
         ]
 
         ordered_network = OrderedDict(self.networks)
-        for index, (key, vnf) in enumerate(ordered_network.items()):
+        for index, vnf in enumerate(ordered_network.values()):
             if ovs_ver >= [2, 7, 0]:
                 dpdk_args = " options:dpdk-devargs=%s" % vnf.get("phy_port")
             dpdk_list.append(ovs_add_port.format(br='br0', port='dpdk%s' % vnf.get("port_num", 0),
@@ -263,7 +263,7 @@ class OvsDpdkContext(Context):
 
         # Bind nics back to kernel
         bind_cmd = "{dpdk_nic_bind} --force -b {driver} {port}"
-        for key, port in self.networks.items():
+        for port in self.networks.values():
             vpci = port.get("phy_port")
             phy_driver = port.get("driver")
             self.connection.execute(bind_cmd.format(dpdk_nic_bind=self.dpdk_nic_bind,
@@ -328,17 +328,17 @@ class OvsDpdkContext(Context):
 
     def configure_nics_for_ovs_dpdk(self):
         portlist = OrderedDict(self.networks)
-        for key, ports in portlist.items():
+        for key in portlist:
             mac = StandaloneContextHelper.get_mac_address()
             portlist[key].update({'mac': mac})
         self.networks = portlist
-        LOG.info("Ports %s" % self.networks)
+        LOG.info("Ports %s", self.networks)
 
     def _enable_interfaces(self, index, vfs, cfg):
         vpath = self.ovs_properties.get("vpath", "/usr/local")
         vf = self.networks[vfs[0]]
         port_num = vf.get('port_num', 0)
-        vpci = PciAddress.parse_address(vf['vpci'].strip(), multi_line=True)
+        vpci = PciAddress(vf['vpci'].strip())
         # Generate the vpci for the interfaces
         slot = index + port_num + 10
         vf['vpci'] = \
@@ -357,9 +357,10 @@ class OvsDpdkContext(Context):
             # 1. Check and delete VM if already exists
             Libvirt.check_if_vm_exists_and_delete(vm_name, self.connection)
 
-            vcpu, mac = Libvirt.build_vm_xml(self.connection, self.vm_flavor, cfg, vm_name, index)
+            _, mac = Libvirt.build_vm_xml(self.connection, self.vm_flavor,
+                                          cfg, vm_name, index)
             # 2: Cleanup already available VMs
-            for idx, (vkey, vfs) in enumerate(OrderedDict(vnf["network_ports"]).items()):
+            for vkey, vfs in OrderedDict(vnf["network_ports"]).items():
                 if vkey == "mgmt":
                     continue
                 self._enable_interfaces(index, vfs, cfg)
@@ -367,7 +368,7 @@ class OvsDpdkContext(Context):
             # copy xml to target...
             self.connection.put(cfg, cfg)
 
-            #    FIXME: launch through libvirt
+            # NOTE: launch through libvirt
             LOG.info("virsh create ...")
             Libvirt.virsh_create_vm(self.connection, cfg)
 
index 69825fb..9d8423b 100644 (file)
@@ -110,7 +110,7 @@ class SriovContext(Context):
             Libvirt.check_if_vm_exists_and_delete(vm, self.connection)
 
         # Bind nics back to kernel
-        for key, ports in self.networks.items():
+        for ports in self.networks.values():
             # enable VFs for given...
             build_vfs = "echo 0 > /sys/bus/pci/devices/{0}/sriov_numvfs"
             self.connection.execute(build_vfs.format(ports.get('phy_port')))
@@ -170,8 +170,7 @@ class SriovContext(Context):
 
     def configure_nics_for_sriov(self):
         vf_cmd = "ip link set {0} vf 0 mac {1}"
-        for key, ports in self.networks.items():
-            vf_pci = []
+        for ports in self.networks.values():
             host_driver = ports.get('driver')
             if host_driver not in self.drivers:
                 self.connection.execute("rmmod %svf" % host_driver)
@@ -187,19 +186,19 @@ class SriovContext(Context):
             if interface is not None:
                 self.connection.execute(vf_cmd.format(interface, mac))
 
-            vf_pci = self.get_vf_data('vf_pci', ports.get('phy_port'), mac, interface)
+            vf_pci = self._get_vf_data(ports.get('phy_port'), mac, interface)
             ports.update({
                 'vf_pci': vf_pci,
                 'mac': mac
             })
 
-        LOG.info("Ports %s" % self.networks)
+        LOG.info('Ports %s', self.networks)
 
     def _enable_interfaces(self, index, idx, vfs, cfg):
         vf_spoofchk = "ip link set {0} vf 0 spoofchk off"
 
         vf = self.networks[vfs[0]]
-        vpci = PciAddress.parse_address(vf['vpci'].strip(), multi_line=True)
+        vpci = PciAddress(vf['vpci'].strip())
         # Generate the vpci for the interfaces
         slot = index + idx + 10
         vf['vpci'] = \
@@ -222,7 +221,7 @@ class SriovContext(Context):
             # 1. Check and delete VM if already exists
             Libvirt.check_if_vm_exists_and_delete(vm_name, self.connection)
 
-            vcpu, mac = Libvirt.build_vm_xml(self.connection, self.vm_flavor, cfg, vm_name, index)
+            _, mac = Libvirt.build_vm_xml(self.connection, self.vm_flavor, cfg, vm_name, index)
             # 2: Cleanup already available VMs
             for idx, (vkey, vfs) in enumerate(OrderedDict(vnf["network_ports"]).items()):
                 if vkey == "mgmt":
@@ -232,7 +231,7 @@ class SriovContext(Context):
             # copy xml to target...
             self.connection.put(cfg, cfg)
 
-            #    FIXME: launch through libvirt
+            # NOTE: launch through libvirt
             LOG.info("virsh create ...")
             Libvirt.virsh_create_vm(self.connection, cfg)
 
@@ -246,15 +245,15 @@ class SriovContext(Context):
 
         return nodes
 
-    def get_vf_data(self, key, value, vfmac, pfif):
+    def _get_vf_data(self, value, vfmac, pfif):
         vf_data = {
             "mac": vfmac,
             "pf_if": pfif
         }
         vfs = StandaloneContextHelper.get_virtual_devices(self.connection, value)
         for k, v in vfs.items():
-            m = PciAddress.parse_address(k.strip(), multi_line=True)
-            m1 = PciAddress.parse_address(value.strip(), multi_line=True)
+            m = PciAddress(k.strip())
+            m1 = PciAddress(value.strip())
             if m.bus == m1.bus:
                 vf_data.update({"vf_pci": str(v)})
                 break
index eac3c81..7a1815e 100644 (file)
@@ -22,6 +22,7 @@ from oslo_config import cfg
 from oslo_config.cfg import NoSuchOptError
 from oslo_utils import encodeutils
 
+
 NSB_ROOT = "/opt/nsb_bin"
 
 CONF = cfg.CONF
@@ -43,30 +44,37 @@ HEXADECIMAL = "[0-9a-zA-Z]"
 
 
 class PciAddress(object):
+    """Class to handle PCI addresses.
 
-    PCI_PATTERN_STR = HEXADECIMAL.join([
-        "(",
-        "{4}):(",  # domain (4 bytes)
-        "{2}):(",  # bus (2 bytes)
-        "{2}).(",  # function (2 bytes)
-        ")",       # slot (1 byte)
-    ])
+    A PCI address could be written in two ways:
+    - Simple BDF notation:
+        00:00.0 # bus:device.function
+    - With domain extension.
+        0000:00:00.0 # domain:bus:device.function
 
-    PCI_PATTERN = re.compile(PCI_PATTERN_STR)
+    Note: in Libvirt, 'device' is called 'slot'.
 
-    @classmethod
-    def parse_address(cls, text, multi_line=False):
-        if multi_line:
-            text = text.replace(os.linesep, '')
-            match = cls.PCI_PATTERN.search(text)
-        return cls(match.group(0))
+    Reference: https://wiki.xenproject.org/wiki/
+               Bus:Device.Function_(BDF)_Notation
+    """
+    PCI_PATTERN_STR = (
+        r"((?P<domain>[0-9a-zA-Z]{4}):)?(?P<bus>[0-9a-zA-Z]{2}):"
+        r"(?P<slot>[0-9a-zA-Z]{2})\.(?P<function>[0-9a-zA-Z]{1})")
 
     def __init__(self, address):
-        super(PciAddress, self).__init__()
-        match = self.PCI_PATTERN.match(address)
+        pci_pattern = re.compile(self.PCI_PATTERN_STR)
+        match = pci_pattern.search(address)
         if not match:
             raise ValueError('Invalid PCI address: {}'.format(address))
-        self.address = address
+
+        self._domain = (match.group('domain') or '0000').lower()
+        self._bus = match.group('bus').lower()
+        self._slot = match.group('slot').lower()
+        self._function = match.group('function').lower()
+        self.address = '{:0>4}:{:0>2}:{:0>2}.{:1}'.format(self.domain,
+                                                          self.bus,
+                                                          self.slot,
+                                                          self.function)
         self.match = match
 
     def __repr__(self):
@@ -74,22 +82,22 @@ class PciAddress(object):
 
     @property
     def domain(self):
-        return self.match.group(1)
+        return self._domain
 
     @property
     def bus(self):
-        return self.match.group(2)
+        return self._bus
 
     @property
     def slot(self):
-        return self.match.group(3)
+        return self._slot
 
     @property
     def function(self):
-        return self.match.group(4)
+        return self._function
 
     def values(self):
-        return [self.match.group(n) for n in range(1, 5)]
+        return [self._domain, self._bus, self._slot, self._function]
 
 
 def get_nsb_option(option, default=None):