[OE-core] [PATCH] connman: get the correct network interface name from dmesg during NFS booting

[Andreas Oberritter]

On 12.10.2016 19:49, Christopher Larson wrote:
> 
> On Wed, Oct 12, 2016 at 12:30 AM, Andreas Oberritter
> <obi at opendreambox.org <mailto:obi at opendreambox.org>> wrote:
> 
>     On 11.10.2016 21:34, Christopher Larson wrote:
>     >
>     > On Tue, Oct 11, 2016 at 12:03 PM, Jagadeesh Krishnanjanappa
>     > <jkrishnanjanappa at mvista.com <mailto:jkrishnanjanappa at mvista.com>
>     <mailto:jkrishnanjanappa at mvista.com
>     <mailto:jkrishnanjanappa at mvista.com>>> wrote:
>     >
>     >     On Tue, Oct 11, 2016 at 9:06 PM, Christopher Larson
>     >     <clarson at kergoth.com <mailto:clarson at kergoth.com>
>     <mailto:clarson at kergoth.com <mailto:clarson at kergoth.com>>> wrote:
>     >
>     >
>     >         On Tue, Oct 11, 2016 at 5:11 AM, Jagadeesh Krishnanjanappa
>     >         <jkrishnanjanappa at mvista.com <mailto:jkrishnanjanappa at mvista.com>
>     >         <mailto:jkrishnanjanappa at mvista.com <mailto:jkrishnanjanappa at mvista.com>>>
>     wrote:
>     >
>     >             Thanks for reviewing the patch.
>     >
>     >
>     >                 I think this is too fragile to land in OE-Core. What
>     >                 happens if a
>     >                 network driver prints "device=eth0"? Or if other kernel
>     >                 messages make
>     >                 the string disappear from dmesg and connman gets
>     >                 restarted later?
>     >
>     >
>     >             True. If device=eth0 gets disappeared/corrupted, then we may
>     >             have problem.
>     >
>     >
>     >                 FWIW, I'm attaching my current wrapper script for
>     >                 connmand. I don't
>     >                 think it's perfect, but at least it doesn't depend on
>     >                 any init manager
>     >                 and works across restarts.
>     >
>     >             The wrapper script attached by you, takes care of the
>     >             missing scenarios. Seems to be more complete.
>     >
>     >
>     >
>     >                 Add these lines to connman's do_install, in case you'd
>     >                 like to test
>     >                 and/or submit it:
>     >
>     >                 mv ${D}${sbindir}/connmand ${D}${sbindir}/connmand.real
>     >                 install -m 755 ${WORKDIR}/connmand-nfsroot.in <http://connmand-nfsroot.in>
>     >                 <http://connmand-nfsroot.in> ${D}${sbindir}/connmand
>     >                 sed -e 's, at sbindir@,${sbindir},g' -i ${D}${sbindir}/connmand
>     >
>     >             I think it would be good idea to integrate your changes into
>     >             the already existing OE-core's connman script, instead of a
>     >             calling original connman script from the wrapper script.
>     >
>     >
>     >         I threw
>     >         together https://github.com/openembedded/openembedded-core/compare/master...kergoth:connman-systemd-nfs
>     <https://github.com/openembedded/openembedded-core/compare/master...kergoth:connman-systemd-nfs>
>     >         <https://github.com/openembedded/openembedded-core/compare/master...kergoth:connman-systemd-nfs
>     <https://github.com/openembedded/openembedded-core/compare/master...kergoth:connman-systemd-nfs>>
>     last
>     >         night, only limited testing. I think using a script in
>     >         libexecdir is rather cleaner than wrapping connmand in place,
>     >         for something like this, personally.
>     >
>     >
>     >     In the above commit,  the last line of connman/connman/start-connman
>     >     file, should be exec @SBINDIR@/connmand "$@" instead of exec
>     >     @SBINDIR@/connman "$@" ; as the actual binary file name is connmand.
>     >
>     >
>     > Thanks, didn’t notice that. Obviously it needs polish and testing before
>     > submission, I mainly linked it to solicit thoughts on the approach :)
> 
>     your proposal adds some complexity to the recipe, which I tried to
>     avoid, because this wrapper script is something we'll probably have to
>     maintain forever.
> 
>     For example, your recipe might break without notice if upstream is going
>     to ship another binary as ${sbindir}/connmand-something in the future;
>     or if upstream decides to change the systemd unit to use a variable
>     instead of a hardcoded path to connmand.
> 
>     Besides that, I don't have a strong opinion on the location of the
>     wrapper script.
> 
> 
> Good points. I think the future-proofing could be addressed with
> something like sed -i -e "s#^ExecStart=.*#ExecStart=$wrapperpath#"
> ${D}${systemd_unitdir}/system/connman.service, and then switch away from
> that weak start-connman script in favor of yours, but keep it in libexec
> for cleanliness.

Agreed.

Regards,
Andreas