From 035490d39eb0b58ca5e138ad3c9f3a5d0aa3f894 Mon Sep 17 00:00:00 2001 From: Richard Hughes Date: Mon, 28 Mar 2022 18:16:06 +0100 Subject: [PATCH] modem-manager: Remove the cache item and use a shadow device instead Fixes https://github.com/fwupd/fwupd/issues/4394 --- plugins/modem-manager/fu-mm-device.c | 74 +++++++------------ plugins/modem-manager/fu-mm-device.h | 26 +------ .../modem-manager/fu-plugin-modem-manager.c | 67 ++++++++++------- 3 files changed, 70 insertions(+), 97 deletions(-) diff --git a/plugins/modem-manager/fu-mm-device.c b/plugins/modem-manager/fu-mm-device.c index 1638f9f05..5d7dd60d4 100644 --- a/plugins/modem-manager/fu-mm-device.c +++ b/plugins/modem-manager/fu-mm-device.c @@ -1776,6 +1776,20 @@ fu_mm_device_setup(FuDevice *device, GError **error) return TRUE; } +static void +fu_mm_device_incorporate(FuDevice *device, FuDevice *donor_device) +{ + FuMmDevice *self = FU_MM_DEVICE(device); + FuMmDevice *donor = FU_MM_DEVICE(donor_device); + + self->update_methods = fu_mm_device_get_update_methods(donor); + self->detach_fastboot_at = g_strdup(fu_mm_device_get_detach_fastboot_at(donor)); + self->inhibition_uid = g_strdup(fu_mm_device_get_inhibition_uid(donor)); + self->port_at_ifnum = fu_mm_device_get_port_at_ifnum(donor); + self->port_qmi_ifnum = fu_mm_device_get_port_qmi_ifnum(donor); + self->port_mbim_ifnum = fu_mm_device_get_port_mbim_ifnum(donor); +} + static void fu_mm_device_set_progress(FuDevice *self, FuProgress *progress) { @@ -1843,6 +1857,7 @@ fu_mm_device_class_init(FuMmDeviceClass *klass) klass_device->write_firmware = fu_mm_device_write_firmware; klass_device->attach = fu_mm_device_attach; klass_device->set_progress = fu_mm_device_set_progress; + klass_device->incorporate = fu_mm_device_incorporate; /** * FuMmDevice::attach-finished: @@ -1873,39 +1888,16 @@ fu_mm_device_new(FuContext *ctx, MMManager *manager, MMObject *omodem) return self; } -FuPluginMmInhibitedDeviceInfo * -fu_plugin_mm_inhibited_device_info_new(FuMmDevice *device) +FuMmDevice * +fu_mm_shadow_device_new(FuMmDevice *device) { - FuPluginMmInhibitedDeviceInfo *info; - - info = g_new0(FuPluginMmInhibitedDeviceInfo, 1); - info->physical_id = g_strdup(fu_device_get_physical_id(FU_DEVICE(device))); - info->vendor = g_strdup(fu_device_get_vendor(FU_DEVICE(device))); - info->name = g_strdup(fu_device_get_name(FU_DEVICE(device))); - info->version = g_strdup(fu_device_get_version(FU_DEVICE(device))); - info->guids = fu_device_get_guids(FU_DEVICE(device)); - info->update_methods = fu_mm_device_get_update_methods(device); - info->detach_fastboot_at = g_strdup(fu_mm_device_get_detach_fastboot_at(device)); - info->port_at_ifnum = fu_mm_device_get_port_at_ifnum(device); - info->port_qmi_ifnum = fu_mm_device_get_port_qmi_ifnum(device); - info->port_mbim_ifnum = fu_mm_device_get_port_mbim_ifnum(device); - info->inhibited_uid = g_strdup(fu_mm_device_get_inhibition_uid(device)); - - return info; -} - -void -fu_plugin_mm_inhibited_device_info_free(FuPluginMmInhibitedDeviceInfo *info) -{ - g_free(info->inhibited_uid); - g_free(info->physical_id); - g_free(info->vendor); - g_free(info->name); - g_free(info->version); - if (info->guids) - g_ptr_array_unref(info->guids); - g_free(info->detach_fastboot_at); - g_free(info); + FuMmDevice *shadow_device = NULL; + shadow_device = g_object_new(FU_TYPE_MM_DEVICE, + "context", + fu_device_get_context(FU_DEVICE(device)), + NULL); + fu_device_incorporate(FU_DEVICE(shadow_device), FU_DEVICE(device)); + return shadow_device; } FuUsbDevice * @@ -1924,23 +1916,13 @@ fu_mm_device_set_usb_device(FuMmDevice *self, FuUsbDevice *usb_device) } FuMmDevice * -fu_mm_device_udev_new(FuContext *ctx, MMManager *manager, FuPluginMmInhibitedDeviceInfo *info) +fu_mm_device_udev_new(FuContext *ctx, MMManager *manager, FuMmDevice *shadow_device) { FuMmDevice *self = g_object_new(FU_TYPE_MM_DEVICE, "context", ctx, NULL); - g_debug("creating udev-based mm device at %s", info->physical_id); + g_debug("creating udev-based mm device at %s", + fu_device_get_physical_id(FU_DEVICE(shadow_device))); self->manager = g_object_ref(manager); - fu_device_set_physical_id(FU_DEVICE(self), info->physical_id); - fu_device_set_vendor(FU_DEVICE(self), info->vendor); - fu_device_set_name(FU_DEVICE(self), info->name); - fu_device_set_version(FU_DEVICE(self), info->version); - self->update_methods = info->update_methods; - self->detach_fastboot_at = g_strdup(info->detach_fastboot_at); - self->port_at_ifnum = info->port_at_ifnum; - self->port_qmi_ifnum = info->port_qmi_ifnum; - - for (guint i = 0; i < info->guids->len; i++) - fu_device_add_guid(FU_DEVICE(self), g_ptr_array_index(info->guids, i)); - + fu_device_incorporate(FU_DEVICE(self), FU_DEVICE(shadow_device)); return self; } diff --git a/plugins/modem-manager/fu-mm-device.h b/plugins/modem-manager/fu-mm-device.h index be70dd083..f82fbfd0c 100644 --- a/plugins/modem-manager/fu-mm-device.h +++ b/plugins/modem-manager/fu-mm-device.h @@ -34,30 +34,10 @@ fu_mm_device_get_port_mbim_ifnum(FuMmDevice *device); MMModemFirmwareUpdateMethod fu_mm_device_get_update_methods(FuMmDevice *device); -/* support for udev-based devices */ -typedef struct { - gchar *inhibited_uid; - gchar *physical_id; - gchar *vendor; - gchar *name; - gchar *version; - GPtrArray *guids; - MMModemFirmwareUpdateMethod update_methods; - gchar *detach_fastboot_at; - gint port_at_ifnum; - gint port_qmi_ifnum; - gint port_mbim_ifnum; -} FuPluginMmInhibitedDeviceInfo; - -FuPluginMmInhibitedDeviceInfo * -fu_plugin_mm_inhibited_device_info_new(FuMmDevice *device); -void -fu_plugin_mm_inhibited_device_info_free(FuPluginMmInhibitedDeviceInfo *info); -G_DEFINE_AUTOPTR_CLEANUP_FUNC(FuPluginMmInhibitedDeviceInfo, - fu_plugin_mm_inhibited_device_info_free); - FuMmDevice * -fu_mm_device_udev_new(FuContext *ctx, MMManager *manager, FuPluginMmInhibitedDeviceInfo *info); +fu_mm_shadow_device_new(FuMmDevice *device); +FuMmDevice * +fu_mm_device_udev_new(FuContext *ctx, MMManager *manager, FuMmDevice *shadow_device); void fu_mm_device_udev_add_port(FuMmDevice *device, const gchar *subsystem, diff --git a/plugins/modem-manager/fu-plugin-modem-manager.c b/plugins/modem-manager/fu-plugin-modem-manager.c index 59082a3dc..cdf83b72f 100644 --- a/plugins/modem-manager/fu-plugin-modem-manager.c +++ b/plugins/modem-manager/fu-plugin-modem-manager.c @@ -31,7 +31,7 @@ struct FuPluginData { /* when a device is inhibited from MM, we store all relevant details * ourselves to recreate a functional device object even without MM */ - FuPluginMmInhibitedDeviceInfo *inhibited; + FuMmDevice *shadow_device; }; static void @@ -40,15 +40,16 @@ fu_plugin_mm_udev_device_removed(FuPlugin *plugin) FuPluginData *priv = fu_plugin_get_data(plugin); FuMmDevice *dev; - if (priv->inhibited == NULL) + if (priv->shadow_device == NULL) return; - dev = fu_plugin_cache_lookup(plugin, priv->inhibited->physical_id); + dev = fu_plugin_cache_lookup(plugin, + fu_device_get_physical_id(FU_DEVICE(priv->shadow_device))); if (dev == NULL) return; /* once the first port is gone, consider device is gone */ - fu_plugin_cache_remove(plugin, priv->inhibited->physical_id); + fu_plugin_cache_remove(plugin, fu_device_get_physical_id(FU_DEVICE(priv->shadow_device))); fu_plugin_device_remove(plugin, FU_DEVICE(dev)); /* no need to wait for more ports, cancel that right away */ @@ -62,17 +63,18 @@ static void fu_plugin_mm_uninhibit_device(FuPlugin *plugin) { FuPluginData *priv = fu_plugin_get_data(plugin); - g_autoptr(FuPluginMmInhibitedDeviceInfo) info = NULL; + g_autoptr(FuMmDevice) shadow_device = NULL; g_clear_object(&priv->udev_client); /* get the device removed from the plugin cache before uninhibiting */ fu_plugin_mm_udev_device_removed(plugin); - info = g_steal_pointer(&priv->inhibited); - if ((priv->manager != NULL) && (info != NULL)) { - g_debug("uninhibit modemmanager device with uid %s", info->inhibited_uid); - mm_manager_uninhibit_device_sync(priv->manager, info->inhibited_uid, NULL, NULL); + shadow_device = g_steal_pointer(&priv->shadow_device); + if (priv->manager != NULL && shadow_device != NULL) { + const gchar *inhibition_uid = fu_mm_device_get_inhibition_uid(shadow_device); + g_debug("uninhibit modemmanager device with uid %s", inhibition_uid); + mm_manager_uninhibit_device_sync(priv->manager, inhibition_uid, NULL, NULL); } } @@ -84,10 +86,11 @@ fu_plugin_mm_udev_device_ports_timeout(gpointer user_data) FuMmDevice *dev; g_autoptr(GError) error = NULL; - g_return_val_if_fail(priv->inhibited != NULL, G_SOURCE_REMOVE); + g_return_val_if_fail(priv->shadow_device != NULL, G_SOURCE_REMOVE); priv->udev_timeout_id = 0; - dev = fu_plugin_cache_lookup(plugin, priv->inhibited->physical_id); + dev = fu_plugin_cache_lookup(plugin, + fu_device_get_physical_id(FU_DEVICE(priv->shadow_device))); if (dev != NULL) { if (!fu_device_probe(FU_DEVICE(dev), &error)) { g_warning("failed to probe MM device: %s", error->message); @@ -104,7 +107,7 @@ fu_plugin_mm_udev_device_ports_timeout_reset(FuPlugin *plugin) { FuPluginData *priv = fu_plugin_get_data(plugin); - g_return_if_fail(priv->inhibited != NULL); + g_return_if_fail(priv->shadow_device != NULL); if (priv->udev_timeout_id != 0) g_source_remove(priv->udev_timeout_id); priv->udev_timeout_id = g_timeout_add_seconds(FU_MM_UDEV_DEVICE_PORTS_TIMEOUT, @@ -122,18 +125,24 @@ fu_plugin_mm_udev_device_port_added(FuPlugin *plugin, FuMmDevice *existing; g_autoptr(FuMmDevice) dev = NULL; - g_return_if_fail(priv->inhibited != NULL); - existing = fu_plugin_cache_lookup(plugin, priv->inhibited->physical_id); + g_return_if_fail(priv->shadow_device != NULL); + + existing = + fu_plugin_cache_lookup(plugin, + fu_device_get_physical_id(FU_DEVICE(priv->shadow_device))); if (existing != NULL) { /* add port to existing device */ fu_mm_device_udev_add_port(existing, subsystem, path, ifnum); fu_plugin_mm_udev_device_ports_timeout_reset(plugin); return; } + /* create device and add to cache */ - dev = fu_mm_device_udev_new(fu_plugin_get_context(plugin), priv->manager, priv->inhibited); + dev = fu_mm_device_udev_new(fu_plugin_get_context(plugin), + priv->manager, + priv->shadow_device); fu_mm_device_udev_add_port(dev, subsystem, path, ifnum); - fu_plugin_cache_add(plugin, priv->inhibited->physical_id, dev); + fu_plugin_cache_add(plugin, fu_device_get_physical_id(FU_DEVICE(priv->shadow_device)), dev); /* wait a bit before probing, in case more ports get added */ fu_plugin_mm_udev_device_ports_timeout_reset(plugin); @@ -154,7 +163,7 @@ fu_plugin_mm_udev_uevent_cb(GUdevClient *udev, g_autofree gchar *device_bus = NULL; gint ifnum = -1; - if (action == NULL || subsystem == NULL || priv->inhibited == NULL || name == NULL) + if (action == NULL || subsystem == NULL || priv->shadow_device == NULL || name == NULL) return TRUE; /* ignore if loading port info fails */ @@ -166,16 +175,17 @@ fu_plugin_mm_udev_uevent_cb(GUdevClient *udev, return TRUE; /* ignore all events for ports not owned by our device */ - if (g_strcmp0(device_sysfs_path, priv->inhibited->physical_id) != 0) + if (g_strcmp0(device_sysfs_path, + fu_device_get_physical_id(FU_DEVICE(priv->shadow_device))) != 0) return TRUE; path = g_strdup_printf("/dev/%s", name); if ((g_str_equal(action, "add")) || (g_str_equal(action, "change"))) { - g_debug("added port to inhibited modem: %s (ifnum %d)", path, ifnum); + g_debug("added port to shadow_device modem: %s (ifnum %d)", path, ifnum); fu_plugin_mm_udev_device_port_added(plugin, subsystem, path, ifnum); } else if (g_str_equal(action, "remove")) { - g_debug("removed port from inhibited modem: %s", path); + g_debug("removed port from shadow_device modem: %s", path); fu_plugin_mm_udev_device_removed(plugin); } @@ -185,20 +195,21 @@ fu_plugin_mm_udev_uevent_cb(GUdevClient *udev, static gboolean fu_plugin_mm_inhibit_device(FuPlugin *plugin, FuDevice *device, GError **error) { + const gchar *inhibition_uid; static const gchar *subsystems[] = {"tty", "usbmisc", "wwan", NULL}; FuPluginData *priv = fu_plugin_get_data(plugin); - g_autoptr(FuPluginMmInhibitedDeviceInfo) info = NULL; + g_autoptr(FuMmDevice) shadow_device = NULL; fu_plugin_mm_uninhibit_device(plugin); - info = fu_plugin_mm_inhibited_device_info_new(FU_MM_DEVICE(device)); - - g_debug("inhibit modemmanager device with uid %s", info->inhibited_uid); - if (!mm_manager_inhibit_device_sync(priv->manager, info->inhibited_uid, NULL, error)) + shadow_device = fu_mm_shadow_device_new(FU_MM_DEVICE(device)); + inhibition_uid = fu_mm_device_get_inhibition_uid(shadow_device); + g_debug("inhibit modemmanager device with uid %s", inhibition_uid); + if (!mm_manager_inhibit_device_sync(priv->manager, inhibition_uid, NULL, error)) return FALSE; - /* setup inhibited device info */ - priv->inhibited = g_steal_pointer(&info); + /* setup shadow_device device info */ + priv->shadow_device = g_steal_pointer(&shadow_device); /* only do modem port monitoring using udev if the module is expected * to reset itself into a fully different layout, e.g. a fastboot device */ @@ -444,7 +455,7 @@ fu_plugin_mm_detach(FuPlugin *plugin, FuDevice *device, FuProgress *progress, GE * lifetime of the FuMmDevice, because that object will only exist for * as long as the ModemManager device exists, and inhibiting will * implicitly remove the device from ModemManager. */ - if (priv->inhibited == NULL) { + if (priv->shadow_device == NULL) { if (!fu_plugin_mm_inhibit_device(plugin, device, error)) return FALSE; }