Skip to content

Commit

Permalink
udiskslinuxdriveata: Prefer udev ID_ATA_* attributes
Browse files Browse the repository at this point in the history 
Looking at the code in udisksata.c vs. udev/ata_id.c it appears
to have equal basis, only the udev part has evolved over time.
Except of one missing property it has feature parity, is better
maintained, runs earlier in the chain and is readily available.
There's no reason not to trust it as an authoritative source
of information.

The existing probing code needs to stay in UDisks for dm-multipath
device use, where udev ata_id is not run.

This needs ID_ATA_READ_LOOKAHEAD support in udev, merged in
systemd-257~devel. libudev version is checked during configure
and if lower, UDisks will fall back to the old probing approach
just for this missing property.

As the udev/ata_id probing code is generally more permissive,
it works on qemu-emulated AHCI+IDE devices as a bonus.
  • Loading branch information
@tbzatek
tbzatek committed Sep 11, 2024
1 parent 367fbc9 commit 98d2896
Show file tree
Hide file tree
Showing 5 changed files with 104 additions and 45 deletions.
6 changes: 6 additions & 0 deletions configure.ac
View file
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,12 @@ if test "x$enable_daemon" = "xyes"; then
AC_SUBST(GUDEV_CFLAGS)
AC_SUBST(GUDEV_LIBS)

PKG_CHECK_MODULES(UDEV, [udev >= 254], [have_udev_257=yes], [have_udev_257=no])
if test "x$have_udev_257" = "xyes"; then
AC_DEFINE([HAVE_UDEV_257], 1, [Define to 1 if udev >= 257 is available])
fi
AC_SUBST(HAVE_UDEV_257)

PKG_CHECK_MODULES(GMODULE, [gmodule-2.0])
AC_SUBST(GMODULE_CFLAGS)
AC_SUBST(GMODULE_LIBS)
Expand Down
16 changes: 13 additions & 3 deletions src/udiskslinuxdevice.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ udisks_linux_device_class_init (UDisksLinuxDeviceClass *klass)
/* ---------------------------------------------------------------------------------------------------- */

static gboolean probe_ata (UDisksLinuxDevice *device,
gboolean force_probe,
GCancellable *cancellable,
GError **error);

Expand Down Expand Up @@ -176,7 +177,7 @@ udisks_linux_device_reprobe_sync (UDisksLinuxDevice *device,
!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))
if (!probe_ata (device, FALSE, cancellable, error))
goto out;
}
else
Expand Down Expand Up @@ -250,7 +251,7 @@ udisks_linux_device_reprobe_sync (UDisksLinuxDevice *device,
break;
}
g_strfreev (slaves);
if (is_ata && !probe_ata (device, cancellable, error))
if (is_ata && !probe_ata (device, TRUE, cancellable, error))
goto out;
}

