Skip to content

Commit

Permalink
udiskslinuxdevice: Fix dm-multipath ATA drives handling
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@tbzatek
tbzatek committed Jun 20, 2024
1 parent 8821a78 commit 4279c5e
Show file tree
Hide file tree
Showing 7 changed files with 132 additions and 32 deletions.
2 changes: 2 additions & 0 deletions doc/udisks2-sections.txt.daemon.sections.in
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
73 changes: 70 additions & 3 deletions src/udiskslinuxdevice.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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;
Expand All @@ -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;
}

Expand All @@ -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)
{
Expand All @@ -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;
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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;
}
7 changes: 6 additions & 1 deletion src/udiskslinuxdevice.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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__ */
2 changes: 1 addition & 1 deletion src/udiskslinuxdrive.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
61 changes: 48 additions & 13 deletions src/udiskslinuxdriveata.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -423,6 +423,24 @@ static gboolean update_io_stats (UDisksLinuxDriveAta *drive, UDisksLinuxDevice *
return noio;
}

static BDExtraArg **
build_smart_extra_args (UDisksLinuxDevice *device)
{
BDExtraArg **extra;

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.
*/
extra = g_new0 (BDExtraArg *, 2);
extra[0] = bd_extra_arg_new ("--device=sat,auto", NULL);
return extra;
}

/**
* udisks_linux_drive_ata_refresh_smart_sync:
* @drive: The #UDisksLinuxDriveAta to refresh.
Expand Down Expand Up @@ -469,7 +487,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,
Expand Down Expand Up @@ -499,6 +517,7 @@ udisks_linux_drive_ata_refresh_smart_sync (UDisksLinuxDriveAta *drive,
}
else
{
BDExtraArg **extra;
guchar count;
gboolean noio = FALSE;
gboolean awake;
Expand All @@ -518,8 +537,11 @@ udisks_linux_drive_ata_refresh_smart_sync (UDisksLinuxDriveAta *drive,
goto out_io;
}

extra = build_smart_extra_args (device);
data = bd_smart_ata_get_info (g_udev_device_get_device_file (device->udev_device),
(const BDExtraArg **) extra,
&l_error);
bd_extra_arg_list_free (extra);
}

if (data == NULL)
Expand Down Expand Up @@ -583,12 +605,13 @@ udisks_linux_drive_ata_smart_selftest_sync (UDisksLinuxDriveAta *drive,
GError *l_error = NULL;
gboolean ret = FALSE;
BDSmartSelfTestOp op;
BDExtraArg **extra = NULL;

object = udisks_daemon_util_dup_object (drive, error);
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,
Expand All @@ -615,8 +638,11 @@ udisks_linux_drive_ata_smart_selftest_sync (UDisksLinuxDriveAta *drive,
goto out;
}

extra = build_smart_extra_args (device);
if (!bd_smart_device_self_test (g_udev_device_get_device_file (device->udev_device),
op, &l_error))
op,
(const BDExtraArg **) extra,
&l_error))
{
g_set_error_literal (error,
UDISKS_ERROR, UDISKS_ERROR_FAILED,
Expand All @@ -628,6 +654,7 @@ udisks_linux_drive_ata_smart_selftest_sync (UDisksLinuxDriveAta *drive,
ret = TRUE;

out:
bd_extra_arg_list_free (extra);
g_clear_object (&device);
g_clear_object (&object);
return ret;
Expand Down Expand Up @@ -661,7 +688,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,
Expand Down Expand Up @@ -818,7 +845,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,
Expand Down Expand Up @@ -1042,7 +1069,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,
Expand Down Expand Up @@ -1145,11 +1172,13 @@ 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;
const gchar *action_id;
uid_t caller_uid;
BDExtraArg **extra = NULL;

object = udisks_daemon_util_dup_object (drive, &error);
if (object == NULL)
Expand All @@ -1169,6 +1198,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 */,
Expand Down Expand Up @@ -1219,8 +1249,11 @@ handle_smart_set_enabled (UDisksDriveAta *_drive,
goto out;
}

extra = build_smart_extra_args (device);
if (!bd_smart_set_enabled (g_udev_device_get_device_file (device->udev_device),
value, &error))
value,
(const BDExtraArg **) extra,
&error))
{
if (g_error_matches (error, BD_SMART_ERROR, BD_SMART_ERROR_TECH_UNAVAIL))
{
Expand Down Expand Up @@ -1274,7 +1307,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);
Expand Down Expand Up @@ -1305,6 +1340,7 @@ handle_smart_set_enabled (UDisksDriveAta *_drive,
udisks_drive_ata_complete_smart_set_enabled (_drive, invocation);

out:
bd_extra_arg_list_free (extra);
g_clear_object (&device);
g_clear_object (&block_object);
g_clear_object (&object);
Expand Down Expand Up @@ -1369,7 +1405,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,
Expand Down Expand Up @@ -1529,7 +1565,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,
Expand Down Expand Up @@ -2030,7 +2066,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,
Expand Down Expand Up @@ -2405,7 +2441,6 @@ handle_security_erase_unit (UDisksDriveAta *_drive,

/* ---------------------------------------------------------------------------------------------------- */


static void
drive_ata_iface_init (UDisksDriveAtaIface *iface)
{
Expand Down
Loading

0 comments on commit 4279c5e

Please sign in to comment.