udiskslinuxdevice: Fix dm-multipath ATA drives handling

The dm-multipath devices are actually capable of tunneling ioctls down to the drive through an active path. It might be actually dangerous to approach the particular paths directly, in parallel or async. This change refrains from any probing on ATA devices which are part of a dm-multipath device. The actual dm-multipath device is probed instead. However the dm-multipath device carries only a small number of udev attributes that still need to be retrieved from the particular path.
storaged-project · Jun 14, 2024 · 7b0a3e5 · 7b0a3e5
1 parent 8821a78
commit 7b0a3e5
Show file tree

Hide file tree

Showing 7 changed files with 115 additions and 30 deletions.
diff --git a/doc/udisks2-sections.txt.daemon.sections.in b/doc/udisks2-sections.txt.daemon.sections.in
@@ -283,6 +283,8 @@ udisks_linux_device_read_sysfs_attr_as_int
 udisks_linux_device_read_sysfs_attr_as_uint64
 udisks_linux_device_subsystem_is_nvme
 udisks_linux_device_nvme_is_fabrics
+udisks_linux_device_is_dm_multipath
+udisks_linux_device_is_mpath_device_path
 <SUBSECTION Standard>
 UDISKS_TYPE_LINUX_DEVICE
 UDISKS_LINUX_DEVICE

