diff --git a/hooks/neutron_ovs_utils.py b/hooks/neutron_ovs_utils.py index 5f39ca5c..10365fff 100644 --- a/hooks/neutron_ovs_utils.py +++ b/hooks/neutron_ovs_utils.py @@ -610,69 +610,68 @@ def configure_sriov(): os.chmod(NEUTRON_SRIOV_INIT_SCRIPT, 0o755) service('enable', 'neutron-openvswitch-networking-sriov') - if charm_config.changed('sriov-numvfs'): - devices = PCINetDevices() - sriov_numvfs = charm_config.get('sriov-numvfs') + devices = PCINetDevices() + sriov_numvfs = charm_config.get('sriov-numvfs') - # automatic configuration of all SR-IOV devices - if sriov_numvfs == 'auto': - log('Configuring SR-IOV device VF functions in auto mode') + # automatic configuration of all SR-IOV devices + if sriov_numvfs == 'auto': + log('Configuring SR-IOV device VF functions in auto mode') + for device in devices.pci_devices: + if device and device.sriov: + log("Configuring SR-IOV device" + " {} with {} VF's".format(device.interface_name, + device.sriov_totalvfs)) + device.set_sriov_numvfs(device.sriov_totalvfs) + else: + # Single int blanket configuration + try: + log('Configuring SR-IOV device VF functions' + ' with blanket setting') for device in devices.pci_devices: if device and device.sriov: - log("Configuring SR-IOV device" - " {} with {} VF's".format(device.interface_name, - device.sriov_totalvfs)) - device.set_sriov_numvfs(device.sriov_totalvfs) - else: - # Single int blanket configuration - try: - log('Configuring SR-IOV device VF functions' - ' with blanket setting') - for device in devices.pci_devices: - if device and device.sriov: - numvfs = min(int(sriov_numvfs), device.sriov_totalvfs) - if int(sriov_numvfs) > device.sriov_totalvfs: - log('Requested value for sriov-numvfs ({}) too ' - 'high for interface {}. Falling back to ' - 'interface totalvfs ' - 'value: {}'.format(sriov_numvfs, - device.interface_name, - device.sriov_totalvfs)) - log("Configuring SR-IOV device {} with {} " - "VFs".format(device.interface_name, numvfs)) - device.set_sriov_numvfs(numvfs) - except ValueError: - # :[ :numvfs] configuration - sriov_numvfs = sriov_numvfs.split() - for device_config in sriov_numvfs: - log('Configuring SR-IOV device VF functions per interface') - interface_name, numvfs = device_config.split(':') - device = devices.get_device_from_interface_name( - interface_name) - if device and device.sriov: - if int(numvfs) > device.sriov_totalvfs: - log('Requested value for sriov-numfs ({}) too ' - 'high for interface {}. Falling back to ' - 'interface totalvfs ' - 'value: {}'.format(numvfs, - device.interface_name, - device.sriov_totalvfs)) - numvfs = device.sriov_totalvfs - log("Configuring SR-IOV device {} with {} " - "VF's".format(device.interface_name, numvfs)) - device.set_sriov_numvfs(int(numvfs)) + numvfs = min(int(sriov_numvfs), device.sriov_totalvfs) + if int(sriov_numvfs) > device.sriov_totalvfs: + log('Requested value for sriov-numvfs ({}) too ' + 'high for interface {}. Falling back to ' + 'interface totalvfs ' + 'value: {}'.format(sriov_numvfs, + device.interface_name, + device.sriov_totalvfs)) + log("Configuring SR-IOV device {} with {} " + "VFs".format(device.interface_name, numvfs)) + device.set_sriov_numvfs(numvfs) + except ValueError: + # :[ :numvfs] configuration + sriov_numvfs = sriov_numvfs.split() + for device_config in sriov_numvfs: + log('Configuring SR-IOV device VF functions per interface') + interface_name, numvfs = device_config.split(':') + device = devices.get_device_from_interface_name( + interface_name) + if device and device.sriov: + if int(numvfs) > device.sriov_totalvfs: + log('Requested value for sriov-numfs ({}) too ' + 'high for interface {}. Falling back to ' + 'interface totalvfs ' + 'value: {}'.format(numvfs, + device.interface_name, + device.sriov_totalvfs)) + numvfs = device.sriov_totalvfs + log("Configuring SR-IOV device {} with {} " + "VF's".format(device.interface_name, numvfs)) + device.set_sriov_numvfs(int(numvfs)) - # Trigger remote restart in parent application - remote_restart('neutron-plugin', 'nova-compute') + # Trigger remote restart in parent application + remote_restart('neutron-plugin', 'nova-compute') - # Restart of SRIOV agent is required after changes to system runtime - # VF configuration - cmp_release = CompareOpenStackReleases( - os_release('neutron-common', base='icehouse')) - if cmp_release >= 'mitaka': - service_restart('neutron-sriov-agent') - else: - service_restart('neutron-plugin-sriov-agent') + # Restart of SRIOV agent is required after changes to system runtime + # VF configuration + cmp_release = CompareOpenStackReleases( + os_release('neutron-common', base='icehouse')) + if cmp_release >= 'mitaka': + service_restart('neutron-sriov-agent') + else: + service_restart('neutron-plugin-sriov-agent') def get_shared_secret(): diff --git a/hooks/pci.py b/hooks/pci.py index a4b431d0..647cbc1e 100644 --- a/hooks/pci.py +++ b/hooks/pci.py @@ -190,7 +190,7 @@ class PCINetDevice(object): @param numvfs: integer to set the current number of VF's to """ - if self.sriov: + if self.sriov and numvfs != self.sriov_numvfs: # NOTE(fnordahl): run-time change of numvfs is disallowed # without resetting to 0 first. self._set_sriov_numvfs(0) diff --git a/unit_tests/test_neutron_ovs_utils.py b/unit_tests/test_neutron_ovs_utils.py index 5f94d271..bf219239 100644 --- a/unit_tests/test_neutron_ovs_utils.py +++ b/unit_tests/test_neutron_ovs_utils.py @@ -795,13 +795,11 @@ class TestNeutronOVSUtils(CharmTestCase): # ports=None whilst port checks are disabled. f.assert_called_once_with('assessor', services='s1', ports=None) - def _configure_sriov_base(self, config, - changed=False): + def _configure_sriov_base(self, config): self.mock_config = MagicMock() self.config.side_effect = None self.config.return_value = self.mock_config self.mock_config.get.side_effect = lambda x: config.get(x) - self.mock_config.changed.return_value = changed self.mock_eth_device = MagicMock() self.mock_eth_device.sriov = False @@ -835,19 +833,6 @@ class TestNeutronOVSUtils(CharmTestCase): mock_pci_devices.get_device_from_interface_name.side_effect = \ lambda x: self.pci_devices.get(x) - @patch('os.chmod') - def test_configure_sriov_no_changes(self, _os_chmod): - self.os_release.return_value = 'kilo' - _config = { - 'enable-sriov': True, - 'sriov-numvfs': 'auto' - } - self._configure_sriov_base(_config, False) - - nutils.configure_sriov() - - self.assertFalse(self.remote_restart.called) - @patch('os.chmod') def test_configure_sriov_auto(self, _os_chmod): self.os_release.return_value = 'mitaka' @@ -855,7 +840,7 @@ class TestNeutronOVSUtils(CharmTestCase): 'enable-sriov': True, 'sriov-numvfs': 'auto' } - self._configure_sriov_base(_config, True) + self._configure_sriov_base(_config) nutils.configure_sriov() @@ -874,7 +859,7 @@ class TestNeutronOVSUtils(CharmTestCase): 'enable-sriov': True, 'sriov-numvfs': '32', } - self._configure_sriov_base(_config, True) + self._configure_sriov_base(_config) nutils.configure_sriov() @@ -890,7 +875,7 @@ class TestNeutronOVSUtils(CharmTestCase): 'enable-sriov': True, 'sriov-numvfs': 'ens0:32 sriov23:64' } - self._configure_sriov_base(_config, True) + self._configure_sriov_base(_config) nutils.configure_sriov() @@ -899,6 +884,24 @@ class TestNeutronOVSUtils(CharmTestCase): self.assertTrue(self.remote_restart.called) + @patch('os.chmod') + def test_configure_sriov_auto_avoid_recall(self, _os_chmod): + self.os_release.return_value = 'mitaka' + _config = { + 'enable-sriov': True, + 'sriov-numvfs': 'auto' + } + self._configure_sriov_base(_config) + + nutils.configure_sriov() + + self.mock_sriov_device2.sriov_numvfs = 64 + self.mock_sriov_device2.set_sriov_numvfs.assert_called_with( + self.mock_sriov_device2.sriov_totalvfs) + self.mock_sriov_device2._set_sriov_numvfs.assert_not_called() + + self.assertTrue(self.remote_restart.called) + @patch.object(nutils, 'subprocess') @patch.object(nutils, 'shutil') def test_install_tmpfilesd_lxd(self, mock_shutil, mock_subprocess): diff --git a/unit_tests/test_pci.py b/unit_tests/test_pci.py index 95ee2084..e388f0d5 100644 --- a/unit_tests/test_pci.py +++ b/unit_tests/test_pci.py @@ -220,6 +220,14 @@ class PCINetDeviceTest(CharmTestCase): mock__set_sriov_numvfs.assert_has_calls([ call(0), call(4)]) + @patch('pci.PCINetDevice._set_sriov_numvfs') + def test_set_sriov_numvfs_avoid_call(self, mock__set_sriov_numvfs): + dev = pci.PCINetDevice('0000:10:00.0') + dev.sriov = True + dev.sriov_numvfs = 4 + dev.set_sriov_numvfs(4) + self.assertFalse(mock__set_sriov_numvfs.called) + class PCINetDevicesTest(CharmTestCase):