proxmox-mirrors/fwupd
1
0
Fork 0
mirror of https://git.proxmox.com/git/fwupd synced 2025-08-13 21:35:02 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

Add a device flag to control child removal

Browse Source 
At the moment the device list auto-removes children when the parent is
removed. This was originally working around the bug that the child
devices had no backend ID, and thus were not matched like the parent
in fu_engine_backend_device_removed_cb() and removed automatically.

Using the workaround means that the children are potentially removed
before the parent depending on the order of the device list -- which
might be modified by replug events.

I think the old cleanup is probably an anti-pattern, and going forward
composite devices should probably define _NO_AUTO_REMOVE_CHILDREN but
we don't want to change this behaviour for plugins already expecting
the old flow, or for devices with no parent backend IDs.
This commit is contained in:
Richard Hughes 2021-07-09 14:14:30 +01:00
parent 30ce27440e
commit f89cb3850b
4 changed files with 74 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
libfwupdplugin/fu-device.c
View File

@ -232,6 +232,8 @@ fu_device_internal_flag_to_string (FuDeviceInternalFlags flag)
return "attach-extra-reset";
if (flag == FU_DEVICE_INTERNAL_FLAG_INHIBIT_CHILDREN)
return "inhibit-children";
if (flag == FU_DEVICE_INTERNAL_FLAG_NO_AUTO_REMOVE_CHILDREN)
return "no-auto-remove-children";
return NULL;
}
@ -278,6 +280,8 @@ fu_device_internal_flag_from_string (const gchar *flag)
return FU_DEVICE_INTERNAL_FLAG_ATTACH_EXTRA_RESET;
if (g_strcmp0 (flag, "inhibit-children") == 0)
return FU_DEVICE_INTERNAL_FLAG_INHIBIT_CHILDREN;
if (g_strcmp0 (flag, "no-auto-remove-children") == 0)
return FU_DEVICE_INTERNAL_FLAG_NO_AUTO_REMOVE_CHILDREN;
return FU_DEVICE_INTERNAL_FLAG_UNKNOWN;
}

2
libfwupdplugin/fu-device.h
View File

@ -238,6 +238,7 @@ FuDevice *fu_device_new (void);
* @FU_DEVICE_INTERNAL_FLAG_AUTO_PARENT_CHILDREN: Automatically assign the parent for children of this device
* @FU_DEVICE_INTERNAL_FLAG_ATTACH_EXTRA_RESET: Device needs resetting twice for attach after the firmware update
* @FU_DEVICE_INTERNAL_FLAG_INHIBIT_CHILDREN: Children of the device are inhibited by the parent
* @FU_DEVICE_INTERNAL_FLAG_NO_AUTO_REMOVE_CHILDREN: Do not auto-remove clildren in the device list
*
* The device internal flags.
**/
@ -258,6 +259,7 @@ typedef enum {
FU_DEVICE_INTERNAL_FLAG_AUTO_PARENT_CHILDREN = (1llu << 12), /* Since: 1.6.2 */
FU_DEVICE_INTERNAL_FLAG_ATTACH_EXTRA_RESET = (1llu << 13), /* Since: 1.6.2 */
FU_DEVICE_INTERNAL_FLAG_INHIBIT_CHILDREN = (1llu << 14), /* Since: 1.6.2 */
FU_DEVICE_INTERNAL_FLAG_NO_AUTO_REMOVE_CHILDREN = (1llu << 15), /* Since: 1.6.2 */
/*< private >*/
FU_DEVICE_INTERNAL_FLAG_UNKNOWN = G_MAXUINT64,
} FuDeviceInternalFlags;

62
src/fu-device-list.c
View File

@ -373,26 +373,29 @@ fu_device_list_device_delayed_remove_cb (gpointer user_data)
{
FuDeviceItem *item = (FuDeviceItem *) user_data;
FuDeviceList *self = FU_DEVICE_LIST (item->self);
GPtrArray *children;
/* no longer valid */
item->remove_id = 0;
/* remove any children associated with device */
children = fu_device_get_children (item->device);
for (guint j = 0; j < children->len; j++) {
FuDevice *child = g_ptr_array_index (children, j);
FuDeviceItem *child_item = fu_device_list_find_by_id (self,
fu_device_get_id (child),
NULL);
if (child_item == NULL) {
g_debug ("device %s not found", fu_device_get_id (child));
continue;
if (!fu_device_has_internal_flag (item->device,
FU_DEVICE_INTERNAL_FLAG_NO_AUTO_REMOVE_CHILDREN)) {
GPtrArray *children = fu_device_get_children (item->device);
for (guint j = 0; j < children->len; j++) {
FuDevice *child = g_ptr_array_index (children, j);
FuDeviceItem *child_item;
child_item = fu_device_list_find_by_id (self,
fu_device_get_id (child),
NULL);
if (child_item == NULL) {
g_debug ("device %s not found", fu_device_get_id (child));
continue;
}
fu_device_list_emit_device_removed (self, child);
g_rw_lock_writer_lock (&self->devices_mutex);
g_ptr_array_remove (self->devices, child_item);
g_rw_lock_writer_unlock (&self->devices_mutex);
}
fu_device_list_emit_device_removed (self, child);
g_rw_lock_writer_lock (&self->devices_mutex);
g_ptr_array_remove (self->devices, child_item);
g_rw_lock_writer_unlock (&self->devices_mutex);
}
/* just remove now */
@ -440,7 +443,6 @@ void
fu_device_list_remove (FuDeviceList *self, FuDevice *device)
{
FuDeviceItem *item;
GPtrArray *children;
g_return_if_fail (FU_IS_DEVICE_LIST (self));
g_return_if_fail (FU_IS_DEVICE (device));
@ -465,20 +467,24 @@ fu_device_list_remove (FuDeviceList *self, FuDevice *device)
}
/* remove any children associated with device */
children = fu_device_get_children (device);
for (guint j = 0; j < children->len; j++) {
FuDevice *child = g_ptr_array_index (children, j);
FuDeviceItem *child_item = fu_device_list_find_by_id (self,
fu_device_get_id (child),
NULL);
if (child_item == NULL) {
g_debug ("device %s not found", fu_device_get_id (child));
continue;
if (!fu_device_has_internal_flag (item->device,
FU_DEVICE_INTERNAL_FLAG_NO_AUTO_REMOVE_CHILDREN)) {
GPtrArray *children = fu_device_get_children (device);
for (guint j = 0; j < children->len; j++) {
FuDevice *child = g_ptr_array_index (children, j);
FuDeviceItem *child_item;
child_item = fu_device_list_find_by_id (self,
fu_device_get_id (child),
NULL);
if (child_item == NULL) {
g_debug ("device %s not found", fu_device_get_id (child));
continue;
}
fu_device_list_emit_device_removed (self, child);
g_rw_lock_writer_lock (&self->devices_mutex);
g_ptr_array_remove (self->devices, child_item);
g_rw_lock_writer_unlock (&self->devices_mutex);
}
fu_device_list_emit_device_removed (self, child);
g_rw_lock_writer_lock (&self->devices_mutex);
g_ptr_array_remove (self->devices, child_item);
g_rw_lock_writer_unlock (&self->devices_mutex);
}
/* remove right now */

34
src/fu-self-test.c
View File

@ -2083,6 +2083,38 @@ _device_list_count_cb (FuDeviceList *device_list, FuDevice *device, gpointer use
(*cnt)++;
}
static void
fu_device_list_no_auto_remove_children_func (gconstpointer user_data)
{
g_autoptr(FuDevice) child = fu_device_new ();
g_autoptr(FuDevice) parent = fu_device_new ();
g_autoptr(FuDeviceList) device_list = fu_device_list_new ();
g_autoptr(GPtrArray) active1 = NULL;
g_autoptr(GPtrArray) active2 = NULL;
g_autoptr(GPtrArray) active3 = NULL;
/* normal behavior, remove child with parent */
fu_device_set_id (parent, "parent");
fu_device_set_id (child, "child");
fu_device_add_child (parent, child);
fu_device_list_add (device_list, parent);
fu_device_list_add (device_list, child);
fu_device_list_remove (device_list, parent);
active1 = fu_device_list_get_active (device_list);
g_assert_cmpint (active1->len, ==, 0);
/* new-style behavior, do not remove child */
fu_device_add_internal_flag (parent, FU_DEVICE_INTERNAL_FLAG_NO_AUTO_REMOVE_CHILDREN);
fu_device_list_add (device_list, parent);
fu_device_list_add (device_list, child);
fu_device_list_remove (device_list, parent);
active2 = fu_device_list_get_active (device_list);
g_assert_cmpint (active2->len, ==, 1);
fu_device_list_remove (device_list, child);
active3 = fu_device_list_get_active (device_list);
g_assert_cmpint (active3->len, ==, 0);
}
static void
fu_device_list_delay_func (gconstpointer user_data)
{
@ -3296,6 +3328,8 @@ main (int argc, char **argv)
fu_device_list_func);
g_test_add_data_func ("/fwupd/device-list{delay}", self,
fu_device_list_delay_func);
g_test_add_data_func ("/fwupd/device-list{no-auto-remove-children}", self,
fu_device_list_no_auto_remove_children_func);
g_test_add_data_func ("/fwupd/device-list{compatible}", self,
fu_device_list_compatible_func);
g_test_add_data_func ("/fwupd/device-list{remove-chain}", self,