pool/udisks2
SHA256
2
0
Fork 1
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Accepting request 1040213 from home:tsaupe:branches:Base:System:udisk2-bsc1120608

Browse Source 
avoid wakening spun-down disks on unrelated events (bsc#1120608)

OBS-URL: https://build.opensuse.org/request/show/1040213
OBS-URL: https://build.opensuse.org/package/show/Base:System/udisks2?expand=0&rev=98
This commit is contained in:
Dominique Leuenberger 2022-12-05 11:29:48 +00:00 committed by Git OBS Bridge
parent 4a663ca811
commit 555ca276bb
5 changed files with 533 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

129
0001-udisksata-Move-the-low-level-PM-state-call.patch Normal file
View File

@ -0,0 +1,129 @@
From a7d9b97c9460f65a726b727e9eaee31ea5016538 Mon Sep 17 00:00:00 2001
From: Tomas Bzatek <tbzatek@redhat.com>
Date: Wed, 5 Jan 2022 20:17:55 +0100
Subject: [PATCH] udisksata: Move the low-level PM state call
(cherry picked from commit 4588dbeecd23c17d1cb7f7fa60afd56702acd455)
---
doc/udisks2-sections.txt.daemon.sections.in | 2 +
src/udisksata.c | 62 +++++++++++++++++++++
src/udisksata.h | 13 +++++
3 files changed, 77 insertions(+)
diff --git a/doc/udisks2-sections.txt.daemon.sections.in b/doc/udisks2-sections.txt.daemon.sections.in
index 12935c4d..9a2bfa03 100644
--- a/doc/udisks2-sections.txt.daemon.sections.in
+++ b/doc/udisks2-sections.txt.daemon.sections.in
@@ -270,6 +270,8 @@ UDisksAtaCommandProtocol
UDisksAtaCommandInput
UDisksAtaCommandOutput
udisks_ata_send_command_sync
+udisks_ata_get_pm_state
+UDISKS_ATA_PM_STATE_AWAKE
</SECTION>
<SECTION>
diff --git a/src/udisksata.c b/src/udisksata.c
index 9491af5e..e6da8c35 100644
--- a/src/udisksata.c
+++ b/src/udisksata.c
@@ -308,3 +308,65 @@ udisks_ata_send_command_sync (gint fd,
out:
return ret;
}
+
+/**
+ * udisks_ata_get_pm_state:
+ * @device: ATA drive block device path.
+ * @error: Return location for error.
+ * @pm_state: Return location for the current power state value.
+ *
+ * Get the current power mode state.
+ *
+ * The format of @pm_state is the result obtained from sending the
+ * ATA command `CHECK POWER MODE` to the drive.
+ *
+ * Known values include:
+ * - `0x00`: Device is in PM2: Standby state.
+ * - `0x40`: Device is in the PM0: Active state, the NV Cache power mode is enabled, and the spindle is spun down or spinning down.
+ * - `0x41`: Device is in the PM0: Active state, the NV Cache power mode is enabled, and the spindle is spun up or spinning up.
+ * - `0x80`: Device is in PM1: Idle state.
+ * - `0xff`: Device is in the PM0: Active state or PM1: Idle State.
+ *
+ * Typically user interfaces will report "Drive is spun down" if @pm_state is
+ * 0x00 and "Drive is spun up" otherwise.
+ *
+ * Returns: %TRUE if the operation succeeded, %FALSE if @error is set.
+ */
+gboolean
+udisks_ata_get_pm_state (const gchar *device, GError **error, guchar *count)
+{
+ int fd;
+ gboolean rc = FALSE;
+ /* ATA8: 7.8 CHECK POWER MODE - E5h, Non-Data */
+ UDisksAtaCommandInput input = {.command = 0xe5};
+ UDisksAtaCommandOutput output = {0};
+
+ g_warn_if_fail (device != NULL);
+
+ fd = open (device, O_RDONLY|O_NONBLOCK);
+ if (fd == -1)
+ {
+ g_set_error (error, UDISKS_ERROR, UDISKS_ERROR_FAILED,
+ "Error opening device file %s while getting PM state: %m",
+ device);
+ goto out;
+ }
+
+ if (!udisks_ata_send_command_sync (fd,
+ -1,
+ UDISKS_ATA_COMMAND_PROTOCOL_NONE,
+ &input,
+ &output,
+ error))
+ {
+ g_prefix_error (error, "Error sending ATA command CHECK POWER MODE: ");
+ goto out;
+ }
+ /* count field is used for the state, see ATA8: table 102 */
+ *count = output.count;
+ rc = TRUE;
+ out:
+ if (fd != -1)
+ close (fd);
+ return rc;
+}
diff --git a/src/udisksata.h b/src/udisksata.h
index 1d4918f1..d652f3ab 100644
--- a/src/udisksata.h
+++ b/src/udisksata.h
@@ -73,6 +73,16 @@ struct _UDisksAtaCommandOutput
guchar *buffer;
};
+/**
+ * UDISKS_ATA_PM_STATE_AWAKE:
+ * @pm_state: The power state value.
+ *
+ * Decodes the power state value as returned by #udisks_ata_get_pm_state.
+ *
+ * Returns: %TRUE when the drive is awake, %FALSE when sleeping.
+*/
+#define UDISKS_ATA_PM_STATE_AWAKE(pm_state) (pm_state >= 0x41)
+
gboolean udisks_ata_send_command_sync (gint fd,
gint timeout_msec,
UDisksAtaCommandProtocol protocol,
@@ -80,6 +90,9 @@ gboolean udisks_ata_send_command_sync (gint fd,
UDisksAtaCommandOutput *output,
GError **error);
+gboolean udisks_ata_get_pm_state (const gchar *device,
+ GError **error,
+ guchar *count);
G_END_DECLS
--
2.38.1

277
0001-udiskslinuxfilesystem-Make-the-size-property-retriev.patch Normal file
View File

@ -0,0 +1,277 @@
From 9a2a96b46803b1d76d105f3bed994188b8205133 Mon Sep 17 00:00:00 2001
From: Tomas Bzatek <tbzatek@redhat.com>
Date: Sun, 2 Jan 2022 23:45:12 +0100
Subject: [PATCH] udiskslinuxfilesystem: Make the 'size' property retrieval
on-demand
Filesystem size value retrieval is very expensive as it typically calls
filesystem tools that read superblock -> doing some I/O. Other
filesystem properties are typically retrieved from existing stateful
sources, either udev or sysfs.
This change overrides the gdbus-codegen-generated GDBusInterfaceSkeleton
property retrieval and adds a custom hook that retrieves the filesystem
size value when actually requested.
One limitation of such approach is that the hook is called with
the GDBusObjectManager lock held and thus it needs to be as minimal
as possible and avoiding access to any GDBusObject.
---
src/udiskslinuxfilesystem.c | 129 +++++++++++++++++++++++++++---------
1 file changed, 97 insertions(+), 32 deletions(-)
diff --git a/src/udiskslinuxfilesystem.c b/src/udiskslinuxfilesystem.c
index a8390a04..413a5a37 100644
--- a/src/udiskslinuxfilesystem.c
+++ b/src/udiskslinuxfilesystem.c
@@ -56,6 +56,7 @@
#include "udiskssimplejob.h"
#include "udiskslinuxdriveata.h"
#include "udiskslinuxmountoptions.h"
+#include "udisksata.h"
/**
* SECTION:udiskslinuxfilesystem
@@ -78,6 +79,10 @@ struct _UDisksLinuxFilesystem
{
UDisksFilesystemSkeleton parent_instance;
GMutex lock;
+ guint64 cached_fs_size;
+ gchar *cached_device_file;
+ gchar *cached_fs_type;
+ gboolean cached_drive_is_ata;
};
struct _UDisksLinuxFilesystemClass
@@ -85,7 +90,14 @@ struct _UDisksLinuxFilesystemClass
UDisksFilesystemSkeletonClass parent_class;
};
+enum
+{
+ PROP_0,
+ PROP_SIZE,
+};
+
static void filesystem_iface_init (UDisksFilesystemIface *iface);
+static guint64 get_filesystem_size (UDisksLinuxFilesystem *filesystem);
G_DEFINE_TYPE_WITH_CODE (UDisksLinuxFilesystem, udisks_linux_filesystem, UDISKS_TYPE_FILESYSTEM_SKELETON,
G_IMPLEMENT_INTERFACE (UDISKS_TYPE_FILESYSTEM, filesystem_iface_init));
@@ -106,6 +118,8 @@ udisks_linux_filesystem_finalize (GObject *object)
UDisksLinuxFilesystem *filesystem = UDISKS_LINUX_FILESYSTEM (object);
g_mutex_clear (&(filesystem->lock));
+ g_free (filesystem->cached_device_file);
+ g_free (filesystem->cached_fs_type);
if (G_OBJECT_CLASS (udisks_linux_filesystem_parent_class)->finalize != NULL)
G_OBJECT_CLASS (udisks_linux_filesystem_parent_class)->finalize (object);
@@ -119,6 +133,44 @@ udisks_linux_filesystem_init (UDisksLinuxFilesystem *filesystem)
G_DBUS_INTERFACE_SKELETON_FLAGS_HANDLE_METHOD_INVOCATIONS_IN_THREAD);
}
+static void
+udisks_linux_filesystem_get_property (GObject *object,
+ guint prop_id,
+ GValue *value,
+ GParamSpec *pspec)
+{
+ UDisksLinuxFilesystem *filesystem = UDISKS_LINUX_FILESYSTEM (object);
+
+ switch (prop_id)
+ {
+ case PROP_SIZE:
+ g_value_set_uint64 (value, get_filesystem_size (filesystem));
+ break;
+
+ default:
+ G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec);
+ break;
+ }
+}
+
+static void
+udisks_linux_filesystem_set_property (GObject *object,
+ guint prop_id,
+ const GValue *value,
+ GParamSpec *pspec)
+{
+ switch (prop_id)
+ {
+ case PROP_SIZE:
+ g_warning ("udisks_linux_filesystem_set_property() should never be called, value = %lu", g_value_get_uint64 (value));
+ break;
+
+ default:
+ G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec);
+ break;
+ }
+}
+
static void
udisks_linux_filesystem_class_init (UDisksLinuxFilesystemClass *klass)
{
@@ -126,6 +178,10 @@ udisks_linux_filesystem_class_init (UDisksLinuxFilesystemClass *klass)
gobject_class = G_OBJECT_CLASS (klass);
gobject_class->finalize = udisks_linux_filesystem_finalize;
+ gobject_class->get_property = udisks_linux_filesystem_get_property;
+ gobject_class->set_property = udisks_linux_filesystem_set_property;
+
+ g_object_class_override_property (gobject_class, PROP_SIZE, "size");
}
/**
@@ -144,49 +200,58 @@ udisks_linux_filesystem_new (void)
/* ---------------------------------------------------------------------------------------------------- */
+/* WARNING: called with GDBusObjectManager lock held, avoid any object lookup */
static guint64
-get_filesystem_size (UDisksLinuxBlockObject *object)
+get_filesystem_size (UDisksLinuxFilesystem *filesystem)
{
guint64 size = 0;
- UDisksLinuxDevice *device;
- gchar *dev;
- const gchar *type;
GError *error = NULL;
- device = udisks_linux_block_object_get_device (object);
- dev = udisks_linux_block_object_get_device_file (object);
- type = g_udev_device_get_property (device->udev_device, "ID_FS_TYPE");
+ if (!filesystem->cached_device_file || !filesystem->cached_fs_type)
+ return 0;
+
+ /* if the drive is ATA and is sleeping, skip filesystem size check to prevent
+ * drive waking up - nothing has changed anyway since it's been sleeping...
+ */
+ if (filesystem->cached_drive_is_ata)
+ {
+ guchar pm_state = 0;
+
+ if (udisks_ata_get_pm_state (filesystem->cached_device_file, NULL, &pm_state))
+ if (!UDISKS_ATA_PM_STATE_AWAKE (pm_state) && filesystem->cached_fs_size > 0)
+ return filesystem->cached_fs_size;
+ }
- if (g_strcmp0 (type, "ext2") == 0)
+ if (g_strcmp0 (filesystem->cached_fs_type, "ext2") == 0)
{
- BDFSExt2Info *info = bd_fs_ext2_get_info (dev, &error);
+ BDFSExt2Info *info = bd_fs_ext2_get_info (filesystem->cached_device_file, &error);
if (info)
{
size = info->block_size * info->block_count;
bd_fs_ext2_info_free (info);
}
}
- else if (g_strcmp0 (type, "ext3") == 0)
+ else if (g_strcmp0 (filesystem->cached_fs_type, "ext3") == 0)
{
- BDFSExt3Info *info = bd_fs_ext3_get_info (dev, &error);
+ BDFSExt3Info *info = bd_fs_ext3_get_info (filesystem->cached_device_file, &error);
if (info)
{
size = info->block_size * info->block_count;
bd_fs_ext3_info_free (info);
}
}
- else if (g_strcmp0 (type, "ext4") == 0)
+ else if (g_strcmp0 (filesystem->cached_fs_type, "ext4") == 0)
{
- BDFSExt4Info *info = bd_fs_ext4_get_info (dev, &error);
+ BDFSExt4Info *info = bd_fs_ext4_get_info (filesystem->cached_device_file, &error);
if (info)
{
size = info->block_size * info->block_count;
bd_fs_ext4_info_free (info);
}
}
- else if (g_strcmp0 (type, "xfs") == 0)
+ else if (g_strcmp0 (filesystem->cached_fs_type, "xfs") == 0)
{
- BDFSXfsInfo *info = bd_fs_xfs_get_info (dev, &error);
+ BDFSXfsInfo *info = bd_fs_xfs_get_info (filesystem->cached_device_file, &error);
if (info)
{
size = info->block_size * info->block_count;
@@ -194,10 +259,9 @@ get_filesystem_size (UDisksLinuxBlockObject *object)
}
}
- g_free (dev);
- g_object_unref (device);
g_clear_error (&error);
+ filesystem->cached_fs_size = size;
return size;
}
@@ -234,14 +298,12 @@ void
udisks_linux_filesystem_update (UDisksLinuxFilesystem *filesystem,
UDisksLinuxBlockObject *object)
{
+ UDisksDriveAta *ata = NULL;
UDisksMountMonitor *mount_monitor;
UDisksLinuxDevice *device;
- UDisksDriveAta *ata = NULL;
GPtrArray *p;
GList *mounts;
GList *l;
- gboolean skip_fs_size = FALSE;
- guchar pm_state;
mount_monitor = udisks_daemon_get_mount_monitor (udisks_linux_block_object_get_daemon (object));
device = udisks_linux_block_object_get_device (object);
@@ -263,20 +325,24 @@ udisks_linux_filesystem_update (UDisksLinuxFilesystem *filesystem,
g_ptr_array_free (p, TRUE);
g_list_free_full (mounts, g_object_unref);
- /* if the drive is ATA and is sleeping, skip filesystem size check to prevent
- * drive waking up - nothing has changed anyway since it's been sleeping...
+ /* cached device properties for on-demand filesystem size retrieval */
+ g_free (filesystem->cached_device_file);
+ g_free (filesystem->cached_fs_type);
+ filesystem->cached_fs_type = g_strdup (g_udev_device_get_property (device->udev_device, "ID_FS_TYPE"));
+ if (g_strcmp0 (filesystem->cached_fs_type, "ext2") == 0 ||
+ g_strcmp0 (filesystem->cached_fs_type, "ext3") == 0 ||
+ g_strcmp0 (filesystem->cached_fs_type, "ext4") == 0 ||
+ g_strcmp0 (filesystem->cached_fs_type, "xfs") == 0)
+ filesystem->cached_device_file = udisks_linux_block_object_get_device_file (object);
+
+ /* TODO: this only looks for a drive object associated with the current
+ * block object. In case of a complex layered structure this needs to walk
+ * the tree and return a list of physical drives to check the powermanagement on.
*/
ata = get_drive_ata (object);
- if (ata != NULL)
- {
- if (udisks_linux_drive_ata_get_pm_state (UDISKS_LINUX_DRIVE_ATA (ata), NULL, &pm_state))
- skip_fs_size = ! UDISKS_LINUX_DRIVE_ATA_IS_AWAKE (pm_state);
- }
+ filesystem->cached_drive_is_ata = ata != NULL && udisks_drive_ata_get_pm_supported (ata);
g_clear_object (&ata);
- if (! skip_fs_size)
- udisks_filesystem_set_size (UDISKS_FILESYSTEM (filesystem), get_filesystem_size (object));
-
g_dbus_interface_skeleton_flush (G_DBUS_INTERFACE_SKELETON (filesystem));
g_object_unref (device);
@@ -1872,10 +1938,9 @@ handle_resize (UDisksFilesystem *filesystem,
/* At least resize2fs might need another uevent after it is done.
*/
+ UDISKS_LINUX_FILESYSTEM (filesystem)->cached_fs_size = 0;
udisks_linux_block_object_trigger_uevent_sync (UDISKS_LINUX_BLOCK_OBJECT (object),
UDISKS_DEFAULT_WAIT_TIMEOUT);
-
- udisks_filesystem_set_size (filesystem, get_filesystem_size (UDISKS_LINUX_BLOCK_OBJECT (object)));
g_dbus_interface_skeleton_flush (G_DBUS_INTERFACE_SKELETON (filesystem));
udisks_filesystem_complete_resize (filesystem, invocation);
udisks_simple_job_complete (UDISKS_SIMPLE_JOB (job), TRUE, NULL);
--
2.38.1

115
0001-udiskslinuxprovider-Only-update-related-objects-on-u.patch Normal file
View File

@ -0,0 +1,115 @@
From ec380135ed8cf57a70501542081dad51d2d11fa8 Mon Sep 17 00:00:00 2001
From: Tomas Bzatek <tbzatek@redhat.com>
Date: Thu, 6 Jan 2022 20:45:45 +0100
Subject: [PATCH] udiskslinuxprovider: Only update related objects on utab
changes
Updating all existing block objects on any utab change
was unnecessary and overly expensive. With the UDisksUtabMonitor
providing specific utab entry, update just block objects
with matching device file.
Note that there is a room for similar optimization in fstab
and crypttab monitoring.
---
src/udiskslinuxprovider.c | 33 ++++++++++++++++++++++++++-------
1 file changed, 26 insertions(+), 7 deletions(-)
diff --git a/src/udiskslinuxprovider.c b/src/udiskslinuxprovider.c
index cfc6a330..4231a33c 100644
--- a/src/udiskslinuxprovider.c
+++ b/src/udiskslinuxprovider.c
@@ -39,6 +39,7 @@
#include "udisksmoduleobject.h"
#include "udisksdaemonutil.h"
#include "udisksconfigmanager.h"
+#include "udisksutabentry.h"
/**
* SECTION:udiskslinuxprovider
@@ -1559,7 +1560,7 @@ on_housekeeping_timeout (gpointer user_data)
/* ---------------------------------------------------------------------------------------------------- */
static void
-update_all_block_objects (UDisksLinuxProvider *provider)
+update_block_objects (UDisksLinuxProvider *provider, const gchar *device_path)
{
GList *objects;
GList *l;
@@ -1572,18 +1573,36 @@ update_all_block_objects (UDisksLinuxProvider *provider)
for (l = objects; l != NULL; l = l->next)
{
UDisksLinuxBlockObject *object = UDISKS_LINUX_BLOCK_OBJECT (l->data);
- udisks_linux_block_object_uevent (object, "change", NULL);
+
+ if (device_path == NULL)
+ udisks_linux_block_object_uevent (object, "change", NULL);
+ else
+ {
+ gchar *block_dev;
+ gboolean match;
+
+ block_dev = udisks_linux_block_object_get_device_file (object);
+ match = g_strcmp0 (block_dev, device_path) == 0;
+ g_free (block_dev);
+ if (match)
+ {
+ udisks_linux_block_object_uevent (object, "change", NULL);
+ break;
+ }
+ }
}
g_list_free_full (objects, g_object_unref);
}
+/* fstab monitoring */
static void
mount_monitor_on_mountpoints_changed (GUnixMountMonitor *monitor,
gpointer user_data)
{
UDisksLinuxProvider *provider = UDISKS_LINUX_PROVIDER (user_data);
- update_all_block_objects (provider);
+ /* TODO: compare differences and only update relevant objects */
+ update_block_objects (provider, NULL);
}
static void
@@ -1592,7 +1611,7 @@ crypttab_monitor_on_entry_added (UDisksCrypttabMonitor *monitor,
gpointer user_data)
{
UDisksLinuxProvider *provider = UDISKS_LINUX_PROVIDER (user_data);
- update_all_block_objects (provider);
+ update_block_objects (provider, NULL);
}
static void
@@ -1601,7 +1620,7 @@ crypttab_monitor_on_entry_removed (UDisksCrypttabMonitor *monitor,
gpointer user_data)
{
UDisksLinuxProvider *provider = UDISKS_LINUX_PROVIDER (user_data);
- update_all_block_objects (provider);
+ update_block_objects (provider, NULL);
}
#ifdef HAVE_LIBMOUNT_UTAB
@@ -1611,7 +1630,7 @@ utab_monitor_on_entry_added (UDisksUtabMonitor *monitor,
gpointer user_data)
{
UDisksLinuxProvider *provider = UDISKS_LINUX_PROVIDER (user_data);
- update_all_block_objects (provider);
+ update_block_objects (provider, udisks_utab_entry_get_source (entry));
}
static void
@@ -1620,6 +1639,6 @@ utab_monitor_on_entry_removed (UDisksUtabMonitor *monitor,
gpointer user_data)
{
UDisksLinuxProvider *provider = UDISKS_LINUX_PROVIDER (user_data);
- update_all_block_objects (provider);
+ update_block_objects (provider, udisks_utab_entry_get_source (entry));
}
#endif
--
2.38.1

9
udisks2.changes
View File

@ -1,3 +1,12 @@
-------------------------------------------------------------------
Tue Nov 22 08:21:22 UTC 2022 - Thomas Blume <thomas.blume@suse.com>
- avoid wakening spun-down disks on unrelated events (bsc#1120608)
* add:
0001-udisksata-Move-the-low-level-PM-state-call.patch
0001-udiskslinuxfilesystem-Make-the-size-property-retriev.patch
0001-udiskslinuxprovider-Only-update-related-objects-on-u.patch
-------------------------------------------------------------------
Wed Nov 16 08:14:50 UTC 2022 - Thomas Blume <thomas.blume@suse.com>

3
udisks2.spec
View File

@ -33,6 +33,9 @@ Source0: %{url}/releases/download/udisks-%{version}/udisks-%{version}.tar
Patch0: harden_udisks2-zram-setup@.service.patch
Patch1: harden_udisks2.service.patch
Patch2: 0001-udiskslinuxmountoptions-Do-not-free-static-daemon-re.patch
Patch3: 0001-udisksata-Move-the-low-level-PM-state-call.patch
Patch4: 0001-udiskslinuxfilesystem-Make-the-size-property-retriev.patch
Patch5: 0001-udiskslinuxprovider-Only-update-related-objects-on-u.patch
BuildRequires: chrpath
BuildRequires: docbook-xsl-stylesheets
BuildRequires: gobject-introspection-devel >= 0.6.2