* [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.