Skip to contentSkip to conversation

systemd: add support to manage user units

PATCH Superseded Chen Qi 4 participants 13 messages Sep 7, 2016 – Sep 9, 2016

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Chen Qi

author
#0 lore raw

cover systemd: add support to manage user units

The following changes since commit 55bb6816aca39bfa25d4f7e2158a57a5f0ac1cca:

oeqa.buildperf: correct globalres time format (2016-09-06 10:24:00 +0100)

are available in the git repository at:

git://git.openembedded.org/openembedded-core-contrib ChenQi/systemd-user-units
http://cgit.openembedded.org/cgit.cgi/openembedded-core-contrib/log/?h=ChenQi/systemd-user-units

Chen Qi (3):
systemd-systemctl: add option to manage user services systemd.bbclass: add support to manage user services
pulseaudio: fix to manage user services corretly

 meta/classes/systemd.bbclass                       | 17 ++++++--
 .../systemd/systemd-systemctl/systemctl            | 45 ++++++++++++++--------
 meta/recipes-multimedia/pulseaudio/pulseaudio.inc  |  4 +-
 3 files changed, 43 insertions(+), 23 deletions(-)
Signature · 1 line
1.9.1
author
#1 lore raw

PATCH 1/3 systemd-systemctl: add option to manage user services

Add '--global' option to our own systemctl script to manage user services.

