* [PATCH 0/3] systemd: add support to manage user units
@ 2016-09-07 9:22 Chen Qi
2016-09-07 9:22 ` [PATCH 1/3] systemd-systemctl: add option to manage user services Chen Qi
` (3 more replies)
0 siblings, 4 replies; 13+ messages in thread
From: Chen Qi @ 2016-09-07 9:22 UTC (permalink / raw)
To: openembedded-core
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(-)
--
1.9.1
^ permalink raw reply [flat|nested] 13+ messages in thread* [PATCH 1/3] systemd-systemctl: add option to manage user services 2016-09-07 9:22 [PATCH 0/3] systemd: add support to manage user units Chen Qi @ 2016-09-07 9:22 ` Chen Qi 2016-09-07 9:22 ` [PATCH 2/3] systemd.bbclass: add support " Chen Qi ` (2 subsequent siblings) 3 siblings, 0 replies; 13+ messages in thread From: Chen Qi @ 2016-09-07 9:22 UTC (permalink / raw) To: openembedded-core Add '--global' option to our own systemctl script to manage user services. [YOCTO #7800] Signed-off-by: Chen Qi <Qi.Chen@windriver.com> --- .../systemd/systemd-systemctl/systemctl | 45 ++++++++++++++-------- 1 file changed, 28 insertions(+), 17 deletions(-) diff --git a/meta/recipes-core/systemd/systemd-systemctl/systemctl b/meta/recipes-core/systemd/systemd-systemctl/systemctl index efad14c..17a7277 100755 --- a/meta/recipes-core/systemd/systemd-systemctl/systemctl +++ b/meta/recipes-core/systemd/systemd-systemctl/systemctl @@ -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 -- 1.9.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 2/3] systemd.bbclass: add support to manage user services 2016-09-07 9:22 [PATCH 0/3] systemd: add support to manage user units Chen Qi 2016-09-07 9:22 ` [PATCH 1/3] systemd-systemctl: add option to manage user services Chen Qi @ 2016-09-07 9:22 ` Chen Qi 2016-09-07 12:14 ` Pau Espin Pedrol 2016-09-07 9:22 ` [PATCH 3/3] pulseaudio: fix to manage user services corretly Chen Qi 2016-09-07 9:43 ` [PATCH 0/3] systemd: add support to manage user units Jérémy Rosen 3 siblings, 1 reply; 13+ messages in thread From: Chen Qi @ 2016-09-07 9:22 UTC (permalink / raw) To: openembedded-core 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] Signed-off-by: Chen Qi <Qi.Chen@windriver.com> --- 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 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, 'ifplugd@eth0.service' 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 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] systemd.bbclass: add support to manage user services 2016-09-07 9:22 ` [PATCH 2/3] systemd.bbclass: add support " Chen Qi @ 2016-09-07 12:14 ` Pau Espin Pedrol 2016-09-08 2:33 ` ChenQi 0 siblings, 1 reply; 13+ messages in thread From: Pau Espin Pedrol @ 2016-09-07 12:14 UTC (permalink / raw) To: Chen Qi; +Cc: OE-core [-- Attachment #1: Type: text/plain, Size: 5157 bytes --] 2016-09-07 11:22 GMT+02:00 Chen Qi <Qi.Chen@windriver.com>: > 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] > > Signed-off-by: Chen Qi <Qi.Chen@windriver.com> > --- > 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 > > 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/openembedded-devel@lists.openembedded.org/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} > 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. > 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, 'ifplugd@eth0.service' > 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 > Openembedded-core@lists.openembedded.org > http://lists.openembedded.org/mailman/listinfo/openembedded-core > [-- Attachment #2: Type: text/html, Size: 7044 bytes --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] systemd.bbclass: add support to manage user services 2016-09-07 12:14 ` Pau Espin Pedrol @ 2016-09-08 2:33 ` ChenQi 2016-09-09 17:48 ` Pau Espin Pedrol 0 siblings, 1 reply; 13+ messages in thread From: ChenQi @ 2016-09-08 2:33 UTC (permalink / raw) To: Pau Espin Pedrol; +Cc: OE-core [-- Attachment #1: Type: text/plain, Size: 6513 bytes --] On 09/07/2016 08:14 PM, Pau Espin Pedrol wrote: > > 2016-09-07 11:22 GMT+02:00 Chen Qi <Qi.Chen@windriver.com > <mailto:Qi.Chen@windriver.com>>: > > 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] > > Signed-off-by: Chen Qi <Qi.Chen@windriver.com > <mailto:Qi.Chen@windriver.com>> > --- > 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 > > 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. > 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 > > > 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} > > > 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 > 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, > 'ifplugd@eth0.service' 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 > 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> > > [-- Attachment #2: Type: text/html, Size: 11249 bytes --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] systemd.bbclass: add support to manage user services 2016-09-08 2:33 ` ChenQi @ 2016-09-09 17:48 ` Pau Espin Pedrol 0 siblings, 0 replies; 13+ messages in thread From: Pau Espin Pedrol @ 2016-09-09 17:48 UTC (permalink / raw) To: ChenQi; +Cc: OE-core [-- Attachment #1: Type: text/plain, Size: 6712 bytes --] Hi, Pau Espin Pedrol 2016-09-08 4:33 GMT+02:00 ChenQi <Qi.Chen@windriver.com>: > On 09/07/2016 08:14 PM, Pau Espin Pedrol wrote: > > > 2016-09-07 11:22 GMT+02:00 Chen Qi <Qi.Chen@windriver.com>: > >> 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] >> >> Signed-off-by: Chen Qi <Qi.Chen@windriver.com> >> --- >> 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 >> >> 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. > 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. > > 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 > > >> >> 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} >> > > 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. > > 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. > Best Regards, > Chen Qi > > > > >> 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, 'ifplugd@eth0.service' >> 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 >> Openembedded-core@lists.openembedded.org >> http://lists.openembedded.org/mailman/listinfo/openembedded-core >> > > > [-- Attachment #2: Type: text/html, Size: 12459 bytes --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 3/3] pulseaudio: fix to manage user services corretly 2016-09-07 9:22 [PATCH 0/3] systemd: add support to manage user units Chen Qi 2016-09-07 9:22 ` [PATCH 1/3] systemd-systemctl: add option to manage user services Chen Qi 2016-09-07 9:22 ` [PATCH 2/3] systemd.bbclass: add support " Chen Qi @ 2016-09-07 9:22 ` Chen Qi 2016-09-07 10:29 ` Pau Espin Pedrol 2016-09-07 9:43 ` [PATCH 0/3] systemd: add support to manage user units Jérémy Rosen 3 siblings, 1 reply; 13+ messages in thread From: Chen Qi @ 2016-09-07 9:22 UTC (permalink / raw) To: openembedded-core 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 <Qi.Chen@windriver.com> --- 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" FILES_${PN}-misc = "${bindir}/* ${libdir}/pulseaudio/libpulsedsp.so" -- 1.9.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 3/3] pulseaudio: fix to manage user services corretly 2016-09-07 9:22 ` [PATCH 3/3] pulseaudio: fix to manage user services corretly Chen Qi @ 2016-09-07 10:29 ` Pau Espin Pedrol 2016-09-08 6:34 ` ChenQi 0 siblings, 1 reply; 13+ messages in thread From: Pau Espin Pedrol @ 2016-09-07 10:29 UTC (permalink / raw) To: Chen Qi; +Cc: OE-core [-- Attachment #1: Type: text/plain, Size: 1659 bytes --] Pau Espin Pedrol 2016-09-07 11:22 GMT+02:00 Chen Qi <Qi.Chen@windriver.com>: > 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 <Qi.Chen@windriver.com> > --- > 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" > > I think specifying "pulseaudio.socket" for SYSTEMD_USER_SERVICE_${PN}-server should be enough, systemd.bbclass is going to add the .service file afair. > FILES_${PN}-misc = "${bindir}/* ${libdir}/pulseaudio/libpulsedsp.so" > > -- > 1.9.1 > > -- > _______________________________________________ > Openembedded-core mailing list > Openembedded-core@lists.openembedded.org > http://lists.openembedded.org/mailman/listinfo/openembedded-core > [-- Attachment #2: Type: text/html, Size: 2726 bytes --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 3/3] pulseaudio: fix to manage user services corretly 2016-09-07 10:29 ` Pau Espin Pedrol @ 2016-09-08 6:34 ` ChenQi 2016-09-08 11:22 ` Tanu Kaskinen 0 siblings, 1 reply; 13+ messages in thread From: ChenQi @ 2016-09-08 6:34 UTC (permalink / raw) To: Pau Espin Pedrol; +Cc: OE-core [-- Attachment #1: Type: text/plain, Size: 2588 bytes --] On 09/07/2016 06:29 PM, Pau Espin Pedrol wrote: > > > Pau Espin Pedrol > > 2016-09-07 11:22 GMT+02:00 Chen Qi <Qi.Chen@windriver.com > <mailto:Qi.Chen@windriver.com>>: > > 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 <Qi.Chen@windriver.com > <mailto:Qi.Chen@windriver.com>> > --- > 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" > > 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 > FILES_${PN}-misc = "${bindir}/* ${libdir}/pulseaudio/libpulsedsp.so" > > -- > 1.9.1 > > -- > _______________________________________________ > 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> > > [-- Attachment #2: Type: text/html, Size: 5203 bytes --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 3/3] pulseaudio: fix to manage user services corretly 2016-09-08 6:34 ` ChenQi @ 2016-09-08 11:22 ` Tanu Kaskinen 2016-09-09 18:10 ` Pau Espin Pedrol 0 siblings, 1 reply; 13+ messages in thread From: Tanu Kaskinen @ 2016-09-08 11:22 UTC (permalink / raw) To: openembedded-core On Thu, 2016-09-08 at 14:34 +0800, ChenQi wrote: > On 09/07/2016 06:29 PM, Pau Espin Pedrol wrote: > > > > > > > > Pau Espin Pedrol > > > > 2016-09-07 11:22 GMT+02:00 Chen Qi <Qi.Chen@windriver.com > > <mailto:Qi.Chen@windriver.com>>: > > > > 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 <Qi.Chen@windriver.com > > <mailto:Qi.Chen@windriver.com>> > > --- > > 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" > > > > 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? 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 3/3] pulseaudio: fix to manage user services corretly 2016-09-08 11:22 ` Tanu Kaskinen @ 2016-09-09 18:10 ` Pau Espin Pedrol 2016-09-09 18:34 ` Tanu Kaskinen 0 siblings, 1 reply; 13+ messages in thread From: Pau Espin Pedrol @ 2016-09-09 18:10 UTC (permalink / raw) To: Tanu Kaskinen, Chen Qi; +Cc: OE-core 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 <tanuk@iki.fi>: > On Thu, 2016-09-08 at 14:34 +0800, ChenQi wrote: >> On 09/07/2016 06:29 PM, Pau Espin Pedrol wrote: >> > >> > >> > >> > Pau Espin Pedrol >> > >> > 2016-09-07 11:22 GMT+02:00 Chen Qi <Qi.Chen@windriver.com >> > <mailto:Qi.Chen@windriver.com>>: >> > >> > 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 <Qi.Chen@windriver.com >> > <mailto:Qi.Chen@windriver.com>> >> > --- >> > 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" >> > >> > 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? > > 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 > Openembedded-core@lists.openembedded.org > http://lists.openembedded.org/mailman/listinfo/openembedded-core ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 3/3] pulseaudio: fix to manage user services corretly 2016-09-09 18:10 ` Pau Espin Pedrol @ 2016-09-09 18:34 ` Tanu Kaskinen 0 siblings, 0 replies; 13+ messages in thread From: Tanu Kaskinen @ 2016-09-09 18:34 UTC (permalink / raw) To: openembedded-core On Fri, 2016-09-09 at 20:10 +0200, Pau Espin Pedrol wrote: > 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! 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 0/3] systemd: add support to manage user units 2016-09-07 9:22 [PATCH 0/3] systemd: add support to manage user units Chen Qi ` (2 preceding siblings ...) 2016-09-07 9:22 ` [PATCH 3/3] pulseaudio: fix to manage user services corretly Chen Qi @ 2016-09-07 9:43 ` Jérémy Rosen 3 siblings, 0 replies; 13+ messages in thread From: Jérémy Rosen @ 2016-09-07 9:43 UTC (permalink / raw) To: openembedded-core It's probably worth updating the documentation too... this is a usefull feature and it deserves the exposure Regards Jeremy Rosen On 07/09/2016 11:22, Chen Qi wrote: > 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(-) > ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2016-09-09 18:34 UTC | newest] Thread overview: 13+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-09-07 9:22 [PATCH 0/3] systemd: add support to manage user units Chen Qi 2016-09-07 9:22 ` [PATCH 1/3] systemd-systemctl: add option to manage user services Chen Qi 2016-09-07 9:22 ` [PATCH 2/3] systemd.bbclass: add support " Chen Qi 2016-09-07 12:14 ` Pau Espin Pedrol 2016-09-08 2:33 ` ChenQi 2016-09-09 17:48 ` Pau Espin Pedrol 2016-09-07 9:22 ` [PATCH 3/3] pulseaudio: fix to manage user services corretly Chen Qi 2016-09-07 10:29 ` Pau Espin Pedrol 2016-09-08 6:34 ` ChenQi 2016-09-08 11:22 ` Tanu Kaskinen 2016-09-09 18:10 ` Pau Espin Pedrol 2016-09-09 18:34 ` Tanu Kaskinen 2016-09-07 9:43 ` [PATCH 0/3] systemd: add support to manage user units Jérémy Rosen
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox