Only set the parent ID when adopting children

We use the ParentGuid quirk key to logically 'tie-together' different discrete
devices into one logical device, for instance making the USB soundcard in a hub
the child of the USB controller on the same PCB.

Setting the discrete child is sometimes correct, for instance when rebooting
the hub, the audio device also goes away -- but it's also sometimes wrong.
If we set the child for a discrete device and the parent does *not* go away
then we get to a situation where the child reference may no longer be valid
if it comes back as a different object.
When we try to remove this no-longer-valid device with the removal timeout the
daemon segfaults. This is realy bad.

Continue to allow using fu_device_add_child() in plugins, where we know the
child lifecycle is is matched by the the parent. In the engine just set the
parent ID directly and let the client use this information to show the tree of
logical devices correctly. There's no benefit to setting up the children as
referenced objects anyway.
This commit is contained in:
Richard Hughes 2020-03-31 13:11:10 +01:00
parent 61f74077ed
commit b4f14e8f0f
4 changed files with 8 additions and 8 deletions

View File

@ -658,8 +658,7 @@ fu_device_set_parent (FuDevice *self, FuDevice *parent)
priv->parent = parent;
/* this is what goes over D-Bus */
fwupd_device_set_parent_id (FWUPD_DEVICE (self),
parent != NULL ? fu_device_get_id (parent) : NULL);
fu_device_set_parent_id (self, parent != NULL ? fu_device_get_id (parent) : NULL);
}
/**

View File

@ -117,6 +117,7 @@ FuDevice *fu_device_new (void);
#define fu_device_set_description(d,v) fwupd_device_set_description(FWUPD_DEVICE(d),v)
#define fu_device_set_flags(d,v) fwupd_device_set_flags(FWUPD_DEVICE(d),v)
#define fu_device_set_modified(d,v) fwupd_device_set_modified(FWUPD_DEVICE(d),v)
#define fu_device_set_parent_id(d,v) fwupd_device_set_parent_id(FWUPD_DEVICE(d),v)
#define fu_device_set_plugin(d,v) fwupd_device_set_plugin(FWUPD_DEVICE(d),v)
#define fu_device_set_serial(d,v) fwupd_device_set_serial(FWUPD_DEVICE(d),v)
#define fu_device_set_summary(d,v) fwupd_device_set_summary(FWUPD_DEVICE(d),v)
@ -142,6 +143,7 @@ FuDevice *fu_device_new (void);
#define fu_device_get_serial(d) fwupd_device_get_serial(FWUPD_DEVICE(d))
#define fu_device_get_summary(d) fwupd_device_get_summary(FWUPD_DEVICE(d))
#define fu_device_get_id(d) fwupd_device_get_id(FWUPD_DEVICE(d))
#define fu_device_get_parent_id(d) fwupd_device_get_parent_id(FWUPD_DEVICE(d))
#define fu_device_get_plugin(d) fwupd_device_get_plugin(FWUPD_DEVICE(d))
#define fu_device_get_update_error(d) fwupd_device_get_update_error(FWUPD_DEVICE(d))
#define fu_device_get_update_state(d) fwupd_device_get_update_state(FWUPD_DEVICE(d))

View File

@ -4361,7 +4361,7 @@ fu_engine_adopt_children (FuEngine *self, FuDevice *device)
fu_device_get_id (device),
fu_device_get_name (device_tmp),
fu_device_get_id (device_tmp));
fu_device_add_child (device_tmp, device);
fu_device_set_parent_id (device, fu_device_get_id (device_tmp));
break;
}
}
@ -4381,7 +4381,7 @@ fu_engine_adopt_children (FuEngine *self, FuDevice *device)
fu_device_get_id (device_tmp),
fu_device_get_name (device),
fu_device_get_id (device));
fu_device_add_child (device, device_tmp);
fu_device_set_parent_id (device_tmp, fu_device_get_id (device));
}
}
}

View File

@ -956,9 +956,8 @@ fu_engine_device_parent_func (gconstpointer user_data)
/* verify both children were adopted */
g_assert (fu_device_get_parent (device3) == device2);
g_assert (fu_device_get_parent (device1) == device2);
g_assert_cmpstr (fu_device_get_vendor (device3), ==, "oem");
g_assert_cmpstr (fu_device_get_vendor (device1), ==, "oem");
g_assert_cmpstr (fu_device_get_parent_id (device1), ==, fu_device_get_id (device2));
/* verify order */
g_assert_cmpint (fu_device_get_order (device1), ==, 0);
@ -2738,11 +2737,11 @@ fu_plugin_composite_func (gconstpointer user_data)
} else if (g_strcmp0 (fu_device_get_id (device),
"c0a0a4aa6480ac28eea1ce164fbb466ca934e1ff") == 0) {
g_assert_cmpstr (fu_device_get_version (device), ==, "1");
g_assert_nonnull (fu_device_get_parent (device));
g_assert_nonnull (fu_device_get_parent_id (device));
} else if (g_strcmp0 (fu_device_get_id (device),
"bf455e9f371d2608d1cb67660fd2b335d3f6ef73") == 0) {
g_assert_cmpstr (fu_device_get_version (device), ==, "10");
g_assert_nonnull (fu_device_get_parent (device));
g_assert_nonnull (fu_device_get_parent_id (device));
}
}