Resolve multipath map paths before flushing them

The Fibre Channel connector flushes a volume's multipath map with

    multipath -f /dev/disk/by-id/dm-uuid-mpath-<WWID>

While multipathd is running that command does not flush locally; it
forwards the request to the daemon as "del map <arg>".  Up to
multipath-tools 0.9.7 the daemon resolved <arg> through libdevmapper,
which accepts any path, and when that failed the multipath client fell
back to a local flush.  Since multipath-tools 0.9.8 (released in 2024;
the Red Hat family ships the same code as device-mapper-multipath with
the same version numbers, so Fedora 42's 0.10.0 and later are affected
as well as Ubuntu 26.04's 0.14.3) cli_del_map() looks the argument up
with find_mp_by_str(), which accepts only a map name, alias, WWID or
dm-N kernel name, so the by-id path comes back as -ENODEV and the
command exits 1 with "device not found".  The client no longer falls
back to a local flush either.

The result on such a host is that every Fibre Channel detach fails:
os-brick retries the flush three times over a minute, raises, and the
caller marks the operation failed even though the map was healthy and
is removed by the multipathd "del map" that follows.  Creating a
volume from an image goes to error because copy_image_to_volume()
cannot detach the destination, and Nova volume detaches fail the same
way.  Observed on Ubuntu 26.04 (multipath-tools 0.14.3, kernel 7.0) in
the Pure Storage third-party CI, where 26 of 26 cinder-volume flushes
and 54 of 54 nova-compute flushes failed while the same job on Ubuntu
24.04 (multipath-tools 0.9.4) passed.  iSCSI is unaffected because
remove_connection() already flushes by the name read from sysfs.

Resolve any absolute path handed to flush_multipath_device() with
os.path.realpath() before running the command, so the daemon receives
/dev/dm-N.  It strips the /dev/ prefix and accepts dm-N.  Daemons
older than 0.9.8 reject dm-N, since libdevmapper treats it as a map
name, but the multipath client of that era then flushes locally by
the full path, which is also how the by-id symlink has been working on
those versions all along, so their behaviour is unchanged.  A bare
map name, as passed by the iSCSI path, is used as is.

The three attempts move from the command to the method so that the
path is resolved afresh on every attempt.  Device-mapper reuses a
freed minor number immediately, so a dm-N resolved once before the
retries could name a different volume's map by the time a retry runs
if the original map were removed in between.  The attempt count, the
20 and 40 second waits and the per-attempt timeout are unchanged.

Closes-Bug: #2167932
Assisted-By: Claude Code claude-fable-5-1
Change-Id: I761914e53fb608467a02d0fb1ab80c582fd58d93
Signed-off-by: Simon Dodsley <simon@purestorage.com>

os_brick/initiator/linuxscsi.py

@@ -467,14 +467,25 @@ class LinuxSCSI(executor.Executor):
                             "device: %(code)s", {'code': exc.exit_code})
                 raise
 
+    # NOTE(geguileo): With 30% connection error rates flush can get stuck,
+    # set timeout to prevent it from hanging here forever.  Retry twice
+    # after 20 and 40 seconds.
+    @utils.retry(putils.ProcessExecutionError, interval=20, retries=3)
     def flush_multipath_device(self, device_map_name: str) -> None:
+        # NOTE: The Fibre Channel connector passes a symlink such as
+        # /dev/disk/by-id/dm-uuid-mpath-<WWID>.  That worked before
+        # multipath-tools 0.9.8, but multipathd now needs the actual block
+        # device (the /dev/dm-N handle) to be able to remove the map.
+        #
+        # NOTE: we intentionally do retries at the function level so that we
+        # get a fresh path before trying the flush (device-mapper is
+        # aggressive about reusing freed minor numbers, and so dm-N could
+        # name a different map when the retry runs).
+        if os.path.isabs(device_map_name):
+            device_map_name = os.path.realpath(device_map_name)
         LOG.debug("Flush multipath device %s", device_map_name)
-        # NOTE(geguileo): With 30% connection error rates flush can get stuck,
-        # set timeout to prevent it from hanging here forever.  Retry twice
-        # after 20 and 40 seconds.
         self._execute('multipath', '-f', device_map_name, run_as_root=True,
-                      attempts=3, timeout=300, interval=10,
-                      root_helper=self._root_helper)
+                      timeout=300, root_helper=self._root_helper)
 
     @utils.retry(exception.VolumeDeviceNotFound)
     def wait_for_path(self, volume_path: str) -> None:

os_brick/tests/initiator/connectors/test_fibre_channel.py

@@ -361,7 +361,7 @@ class FibreChannelConnectorTestCase(test_connector.ConnectorTestCase):
         self.connector.disconnect_volume(connection_info['data'],
                                          devices['devices'][0])
         expected_commands = [
-            'multipath -f ' + find_mp_device_path_mock.return_value,
+            'multipath -f ' + realpath_mock.return_value,
             'tee -a /sys/block/sdb/device/delete',
             'tee -a /sys/block/sdc/device/delete',
         ]
