Re: [PATCH 1/9] Ignore rild modem devices.

[Tony Espy <espy canonical com>]

On 06/08/2016 05:37 PM, Dan Williams wrote:
> On Wed, 2016-06-08 at 16:20 -0400, Tony Espy wrote:
> > From: Mathieu Trudel-Lapierre <mathieu trudel-lapierre canonical com>
> >
> > Gbp-Pq: Name Ignore-rild-modem-devices.patch
> > ---
> >   src/nm-manager.c | 8 ++++++++
> >   1 file changed, 8 insertions(+)
> >
> > diff --git a/src/nm-manager.c b/src/nm-manager.c
> > index 10aa3d7..c2ed1da 100644
> > --- a/src/nm-manager.c
> > +++ b/src/nm-manager.c
> > @@ -2089,6 +2089,14 @@ platform_link_added (NMManager *self,
> >                 gboolean ignore = FALSE;
> >                 gs_free_error GError *error = NULL;
> >
> > +               /* Ignore rild modem devices, which will be handled
> > by their modem parent */
> > +               if (g_strstr_len (plink->name, NM_STRLEN ("rmnet"),
> > "rmnet") ||
> > +                       g_strstr_len (plink->name, NM_STRLEN
> > ("rev_rmnet"), "rev_rmnet") ||
> > +                       g_strstr_len (plink->name, NM_STRLEN
> > ("ccmni"), "ccmni")) {
> > +                       _LOGW (LOGD_HW, "Ignoring rild modem device:
> > %s", plink->name);
> > +                       return;
> > +               }
>
> I'd actually put this into the WWAN factory, see just below:
>
> >                 device = nm_device_factory_create_device (factory,
> > plink->name, plink, NULL, &ignore, &error);
> >                 if (!device) {
> >                         if (!ignore) {
>
> where &ignore is; the factory can indicate to the manager that it wants
> certain devices ignored through that variable.  So you'd toss this into
> src/devices/wwan/nm-wwan-factory.c::create_device() instead.

The one area of concern I have is whether or not these devices are 
reported as the proper type, otherwise they won't get passed to the wwan 
factory.

If this isn't a problem for us, then this seems like a reasonable 
approach... except for the fact that we've hard-coded these device 
matching expressions.  Over time this won't scale.

Any thoughts about how this could be made runtime configurable?  I'd 
love to get rid of one more source patch.  ;)

/tony