[YOCTO #7800]

---
 .../systemd/systemd-systemctl/systemctl            | 45 ++++++++++++++--------
 1 file changed, 28 insertions(+), 17 deletions(-)
meta/recipes-core/systemd/systemd-systemctl/systemctl +28 -17
index efad14c..17a7277 100755
@@ -2,7 +2,8 @@
 echo "Started $0 $*"
 
 ROOT=
-
+USER_SERVICE=no
+location=system
 # parse command line params
 action=
 while [ $# != 0 ]; do
@@ -46,6 +47,11 @@ while [ $# != 0 ]; do
 			cmd_args="0"
 			shift
 			;;
+		--global)
+			USER_SERVICE=yes
+			cmd_args="0"
+			shift
+			;;
 		*)
 			if [ "$cmd_args" = "1" ]; then
 				services="$services $opt" 
@@ -57,8 +63,13 @@ while [ $# != 0 ]; do
 			;;
 	esac
 done
+
+if [ "$USER_SERVICE" = "yes" ]; then
+	location=user
+fi
+
 if [ "$action" = "preset" -a "$service_file" = "" ]; then
-	services=$(for f in `find $ROOT/etc/systemd/system $ROOT/lib/systemd/system $ROOT/usr/lib/systemd/system -type f 2>1`; do basename $f; done)
+	services=$(for f in `find $ROOT/etc/systemd/$location $ROOT/lib/systemd/$location $ROOT/usr/lib/systemd/$location -type f 2>1`; do basename $f; done)
 	services="$services $opt"
 	presetall=1
 fi
@@ -68,10 +79,10 @@ for service in $services; do
 		action="preset"
 	fi
 	if [ "$action" = "mask" ]; then
-		if [ ! -d $ROOT/etc/systemd/system/ ]; then
-			mkdir -p $ROOT/etc/systemd/system/
+		if [ ! -d $ROOT/etc/systemd/$location/ ]; then
+			mkdir -p $ROOT/etc/systemd/$location/
 		fi
-		cmd="ln -s /dev/null $ROOT/etc/systemd/system/$service"
+		cmd="ln -s /dev/null $ROOT/etc/systemd/$location/$service"
 		echo "$cmd"
 		$cmd
 		exit 0
@@ -92,9 +103,9 @@ for service in $services; do
 	fi
 
 	# find service file
-	for p in $ROOT/etc/systemd/system \
-		 $ROOT/lib/systemd/system \
-		 $ROOT/usr/lib/systemd/system; do
+	for p in $ROOT/etc/systemd/$location \
+		 $ROOT/lib/systemd/$location \
+		 $ROOT/usr/lib/systemd/$location; do
 		if [ -e $p/$service_base_file ]; then
 			service_file=$p/$service_base_file
 			service_file=${service_file##$ROOT}
@@ -151,18 +162,18 @@ for service in $services; do
 						enable_service=$(echo $service | sed "s/@/@$(echo $default_instance | sed 's/\\/\\\\/g')/")
 					fi
 				fi
-				mkdir -p $ROOT/etc/systemd/system/$r.$suffix
-				ln -s $service_file $ROOT/etc/systemd/system/$r.$suffix/$enable_service
+				mkdir -p $ROOT/etc/systemd/$location/$r.$suffix
+				ln -s $service_file $ROOT/etc/systemd/$location/$r.$suffix/$enable_service
 				echo "Enabled $enable_service for $r."
 			else
 				if [ "$service_template" = true -a "$instance_specified" = false ]; then
-					disable_service="$ROOT/etc/systemd/system/$r.$suffix/`echo $service | sed 's/@/@*/'`"
+					disable_service="$ROOT/etc/systemd/$location/$r.$suffix/`echo $service | sed 's/@/@*/'`"
 				else
-					disable_service="$ROOT/etc/systemd/system/$r.$suffix/$service"
+					disable_service="$ROOT/etc/systemd/$location/$r.$suffix/$service"
 				fi
 				rm -f $disable_service
-				[ -d $ROOT/etc/systemd/system/$r.$suffix ] && rmdir --ignore-fail-on-non-empty -p $ROOT/etc/systemd/system/$r.$suffix
-				echo "Disabled ${disable_service##$ROOT/etc/systemd/system/$r.$suffix/} for $r."
+				[ -d $ROOT/etc/systemd/$location/$r.$suffix ] && rmdir --ignore-fail-on-non-empty -p $ROOT/etc/systemd/$location/$r.$suffix
+				echo "Disabled ${disable_service##$ROOT/etc/systemd/$location/$r.$suffix/} for $r."
 			fi
 		done
 	done
@@ -174,11 +185,11 @@ for service in $services; do
 
 	for r in $alias; do
 		if [ "$action" = "enable" ]; then
-			mkdir -p $ROOT/etc/systemd/system
-			ln -s $service_file $ROOT/etc/systemd/system/$r
+			mkdir -p $ROOT/etc/systemd/$location
+			ln -s $service_file $ROOT/etc/systemd/$location/$r
 			echo "Enabled $service for $alias."
 		else
-			rm -f $ROOT/etc/systemd/system/$r
+			rm -f $ROOT/etc/systemd/$location/$r
 			echo "Disabled $service for $alias."
 		fi
 	done
Signature · 1 line
1.9.1
author
#2 lore raw

PATCH 2/3 systemd.bbclass: add support to manage user services

In reply to Chen Qi

Add new variable SYSTEMD_USER_SERVICE and SYSTEM_USER_AUTO_ENABLE to manage user services. Their usage is like SYSTEMD_SERVICE and SYSTEMD_AUTO_ENABLE.

[YOCTO #7800]

---
 meta/classes/systemd.bbclass | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)
meta/classes/systemd.bbclass +13 -4
index db7873f..78cec97 100644
@@ -7,6 +7,7 @@ SYSTEMD_PACKAGES_class-nativesdk ?= ""
 
 # Whether to enable or disable the services on installation.
 SYSTEMD_AUTO_ENABLE ??= "enable"
+SYSTEMD_USER_AUTO_ENABLE ??= "enable"
 
 # This class will be included in any recipe that supports systemd init scripts,
 # even if systemd is not in DISTRO_FEATURES.  As such don't make any changes
@@ -30,10 +31,14 @@ fi
 
 if type systemctl >/dev/null 2>/dev/null; then
 	systemctl $OPTS ${SYSTEMD_AUTO_ENABLE} ${SYSTEMD_SERVICE}
+	systemctl $OPTS --global ${SYSTEMD_USER_AUTO_ENABLE} ${SYSTEMD_USER_SERVICE}
 
 	if [ -z "$D" -a "${SYSTEMD_AUTO_ENABLE}" = "enable" ]; then
 		systemctl restart ${SYSTEMD_SERVICE}
 	fi
+	if [ -z "$D" -a "${SYSTEMD_USER_AUTO_ENABLE}" = "enable" ]; then
+		systemctl --global restart ${SYSTEMD_USER_SERVICE}
+	fi
 fi
 }
 
@@ -47,9 +52,11 @@ fi
 if type systemctl >/dev/null 2>/dev/null; then
 	if [ -z "$D" ]; then
 		systemctl stop ${SYSTEMD_SERVICE}
+		systemctl --global stop ${SYSTEMD_USER_SERVICE}
 	fi
 
 	systemctl $OPTS disable ${SYSTEMD_SERVICE}
+	systemctl $OPTS --global disable ${SYSTEMD_USER_SERVICE}
 fi
 }
 
@@ -139,12 +146,14 @@ python systemd_populate_packages() {
     def systemd_check_services():
         searchpaths = [oe.path.join(d.getVar("sysconfdir", True), "systemd", "system"),]
         searchpaths.append(d.getVar("systemd_system_unitdir", True))
+        searchpaths.append(oe.path.join(d.getVar("sysconfdir", True), "systemd", "user"))
+        searchpaths.append(d.getVar("systemd_user_unitdir", True))
         systemd_packages = d.getVar('SYSTEMD_PACKAGES', True)
 
         keys = 'Also'
         # scan for all in SYSTEMD_SERVICE[]
         for pkg_systemd in systemd_packages.split():
-            for service in get_package_var(d, 'SYSTEMD_SERVICE', pkg_systemd).split():
+            for service in (get_package_var(d, 'SYSTEMD_SERVICE', pkg_systemd) + get_package_var(d, 'SYSTEMD_USER_SERVICE', pkg_systemd)).split():
                 path_found = ''
 
                 # Deal with adding, for example, '[email protected]' from
@@ -165,14 +174,14 @@ python systemd_populate_packages() {
                 if path_found != '':
                     systemd_add_files_and_parse(pkg_systemd, path_found, service, keys)
                 else:
-                    raise bb.build.FuncFailed("SYSTEMD_SERVICE_%s value %s does not exist" % \
-                        (pkg_systemd, service))
+                    raise bb.build.FuncFailed("SYSTEMD_SERVICE_%s or SYSTEMD_USER_SERVICE_%s value %s does not exist" % \
+                        (pkg_systemd, pkg_systemd, service))
 
     # Run all modifications once when creating package
     if os.path.exists(d.getVar("D", True)):
         for pkg in d.getVar('SYSTEMD_PACKAGES', True).split():
             systemd_check_package(pkg)
-            if d.getVar('SYSTEMD_SERVICE_' + pkg, True):
+            if d.getVar('SYSTEMD_SERVICE_' + pkg, True) or d.getVar('SYSTEMD_USER_SERVICE_' + pkg, True):
                 systemd_generate_package_scripts(pkg)
         systemd_check_services()
 }
Signature · 1 line
1.9.1
#3 lore raw

Re: [PATCH 2/3] systemd.bbclass: add support to manage user services

2016-09-07 11:22 GMT+02:00 Chen Qi <[email protected]>:

Quoted 26 lines

Add new variable SYSTEMD_USER_SERVICE and SYSTEM_USER_AUTO_ENABLE to manage user services. Their usage is like SYSTEMD_SERVICE and SYSTEMD_AUTO_ENABLE.

[YOCTO #7800]

---
 meta/classes/systemd.bbclass | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)
meta/classes/systemd.bbclass +1 -0
index db7873f..78cec97 100644
@@ -7,6 +7,7 @@ SYSTEMD_PACKAGES_class-nativesdk ?= ""
 
 # Whether to enable or disable the services on installation.
 SYSTEMD_AUTO_ENABLE ??= "enable"
+SYSTEMD_USER_AUTO_ENABLE ??= "enable"
 
 # This class will be included in any recipe that supports systemd init
 scripts,
 # even if systemd is not in DISTRO_FEATURES.  As such don't make any
changes

meta/classes/systemd.bbclass fi · line 36

Quoted 6 lines
meta/classes/systemd.bbclass +1 -0
@@ -30,10 +31,14 @@ fi
 
 if type systemctl >/dev/null 2>/dev/null; then
        systemctl $OPTS ${SYSTEMD_AUTO_ENABLE} ${SYSTEMD_SERVICE}
+       systemctl $OPTS --global ${SYSTEMD_USER_AUTO_ENABLE}
 ${SYSTEMD_USER_SERVICE}
 

I'm not sure having these 2 systemctl being executed together everytime is a good idea. What if a recipe has a user service and no system service? We are calling the first one with an empty system service? Or we are may be enabling a systemd system service which should not be enabled according to the recipe? I have the feeling this kind of cases are not being catch in here and other pkg scripts in this commit.

It is far from perfect, but in case you didn't, you may want to have a look at my initial/previous commit to try to fix the issue:
https://www.mail-archive.com/[email protected]/msg42187.html

Quoted 11 lines
meta/classes/systemd.bbclass +3 -0
        if [ -z "$D" -a "${SYSTEMD_AUTO_ENABLE}" = "enable" ]; then
                systemctl restart ${SYSTEMD_SERVICE}
        fi
+       if [ -z "$D" -a "${SYSTEMD_USER_AUTO_ENABLE}" = "enable" ]; then
+               systemctl --global restart ${SYSTEMD_USER_SERVICE}
+       fi
 fi
 }

meta/classes/systemd.bbclass fi · line 56

Quoted 5 lines
meta/classes/systemd.bbclass +1 -0
@@ -47,9 +52,11 @@ fi
 if type systemctl >/dev/null 2>/dev/null; then
        if [ -z "$D" ]; then
                systemctl stop ${SYSTEMD_SERVICE}
+               systemctl --global stop ${SYSTEMD_USER_SERVICE}
 

I think this is not gonna work, you cannot call --global with stop afair. I'm not sure which is the good solution for this though, but you should ideally go through all systemd user sessions and call "systemctl --user stop". No idea if that's actually easily feasible.

Quoted 60 lines
meta/classes/systemd.bbclass +1 -0
        fi
 
        systemctl $OPTS disable ${SYSTEMD_SERVICE}
+       systemctl $OPTS --global disable ${SYSTEMD_USER_SERVICE}
 fi
 }
