* [PATCH 1/2] systemctl-native: add target.wants to target regex @ 2017-10-16 16:31 Martin Kelly 2017-10-16 16:31 ` [PATCH 2/2] systemctl-native: explicitly check target validity Martin Kelly 2017-11-08 17:40 ` [PATCH 1/2] systemctl-native: add target.wants to target regex Martin Kelly 0 siblings, 2 replies; 6+ messages in thread From: Martin Kelly @ 2017-10-16 16:31 UTC (permalink / raw) To: openembedded-core The regex for acceptable systemd WantedBy/RequiredBy targets does not include target.wants, so a line like this: WantedBy=multi-user.target.wants gets silently ignored, even though it works fine on a real system. Signed-off-by: Martin Kelly <mkelly@xevo.com> --- meta/recipes-core/systemd/systemd-systemctl/systemctl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/meta/recipes-core/systemd/systemd-systemctl/systemctl b/meta/recipes-core/systemd/systemd-systemctl/systemctl index efad14ce17..6e5a1b7181 100755 --- a/meta/recipes-core/systemd/systemd-systemctl/systemctl +++ b/meta/recipes-core/systemd/systemd-systemctl/systemctl @@ -108,7 +108,7 @@ for service in $services; do # If any new unit types are added to systemd they should be added # to this regular expression. - unit_types_re='\.\(service\|socket\|device\|mount\|automount\|swap\|target\|path\|timer\|snapshot\)\s*$' + unit_types_re='\.\(service\|socket\|device\|mount\|automount\|swap\|target\|target\.wants\|path\|timer\|snapshot\)\s*$' if [ "$action" = "preset" ]; then action=`egrep -sh $service $ROOT/etc/systemd/user-preset/*.preset | cut -f1 -d' '` if [ -z "$action" ]; then -- 2.11.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] systemctl-native: explicitly check target validity 2017-10-16 16:31 [PATCH 1/2] systemctl-native: add target.wants to target regex Martin Kelly @ 2017-10-16 16:31 ` Martin Kelly 2017-11-08 17:40 ` [PATCH 1/2] systemctl-native: add target.wants to target regex Martin Kelly 1 sibling, 0 replies; 6+ messages in thread From: Martin Kelly @ 2017-10-16 16:31 UTC (permalink / raw) To: openembedded-core Currently, we parse systemd service files looking for valid WantedBy and RequiredBy lines. However, invalid lines get silently rejected, causing recipes to build correctly but then not get enabled in the rootfs. Add explicit checks for validity and print a message when an invalid target is found so that the user can fix these issues. Signed-off-by: Martin Kelly <mkelly@xevo.com> --- .../systemd/systemd-systemctl/systemctl | 37 ++++++++++++++++++---- 1 file changed, 30 insertions(+), 7 deletions(-) diff --git a/meta/recipes-core/systemd/systemd-systemctl/systemctl b/meta/recipes-core/systemd/systemd-systemctl/systemctl index 6e5a1b7181..3bed407d9e 100755 --- a/meta/recipes-core/systemd/systemd-systemctl/systemctl +++ b/meta/recipes-core/systemd/systemd-systemctl/systemctl @@ -1,4 +1,24 @@ #!/bin/sh + +check_target_validity() { + local line_reg="$1" + local service_file="$2" + local unit_types_re="$3" + + local line="$(sed $line_reg "$service_file")" + if [ -z "$line" ]; then + # Line was not found; there's nothing to check. + return + fi + + if ! echo $line | grep -q $unit_types_re; then + echo "One or more of the target types in the service were not recognized as valid." + echo "Service: $service_file" + echo "Targets: $line" + exit 1 + fi +} + echo "Started $0 $*" ROOT= @@ -120,15 +140,18 @@ for service in $services; do fi fi fi - # create the required symbolic links - wanted_by=$(sed '/^WantedBy[[:space:]]*=/s,[^=]*=,,p;d' "$ROOT/$service_file" \ - | tr ',' '\n' \ - | grep "$unit_types_re") - required_by=$(sed '/^RequiredBy[[:space:]]*=/s,[^=]*=,,p;d' "$ROOT/$service_file" \ - | tr ',' '\n' \ - | grep "$unit_types_re") + check_target_validity \ + '/^WantedBy[[:space:]]*=/s,[^=]*=,,p;d' \ + "$ROOT/$service_file" \ + "$unit_types_re" + check_target_validity \ + '/^RequiredBy[[:space:]]*=/s,[^=]*=,,p;d' \ + "$ROOT/$service_file" \ + "$unit_types_re" + + # create the required symbolic links for dependency in WantedBy RequiredBy; do if [ "$dependency" = "WantedBy" ]; then suffix="wants" -- 2.11.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] systemctl-native: add target.wants to target regex 2017-10-16 16:31 [PATCH 1/2] systemctl-native: add target.wants to target regex Martin Kelly 2017-10-16 16:31 ` [PATCH 2/2] systemctl-native: explicitly check target validity Martin Kelly @ 2017-11-08 17:40 ` Martin Kelly 2017-11-08 20:31 ` Burton, Ross 1 sibling, 1 reply; 6+ messages in thread From: Martin Kelly @ 2017-11-08 17:40 UTC (permalink / raw) To: Martin Kelly, Patches and discussions about the oe-core layer (ping) for this patch series. On 10/16/2017 09:31 AM, Martin Kelly wrote: > The regex for acceptable systemd WantedBy/RequiredBy targets does not include > target.wants, so a line like this: > > WantedBy=multi-user.target.wants > > gets silently ignored, even though it works fine on a real system. > > Signed-off-by: Martin Kelly <mkelly@xevo.com> > --- > meta/recipes-core/systemd/systemd-systemctl/systemctl | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/meta/recipes-core/systemd/systemd-systemctl/systemctl b/meta/recipes-core/systemd/systemd-systemctl/systemctl > index efad14ce17..6e5a1b7181 100755 > --- a/meta/recipes-core/systemd/systemd-systemctl/systemctl > +++ b/meta/recipes-core/systemd/systemd-systemctl/systemctl > @@ -108,7 +108,7 @@ for service in $services; do > > # If any new unit types are added to systemd they should be added > # to this regular expression. > - unit_types_re='\.\(service\|socket\|device\|mount\|automount\|swap\|target\|path\|timer\|snapshot\)\s*$' > + unit_types_re='\.\(service\|socket\|device\|mount\|automount\|swap\|target\|target\.wants\|path\|timer\|snapshot\)\s*$' > if [ "$action" = "preset" ]; then > action=`egrep -sh $service $ROOT/etc/systemd/user-preset/*.preset | cut -f1 -d' '` > if [ -z "$action" ]; then > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] systemctl-native: add target.wants to target regex 2017-11-08 17:40 ` [PATCH 1/2] systemctl-native: add target.wants to target regex Martin Kelly @ 2017-11-08 20:31 ` Burton, Ross 2017-11-10 1:01 ` Martin Kelly 0 siblings, 1 reply; 6+ messages in thread From: Burton, Ross @ 2017-11-08 20:31 UTC (permalink / raw) To: Martin Kelly; +Cc: Patches and discussions about the oe-core layer [-- Attachment #1: Type: text/plain, Size: 2036 bytes --] Thanks for reminding me. :) 1/2 is queued but 2/2 was implicated in a number of systemd-related boot failures on the autobuilder ( https://autobuilder.yocto.io/builders/nightly-qa-extras/builds/553). I've not yet got around to looking at exactly what sanity test 5 and 7 do to trigger this. Ross On 8 November 2017 at 17:40, Martin Kelly <mkelly@xevo.com> wrote: > (ping) for this patch series. > > On 10/16/2017 09:31 AM, Martin Kelly wrote: > >> The regex for acceptable systemd WantedBy/RequiredBy targets does not >> include >> target.wants, so a line like this: >> >> WantedBy=multi-user.target.wants >> >> gets silently ignored, even though it works fine on a real system. >> >> Signed-off-by: Martin Kelly <mkelly@xevo.com> >> --- >> meta/recipes-core/systemd/systemd-systemctl/systemctl | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/meta/recipes-core/systemd/systemd-systemctl/systemctl >> b/meta/recipes-core/systemd/systemd-systemctl/systemctl >> index efad14ce17..6e5a1b7181 100755 >> --- a/meta/recipes-core/systemd/systemd-systemctl/systemctl >> +++ b/meta/recipes-core/systemd/systemd-systemctl/systemctl >> @@ -108,7 +108,7 @@ for service in $services; do >> # If any new unit types are added to systemd they should be added >> # to this regular expression. >> - unit_types_re='\.\(service\|socket\|device\|mount\|automoun >> t\|swap\|target\|path\|timer\|snapshot\)\s*$' >> + unit_types_re='\.\(service\|socket\|device\|mount\|automoun >> t\|swap\|target\|target\.wants\|path\|timer\|snapshot\)\s*$' >> if [ "$action" = "preset" ]; then >> action=`egrep -sh $service $ROOT/etc/systemd/user-preset/*.preset >> | cut -f1 -d' '` >> if [ -z "$action" ]; then >> >> -- > _______________________________________________ > Openembedded-core mailing list > Openembedded-core@lists.openembedded.org > http://lists.openembedded.org/mailman/listinfo/openembedded-core > [-- Attachment #2: Type: text/html, Size: 3119 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] systemctl-native: add target.wants to target regex 2017-11-08 20:31 ` Burton, Ross @ 2017-11-10 1:01 ` Martin Kelly 2017-11-16 1:20 ` Martin Kelly 0 siblings, 1 reply; 6+ messages in thread From: Martin Kelly @ 2017-11-10 1:01 UTC (permalink / raw) To: Burton, Ross; +Cc: Patches and discussions about the oe-core layer Got it, thanks. My patch *should* just convert runtime failures into compile-time failures, but it looks like we're seeing actual runtime failures, so I'll try to figure out what happened. On 11/08/2017 12:31 PM, Burton, Ross wrote: > Thanks for reminding me. :) > > 1/2 is queued but 2/2 was implicated in a number of systemd-related boot > failures on the autobuilder > (https://autobuilder.yocto.io/builders/nightly-qa-extras/builds/553). > > I've not yet got around to looking at exactly what sanity test 5 and 7 > do to trigger this. > > Ross > > On 8 November 2017 at 17:40, Martin Kelly <mkelly@xevo.com > <mailto:mkelly@xevo.com>> wrote: > > (ping) for this patch series. > > On 10/16/2017 09:31 AM, Martin Kelly wrote: > > The regex for acceptable systemd WantedBy/RequiredBy targets > does not include > target.wants, so a line like this: > > WantedBy=multi-user.target.wants > > gets silently ignored, even though it works fine on a real system. > > Signed-off-by: Martin Kelly <mkelly@xevo.com > <mailto:mkelly@xevo.com>> > --- > meta/recipes-core/systemd/systemd-systemctl/systemctl | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git > a/meta/recipes-core/systemd/systemd-systemctl/systemctl > b/meta/recipes-core/systemd/systemd-systemctl/systemctl > index efad14ce17..6e5a1b7181 100755 > --- a/meta/recipes-core/systemd/systemd-systemctl/systemctl > +++ b/meta/recipes-core/systemd/systemd-systemctl/systemctl > @@ -108,7 +108,7 @@ for service in $services; do > # If any new unit types are added to systemd they > should be added > # to this regular expression. > - > unit_types_re='\.\(service\|socket\|device\|mount\|automount\|swap\|target\|path\|timer\|snapshot\)\s*$' > + > unit_types_re='\.\(service\|socket\|device\|mount\|automount\|swap\|target\|target\.wants\|path\|timer\|snapshot\)\s*$' > if [ "$action" = "preset" ]; then > action=`egrep -sh $service > $ROOT/etc/systemd/user-preset/*.preset | cut -f1 -d' '` > if [ -z "$action" ]; then > > -- > _______________________________________________ > Openembedded-core mailing list > Openembedded-core@lists.openembedded.org > <mailto:Openembedded-core@lists.openembedded.org> > http://lists.openembedded.org/mailman/listinfo/openembedded-core > <http://lists.openembedded.org/mailman/listinfo/openembedded-core> > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] systemctl-native: add target.wants to target regex 2017-11-10 1:01 ` Martin Kelly @ 2017-11-16 1:20 ` Martin Kelly 0 siblings, 0 replies; 6+ messages in thread From: Martin Kelly @ 2017-11-16 1:20 UTC (permalink / raw) To: Burton, Ross; +Cc: Patches and discussions about the oe-core layer I looked again at the log failures, and it's hard to conclude anything without more information. Is it possible to get a baseline without this patch? For example, step 53 fails with SSH refusing the connection. Does this ever happen without the patch? Step 47 fails because connman is not running and because test_check_rpm_install_removal_log_file_size fails. I'm wondering if that ever happens in normal test runs. Another useful thing would be getting the output of the rootfs creation logs, which would list the exact systemctl-native commands used so we could see if some service wasn't enabled when it should be. Again, systemctl *should* fail and cause a build failure if that happens, but there could be some bug. On 11/09/2017 05:01 PM, Martin Kelly wrote: > Got it, thanks. My patch *should* just convert runtime failures into > compile-time failures, but it looks like we're seeing actual runtime > failures, so I'll try to figure out what happened. > > On 11/08/2017 12:31 PM, Burton, Ross wrote: >> Thanks for reminding me. :) >> >> 1/2 is queued but 2/2 was implicated in a number of systemd-related >> boot failures on the autobuilder >> (https://autobuilder.yocto.io/builders/nightly-qa-extras/builds/553). >> >> I've not yet got around to looking at exactly what sanity test 5 and 7 >> do to trigger this. >> >> Ross >> >> On 8 November 2017 at 17:40, Martin Kelly <mkelly@xevo.com >> <mailto:mkelly@xevo.com>> wrote: >> >> (ping) for this patch series. >> >> On 10/16/2017 09:31 AM, Martin Kelly wrote: >> >> The regex for acceptable systemd WantedBy/RequiredBy targets >> does not include >> target.wants, so a line like this: >> >> WantedBy=multi-user.target.wants >> >> gets silently ignored, even though it works fine on a real >> system. >> >> Signed-off-by: Martin Kelly <mkelly@xevo.com >> <mailto:mkelly@xevo.com>> >> --- >> meta/recipes-core/systemd/systemd-systemctl/systemctl | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git >> a/meta/recipes-core/systemd/systemd-systemctl/systemctl >> b/meta/recipes-core/systemd/systemd-systemctl/systemctl >> index efad14ce17..6e5a1b7181 100755 >> --- a/meta/recipes-core/systemd/systemd-systemctl/systemctl >> +++ b/meta/recipes-core/systemd/systemd-systemctl/systemctl >> @@ -108,7 +108,7 @@ for service in $services; do >> # If any new unit types are added to systemd they >> should be added >> # to this regular expression. >> - >> unit_types_re='\.\(service\|socket\|device\|mount\|automount\|swap\|target\|path\|timer\|snapshot\)\s*$' >> >> + >> unit_types_re='\.\(service\|socket\|device\|mount\|automount\|swap\|target\|target\.wants\|path\|timer\|snapshot\)\s*$' >> >> if [ "$action" = "preset" ]; then >> action=`egrep -sh $service >> $ROOT/etc/systemd/user-preset/*.preset | cut -f1 -d' '` >> if [ -z "$action" ]; then >> >> -- _______________________________________________ >> Openembedded-core mailing list >> Openembedded-core@lists.openembedded.org >> <mailto:Openembedded-core@lists.openembedded.org> >> http://lists.openembedded.org/mailman/listinfo/openembedded-core >> <http://lists.openembedded.org/mailman/listinfo/openembedded-core> >> >> ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-11-16 1:21 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-10-16 16:31 [PATCH 1/2] systemctl-native: add target.wants to target regex Martin Kelly 2017-10-16 16:31 ` [PATCH 2/2] systemctl-native: explicitly check target validity Martin Kelly 2017-11-08 17:40 ` [PATCH 1/2] systemctl-native: add target.wants to target regex Martin Kelly 2017-11-08 20:31 ` Burton, Ross 2017-11-10 1:01 ` Martin Kelly 2017-11-16 1:20 ` Martin Kelly
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.