From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail1.windriver.com (mail1.windriver.com [147.11.146.13]) by mail.openembedded.org (Postfix) with ESMTP id F281760620 for ; Thu, 8 Sep 2016 02:32:54 +0000 (UTC) Received: from ALA-HCA.corp.ad.wrs.com (ala-hca.corp.ad.wrs.com [147.11.189.40]) by mail1.windriver.com (8.15.2/8.15.1) with ESMTPS id u882Wq13006302 (version=TLSv1 cipher=AES128-SHA bits=128 verify=FAIL); Wed, 7 Sep 2016 19:32:53 -0700 (PDT) Received: from [128.224.162.229] (128.224.162.229) by ALA-HCA.corp.ad.wrs.com (147.11.189.50) with Microsoft SMTP Server (TLS) id 14.3.248.2; Wed, 7 Sep 2016 19:32:51 -0700 To: Pau Espin Pedrol References: <4e703d64c6d15986135603c94ad9730b7f6d66c7.1473240119.git.Qi.Chen@windriver.com> From: ChenQi Message-ID: <57D0CDFE.7090101@windriver.com> Date: Thu, 8 Sep 2016 10:33:34 +0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.7.2 MIME-Version: 1.0 In-Reply-To: X-Originating-IP: [128.224.162.229] Cc: OE-core Subject: Re: [PATCH 2/3] systemd.bbclass: add support to manage user services X-BeenThere: openembedded-core@lists.openembedded.org X-Mailman-Version: 2.1.12 Precedence: list List-Id: Patches and discussions about the oe-core layer List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 08 Sep 2016 02:32:55 -0000 Content-Type: multipart/alternative; boundary="------------070809050002000002080700" --------------070809050002000002080700 Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit On 09/07/2016 08:14 PM, Pau Espin Pedrol wrote: > > 2016-09-07 11:22 GMT+02:00 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] > > Signed-off-by: Chen Qi > > --- > 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 > > http://lists.openembedded.org/mailman/listinfo/openembedded-core > > > --------------070809050002000002080700 Content-Type: text/html; charset="utf-8" Content-Transfer-Encoding: 8bit
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.

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
http://lists.openembedded.org/mailman/listinfo/openembedded-core


--------------070809050002000002080700--