dell: Adjust for changes in daemon around USB removal

1) Switch to daemon provided vfuncs for USB
2) Set quirks so that the plugin only runs when Realtek NIC shows up
3) Rely on the daemon to process all removals by parent tree

plugins/dell/dell.quirk

@@ -1,3 +1,7 @@
+# Realtek NIC in Dell docks
+[DeviceInstanceId=USB\VID_0BDA&PID_8153]
+Plugin = dell
+
 [SmbiosManufacturer=Dell Inc.]
 UefiVersionFormat = quad
 

plugins/dell/fu-plugin-dell.c

@@ -254,20 +254,6 @@ fu_plugin_dell_get_version_format (FuPlugin *plugin)
 	return AS_VERSION_PARSE_FLAG_USE_TRIPLET;
 }
 
-static gchar *
-fu_plugin_get_dock_key (FuPlugin *plugin,
-			GUsbDevice *device, const gchar *guid)
-{
-	FuPluginData *data = fu_plugin_get_data (plugin);
-	const gchar* platform_id;
-
-	if (data->smi_obj->fake_smbios)
-		platform_id = "fake";
-	else
-		platform_id = g_usb_device_get_platform_id (device);
-	return g_strdup_printf ("%s_%s", platform_id, guid);
-}
-
 static gboolean
 fu_plugin_dell_capsule_supported (FuPlugin *plugin)
 {
@@ -277,13 +263,12 @@ fu_plugin_dell_capsule_supported (FuPlugin *plugin)
 }
 
 static gboolean