@@ -139,12 +146,14 @@ python systemd_populate_packages() {
     def systemd_check_services():
         searchpaths = [oe.path.join(d.getVar("sysconfdir", True),

"systemd", "system"),]
searchpaths.append(d.getVar("systemd_system_unitdir", True))
+ searchpaths.append(oe.path.join(d.getVar("sysconfdir", True),
"systemd", "user"))

meta/classes/systemd.bbclass +1 -1
+        searchpaths.append(d.getVar("systemd_user_unitdir", True))
         systemd_packages = d.getVar('SYSTEMD_PACKAGES', True)
 
         keys = 'Also'
         # scan for all in SYSTEMD_SERVICE[]
         for pkg_systemd in systemd_packages.split():
-            for service in get_package_var(d, 'SYSTEMD_SERVICE',

pkg_systemd).split():
+ for service in (get_package_var(d, 'SYSTEMD_SERVICE',
pkg_systemd) + get_package_var(d, 'SYSTEMD_USER_SERVICE', pkg_systemd)).split():
path_found = ''

# Deal with adding, for example, '[email protected]'
from

meta/classes/systemd.bbclass +3 -3
@@ -165,14 +174,14 @@ python systemd_populate_packages() {
                 if path_found != '':
                     systemd_add_files_and_parse(pkg_systemd, path_found,
 service, keys)
                 else:
-                    raise bb.build.FuncFailed("SYSTEMD_SERVICE_%s value
 %s does not exist" % \
-                        (pkg_systemd, service))
+                    raise bb.build.FuncFailed("SYSTEMD_SERVICE_%s or
 SYSTEMD_USER_SERVICE_%s value %s does not exist" % \
+                        (pkg_systemd, pkg_systemd, service))
 
     # Run all modifications once when creating package
     if os.path.exists(d.getVar("D", True)):
         for pkg in d.getVar('SYSTEMD_PACKAGES', True).split():
             systemd_check_package(pkg)
-            if d.getVar('SYSTEMD_SERVICE_' + pkg, True):
+            if d.getVar('SYSTEMD_SERVICE_' + pkg, True) or

d.getVar('SYSTEMD_USER_SERVICE_' + pkg, True):
systemd_generate_package_scripts(pkg)
systemd_check_services()
}
--
1.9.1

--
_______________________________________________
Openembedded-core mailing list
[email protected]
http://lists.openembedded.org/mailman/listinfo/openembedded-core

author
#4 lore raw

Re: [PATCH 2/3] systemd.bbclass: add support to manage user services

Quoted 32 lines · Pau Espin Pedrol

2016-09-07 11:22 GMT+02:00 Chen Qi <[email protected] <mailto:[email protected]>>:

Add new variable SYSTEMD_USER_SERVICE and SYSTEM_USER_AUTO_ENABLE to manage user services. Their usage is like SYSTEMD_SERVICE and SYSTEMD_AUTO_ENABLE.

[YOCTO #7800]

<mailto:[email protected]>>
---
meta/classes/systemd.bbclass | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/meta/classes/systemd.bbclass b/meta/classes/systemd.bbclass
index db7873f..78cec97 100644
--- a/meta/classes/systemd.bbclass
+++ b/meta/classes/systemd.bbclass
@@ -7,6 +7,7 @@ SYSTEMD_PACKAGES_class-nativesdk ?= ""

# Whether to enable or disable the services on installation. SYSTEMD_AUTO_ENABLE ??= "enable"
+SYSTEMD_USER_AUTO_ENABLE ??= "enable"

# This class will be included in any recipe that supports systemd
init scripts,
# even if systemd is not in DISTRO_FEATURES. As such don't make
any changes
@@ -30,10 +31,14 @@ fi

Quoted 7 lines

if type systemctl >/dev/null 2>/dev/null; then
systemctl $OPTS ${SYSTEMD_AUTO_ENABLE} ${SYSTEMD_SERVICE}
+ systemctl $OPTS --global ${SYSTEMD_USER_AUTO_ENABLE}
${SYSTEMD_USER_SERVICE}

I'm not sure having these 2 systemctl being executed together everytime is a good idea. What if a recipe has a user service and no system service?

Hi Pau Espin Pedrol,

Thanks for your review.

The postinstall script runs successfully with expected result at rootfs time. But your question reminds me of the situation of the on-target install/remove situation. I think I'll need to make a new patch to make sure things work in both situations.

The key point here is that 'systemctl' at rootfs time is a shell script written by ourselves and 'systemctl' on target is that provided by systemd.

Quoted 22 lines

We are calling the first one with an empty system service? Or we are may be enabling a systemd system service which should not be enabled according to the recipe? I have the feeling this kind of cases are not being catch in here and other pkg scripts in this commit.

It is far from perfect, but in case you didn't, you may want to have a look at my initial/previous commit to try to fix the issue: https://www.mail-archive.com/[email protected]/msg42187.html

            if [ -z "$D" -a "${SYSTEMD_AUTO_ENABLE}" = "enable" ]; then
                    systemctl restart ${SYSTEMD_SERVICE}
            fi
    +       if [ -z "$D" -a "${SYSTEMD_USER_AUTO_ENABLE}" = "enable"
    ]; then
    +               systemctl --global restart ${SYSTEMD_USER_SERVICE}
    +       fi
     fi
     }

    @@ -47,9 +52,11 @@ fi
     if type systemctl >/dev/null 2>/dev/null; then
            if [ -z "$D" ]; then
                    systemctl stop ${SYSTEMD_SERVICE}
    +               systemctl --global stop ${SYSTEMD_USER_SERVICE}
Quoted 1 line

I think this is not gonna work, you cannot call --global with stop afair. I'm not sure which is the good solution for this though, but you should ideally go through all systemd user sessions and call "systemctl --user stop". No idea if that's actually easily feasible.

The service is enabled and started with '--global' option, and stopping it with '--global' option seems reasonable to me. If there's some special case, let's look into it then.

Best Regards,
Chen Qi

Quoted 63 lines

fi

systemctl $OPTS disable ${SYSTEMD_SERVICE}
+ systemctl $OPTS --global disable ${SYSTEMD_USER_SERVICE}
fi
}

@@ -139,12 +146,14 @@ python systemd_populate_packages() {
def systemd_check_services():
searchpaths = [oe.path.join(d.getVar("sysconfdir", True),
"systemd", "system"),]
searchpaths.append(d.getVar("systemd_system_unitdir", True))
+ searchpaths.append(oe.path.join(d.getVar("sysconfdir",
True), "systemd", "user"))
+ searchpaths.append(d.getVar("systemd_user_unitdir", True))
systemd_packages = d.getVar('SYSTEMD_PACKAGES', True)

keys = 'Also'
# scan for all in SYSTEMD_SERVICE[] for pkg_systemd in systemd_packages.split():
- for service in get_package_var(d, 'SYSTEMD_SERVICE',
pkg_systemd).split():
+ for service in (get_package_var(d, 'SYSTEMD_SERVICE',
pkg_systemd) + get_package_var(d, 'SYSTEMD_USER_SERVICE', pkg_systemd)).split():
path_found = ''

# Deal with adding, for example,
'[email protected]' from
@@ -165,14 +174,14 @@ python systemd_populate_packages() {
if path_found != '':
systemd_add_files_and_parse(pkg_systemd,
path_found, service, keys)
else:
- raise bb.build.FuncFailed("SYSTEMD_SERVICE_%s
value %s does not exist" % \
- (pkg_systemd, service))
+ raise bb.build.FuncFailed("SYSTEMD_SERVICE_%s
or SYSTEMD_USER_SERVICE_%s value %s does not exist" % \
+ (pkg_systemd, pkg_systemd, service))

# Run all modifications once when creating package if os.path.exists(d.getVar("D", True)):
for pkg in d.getVar('SYSTEMD_PACKAGES', True).split():
systemd_check_package(pkg)
- if d.getVar('SYSTEMD_SERVICE_' + pkg, True):
+ if d.getVar('SYSTEMD_SERVICE_' + pkg, True) or
d.getVar('SYSTEMD_USER_SERVICE_' + pkg, True):
systemd_generate_package_scripts(pkg)
systemd_check_services()
}
--
1.9.1

--
_______________________________________________
Openembedded-core mailing list [email protected] <mailto:[email protected]>
http://lists.openembedded.org/mailman/listinfo/openembedded-core
<http://lists.openembedded.org/mailman/listinfo/openembedded-core>

#5 lore raw

Re: [PATCH 2/3] systemd.bbclass: add support to manage user services

Hi,

Pau Espin Pedrol

2016-09-08 4:33 GMT+02:00 ChenQi <[email protected]>:

Quoted 46 lines · Pau Espin Pedrol

2016-09-07 11:22 GMT+02:00 Chen Qi <[email protected]>:

Quoted 26 lines

Add new variable SYSTEMD_USER_SERVICE and SYSTEM_USER_AUTO_ENABLE to manage user services. Their usage is like SYSTEMD_SERVICE and SYSTEMD_AUTO_ENABLE.

[YOCTO #7800]

---
 meta/classes/systemd.bbclass | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)
meta/classes/systemd.bbclass +1 -0
index db7873f..78cec97 100644
@@ -7,6 +7,7 @@ SYSTEMD_PACKAGES_class-nativesdk ?= ""
 
 # Whether to enable or disable the services on installation.
 SYSTEMD_AUTO_ENABLE ??= "enable"
+SYSTEMD_USER_AUTO_ENABLE ??= "enable"
 
 # This class will be included in any recipe that supports systemd init
 scripts,
 # even if systemd is not in DISTRO_FEATURES.  As such don't make any
changes

meta/classes/systemd.bbclass fi · line 36

Quoted 6 lines
meta/classes/systemd.bbclass +1 -0
@@ -30,10 +31,14 @@ fi
 
 if type systemctl >/dev/null 2>/dev/null; then
        systemctl $OPTS ${SYSTEMD_AUTO_ENABLE} ${SYSTEMD_SERVICE}
+       systemctl $OPTS --global ${SYSTEMD_USER_AUTO_ENABLE}
 ${SYSTEMD_USER_SERVICE}
 

I'm not sure having these 2 systemctl being executed together everytime is a good idea. What if a recipe has a user service and no system service?

Hi Pau Espin Pedrol,

Thanks for your review.

Quoted 8 lines

The postinstall script runs successfully with expected result at rootfs time. But your question reminds me of the situation of the on-target install/remove situation. I think I'll need to make a new patch to make sure things work in both situations.

The key point here is that 'systemctl' at rootfs time is a shell script written by ourselves and 'systemctl' on target is that provided by systemd.

Yes, I was unaware of the existance of that OE script when I worked on this. Why are we using that script at build time instead of using systemd's systemctl? Any good reason for that? git log only shows it was imported from another repo a while ago.

Quoted 35 lines

We are calling the first one with an empty system service? Or we are may be enabling a systemd system service which should not be enabled according to the recipe? I have the feeling this kind of cases are not being catch in here and other pkg scripts in this commit.

It is far from perfect, but in case you didn't, you may want to have a look at my initial/previous commit to try to fix the issue:
https://www.mail-archive.com/openembedded-devel@lists.
openembedded.org/msg42187.html

Quoted 11 lines
meta/classes/systemd.bbclass +3 -0
        if [ -z "$D" -a "${SYSTEMD_AUTO_ENABLE}" = "enable" ]; then
                systemctl restart ${SYSTEMD_SERVICE}
        fi
+       if [ -z "$D" -a "${SYSTEMD_USER_AUTO_ENABLE}" = "enable" ]; then
+               systemctl --global restart ${SYSTEMD_USER_SERVICE}
+       fi
 fi
 }

meta/classes/systemd.bbclass fi · line 56

Quoted 5 lines
meta/classes/systemd.bbclass +1 -0
@@ -47,9 +52,11 @@ fi
 if type systemctl >/dev/null 2>/dev/null; then
        if [ -z "$D" ]; then
                systemctl stop ${SYSTEMD_SERVICE}
+               systemctl --global stop ${SYSTEMD_USER_SERVICE}
 

I think this is not gonna work, you cannot call --global with stop afair. I'm not sure which is the good solution for this though, but you should ideally go through all systemd user sessions and call "systemctl --user stop". No idea if that's actually easily feasible.

Quoted 5 lines

The service is enabled and started with '--global' option, and stopping it with '--global' option seems reasonable to me. If there's some special case, let's look into it then.

Did you try running it in a running image? As in taking the ipkg, and reinstalling it live in the image with systemd's systemctl. I bet you will get an error from systemctl for both "--gobal restart" and "--global stop". At least it used to be like that afair.

Quoted 69 lines

Best Regards,
Chen Qi

Quoted 60 lines
meta/classes/systemd.bbclass +1 -0
        fi
 
        systemctl $OPTS disable ${SYSTEMD_SERVICE}
+       systemctl $OPTS --global disable ${SYSTEMD_USER_SERVICE}
 fi
 }
@@ -139,12 +146,14 @@ python systemd_populate_packages() {
     def systemd_check_services():
         searchpaths = [oe.path.join(d.getVar("sysconfdir", True),

"systemd", "system"),]
searchpaths.append(d.getVar("systemd_system_unitdir", True))
+ searchpaths.append(oe.path.join(d.getVar("sysconfdir", True),
"systemd", "user"))

meta/classes/systemd.bbclass +1 -1
+        searchpaths.append(d.getVar("systemd_user_unitdir", True))
         systemd_packages = d.getVar('SYSTEMD_PACKAGES', True)
 
         keys = 'Also'
         # scan for all in SYSTEMD_SERVICE[]
         for pkg_systemd in systemd_packages.split():
-            for service in get_package_var(d, 'SYSTEMD_SERVICE',

pkg_systemd).split():
+ for service in (get_package_var(d, 'SYSTEMD_SERVICE',
pkg_systemd) + get_package_var(d, 'SYSTEMD_USER_SERVICE', pkg_systemd)).split():
path_found = ''

# Deal with adding, for example, '[email protected]'
from

meta/classes/systemd.bbclass +3 -3
@@ -165,14 +174,14 @@ python systemd_populate_packages() {
                 if path_found != '':
                     systemd_add_files_and_parse(pkg_systemd,
 path_found, service, keys)
                 else:
-                    raise bb.build.FuncFailed("SYSTEMD_SERVICE_%s value
 %s does not exist" % \
-                        (pkg_systemd, service))
+                    raise bb.build.FuncFailed("SYSTEMD_SERVICE_%s or
 SYSTEMD_USER_SERVICE_%s value %s does not exist" % \
+                        (pkg_systemd, pkg_systemd, service))
 
     # Run all modifications once when creating package
     if os.path.exists(d.getVar("D", True)):
         for pkg in d.getVar('SYSTEMD_PACKAGES', True).split():
             systemd_check_package(pkg)
-            if d.getVar('SYSTEMD_SERVICE_' + pkg, True):
+            if d.getVar('SYSTEMD_SERVICE_' + pkg, True) or

d.getVar('SYSTEMD_USER_SERVICE_' + pkg, True):
systemd_generate_package_scripts(pkg)
systemd_check_services()
}
--
1.9.1

--
_______________________________________________
Openembedded-core mailing list
[email protected]
http://lists.openembedded.org/mailman/listinfo/openembedded-core

author
#6 lore raw

PATCH 3/3 pulseaudio: fix to manage user services corretly

In reply to Chen Qi

Make use of the new SYSTEMD_USER_SERVICE variable added in systemd.bbclass to manage user services in pulseaudio-server package.

---
 meta/recipes-multimedia/pulseaudio/pulseaudio.inc | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
meta/recipes-multimedia/pulseaudio/pulseaudio.inc +2 -2
index 6ed79ef..f3754d7 100644
@@ -124,8 +124,8 @@ FILES_${PN}-conf = "${sysconfdir}"
 FILES_${PN}-bin += "${sysconfdir}/default/volatiles/volatiles.04_pulse"
 FILES_${PN}-server = "${bindir}/pulseaudio ${bindir}/start-* ${sysconfdir} ${bindir}/pactl */udev/rules.d/*.rules */*/udev/rules.d/*.rules ${systemd_user_unitdir}/*"
 
-#SYSTEMD_PACKAGES = "${PN}-server"
-SYSTEMD_SERVICE_${PN}-server = "pulseaudio.service"
+SYSTEMD_PACKAGES = "${PN}-server"
+SYSTEMD_USER_SERVICE_${PN}-server = "pulseaudio.service pulseaudio.socket"
 
 FILES_${PN}-misc = "${bindir}/* ${libdir}/pulseaudio/libpulsedsp.so"
 
Signature · 1 line
1.9.1
#7 lore raw

Re: [PATCH 3/3] pulseaudio: fix to manage user services corretly

Pau Espin Pedrol

2016-09-07 11:22 GMT+02:00 Chen Qi <[email protected]>:

Quoted 25 lines

Make use of the new SYSTEMD_USER_SERVICE variable added in systemd.bbclass to manage user services in pulseaudio-server package.

---
 meta/recipes-multimedia/pulseaudio/pulseaudio.inc | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
meta/recipes-multimedia/pulseaudio/pulseaudio.inc +0 -0
index 6ed79ef..f3754d7 100644
@@ -124,8 +124,8 @@ FILES_${PN}-conf = "${sysconfdir}"
 FILES_${PN}-bin += "${sysconfdir}/default/volatiles/volatiles.04_pulse"
 FILES_${PN}-server = "${bindir}/pulseaudio ${bindir}/start-*

${sysconfdir} ${bindir}/pactl */udev/rules.d/*.rules */*/udev/rules.d/*.rules ${systemd_user_unitdir}/*"

meta/recipes-multimedia/pulseaudio/pulseaudio.inc +2 -2
-#SYSTEMD_PACKAGES = "${PN}-server"
-SYSTEMD_SERVICE_${PN}-server = "pulseaudio.service"
+SYSTEMD_PACKAGES = "${PN}-server"
+SYSTEMD_USER_SERVICE_${PN}-server = "pulseaudio.service

pulseaudio.socket"

Quoted 1 line

I think specifying "pulseaudio.socket" for SYSTEMD_USER_SERVICE_${PN}-server

should be enough, systemd.bbclass is going to add the .service file afair.

Quoted 11 lines

FILES_${PN}-misc = "${bindir}/* ${libdir}/pulseaudio/libpulsedsp.so"

--
1.9.1

--
_______________________________________________
Openembedded-core mailing list
[email protected]
http://lists.openembedded.org/mailman/listinfo/openembedded-core

author
#8 lore raw

Re: [PATCH 3/3] pulseaudio: fix to manage user services corretly

Quoted 34 lines · Pau Espin Pedrol

Pau Espin Pedrol

2016-09-07 11:22 GMT+02:00 Chen Qi <[email protected] <mailto:[email protected]>>:

    Make use of the new SYSTEMD_USER_SERVICE variable added in
    systemd.bbclass
    to manage user services in pulseaudio-server package.

    Signed-off-by: Chen Qi <[email protected]
    <mailto:[email protected]>>
    ---
     meta/recipes-multimedia/pulseaudio/pulseaudio.inc | 4 ++--
     1 file changed, 2 insertions(+), 2 deletions(-)

    diff --git a/meta/recipes-multimedia/pulseaudio/pulseaudio.inc
    b/meta/recipes-multimedia/pulseaudio/pulseaudio.inc
    index 6ed79ef..f3754d7 100644
    --- a/meta/recipes-multimedia/pulseaudio/pulseaudio.inc
    +++ b/meta/recipes-multimedia/pulseaudio/pulseaudio.inc
    @@ -124,8 +124,8 @@ FILES_${PN}-conf = "${sysconfdir}"
     FILES_${PN}-bin +=
    "${sysconfdir}/default/volatiles/volatiles.04_pulse"
     FILES_${PN}-server = "${bindir}/pulseaudio ${bindir}/start-*
    ${sysconfdir} ${bindir}/pactl */udev/rules.d/*.rules
    */*/udev/rules.d/*.rules ${systemd_user_unitdir}/*"

    -#SYSTEMD_PACKAGES = "${PN}-server"
    -SYSTEMD_SERVICE_${PN}-server = "pulseaudio.service"
    +SYSTEMD_PACKAGES = "${PN}-server"
    +SYSTEMD_USER_SERVICE_${PN}-server = "pulseaudio.service
    pulseaudio.socket"
Quoted 1 line

I think specifying "pulseaudio.socket" for SYSTEMD_USER_SERVICE_${PN}-server should be enough, systemd.bbclass is going to add the .service file afair.

Add both:
chenqi@pek-hostel-deb01:~/poky/build-systemd [1] $ ls tmp/work/qemux86_64-poky-linux/core-image-minimal/1.0-r0/rootfs/etc/systemd/user default.target.wants sockets.target.wants

Add pulseaudio.socket:
chenqi@pek-hostel-deb01:~/poky/build-systemd [1] $ ls tmp/work/qemux86_64-poky-linux/core-image-minimal/1.0-r0/rootfs/etc/systemd/user sockets.target.wants

They have different results.
I don't know a lot about pulseaudio. Can you confirm that pulseaudio.socket is enough?

Regards,
Chen Qi

Quoted 14 lines
     FILES_${PN}-misc = "${bindir}/* ${libdir}/pulseaudio/libpulsedsp.so"

    --
    1.9.1

    --
    _______________________________________________
    Openembedded-core mailing list
    [email protected]
    <mailto:[email protected]>
    http://lists.openembedded.org/mailman/listinfo/openembedded-core
    <http://lists.openembedded.org/mailman/listinfo/openembedded-core>
#9 lore raw

Re: [PATCH 3/3] pulseaudio: fix to manage user services corretly

Quoted 46 lines · ChenQi
Quoted 36 lines · Pau Espin Pedrol

Pau Espin Pedrol

2016-09-07 11:22 GMT+02:00 Chen Qi <[email protected] <mailto:[email protected]>>:

    Make use of the new SYSTEMD_USER_SERVICE variable added in
    systemd.bbclass
    to manage user services in pulseaudio-server package.

    Signed-off-by: Chen Qi <[email protected]
    <mailto:[email protected]>>
    ---
     meta/recipes-multimedia/pulseaudio/pulseaudio.inc | 4 ++--
     1 file changed, 2 insertions(+), 2 deletions(-)

    diff --git a/meta/recipes-multimedia/pulseaudio/pulseaudio.inc
    b/meta/recipes-multimedia/pulseaudio/pulseaudio.inc
    index 6ed79ef..f3754d7 100644
    --- a/meta/recipes-multimedia/pulseaudio/pulseaudio.inc
    +++ b/meta/recipes-multimedia/pulseaudio/pulseaudio.inc
    @@ -124,8 +124,8 @@ FILES_${PN}-conf = "${sysconfdir}"
     FILES_${PN}-bin +=
    "${sysconfdir}/default/volatiles/volatiles.04_pulse"
     FILES_${PN}-server = "${bindir}/pulseaudio ${bindir}/start-*
    ${sysconfdir} ${bindir}/pactl */udev/rules.d/*.rules
    */*/udev/rules.d/*.rules ${systemd_user_unitdir}/*"

    -#SYSTEMD_PACKAGES = "${PN}-server"
    -SYSTEMD_SERVICE_${PN}-server = "pulseaudio.service"
    +SYSTEMD_PACKAGES = "${PN}-server"
    +SYSTEMD_USER_SERVICE_${PN}-server = "pulseaudio.service
    pulseaudio.socket"
Quoted 3 lines

I think specifying "pulseaudio.socket" for SYSTEMD_USER_SERVICE_${PN}-server should be enough, systemd.bbclass is going to add the .service file afair.

Add both:
chenqi@pek-hostel-deb01:~/poky/build-systemd [1] $ ls tmp/work/qemux86_64-poky-linux/core-image-minimal/1.0-r0/rootfs/etc/systemd/user default.target.wants sockets.target.wants

Quoted 8 lines

Add pulseaudio.socket:
chenqi@pek-hostel-deb01:~/poky/build-systemd [1] $ ls tmp/work/qemux86_64-poky-linux/core-image-minimal/1.0-r0/rootfs/etc/systemd/user sockets.target.wants

They have different results.
I don't know a lot about pulseaudio. Can you confirm that pulseaudio.socket is enough?

If I understood correctly, Pau's argument was that there would be no difference, which doesn't seem to be the case. However, if pulseaudio.service isn't included in default.target.wants, that shouldn't be a big problem, because socket activation will anyway start the service when needed. On the other hand, I don't
think SYSTEMD_USER_SERVICE is intended to be used for controlling which units to enable - it looks more like a variable that should contain all units contained in the package. If my understanding is correct, you should list both units in the variable.

The distributions that I've seen to use systemd to manage pulseaudio only enable pulseaudio.socket, so that socket activation is used rather than starting pulseaudio automatically as part of the user session. I don't know if OE supports that - it looks like there's only a toggle to enable all units or none. Other distributions also patch client.conf to disable pulseaudio's own autospawning mechanism, since it's redundant when using socket activation. Leaving autospawning enabled shouldn't break anything in normal use, but it can cause some confusing behaviour if the user tries to manually disable automatic pulseaudio starting by disabling the systemd units.

--
Tanu

#10 lore raw

Re: [PATCH 3/3] pulseaudio: fix to manage user services corretly

Hi,

I think I didn't express myself correctly. Please note I did the longer investigations quite a while ago and I'm mainly talking from memory, so I may be wrong in some of the assumptions.

From systemd.bbclass, you can see this:
if service.find('.socket') != -1:
# for *.socket add *.service and *@.service service_base = service.replace('.socket', '') systemd_add_files_and_parse(pkg_systemd, path,
service_base + '.service', keys)
systemd_add_files_and_parse(pkg_systemd, path,
service_base + '@.service', keys)

So, for installation purposes, no .service is required in SYSTEMD_USER_SERVICE. Setting it .socket should also install (but not enable) the .service file together with the .socket one into the image, as actually the .socket one usually depends on the .service file at runtime.

That being said, of course it's not the same behavior if you add only one of them, the other or both:
1- Adding only .service -> It will install + enable the .service 2- Adding only the .socket -> It will install both. It will enable .socket 3- Adding .service + .socket -> It will install both. It will enable both.

Usually, you want either only the .service to be enabled (1, which will start it automatically at startup of user session), or only the .socket enabled (2, which will start only the socket at startup of user session, and only start pulseaudio process when required by some pulseaudio client). The third case, that is, installing + enabling both is usually not a good idea, at least it makes no sense to me.

So, we should consider only "1" or "2". I would personally go for 2nd option, and probably disable pulseaudio own autospawn system as explained by Tanu. This way we don't start pulseaudio unless when it's actually needed.

That is:
+SYSTEMD_PACKAGES = "${PN}-server" +SYSTEMD_USER_SERVICE_${PN}-server = "pulseaudio.socket"

And by the way, you should be able to remove "${systemd_user_unitdir}/*" from FILES_${PN}-server as it should be handled automatically by systemd.bbclass. If that's not the case, then probably there's some error in systemd.bbclass you should look into.

Hope I explained myself better now. Regards!

2016-09-08 13:22 GMT+02:00 Tanu Kaskinen <[email protected]>:

Quoted 84 lines · ChenQi
Quoted 46 lines · Pau Espin Pedrol
Quoted 36 lines

Pau Espin Pedrol

2016-09-07 11:22 GMT+02:00 Chen Qi <[email protected] <mailto:[email protected]>>:

    Make use of the new SYSTEMD_USER_SERVICE variable added in
    systemd.bbclass
    to manage user services in pulseaudio-server package.

    Signed-off-by: Chen Qi <[email protected]
    <mailto:[email protected]>>
    ---
     meta/recipes-multimedia/pulseaudio/pulseaudio.inc | 4 ++--
     1 file changed, 2 insertions(+), 2 deletions(-)

    diff --git a/meta/recipes-multimedia/pulseaudio/pulseaudio.inc
    b/meta/recipes-multimedia/pulseaudio/pulseaudio.inc
    index 6ed79ef..f3754d7 100644
    --- a/meta/recipes-multimedia/pulseaudio/pulseaudio.inc
    +++ b/meta/recipes-multimedia/pulseaudio/pulseaudio.inc
    @@ -124,8 +124,8 @@ FILES_${PN}-conf = "${sysconfdir}"
     FILES_${PN}-bin +=
    "${sysconfdir}/default/volatiles/volatiles.04_pulse"
     FILES_${PN}-server = "${bindir}/pulseaudio ${bindir}/start-*
    ${sysconfdir} ${bindir}/pactl */udev/rules.d/*.rules
    */*/udev/rules.d/*.rules ${systemd_user_unitdir}/*"

    -#SYSTEMD_PACKAGES = "${PN}-server"
    -SYSTEMD_SERVICE_${PN}-server = "pulseaudio.service"
    +SYSTEMD_PACKAGES = "${PN}-server"
    +SYSTEMD_USER_SERVICE_${PN}-server = "pulseaudio.service
    pulseaudio.socket"
Quoted 3 lines

I think specifying "pulseaudio.socket" for SYSTEMD_USER_SERVICE_${PN}-server should be enough, systemd.bbclass is going to add the .service file afair.

Add both:
chenqi@pek-hostel-deb01:~/poky/build-systemd [1] $ ls tmp/work/qemux86_64-poky-linux/core-image-minimal/1.0-r0/rootfs/etc/systemd/user default.target.wants sockets.target.wants

Quoted 8 lines

Add pulseaudio.socket:
chenqi@pek-hostel-deb01:~/poky/build-systemd [1] $ ls tmp/work/qemux86_64-poky-linux/core-image-minimal/1.0-r0/rootfs/etc/systemd/user sockets.target.wants

They have different results.
I don't know a lot about pulseaudio. Can you confirm that pulseaudio.socket is enough?

If I understood correctly, Pau's argument was that there would be no difference, which doesn't seem to be the case. However, if pulseaudio.service isn't included in default.target.wants, that shouldn't be a big problem, because socket activation will anyway start the service when needed. On the other hand, I don't
think SYSTEMD_USER_SERVICE is intended to be used for controlling which units to enable - it looks more like a variable that should contain all units contained in the package. If my understanding is correct, you should list both units in the variable.

The distributions that I've seen to use systemd to manage pulseaudio only enable pulseaudio.socket, so that socket activation is used rather than starting pulseaudio automatically as part of the user session. I don't know if OE supports that - it looks like there's only a toggle to enable all units or none. Other distributions also patch client.conf to disable pulseaudio's own autospawning mechanism, since it's redundant when using socket activation. Leaving autospawning enabled shouldn't break anything in normal use, but it can cause some confusing behaviour if the user tries to manually disable automatic pulseaudio starting by disabling the systemd units.

--
Tanu
--
_______________________________________________
Openembedded-core mailing list
[email protected]
http://lists.openembedded.org/mailman/listinfo/openembedded-core

#11 lore raw

Re: [PATCH 3/3] pulseaudio: fix to manage user services corretly

Quoted 44 lines · Pau Espin Pedrol

Hi,

I think I didn't express myself correctly. Please note I did the longer investigations quite a while ago and I'm mainly talking from memory, so I may be wrong in some of the assumptions.

From systemd.bbclass, you can see this:
if service.find('.socket') != -1:
# for *.socket add *.service and *@.service service_base = service.replace('.socket', '') systemd_add_files_and_parse(pkg_systemd, path,
service_base + '.service', keys)
systemd_add_files_and_parse(pkg_systemd, path,
service_base + '@.service', keys)

So, for installation purposes, no .service is required in SYSTEMD_USER_SERVICE. Setting it .socket should also install (but not enable) the .service file together with the .socket one into the image, as actually the .socket one usually depends on the .service file at runtime.

That being said, of course it's not the same behavior if you add only one of them, the other or both:
1- Adding only .service -> It will install + enable the .service 2- Adding only the .socket -> It will install both. It will enable .socket 3- Adding .service + .socket -> It will install both. It will enable both.

Usually, you want either only the .service to be enabled (1, which will start it automatically at startup of user session), or only the .socket enabled (2, which will start only the socket at startup of user session, and only start pulseaudio process when required by some pulseaudio client). The third case, that is, installing + enabling both is usually not a good idea, at least it makes no sense to me.

So, we should consider only "1" or "2". I would personally go for 2nd option, and probably disable pulseaudio own autospawn system as explained by Tanu. This way we don't start pulseaudio unless when it's actually needed.

That is:

(quoted diff) +2 -0
+SYSTEMD_PACKAGES = "${PN}-server"
+SYSTEMD_USER_SERVICE_${PN}-server = "pulseaudio.socket"
Quoted 6 lines

And by the way, you should be able to remove "${systemd_user_unitdir}/*" from FILES_${PN}-server as it should be handled automatically by systemd.bbclass. If that's not the case, then probably there's some error in systemd.bbclass you should look into.

Hope I explained myself better now. Regards!

Thanks for the explanation. Part of my confusion was that I incorrectly assumed that systemd.bbclass couldn't guess that the pulseaudio-server package contains systemd units (due to the -server suffix), and therefore it seemed to me that SYSTEMD_USER_SERVICE is expected to contain all units, but that problem is fixed by the SYSTEMD_PACKAGES variable.

Still, I think SYSTEMD_USER_SERVICE is a misleading variable name, if the actual purpose of that variable is only to control which units to enable during installation. Something like SYSTEMD_USER_ENABLE_UNITS would be better. However, this problem is older than these patches (due to SYSTEMD_SERVICE for system units), so I don't suggest that the patches should be changed because of this. It indeed seems like the best course of action would be to only add the pulseaudio.socket unit to SYSTEMD_USER_SERVICE. Patching client.conf to disable autospawning in case systemd is enabled would be a good addition, although not strictly necessary.

--
Tanu

#12 lore raw
In reply to Chen Qi

It's probably worth updating the documentation too... this is a usefull feature and it deserves the exposure

Regards

Jeremy Rosen

Quoted 19 lines · Chen Qi

The following changes since commit 55bb6816aca39bfa25d4f7e2158a57a5f0ac1cca:

   oeqa.buildperf: correct globalres time format (2016-09-06 10:24:00 +0100)

are available in the git repository at:

git://git.openembedded.org/openembedded-core-contrib ChenQi/systemd-user-units
http://cgit.openembedded.org/cgit.cgi/openembedded-core-contrib/log/?h=ChenQi/systemd-user-units

Chen Qi (3):
systemd-systemctl: add option to manage user services systemd.bbclass: add support to manage user services
pulseaudio: fix to manage user services corretly

  meta/classes/systemd.bbclass                       | 17 ++++++--
  .../systemd/systemd-systemctl/systemctl            | 45 ++++++++++++++--------
  meta/recipes-multimedia/pulseaudio/pulseaudio.inc  |  4 +-
  3 files changed, 43 insertions(+), 23 deletions(-)