From 81c427ca6d6fbd1bac2d603642cd64122fcbf45c Mon Sep 17 00:00:00 2001 From: Richard Hughes Date: Mon, 6 Aug 2018 15:20:17 +0100 Subject: [PATCH] Allow different plugins to add the same device In this instance, we define the 'same device' to be a FuDevice that has at least one matching GUID. We allow the plugins to define which one is 'better' than other plugins, and use this to only have one FuDevice for the physical device. Alternative to https://github.com/hughsie/fwupd/pull/604 --- src/fu-device-list.c | 27 ++++++++++++++++++++++++++ src/fu-device-private.h | 3 +++ src/fu-device.c | 34 ++++++++++++++++++++++++++++++++ src/fu-engine.c | 1 + src/fu-plugin-list.c | 30 ++++++++++++++++++++++++++++ src/fu-plugin-private.h | 3 +++ src/fu-plugin.c | 30 ++++++++++++++++++++++++++++ src/fu-plugin.h | 2 ++ src/fu-self-test.c | 43 +++++++++++++++++++++++++++++++++++++++++ 9 files changed, 173 insertions(+) diff --git a/src/fu-device-list.c b/src/fu-device-list.c index 0f7fb81d7..6f7f307dd 100644 --- a/src/fu-device-list.c +++ b/src/fu-device-list.c @@ -436,6 +436,33 @@ fu_device_list_add (FuDeviceList *self, FuDevice *device) return; } + /* added the same device from a different plugin */ + if (item != NULL && g_strcmp0 (fu_device_get_plugin (item->device), + fu_device_get_plugin (device)) != 0) { + if (fu_device_get_priority (device) < fu_device_get_priority (item->device)) { + g_debug ("ignoring device %s [%s] as better device %s [%s] already exists", + fu_device_get_id (device), + fu_device_get_plugin (device), + fu_device_get_id (item->device), + fu_device_get_plugin (item->device)); + return; + } + if (fu_device_get_priority (device) == fu_device_get_priority (item->device)) { + g_warning ("ignoring device %s [%s] existing device %s [%s] already exists", + fu_device_get_id (device), + fu_device_get_plugin (device), + fu_device_get_id (item->device), + fu_device_get_plugin (item->device)); + return; + } + g_debug ("removing device %s [%s] as better device %s [%s] added", + fu_device_get_id (item->device), + fu_device_get_plugin (item->device), + fu_device_get_id (device), + fu_device_get_plugin (device)); + fu_device_list_remove (self, item->device); + } + /* add helper */ item = g_new0 (FuDeviceItem, 1); item->self = self; /* no ref */ diff --git a/src/fu-device-private.h b/src/fu-device-private.h index 7832b04b7..38dec12f9 100644 --- a/src/fu-device-private.h +++ b/src/fu-device-private.h @@ -18,6 +18,9 @@ gboolean fu_device_has_parent_guid (FuDevice *device, guint fu_device_get_order (FuDevice *device); void fu_device_set_order (FuDevice *device, guint order); +guint fu_device_get_priority (FuDevice *device); +void fu_device_set_priority (FuDevice *device, + guint priority); void fu_device_set_alternate (FuDevice *device, FuDevice *alternate); diff --git a/src/fu-device.c b/src/fu-device.c index a205ba2e7..953eefc49 100644 --- a/src/fu-device.c +++ b/src/fu-device.c @@ -39,6 +39,7 @@ typedef struct { FwupdStatus status; guint progress; guint order; + guint priority; } FuDevicePrivate; enum { @@ -137,6 +138,39 @@ fu_device_set_order (FuDevice *device, guint order) priv->order = order; } +/** + * fu_device_get_priority: + * @device: a #FuPlugin + * + * Gets the device priority, where higher numbers are better. + * + * Returns: the integer value + * + * Since: 1.1.1 + **/ +guint +fu_device_get_priority (FuDevice *device) +{ + FuDevicePrivate *priv = fu_device_get_instance_private (device); + return priv->priority; +} + +/** + * fu_device_set_priority: + * @device: a #FuDevice + * @priority: a integer value + * + * Sets the device priority, where higher numbers are better. + * + * Since: 1.1.1 + **/ +void +fu_device_set_priority (FuDevice *device, guint priority) +{ + FuDevicePrivate *priv = fu_device_get_instance_private (device); + priv->priority = priority; +} + const gchar * fu_device_get_equivalent_id (FuDevice *device) { diff --git a/src/fu-engine.c b/src/fu-engine.c index 3bf3ae07e..9bd282452 100644 --- a/src/fu-engine.c +++ b/src/fu-engine.c @@ -2813,6 +2813,7 @@ fu_engine_plugin_device_added_cb (FuPlugin *plugin, gpointer user_data) { FuEngine *self = (FuEngine *) user_data; + fu_device_set_priority (device, fu_plugin_get_priority (plugin)); fu_engine_add_device (self, device); } diff --git a/src/fu-plugin-list.c b/src/fu-plugin-list.c index ab8199cf1..e2dd40450 100644 --- a/src/fu-plugin-list.c +++ b/src/fu-plugin-list.c @@ -198,6 +198,36 @@ fu_plugin_list_depsolve (FuPluginList *self, GError **error) } } + /* set priority as well */ + for (guint i = 0; i < self->plugins->len; i++) { + FuPlugin *plugin = g_ptr_array_index (self->plugins, i); + deps = fu_plugin_get_rules (plugin, FU_PLUGIN_RULE_BETTER_THAN); + for (guint j = 0; j < deps->len && !changes; j++) { + const gchar *plugin_name = g_ptr_array_index (deps, j); + dep = fu_plugin_list_find_by_name (self, plugin_name, NULL); + if (dep == NULL) { + g_debug ("cannot find plugin '%s' " + "referenced by '%s'", + plugin_name, + fu_plugin_get_name (plugin)); + continue; + } + if (!fu_plugin_get_enabled (dep)) + continue; + if (fu_plugin_get_priority (plugin) <= fu_plugin_get_priority (dep)) { + g_debug ("%s [%u] better than %s [%u] " + "so bumping to [%u]", + fu_plugin_get_name (plugin), + fu_plugin_get_priority (plugin), + fu_plugin_get_name (dep), + fu_plugin_get_priority (dep), + fu_plugin_get_priority (dep) + 1); + fu_plugin_set_priority (plugin, fu_plugin_get_priority (dep) + 1); + changes = TRUE; + } + } + } + /* check we're not stuck */ if (dep_loop_check++ > 100) { g_set_error_literal (error, diff --git a/src/fu-plugin-private.h b/src/fu-plugin-private.h index f7c352eb1..8af9744c0 100644 --- a/src/fu-plugin-private.h +++ b/src/fu-plugin-private.h @@ -35,6 +35,9 @@ void fu_plugin_set_smbios (FuPlugin *plugin, guint fu_plugin_get_order (FuPlugin *plugin); void fu_plugin_set_order (FuPlugin *plugin, guint order); +guint fu_plugin_get_priority (FuPlugin *plugin); +void fu_plugin_set_priority (FuPlugin *plugin, + guint priority); void fu_plugin_set_name (FuPlugin *plugin, const gchar *name); GPtrArray *fu_plugin_get_rules (FuPlugin *plugin, diff --git a/src/fu-plugin.c b/src/fu-plugin.c index 3f7c00364..8adda50b7 100644 --- a/src/fu-plugin.c +++ b/src/fu-plugin.c @@ -39,6 +39,7 @@ typedef struct { GUsbContext *usb_ctx; gboolean enabled; guint order; + guint priority; GPtrArray *rules[FU_PLUGIN_RULE_LAST]; gchar *name; FuHwids *hwids; @@ -1475,6 +1476,35 @@ fu_plugin_set_order (FuPlugin *plugin, guint order) priv->order = order; } +/** + * fu_plugin_get_priority: + * @plugin: a #FuPlugin + * + * Gets the plugin priority, where higher numbers are better. + * + * Returns: the integer value + **/ +guint +fu_plugin_get_priority (FuPlugin *plugin) +{ + FuPluginPrivate *priv = fu_plugin_get_instance_private (plugin); + return priv->priority; +} + +/** + * fu_plugin_set_priority: + * @plugin: a #FuPlugin + * @priority: a integer value + * + * Sets the plugin priority, where higher numbers are better. + **/ +void +fu_plugin_set_priority (FuPlugin *plugin, guint priority) +{ + FuPluginPrivate *priv = fu_plugin_get_instance_private (plugin); + priv->priority = priority; +} + /** * fu_plugin_add_rule: * @plugin: a #FuPlugin diff --git a/src/fu-plugin.h b/src/fu-plugin.h index 4014cd047..9cde5e5ef 100644 --- a/src/fu-plugin.h +++ b/src/fu-plugin.h @@ -64,6 +64,7 @@ typedef enum { * @FU_PLUGIN_RULE_RUN_AFTER: Order the plugin after another * @FU_PLUGIN_RULE_RUN_BEFORE: Order the plugin before another * @FU_PLUGIN_RULE_REQUIRES_QUIRK: Requires a specific quirk + * @FU_PLUGIN_RULE_BETTER_THAN: Is better than another plugin * * The rules used for ordering plugins. * Plugins are expected to add rules in fu_plugin_initialize(). @@ -73,6 +74,7 @@ typedef enum { FU_PLUGIN_RULE_RUN_AFTER, FU_PLUGIN_RULE_RUN_BEFORE, FU_PLUGIN_RULE_REQUIRES_QUIRK, + FU_PLUGIN_RULE_BETTER_THAN, /*< private >*/ FU_PLUGIN_RULE_LAST } FuPluginRule; diff --git a/src/fu-self-test.c b/src/fu-self-test.c index d2effc491..8fc4c58c4 100644 --- a/src/fu-self-test.c +++ b/src/fu-self-test.c @@ -186,6 +186,48 @@ fu_engine_requirements_device_func (void) g_assert (ret); } +static void +fu_engine_device_priority_func (void) +{ + g_autoptr(FuDevice) device1 = fu_device_new (); + g_autoptr(FuDevice) device2 = fu_device_new (); + g_autoptr(FuDevice) device3 = fu_device_new (); + g_autoptr(FuDevice) device = NULL; + g_autoptr(FuEngine) engine = fu_engine_new (FU_APP_FLAGS_NONE); + g_autoptr(GError) error = NULL; + + /* add low prio then high then low */ + fu_device_set_id (device1, "id1"); + fu_device_set_priority (device1, 0); + fu_device_set_plugin (device1, "udev"); + fu_device_add_guid (device1, "GUID1"); + fu_engine_add_device (engine, device1); + fu_device_set_id (device2, "id2"); + fu_device_set_priority (device2, 1); + fu_device_set_plugin (device2, "redfish"); + fu_device_add_guid (device2, "GUID1"); + fu_engine_add_device (engine, device2); + fu_device_set_id (device3, "id3"); + fu_device_set_priority (device3, 0); + fu_device_set_plugin (device3, "uefi"); + fu_device_add_guid (device3, "GUID1"); + fu_engine_add_device (engine, device3); + + /* get the high prio device */ + device = fu_engine_get_device (engine, "867d5f8110f8aa79dd63d7440f21724264f10430", &error); + g_assert_no_error (error); + g_assert_cmpint (fu_device_get_priority (device), ==, 1); + + /* the now-removed low-prio device */ + device = fu_engine_get_device (engine, "4e89d81a2e6fb4be2578d245fd8511c1f4ad0b58", &error); + g_assert_error (error, FWUPD_ERROR, FWUPD_ERROR_NOT_FOUND); + g_clear_error (&error); + + /* the never-added 2nd low-prio device */ + device = fu_engine_get_device (engine, "c48feddbbcfee514f530ce8f7f2dccd98b6cc150", &error); + g_assert_error (error, FWUPD_ERROR, FWUPD_ERROR_NOT_FOUND); +} + static void fu_engine_device_parent_func (void) { @@ -2271,6 +2313,7 @@ main (int argc, char **argv) g_test_add_func ("/fwupd/engine{requirements-unsupported}", fu_engine_requirements_unsupported_func); g_test_add_func ("/fwupd/engine{requirements-device}", fu_engine_requirements_device_func); g_test_add_func ("/fwupd/engine{device-auto-parent}", fu_engine_device_parent_func); + g_test_add_func ("/fwupd/engine{device-priority}", fu_engine_device_priority_func); g_test_add_func ("/fwupd/hwids", fu_hwids_func); g_test_add_func ("/fwupd/smbios", fu_smbios_func); g_test_add_func ("/fwupd/smbios3", fu_smbios3_func);