-fu_plugin_dock_node (FuPlugin *plugin, GUsbDevice *device,
+fu_plugin_dock_node (FuPlugin *plugin, const gchar *platform,
 		     guint8 type, const gchar *component_guid,
 		     const gchar *component_desc, const gchar *version)
 {
 	const gchar *dock_type;
 	g_autofree gchar *dock_id = NULL;
-	g_autofree gchar *dock_key = NULL;
 	g_autofree gchar *dock_name = NULL;
 	g_autoptr(FuDevice) dev = NULL;
 
@@ -293,13 +278,8 @@ fu_plugin_dock_node (FuPlugin *plugin, GUsbDevice *device,
 		return FALSE;
 	}
 
-	dock_key = fu_plugin_get_dock_key (plugin, device, component_guid);
-	if (fu_plugin_cache_lookup (plugin, dock_key) != NULL) {
-		g_debug ("%s is already registered.", dock_key);
-		return FALSE;
-	}
-
 	dev = fu_device_new ();
+	fu_device_set_platform_id (dev, platform);
 	dock_id = g_strdup_printf ("DELL-%s" G_GUINT64_FORMAT, component_guid);
 	if (component_desc != NULL) {
 		dock_name = g_strdup_printf ("Dell %s %s", dock_type,
@@ -332,15 +312,13 @@ fu_plugin_dock_node (FuPlugin *plugin, GUsbDevice *device,
 	}
 
 	fu_plugin_device_register (plugin, dev);
-	fu_plugin_cache_add (plugin, dock_key, dev);
 	return TRUE;
 }
 
-
-void
-fu_plugin_dell_device_added_cb (GUsbContext *ctx,
+gboolean
+fu_plugin_usb_device_added (FuPlugin *plugin,
 			    GUsbDevice *device,
-				FuPlugin *plugin)
+			    GError **error)
 {
 	FuPluginData *data = fu_plugin_get_data (plugin);
 	AsVersionParseFlag parse_flags;
@@ -349,6 +327,7 @@ fu_plugin_dell_device_added_cb (GUsbContext *ctx,
 	const gchar *query_str;
 	const gchar *component_guid = NULL;
 	const gchar *component_name = NULL;
+	const gchar *platform;
 	DOCK_UNION buf;
 	DOCK_INFO *dock_info;
 	gboolean old_ec = FALSE;
@@ -359,25 +338,32 @@ fu_plugin_dell_device_added_cb (GUsbContext *ctx,
 	if (!data->smi_obj->fake_smbios) {
 		vid = g_usb_device_get_vid (device);
 		pid = g_usb_device_get_pid (device);
+		platform = g_usb_device_get_platform_id (device);
 	} else {
 		vid = data->fake_vid;
 		pid = data->fake_pid;
+		platform = "fake";
 	}
 
 	/* we're going to match on the Realtek NIC in the dock */
-	if (vid != DOCK_NIC_VID || pid != DOCK_NIC_PID)
-		return;
+	if (vid != DOCK_NIC_VID || pid != DOCK_NIC_PID) {
+		g_set_error (error, FWUPD_ERROR, FWUPD_ERROR_NOT_SUPPORTED,
+			     "wrong VID/PID %04x:%04x", vid, pid);
+		return FALSE;
+	}
 
 	buf.buf = NULL;
 	if (!fu_dell_query_dock (data->smi_obj, &buf)) {
-		g_debug ("No dock detected.");
-		return;
+		g_set_error_literal (error, FWUPD_ERROR, FWUPD_ERROR_NOT_SUPPORTED,
+				     "no dock detected");
+		return FALSE;
 	}
 
 	if (buf.record->dock_info_header.dir_version != 1) {
-		g_debug ("Dock info header version unknown: %d",
+		g_set_error (error, FWUPD_ERROR, FWUPD_ERROR_NOT_SUPPORTED,
+			     "dock info header version unknown %d",
 			     buf.record->dock_info_header.dir_version);
-		return;
+		return FALSE;
 	}
 
 	dock_info = &buf.record->dock_info;
@@ -403,15 +389,16 @@ fu_plugin_dell_device_added_cb (GUsbContext *ctx,
 		query_str = g_strrstr (dock_info->components[i].description,
 				       "Query ");
 		if (query_str == NULL) {
-			g_debug ("Invalid dock component request");
-			return;
+			g_set_error_literal (error, FWUPD_ERROR, FWUPD_ERROR_NOT_SUPPORTED,
+					     "invalid dock component request");
+			return FALSE;
 		}
 		if (!fu_plugin_dell_match_dock_component (query_str + 6,
 							  &component_guid,
 							  &component_name)) {
-			g_debug ("Unable to match dock component %s",
-				query_str);
-			return;
+			g_set_error (error, FWUPD_ERROR, FWUPD_ERROR_NOT_SUPPORTED,
+				     "invalid dock component request %s", query_str);
+			return FALSE;
 		}
 
 		/* dock EC hasn't been updated for first time */
@@ -430,13 +417,14 @@ fu_plugin_dell_device_added_cb (GUsbContext *ctx,
 		fw_str = as_utils_version_from_uint32 (dock_info->components[i].fw_version,
 						       parse_flags);
 		if (!fu_plugin_dock_node (plugin,
-						 device,
+					  platform,
 					  buf.record->dock_info_header.dock_type,
 					  component_guid,
 					  component_name,
 					  fw_str)) {
-			g_debug ("Failed to create %s", component_name);
-			return;
+			g_set_error (error, FWUPD_ERROR, FWUPD_ERROR_INTERNAL,
+				     "failed to create %s", component_name);
+			return FALSE;
 		}
 	}
 
@@ -445,61 +433,21 @@ fu_plugin_dell_device_added_cb (GUsbContext *ctx,
 		flash_ver_str = as_utils_version_from_uint32 (dock_info->flash_pkg_version,
 							      parse_flags);
 	if (!fu_plugin_dock_node (plugin,
-				  device,
+				  platform,
 				  buf.record->dock_info_header.dock_type,
 				  DOCK_FLASH_GUID,
 				  NULL,
 				  flash_ver_str)) {
-		g_debug ("Failed to create top dock node");
-		return;
+		g_set_error_literal (error, FWUPD_ERROR, FWUPD_ERROR_INTERNAL,
+				    "failed to create top dock node");
+
+		return FALSE;
 	}
 
 #if defined (HAVE_SYNAPTICS)
 	fu_plugin_request_recoldplug (plugin);
 #endif
-}
-
-void
-fu_plugin_dell_device_removed_cb (GUsbContext *ctx,
-				  GUsbDevice *device,
-				  FuPlugin *plugin)
-{
-	FuPluginData *data = fu_plugin_get_data (plugin);
-	const gchar *guids[] = { WD15_EC_GUID, TB16_EC_GUID, TB16_PC2_GUID,
-				 TB16_PC1_GUID, WD15_PC1_GUID,
-				 LEGACY_CBL_GUID, UNIV_CBL_GUID,
-				 TBT_CBL_GUID, DOCK_FLASH_GUID};
-	guint16 pid;
-	guint16 vid;
-	FuDevice *dev = NULL;
-
-	if (!data->smi_obj->fake_smbios) {
-		vid = g_usb_device_get_vid (device);
-		pid = g_usb_device_get_pid (device);
-	} else {
-		vid = data->fake_vid;
-		pid = data->fake_pid;
-	}
-
-	/* we're going to match on the Realtek NIC in the dock */
-	if (vid != DOCK_NIC_VID || pid != DOCK_NIC_PID)
-		return;
-
-	/* remove any components already in database? */
-	for (guint i = 0; i < G_N_ELEMENTS (guids); i++) {
-		g_autofree gchar *dock_key = NULL;
-		dock_key = fu_plugin_get_dock_key (plugin, device,
-							  guids[i]);
-		dev = fu_plugin_cache_lookup (plugin, dock_key);
-		if (dev != NULL) {
-			fu_plugin_device_remove (plugin,
-						   dev);
-			fu_plugin_cache_remove (plugin, dock_key);
-		}
-	}
-#if defined (HAVE_SYNAPTICS)
-	fu_plugin_request_recoldplug (plugin);
-#endif
+	return TRUE;
 }
 
 gboolean
@@ -914,6 +862,7 @@ fu_plugin_init (FuPlugin *plugin)
 	data->smi_obj->fake_smbios = FALSE;
 	if (g_getenv ("FWUPD_DELL_FAKE_SMBIOS") != NULL)
 		data->smi_obj->fake_smbios = TRUE;
+	fu_plugin_add_rule (plugin, FU_PLUGIN_RULE_REQUIRES_QUIRK, FU_QUIRKS_PLUGIN);
 }
 
 void
@@ -929,7 +878,6 @@ gboolean
 fu_plugin_startup (FuPlugin *plugin, GError **error)
 {
 	FuPluginData *data = fu_plugin_get_data (plugin);
-	GUsbContext *usb_ctx = fu_plugin_get_usb_context (plugin);
 	g_autofree gchar *sysfsfwdir = NULL;
 	g_autofree gchar *esrtdir = NULL;
 
@@ -969,15 +917,6 @@ fu_plugin_startup (FuPlugin *plugin, GError **error)
 		g_debug ("UEFI capsule firmware updating not supported");
 	}
 
-	if (usb_ctx != NULL) {
-		g_signal_connect (usb_ctx, "device-added",
-				  G_CALLBACK (fu_plugin_dell_device_added_cb),
-				  plugin);
-		g_signal_connect (usb_ctx, "device-removed",
-				  G_CALLBACK (fu_plugin_dell_device_removed_cb),
-				  plugin);
-	}
-
 	return TRUE;
 }
 

plugins/dell/fu-plugin-dell.h

@@ -27,16 +27,6 @@ fu_plugin_dell_inject_fake_data (FuPlugin *plugin,
 gboolean
 fu_plugin_dell_detect_tpm (FuPlugin *plugin, GError **error);
 
-void
-fu_plugin_dell_device_added_cb (GUsbContext *ctx,
-				GUsbDevice *device,
-				FuPlugin *plugin);
-
-void
-fu_plugin_dell_device_removed_cb (GUsbContext *ctx,
-				  GUsbDevice *device,
-				  FuPlugin *plugin);
-
 /* These are nodes that will indicate information about
  * the TPM status
  */

plugins/dell/fu-self-test.c

@@ -14,6 +14,7 @@
 #include "fu-device-private.h"
 #include "fu-plugin-private.h"
 #include "fu-plugin-dell.h"
+#include "fu-plugin-vfuncs.h"
 
 static FuDevice *
 _find_device_by_id (GPtrArray *devices, const gchar *device_id)
@@ -288,7 +289,9 @@ fu_plugin_dell_dock_func (void)
 	fu_plugin_dell_inject_fake_data (plugin_dell,
 					   (guint32 *) &out,
 					   0x1234, 0x4321, NULL, FALSE);
-	fu_plugin_dell_device_added_cb (NULL, NULL, plugin_dell);
+	ret = fu_plugin_usb_device_added (plugin_dell, NULL, &error);
+	g_assert_false (ret);
+	g_clear_error (&error);
 	g_assert_cmpint (devices->len, ==, 0);
 
 	/* inject a USB dongle matching correct VID/PID */
@@ -298,7 +301,9 @@ fu_plugin_dell_dock_func (void)
 					   (guint32 *) &out,
 					   DOCK_NIC_VID, DOCK_NIC_PID,
 					   NULL, FALSE);
-	fu_plugin_dell_device_added_cb (NULL, NULL, plugin_dell);
+	ret = fu_plugin_usb_device_added (plugin_dell, NULL, &error);
+	g_assert_false (ret);
+	g_clear_error (&error);
 	g_assert_cmpint (devices->len, ==, 0);
 
 	/* inject valid TB16 dock w/ invalid flash pkg version */
@@ -330,12 +335,11 @@ fu_plugin_dell_dock_func (void)
 					   (guint32 *) &out,
 					   DOCK_NIC_VID, DOCK_NIC_PID,
 					   buf.buf, FALSE);
-	fu_plugin_dell_device_added_cb (NULL, NULL, plugin_dell);
+	ret = fu_plugin_usb_device_added (plugin_dell, NULL, NULL);
+	g_assert (ret);
 	g_assert_cmpint (devices->len, ==, 4);
 	g_ptr_array_set_size (devices, 0);
 	g_free (buf.record);
-	fu_plugin_dell_device_removed_cb (NULL, NULL,
-					    plugin_dell);
 
 	/* inject valid TB16 dock w/ older system EC */
 	buf.record = g_malloc0 (sizeof(DOCK_INFO_RECORD));
@@ -366,13 +370,11 @@ fu_plugin_dell_dock_func (void)
 					   (guint32 *) &out,
 					   DOCK_NIC_VID, DOCK_NIC_PID,
 					   buf.buf, FALSE);
-	fu_plugin_dell_device_added_cb (NULL, NULL, plugin_dell);
+	ret = fu_plugin_usb_device_added (plugin_dell, NULL, NULL);
+	g_assert (ret);
 	g_assert_cmpint (devices->len, ==, 3);
 	g_ptr_array_set_size (devices, 0);
 	g_free (buf.record);
-	fu_plugin_dell_device_removed_cb (NULL, NULL,
-					    plugin_dell);
-
 
 	/* inject valid WD15 dock w/ invalid flash pkg version */
 	buf.record = g_malloc0 (sizeof(DOCK_INFO_RECORD));
@@ -400,13 +402,12 @@ fu_plugin_dell_dock_func (void)
 					   (guint32 *) &out,
 					   DOCK_NIC_VID, DOCK_NIC_PID,
 					   buf.buf, FALSE);
-	fu_plugin_dell_device_added_cb (NULL, NULL, plugin_dell);
+	ret = fu_plugin_usb_device_added (plugin_dell, NULL, &error);
+	g_assert (ret);
+	g_assert_no_error (error);
 	g_assert_cmpint (devices->len, ==, 3);
 	g_ptr_array_set_size (devices, 0);
 	g_free (buf.record);
-	fu_plugin_dell_device_removed_cb (NULL, NULL,
-					    plugin_dell);
-
 
 	/* inject valid WD15 dock w/ older system EC */
 	buf.record = g_malloc0 (sizeof(DOCK_INFO_RECORD));
@@ -434,12 +435,12 @@ fu_plugin_dell_dock_func (void)
 					 (guint32 *) &out,
 					 DOCK_NIC_VID, DOCK_NIC_PID,
 					 buf.buf, FALSE);
-	fu_plugin_dell_device_added_cb (NULL, NULL, plugin_dell);
+	ret = fu_plugin_usb_device_added (plugin_dell, NULL, &error);
+	g_assert (ret);
+	g_assert_no_error (error);
 	g_assert_cmpint (devices->len, ==, 2);
 	g_ptr_array_set_size (devices, 0);
 	g_free (buf.record);
-	fu_plugin_dell_device_removed_cb (NULL, NULL,
-					    plugin_dell);
 
 	/* inject an invalid future dock */
 	buf.record = g_malloc0 (sizeof(DOCK_INFO_RECORD));
@@ -461,7 +462,8 @@ fu_plugin_dell_dock_func (void)
 					 (guint32 *) &out,
 					 DOCK_NIC_VID, DOCK_NIC_PID,
 					 buf.buf, FALSE);
-	fu_plugin_dell_device_added_cb (NULL, NULL, plugin_dell);
+	ret = fu_plugin_usb_device_added (plugin_dell, NULL, &error);
+	g_assert_false (ret);
 	g_assert_cmpint (devices->len, ==, 0);
 	g_free (buf.record);
 }