From 16ba8ed3967999ef8535074aa6d6cc2b09bca54d Mon Sep 17 00:00:00 2001 From: Gorka Eguileor Date: Thu, 14 Sep 2023 14:34:07 +0200 Subject: [PATCH] NVMe-oF: Support nvme cli v2 The nvme cli has changed its behavior, now they no longer differentiate between errors returning a different exit code. Exit code 1 is for errors and 0 for success. This patch fixes the detection of race conditions to also look for the message in case it's a newer CLI version. Together with change I318f167baa0ba7789f4ca2c7c12a8de5568195e0 we are ready for nvme CLI v2. Closes-Bug: #1961222 Change-Id: Idf4d79527e1f03cec754ad708d069b2905b90d3f --- os_brick/initiator/connectors/nvmeof.py | 8 +++- .../tests/initiator/connectors/test_nvmeof.py | 38 ++++++++++++------- .../nvmeof-support-v2-0d3a423c26eee003.yaml | 6 +++ 3 files changed, 37 insertions(+), 15 deletions(-) create mode 100644 releasenotes/notes/nvmeof-support-v2-0d3a423c26eee003.yaml diff --git a/os_brick/initiator/connectors/nvmeof.py b/os_brick/initiator/connectors/nvmeof.py index e38fe20a3..7f55c126c 100644 --- a/os_brick/initiator/connectors/nvmeof.py +++ b/os_brick/initiator/connectors/nvmeof.py @@ -926,8 +926,12 @@ class NVMeOFConnector(base.BaseLinuxConnector): # c-vol running on same node with different lock paths) or # an admin is touching things manually. Not passing these # exit codes in check_exit_code parameter to _execute so we - # can log it. - if exc.exit_code not in (70, errno.EALREADY): + # can log it. nvme cli v2 returns 1, so we parse the + # message. Some nvme cli versions return errors in stdout, + # so we look in stderr and stdout. + if not (exc.exit_code in (70, errno.EALREADY) or + (exc.exit_code == 1 and + 'already connected' in exc.stderr + exc.stdout)): LOG.error('Could not connect to %s: exit_code: %s, ' 'stdout: "%s", stderr: "%s",', portal, exc.exit_code, exc.stdout, exc.stderr) diff --git a/os_brick/tests/initiator/connectors/test_nvmeof.py b/os_brick/tests/initiator/connectors/test_nvmeof.py index e36b66b01..6145e387e 100644 --- a/os_brick/tests/initiator/connectors/test_nvmeof.py +++ b/os_brick/tests/initiator/connectors/test_nvmeof.py @@ -1503,7 +1503,11 @@ class NVMeOFConnectorTestCase(test_connector.ConnectorTestCase): '-n', target.nqn, '-Q', '128', '-l', '-1']) for portal in target.portals]) - @ddt.data(70, errno.EALREADY) + @ddt.data((70, '', ''), + (errno.EALREADY, '', ''), + (1, '', 'already connected'), + (1, 'already connected', '')) + @ddt.unpack @mock.patch.object(nvmeof.LOG, 'warning') @mock.patch.object(nvmeof.Target, 'find_device') @mock.patch.object(nvmeof.Target, 'set_portals_controllers') @@ -1511,14 +1515,14 @@ class NVMeOFConnectorTestCase(test_connector.ConnectorTestCase): @mock.patch.object(nvmeof.NVMeOFConnector, 'rescan') @mock.patch.object(nvmeof.Portal, 'state', new_callable=mock.PropertyMock) def test__connect_target_race( - self, exit_code, mock_state, mock_rescan, mock_cli, + self, exit_code, stdout, stderr, mock_state, mock_rescan, mock_cli, mock_set_ctrls, mock_find_dev, mock_log): """Treat race condition with sysadmin as success.""" mock_state.side_effect = ['connecting', 'connecting', None, 'live'] dev_path = '/dev/nvme0n1' mock_find_dev.return_value = dev_path mock_cli.side_effect = putils.ProcessExecutionError( - exit_code=exit_code) + exit_code=exit_code, stdout=stdout, stderr=stderr) target = self.conn_props.targets[0] @@ -1537,7 +1541,11 @@ class NVMeOFConnectorTestCase(test_connector.ConnectorTestCase): portal.transport, '-n', target.nqn, '-Q', '128', '-l', '-1']) self.assertEqual(1, mock_log.call_count) - @ddt.data(70, errno.EALREADY) + @ddt.data((70, '', ''), + (errno.EALREADY, '', ''), + (1, '', 'already connected'), + (1, 'already connected', '')) + @ddt.unpack @mock.patch.object(nvmeof.LOG, 'warning') @mock.patch('time.sleep') @mock.patch('time.time', side_effect=[0, 0.1, 0.6]) @@ -1551,12 +1559,12 @@ class NVMeOFConnectorTestCase(test_connector.ConnectorTestCase): @mock.patch.object(nvmeof.NVMeOFConnector, 'rescan') @mock.patch.object(nvmeof.Portal, 'state', new_callable=mock.PropertyMock) def test__connect_target_race_connecting( - self, exit_code, mock_state, mock_rescan, mock_cli, mock_set_ctrls, - mock_find_dev, mock_is_live, mock_delay, mock_time, mock_sleep, - mock_log): + self, exit_code, stdout, stderr, mock_state, mock_rescan, mock_cli, + mock_set_ctrls, mock_find_dev, mock_is_live, mock_delay, mock_time, + mock_sleep, mock_log): """Test connect target when portal is reconnecting after race.""" mock_cli.side_effect = putils.ProcessExecutionError( - exit_code=exit_code) + exit_code=exit_code, stdout=stdout, stderr=stderr) mock_state.side_effect = ['connecting', 'connecting', None, 'connecting'] mock_is_live.side_effect = [False, False, False, False, True] @@ -1579,7 +1587,11 @@ class NVMeOFConnectorTestCase(test_connector.ConnectorTestCase): portal.transport, '-n', target.nqn, '-Q', '128', '-l', '-1']) self.assertEqual(1, mock_log.call_count) - @ddt.data(70, errno.EALREADY) + @ddt.data((70, '', ''), + (errno.EALREADY, '', ''), + (1, '', 'already connected'), + (1, 'already connected', '')) + @ddt.unpack @mock.patch.object(nvmeof.LOG, 'warning') @mock.patch.object(nvmeof.LOG, 'error') @mock.patch('time.sleep') @@ -1594,12 +1606,12 @@ class NVMeOFConnectorTestCase(test_connector.ConnectorTestCase): @mock.patch.object(nvmeof.NVMeOFConnector, 'rescan') @mock.patch.object(nvmeof.Portal, 'state', new_callable=mock.PropertyMock) def test__connect_target_race_unknown( - self, exit_code, mock_state, mock_rescan, mock_cli, mock_set_ctrls, - mock_find_dev, mock_is_live, mock_delay, mock_time, mock_sleep, - mock_log_err, mock_log_warn): + self, exit_code, stdout, stderr, mock_state, mock_rescan, mock_cli, + mock_set_ctrls, mock_find_dev, mock_is_live, mock_delay, mock_time, + mock_sleep, mock_log_err, mock_log_warn): """Test connect target when portal is unknown after race.""" mock_cli.side_effect = putils.ProcessExecutionError( - exit_code=exit_code) + exit_code=exit_code, stdout=stdout, stderr=stderr) mock_state.side_effect = ['connecting', 'connecting', None, 'unknown'] mock_is_live.side_effect = [False, False, False, True] diff --git a/releasenotes/notes/nvmeof-support-v2-0d3a423c26eee003.yaml b/releasenotes/notes/nvmeof-support-v2-0d3a423c26eee003.yaml new file mode 100644 index 000000000..2d0f82268 --- /dev/null +++ b/releasenotes/notes/nvmeof-support-v2-0d3a423c26eee003.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - | + NVMe-oF connector `bug #1961222 + `_: Fixed + support of newer NVMe CLI v2.