@@ -910,7 +910,7 @@ class FibreChannelConnectorTestCase(test_connector.ConnectorTestCase):
         self.connector.disconnect_volume(connection_info['data'],
                                          devices['devices'][0])
         expected_commands = [
-            'multipath -f ' + find_mp_device_path_mock.return_value,
+            'multipath -f ' + realpath_mock.return_value,
             'tee -a /sys/block/sdb/device/delete',
             'tee -a /sys/block/sdc/device/delete',
         ]

os_brick/tests/initiator/test_linuxscsi.py

@@ -122,14 +122,62 @@ class LinuxSCSITestCase(base.TestCase):
                                       for name in names] * 2)
         self.assertEqual(1, sleep_mock.call_count)
 
-    def test_flush_multipath_device(self):
+    @mock.patch.object(os.path, 'realpath')
+    def test_flush_multipath_device(self, realpath_mock):
+        """A map name, as the iSCSI path passes, is used as is."""
         dm_map_name = '3600d0230000000000e13955cc3757800'
         with mock.patch.object(self.linuxscsi, '_execute') as exec_mock:
             self.linuxscsi.flush_multipath_device(dm_map_name)
 
+        realpath_mock.assert_not_called()
         exec_mock.assert_called_once_with(
-            'multipath', '-f', dm_map_name, run_as_root=True, attempts=3,
-            timeout=300, interval=10, root_helper=self.linuxscsi._root_helper)
+            'multipath', '-f', dm_map_name, run_as_root=True, timeout=300,
+            root_helper=self.linuxscsi._root_helper)
+
+    @mock.patch.object(os.path, 'realpath', return_value='/dev/dm-3')
+    def test_flush_multipath_device_by_path(self, realpath_mock):
+        """A path is resolved to its dm-N node, which multipathd accepts."""
+        wwid = '3600d0230000000000e13955cc3757800'
+        path = '/dev/disk/by-id/dm-uuid-mpath-' + wwid
+        with mock.patch.object(self.linuxscsi, '_execute') as exec_mock:
+            self.linuxscsi.flush_multipath_device(path)
+
+        realpath_mock.assert_called_once_with(path)
+        exec_mock.assert_called_once_with(
+            'multipath', '-f', '/dev/dm-3', run_as_root=True, timeout=300,
+            root_helper=self.linuxscsi._root_helper)
+
+    @mock.patch.object(os.path, 'realpath',
+                       side_effect=['/dev/dm-3', '/dev/dm-7'])
+    def test_flush_multipath_device_retry_resolves_path_again(
+            self, realpath_mock):
+        """Each attempt resolves the path, so a reused minor is not hit."""
+        wwid = '3600d0230000000000e13955cc3757800'
+        path = '/dev/disk/by-id/dm-uuid-mpath-' + wwid
+        exec_mock = mock.Mock(side_effect=[putils.ProcessExecutionError(),
+                                           ('', None)])
+        with mock.patch.object(self.linuxscsi, '_execute', exec_mock):
+            self.linuxscsi.flush_multipath_device(path)
+
+        self.assertEqual([mock.call(path)] * 2, realpath_mock.call_args_list)
+        kwargs = dict(run_as_root=True, timeout=300,
+                      root_helper=self.linuxscsi._root_helper)
+        self.assertEqual(
+            [mock.call('multipath', '-f', '/dev/dm-3', **kwargs),
+             mock.call('multipath', '-f', '/dev/dm-7', **kwargs)],
+            exec_mock.call_args_list)
+
+    @mock.patch.object(os.path, 'realpath', return_value='/dev/dm-3')
+    def test_flush_multipath_device_fails_after_retries(self, realpath_mock):
+        """Three failed attempts raise the last error to the caller."""
+        exec_mock = mock.Mock(side_effect=putils.ProcessExecutionError())
+        with mock.patch.object(self.linuxscsi, '_execute', exec_mock):
+            self.assertRaises(putils.ProcessExecutionError,
+                              self.linuxscsi.flush_multipath_device,
+                              '/dev/mapper/mpatha')
+
+        self.assertEqual(3, exec_mock.call_count)
+        self.assertEqual(3, realpath_mock.call_count)
 
     def test_get_scsi_wwn(self):
         fake_path = '/dev/disk/by-id/somepath'

releasenotes/notes/bug-2167932-multipath-flush-by-path-28be1cbb0010321b.yaml

@@ -0,0 +1,10 @@
+---
+fixes:
+  - |
+    `Bug #2167932 <https://bugs.launchpad.net/os-brick/+bug/2167932>`_:
+    Fixed Fibre Channel volume detach failing with ``multipath -f``
+    reporting ``device not found`` on hosts running multipath-tools
+    0.9.8 or later, for example Ubuntu 26.04.  The Fibre Channel
+    connector flushed the multipath map by its ``/dev/disk/by-id``
+    symlink, which multipathd no longer resolves.  os-brick now resolves
+    any path to its ``/dev/dm-N`` node before flushing.