diff --git a/src/udiskslinuxdevice.c b/src/udiskslinuxdevice.c
@@ -99,6 +99,7 @@ static gboolean probe_ata (UDisksLinuxDevice  *device,
 /**
  * udisks_linux_device_new_sync:
  * @udev_device: A #GUdevDevice.
+ * @udev_client: A #GUdevClient.
  *
  * Creates a new #UDisksLinuxDevice from @udev_device which includes
  * probing the device for more information, if applicable.
@@ -109,7 +110,7 @@ static gboolean probe_ata (UDisksLinuxDevice  *device,
  * Returns: A #UDisksLinuxDevice.
  */
 UDisksLinuxDevice *
-udisks_linux_device_new_sync (GUdevDevice *udev_device)
+udisks_linux_device_new_sync (GUdevDevice *udev_device, GUdevClient *udev_client)
 {
   UDisksLinuxDevice *device;
   GError *error = NULL;
@@ -122,7 +123,7 @@ udisks_linux_device_new_sync (GUdevDevice *udev_device)
   /* No point in probing on remove events */
   if (!(g_strcmp0 (g_udev_device_get_action (udev_device), "remove") == 0))
     {
-      if (!udisks_linux_device_reprobe_sync (device, NULL, &error))
+      if (!udisks_linux_device_reprobe_sync (device, udev_client, NULL, &error))
         goto out;
     }
 
@@ -142,17 +143,22 @@ udisks_linux_device_new_sync (GUdevDevice *udev_device)
 /**
  * udisks_linux_device_reprobe_sync:
  * @device: A #UDisksLinuxDevice.
+ * @udev_client: A #GUdevClient.
  * @cancellable: (allow-none): A #GCancellable or %NULL.
  * @error: Return location for error or %NULL.
  *
  * Forcibly reprobe information on @device. The calling thread may be
  * blocked for a non-trivial amount of time while the probing is
  * underway.
  *
+ * Probing is dm-multipath aware in which case an active path
+ * is looked up and udev attributes are fetched from there.
+ *
  * Returns: %TRUE if reprobing succeeded, %FALSE otherwise.
  */
 gboolean
 udisks_linux_device_reprobe_sync (UDisksLinuxDevice  *device,
+                                  GUdevClient        *udev_client,
                                   GCancellable       *cancellable,
                                   GError            **error)
 {
@@ -167,7 +173,8 @@ udisks_linux_device_reprobe_sync (UDisksLinuxDevice  *device,
       g_udev_device_get_property_as_boolean (device->udev_device, "ID_ATA") &&
       !g_udev_device_has_property (device->udev_device, "ID_USB_TYPE") &&
       !g_udev_device_has_property (device->udev_device, "ID_USB_DRIVER") &&
-      !g_udev_device_has_property (device->udev_device, "ID_USB_MODEL"))
+      !g_udev_device_has_property (device->udev_device, "ID_USB_MODEL") &&
+      !udisks_linux_device_is_mpath_device_path (device))
     {
       if (!probe_ata (device, cancellable, error))
         goto out;
@@ -218,6 +225,34 @@ udisks_linux_device_reprobe_sync (UDisksLinuxDevice  *device,
       if (!device->nvme_ns_info)
         goto out;
     }
+  else
+  /* Probe the dm-multipath devices */
+  if (g_strcmp0 (g_udev_device_get_subsystem (device->udev_device), "block") == 0 &&
+      g_strcmp0 (g_udev_device_get_devtype (device->udev_device), "disk") == 0 &&
+      udisks_linux_device_is_dm_multipath (device))
+    {
+      gboolean is_ata = FALSE;
+      gchar **slaves;
+      guint n;
+
+      slaves = udisks_daemon_util_resolve_links (g_udev_device_get_sysfs_path (device->udev_device), "slaves");
+      for (n = 0; slaves[n] != NULL; n++)
+        {
+          GUdevDevice *slave;
+
+          slave = g_udev_client_query_by_sysfs_path (udev_client, slaves[n]);
+          if (slave != NULL)
+            {
+              is_ata |= g_udev_device_get_property_as_boolean (slave, "ID_ATA");
+              g_object_unref (slave);
+            }
+          if (is_ata)
+            break;
+        }
+      g_strfreev (slaves);
+      if (is_ata && !probe_ata (device, cancellable, error))
+        goto out;
+    }
 
   ret = TRUE;
 
@@ -463,3 +498,35 @@ udisks_linux_device_nvme_is_fabrics (UDisksLinuxDevice *device)
 
   return FALSE;
 }
+
+/**
+ * udisks_linux_device_is_dm_multipath:
+ * @device: A #UDisksLinuxDevice.
+ *
+ * Returns: %TRUE if @device represents a dm-multipath block device,
+ *          %FALSE otherwise.
+ */
+gboolean
+udisks_linux_device_is_dm_multipath (UDisksLinuxDevice *device)
+{
+  const gchar *dm_uuid;
+
+  if (g_udev_device_get_property_as_int (device->udev_device, "MPATH_DEVICE_READY") == 1)
+    return TRUE;
+
+  dm_uuid = g_udev_device_get_sysfs_attr (device->udev_device, "dm/uuid");
+  return dm_uuid != NULL && g_str_has_prefix (dm_uuid, "mpath-");
+}
+
+/**
+ * udisks_linux_device_is_mpath_device_path:
+ * @device: A #UDisksLinuxDevice.
+ *
+ * Returns: %TRUE if @device is an I/O path that is part of a dm-multipath
+ *          device, %FALSE otherwise.
+ */
+gboolean
+udisks_linux_device_is_mpath_device_path (UDisksLinuxDevice *device)
+{
+  return g_udev_device_get_property_as_int (device->udev_device, "DM_MULTIPATH_DEVICE_PATH") == 1;
+}
diff --git a/src/udiskslinuxdevice.h b/src/udiskslinuxdevice.h
@@ -57,8 +57,10 @@ struct _UDisksLinuxDevice
 };
 
 GType              udisks_linux_device_get_type     (void) G_GNUC_CONST;
-UDisksLinuxDevice *udisks_linux_device_new_sync     (GUdevDevice *udev_device);
+UDisksLinuxDevice *udisks_linux_device_new_sync     (GUdevDevice        *udev_device,
+                                                     GUdevClient        *udev_client);
 gboolean           udisks_linux_device_reprobe_sync (UDisksLinuxDevice  *device,
+                                                     GUdevClient        *udev_client,
                                                      GCancellable       *cancellable,
                                                      GError            **error);
 
@@ -75,6 +77,9 @@ guint64            udisks_linux_device_read_sysfs_attr_as_uint64 (UDisksLinuxDev
 gboolean           udisks_linux_device_subsystem_is_nvme         (UDisksLinuxDevice  *device);
 gboolean           udisks_linux_device_nvme_is_fabrics           (UDisksLinuxDevice  *device);
 
+gboolean           udisks_linux_device_is_dm_multipath           (UDisksLinuxDevice  *device);
+gboolean           udisks_linux_device_is_mpath_device_path      (UDisksLinuxDevice  *device);
+
 G_END_DECLS
 
 #endif /* __UDISKS_LINUX_DEVICE_H__ */
diff --git a/src/udiskslinuxdrive.c b/src/udiskslinuxdrive.c
@@ -1630,7 +1630,7 @@ handle_power_off (UDisksDrive           *_drive,
     }
   fd = -1;
 
-  device = udisks_linux_drive_object_get_device (object, TRUE /* get_hw */);
+  device = udisks_linux_drive_object_get_device (object, FALSE /* get_hw */);
   if (device == NULL)
     {
       g_dbus_method_invocation_return_error (invocation,

diff --git a/src/udiskslinuxdriveata.c b/src/udiskslinuxdriveata.c
@@ -372,7 +372,7 @@ udisks_linux_drive_ata_update (UDisksLinuxDriveAta    *drive,
                                UDisksLinuxDriveObject *object)
 {
   UDisksLinuxDevice *device;
-  device = udisks_linux_drive_object_get_device (object, TRUE /* get_hw */);
+  device = udisks_linux_drive_object_get_device (object, FALSE /* get_hw */);
   if (device == NULL)
     goto out;
 
@@ -423,6 +423,20 @@ static gboolean update_io_stats (UDisksLinuxDriveAta *drive, UDisksLinuxDevice *
   return noio;
 }
 
+static const gchar *
+get_smart_device_type (UDisksLinuxDevice *device)
+{
+  if (!udisks_linux_device_is_dm_multipath (device))
+    return NULL;
+
+  /* Now that we're in UDisksLinuxDriveAta it means the device already passed
+   * earlier selection for hardware type. The most likely scenario here are
+   * SATA drives connected to a SAS HBA, so hardcode the SCSI to ATA Translation (SAT)
+   * protocol for ATA PASS THROUGH SCSI.
+   */
+  return "sat,auto";
+}
+
 /**
  * udisks_linux_drive_ata_refresh_smart_sync:
  * @drive: The #UDisksLinuxDriveAta to refresh.
@@ -469,7 +483,7 @@ udisks_linux_drive_ata_refresh_smart_sync (UDisksLinuxDriveAta  *drive,
       goto out;
     }
 
-  device = udisks_linux_drive_object_get_device (object, TRUE /* get_hw */);
+  device = udisks_linux_drive_object_get_device (object, FALSE /* get_hw */);
   if (device == NULL)
     {
       g_set_error_literal (error, UDISKS_ERROR, UDISKS_ERROR_FAILED,
@@ -519,6 +533,7 @@ udisks_linux_drive_ata_refresh_smart_sync (UDisksLinuxDriveAta  *drive,
         }
 
       data = bd_smart_ata_get_info (g_udev_device_get_device_file (device->udev_device),
+                                    get_smart_device_type (device),
                                     &l_error);
     }
 
@@ -588,7 +603,7 @@ udisks_linux_drive_ata_smart_selftest_sync (UDisksLinuxDriveAta  *drive,
   if (object == NULL)
     goto out;
 
-  device = udisks_linux_drive_object_get_device (object, TRUE /* get_hw */);
+  device = udisks_linux_drive_object_get_device (object, FALSE /* get_hw */);
   if (device == NULL)
     {
       g_set_error_literal (error, UDISKS_ERROR, UDISKS_ERROR_FAILED,
@@ -616,6 +631,7 @@ udisks_linux_drive_ata_smart_selftest_sync (UDisksLinuxDriveAta  *drive,
     }
 
   if (!bd_smart_device_self_test (g_udev_device_get_device_file (device->udev_device),
+                                  get_smart_device_type (device),
                                   op, &l_error))
     {
       g_set_error_literal (error,
@@ -661,7 +677,7 @@ handle_smart_update (UDisksDriveAta        *_drive,
     }
 
   daemon = udisks_linux_drive_object_get_daemon (object);
-  block_object = udisks_linux_drive_object_get_block (object, TRUE);
+  block_object = udisks_linux_drive_object_get_block (object, FALSE);
   if (block_object == NULL)
     {
       g_dbus_method_invocation_return_error (invocation,
@@ -818,7 +834,7 @@ handle_smart_selftest_abort (UDisksDriveAta        *_drive,
     }
 
   daemon = udisks_linux_drive_object_get_daemon (object);
-  block_object = udisks_linux_drive_object_get_block (object, TRUE);
+  block_object = udisks_linux_drive_object_get_block (object, FALSE);
   if (block_object == NULL)
     {
       g_dbus_method_invocation_return_error (invocation,
@@ -1042,7 +1058,7 @@ handle_smart_selftest_start (UDisksDriveAta        *_drive,
     }
 
   daemon = udisks_linux_drive_object_get_daemon (object);
-  block_object = udisks_linux_drive_object_get_block (object, TRUE);
+  block_object = udisks_linux_drive_object_get_block (object, FALSE);
   if (block_object == NULL)
     {
       g_dbus_method_invocation_return_error (invocation,
@@ -1145,6 +1161,7 @@ handle_smart_set_enabled (UDisksDriveAta        *_drive,
   UDisksLinuxDriveObject *object = NULL;
   UDisksLinuxBlockObject *block_object = NULL;
   UDisksDaemon *daemon;
+  UDisksLinuxProvider *provider;
   GError *error = NULL;
   UDisksLinuxDevice *device = NULL;
   const gchar *message;
@@ -1169,6 +1186,7 @@ handle_smart_set_enabled (UDisksDriveAta        *_drive,
     }
 
   daemon = udisks_linux_drive_object_get_daemon (object);
+  provider = udisks_daemon_get_linux_provider (daemon);
   if (!udisks_daemon_util_get_caller_uid_sync (daemon,
                                                invocation,
                                                NULL /* GCancellable */,
@@ -1220,6 +1238,7 @@ handle_smart_set_enabled (UDisksDriveAta        *_drive,
     }
 
   if (!bd_smart_set_enabled (g_udev_device_get_device_file (device->udev_device),
+                             get_smart_device_type (device),
                              value, &error))
     {
       if (g_error_matches (error, BD_SMART_ERROR, BD_SMART_ERROR_TECH_UNAVAIL))
@@ -1274,7 +1293,9 @@ handle_smart_set_enabled (UDisksDriveAta        *_drive,
     }
 
   /* Reread new IDENTIFY data */
-  if (!udisks_linux_device_reprobe_sync (device, NULL, &error))
+  if (!udisks_linux_device_reprobe_sync (device,
+                                         udisks_linux_provider_get_udev_client (provider),
+                                         NULL, &error))
     {
       g_prefix_error (&error, "Error reprobing device: ");
       g_dbus_method_invocation_take_error (invocation, error);
@@ -1369,7 +1390,7 @@ udisks_linux_drive_ata_get_pm_state (UDisksLinuxDriveAta  *drive,
 
   /* TODO: some SSD controllers may block for considerable time when trimming large amount of blocks */
 
-  device = udisks_linux_drive_object_get_device (object, TRUE /* get_hw */);
+  device = udisks_linux_drive_object_get_device (object, FALSE /* get_hw */);
   if (device == NULL)
     {
       g_set_error_literal (error, UDISKS_ERROR, UDISKS_ERROR_FAILED,
@@ -1529,7 +1550,7 @@ handle_pm_standby_wakeup (UDisksDriveAta        *_drive,
                                                     invocation))
     goto out;
 
-  device = udisks_linux_drive_object_get_device (object, TRUE /* get_hw */);
+  device = udisks_linux_drive_object_get_device (object, FALSE /* get_hw */);
   if (device == NULL)
     {
       g_dbus_method_invocation_return_error (invocation, UDISKS_ERROR, UDISKS_ERROR_FAILED,
@@ -2030,7 +2051,7 @@ udisks_linux_drive_ata_secure_erase_sync (UDisksLinuxDriveAta  *drive,
 
   daemon = udisks_linux_drive_object_get_daemon (object);
 
-  device = udisks_linux_drive_object_get_device (object, TRUE /* get_hw */);
+  device = udisks_linux_drive_object_get_device (object, FALSE /* get_hw */);
   if (device == NULL)
     {
       g_set_error (&local_error, UDISKS_ERROR, UDISKS_ERROR_FAILED,
@@ -2405,7 +2426,6 @@ handle_security_erase_unit (UDisksDriveAta        *_drive,
 
 /* ---------------------------------------------------------------------------------------------------- */
 
-
 static void
 drive_ata_iface_init (UDisksDriveAtaIface *iface)
 {

diff --git a/src/udiskslinuxdriveobject.c b/src/udiskslinuxdriveobject.c
@@ -234,15 +234,6 @@ strip_and_replace_with_uscore (gchar *s)
   ;
 }
 
-static gboolean
-is_dm_multipath (UDisksLinuxDevice *device)
-{
-  const gchar *dm_uuid;
-
-  dm_uuid = g_udev_device_get_sysfs_attr (device->udev_device, "dm/uuid");
-  return dm_uuid != NULL && g_str_has_prefix (dm_uuid, "mpath-");
-}
-
 static void
 udisks_linux_drive_object_constructed (GObject *_object)
 {
@@ -437,7 +428,7 @@ udisks_linux_drive_object_get_device (UDisksLinuxDriveObject *object,
   g_mutex_lock (&object->devices_mutex);
   for (devices = object->devices; devices; devices = devices->next)
     {
-      if (!get_hw || !is_dm_multipath (UDISKS_LINUX_DEVICE (devices->data)))
+      if (!get_hw || !udisks_linux_device_is_dm_multipath (UDISKS_LINUX_DEVICE (devices->data)))
         {
           ret = devices->data;
           break;
@@ -486,7 +477,7 @@ udisks_linux_drive_object_get_block (UDisksLinuxDriveObject *object,
 
       device = udisks_linux_block_object_get_device (UDISKS_LINUX_BLOCK_OBJECT (iter_object));
       skip = (g_strcmp0 (g_udev_device_get_devtype (device->udev_device), "disk") != 0
-              || (get_hw && is_dm_multipath (device)));
+              || (get_hw && udisks_linux_device_is_dm_multipath (device)));
       g_object_unref (device);
 
       if (skip)
@@ -1049,7 +1040,7 @@ udisks_linux_drive_object_should_include_device (GUdevClient        *client,
         }
 
       /* dm-multipath */
-      if (is_dm_multipath (device))
+      if (udisks_linux_device_is_dm_multipath (device))
         {
           gchar **slaves;
           guint n;

diff --git a/src/udiskslinuxprovider.c b/src/udiskslinuxprovider.c
@@ -327,7 +327,7 @@ probe_request_thread_func (gpointer user_data)
         continue;
 
       /* probe the device - this may take a while */
-      request->udisks_device = udisks_linux_device_new_sync (request->udev_device);
+      request->udisks_device = udisks_linux_device_new_sync (request->udev_device, provider->gudev_client);
 
       /* now that we've probed the device, post the request back to the main thread */
       g_idle_add (on_idle_with_probed_uevent, request);
@@ -565,7 +565,7 @@ get_udisks_devices (UDisksLinuxProvider *provider)
       GUdevDevice *device = G_UDEV_DEVICE (l->data);
       if (!g_udev_device_get_is_initialized (device))
         continue;
-      udisks_devices = g_list_prepend (udisks_devices, udisks_linux_device_new_sync (device));
+      udisks_devices = g_list_prepend (udisks_devices, udisks_linux_device_new_sync (device, provider->gudev_client));
     }
   udisks_devices = g_list_reverse (udisks_devices);
   g_list_free_full (devices, g_object_unref);