Expand All @@ -264,6 +265,7 @@ udisks_linux_device_reprobe_sync (UDisksLinuxDevice *device,

static gboolean
probe_ata (UDisksLinuxDevice *device,
gboolean force_probe,
GCancellable *cancellable,
GError **error)
{
Expand All @@ -273,6 +275,15 @@ probe_ata (UDisksLinuxDevice *device,
UDisksAtaCommandInput input = {0};
UDisksAtaCommandOutput output = {0};

if (!force_probe
#ifndef HAVE_UDEV_257
/* added in https://github.com/systemd/systemd/pull/34343 */
&& g_udev_device_has_property (device->udev_device, "ID_ATA_READ_LOOKAHEAD")
#endif
)
/* udev carries everything we need, no need for explicit probing */
return TRUE;

device_file = g_udev_device_get_device_file (device->udev_device);
fd = open (device_file, O_RDONLY|O_NONBLOCK);
if (fd == -1)
Expand All @@ -283,7 +294,6 @@ probe_ata (UDisksLinuxDevice *device,
goto out;
}


if (ioctl (fd, CDROM_GET_CAPABILITY, NULL) == -1)
{
/* ATA8: 7.16 IDENTIFY DEVICE - ECh, PIO Data-In */
Expand Down
4 changes: 3 additions & 1 deletion src/udiskslinuxdrive.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -583,7 +583,9 @@ set_rotation_rate (UDisksDrive *iface,
else
{
rate = -1;
if (device->ata_identify_device_data != NULL)
if (g_udev_device_has_property (device->udev_device, "ID_ATA_ROTATION_RATE_RPM"))
rate = g_udev_device_get_property_as_int (device->udev_device, "ID_ATA_ROTATION_RATE_RPM");
else if (device->ata_identify_device_data != NULL)
{
guint word_217 = 0;

Expand Down
120 changes: 80 additions & 40 deletions src/udiskslinuxdriveata.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -207,13 +207,16 @@ update_smart (UDisksLinuxDriveAta *drive,
guint16 word_85 = 0;

#ifdef HAVE_SMART
/* ATA8: 7.16 IDENTIFY DEVICE - ECh, PIO Data-In - Table 29 IDENTIFY DEVICE data */
word_82 = udisks_ata_identify_get_word (device->ata_identify_device_data, 82);
word_85 = udisks_ata_identify_get_word (device->ata_identify_device_data, 85);
supported = word_82 & (1<<0);
enabled = word_85 & (1<<0);
#else
supported = enabled = FALSE;
supported = g_udev_device_get_property_as_boolean (device->udev_device, "ID_ATA_FEATURE_SET_SMART");
enabled = g_udev_device_get_property_as_boolean (device->udev_device, "ID_ATA_FEATURE_SET_SMART_ENABLED");
if (!supported && device->ata_identify_device_data)
{
/* ATA8: 7.16 IDENTIFY DEVICE - ECh, PIO Data-In - Table 29 IDENTIFY DEVICE data */
word_82 = udisks_ata_identify_get_word (device->ata_identify_device_data, 82);
word_85 = udisks_ata_identify_get_word (device->ata_identify_device_data, 85);
supported = word_82 & (1<<0);
enabled = word_85 & (1<<0);
}
#endif

G_LOCK (object_lock);
Expand Down Expand Up @@ -277,25 +280,52 @@ update_pm (UDisksLinuxDriveAta *drive,
guint16 word_86 = 0;
guint16 word_94 = 0;

/* ATA8: 7.16 IDENTIFY DEVICE - ECh, PIO Data-In - Table 29 IDENTIFY DEVICE data */
word_82 = udisks_ata_identify_get_word (device->ata_identify_device_data, 82);
word_83 = udisks_ata_identify_get_word (device->ata_identify_device_data, 83);
word_85 = udisks_ata_identify_get_word (device->ata_identify_device_data, 85);
word_86 = udisks_ata_identify_get_word (device->ata_identify_device_data, 86);
word_94 = udisks_ata_identify_get_word (device->ata_identify_device_data, 94);

pm_supported = word_82 & (1<<3);
pm_enabled = word_85 & (1<<3);
apm_supported = word_83 & (1<<3);
apm_enabled = word_86 & (1<<3);
aam_supported = word_83 & (1<<9);
aam_enabled = word_86 & (1<<9);
if (aam_supported)
aam_vendor_recommended_value = (word_94 >> 8);
write_cache_supported = word_82 & (1<<5);
write_cache_enabled = word_85 & (1<<5);
read_lookahead_supported = word_82 & (1<<6);
read_lookahead_enabled = word_85 & (1<<6);
pm_supported = g_udev_device_get_property_as_boolean (device->udev_device, "ID_ATA_FEATURE_SET_PM");
pm_enabled = g_udev_device_get_property_as_boolean (device->udev_device, "ID_ATA_FEATURE_SET_PM_ENABLED");
apm_supported = g_udev_device_get_property_as_boolean (device->udev_device, "ID_ATA_FEATURE_SET_APM");
apm_enabled = g_udev_device_get_property_as_boolean (device->udev_device, "ID_ATA_FEATURE_SET_APM_ENABLED");
aam_supported = g_udev_device_get_property_as_boolean (device->udev_device, "ID_ATA_FEATURE_SET_AAM");
aam_enabled = g_udev_device_get_property_as_boolean (device->udev_device, "ID_ATA_FEATURE_SET_AAM_ENABLED");
write_cache_supported = g_udev_device_get_property_as_boolean (device->udev_device, "ID_ATA_WRITE_CACHE");
write_cache_enabled = g_udev_device_get_property_as_boolean (device->udev_device, "ID_ATA_WRITE_CACHE_ENABLED");
read_lookahead_supported = g_udev_device_get_property_as_boolean (device->udev_device, "ID_ATA_READ_LOOKAHEAD");
read_lookahead_enabled = g_udev_device_get_property_as_boolean (device->udev_device, "ID_ATA_READ_LOOKAHEAD_ENABLED");
aam_vendor_recommended_value = g_udev_device_get_property_as_int (device->udev_device, "ID_ATA_FEATURE_SET_AAM_VENDOR_RECOMMENDED_VALUE");

if (device->ata_identify_device_data)
{
/* ATA8: 7.16 IDENTIFY DEVICE - ECh, PIO Data-In - Table 29 IDENTIFY DEVICE data */
word_82 = udisks_ata_identify_get_word (device->ata_identify_device_data, 82);
word_85 = udisks_ata_identify_get_word (device->ata_identify_device_data, 85);

/* available in udev since < 2012 */
if (!g_udev_device_get_property_as_boolean (device->udev_device, "ID_ATA"))
{
word_83 = udisks_ata_identify_get_word (device->ata_identify_device_data, 83);
word_86 = udisks_ata_identify_get_word (device->ata_identify_device_data, 86);
word_94 = udisks_ata_identify_get_word (device->ata_identify_device_data, 94);

pm_supported = word_82 & (1<<3);
pm_enabled = word_85 & (1<<3);
apm_supported = word_83 & (1<<3);
apm_enabled = word_86 & (1<<3);
aam_supported = word_83 & (1<<9);
aam_enabled = word_86 & (1<<9);
if (aam_supported)
aam_vendor_recommended_value = (word_94 >> 8);
write_cache_supported = word_82 & (1<<5);
write_cache_enabled = word_85 & (1<<5);
}

/* added recently, unable to distinguish between "not supported by device"
* and "not implemented by udev"
*/
if (!read_lookahead_supported)
{
read_lookahead_supported = word_82 & (1<<6);
read_lookahead_enabled = word_85 & (1<<6);
}
}

g_object_freeze_notify (G_OBJECT (drive));
udisks_drive_ata_set_pm_supported (UDISKS_DRIVE_ATA (drive), !!pm_supported);
Expand Down Expand Up @@ -329,21 +359,31 @@ update_security (UDisksLinuxDriveAta *drive,
guint16 word_90 = 0;
guint16 word_128 = 0;

/* ATA8: 7.16 IDENTIFY DEVICE - ECh, PIO Data-In - Table 29 IDENTIFY DEVICE data */
word_82 = udisks_ata_identify_get_word (device->ata_identify_device_data, 82);
word_85 = udisks_ata_identify_get_word (device->ata_identify_device_data, 85);
word_89 = udisks_ata_identify_get_word (device->ata_identify_device_data, 89);
word_90 = udisks_ata_identify_get_word (device->ata_identify_device_data, 90);
word_128 = udisks_ata_identify_get_word (device->ata_identify_device_data, 128);

security_supported = word_82 & (1<<1);
security_enabled = word_85 & (1<<1);
if (security_supported)
{
erase_unit = (word_89 & 0xff) * 2;
enhanced_erase_unit = (word_90 & 0xff) * 2;
security_supported = g_udev_device_get_property_as_boolean (device->udev_device, "ID_ATA_FEATURE_SET_SECURITY");
security_enabled = g_udev_device_get_property_as_boolean (device->udev_device, "ID_ATA_FEATURE_SET_SECURITY_ENABLED");
erase_unit = g_udev_device_get_property_as_int (device->udev_device, "ID_ATA_FEATURE_SET_SECURITY_ERASE_UNIT_MIN");
enhanced_erase_unit = g_udev_device_get_property_as_int (device->udev_device, "ID_ATA_FEATURE_SET_SECURITY_ENHANCED_ERASE_UNIT_MIN");
frozen = g_udev_device_get_property_as_boolean (device->udev_device, "ID_ATA_FEATURE_SET_SECURITY_FROZEN");

if (!g_udev_device_get_property_as_boolean (device->udev_device, "ID_ATA")
&& device->ata_identify_device_data)
{
/* ATA8: 7.16 IDENTIFY DEVICE - ECh, PIO Data-In - Table 29 IDENTIFY DEVICE data */
word_82 = udisks_ata_identify_get_word (device->ata_identify_device_data, 82);
word_85 = udisks_ata_identify_get_word (device->ata_identify_device_data, 85);
word_89 = udisks_ata_identify_get_word (device->ata_identify_device_data, 89);
word_90 = udisks_ata_identify_get_word (device->ata_identify_device_data, 90);
word_128 = udisks_ata_identify_get_word (device->ata_identify_device_data, 128);

security_supported = word_82 & (1<<1);
security_enabled = word_85 & (1<<1);
if (security_supported)
{
erase_unit = (word_89 & 0xff) * 2;
enhanced_erase_unit = (word_90 & 0xff) * 2;
}
frozen = word_128 & (1<<3);
}
frozen = word_128 & (1<<3);

g_object_freeze_notify (G_OBJECT (drive));
/* TODO: export Security{Supported,Enabled} properties
Expand Down
3 changes: 2 additions & 1 deletion src/udiskslinuxdriveobject.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -605,7 +605,8 @@ drive_ata_check (UDisksObject *object)
goto out;

device = drive_object->devices->data;
if (device->ata_identify_device_data != NULL || device->ata_identify_packet_device_data != NULL)
if (g_udev_device_get_property_as_boolean (device->udev_device, "ID_ATA") ||
device->ata_identify_device_data != NULL || device->ata_identify_packet_device_data != NULL)
ret = TRUE;

out:
Expand Down

0 comments on commit 98d2896

Please sign in to comment.