[V3] update-rc.d: support enable/disable options

Submitted by changqing.li@windriver.com on April 30, 2019, 6:56 a.m. | Patch ID: 160782

Details

Message ID 1556607386-98960-1-git-send-email-changqing.li@windriver.com
State New
Headers show

Commit Message

changqing.li@windriver.com April 30, 2019, 6:56 a.m.
From: Changqing Li <changqing.li@windriver.com>

* update-rc.d has added support of enable/disable options, which are
  expected to keep the previous configuration even after upgrade the packages.
  With support for these options, it will only create start/stop link
  when there are none, or it will keep the previous configuration.

  Our preinst uses "-f remove" to remove any links under the /etc/rcrunlevel.d
  which is conflicting behavior with disable/enable options, so remove it.

  For example, if a user disabled one service before upgrade,
  then after upgrade the service could be started. This happens because during preinst,
  all links have been deleted, then postinst may create the link to start service.

  With this change, we remove preinst and therefore keep the previous links
  so that after upgrade, if a link existed for the package, then the postinst
  will not create new start/stop links.

* remove '-f' for postinst. Previously, the keepalived recipe used 'remove'
  during postinst, so we needed the -f, but now the keepalived recipe has fixed
  this problem, so it's safe to remove '-f'.

[Yocto #12955]

Signed-off-by: Changqing Li <changqing.li@windriver.com>
---
 documentation/ref-manual/ref-variables.xml |  2 +-
 meta/classes/update-rc.d.bbclass           | 28 ++++------------------------
 2 files changed, 5 insertions(+), 25 deletions(-)

Patch hide | download patch | download mbox

diff --git a/documentation/ref-manual/ref-variables.xml b/documentation/ref-manual/ref-variables.xml
index 536bd15..8fc48c3 100644
--- a/documentation/ref-manual/ref-variables.xml
+++ b/documentation/ref-manual/ref-variables.xml
@@ -7243,7 +7243,7 @@ 
                     to the <filename>update-rc.d</filename> command.
                     For more information on valid parameters, please see the
                     <filename>update-rc.d</filename> manual page at
-                    <ulink url='http://www.tin.org/bin/man.cgi?section=8&amp;topic=update-rc.d'></ulink>.
+                    <ulink url='https://manpages.debian.org/wheezy/sysv-rc/update-rc.d.8.en.html'></ulink>.
                 </para>
             </glossdef>
         </glossentry>
diff --git a/meta/classes/update-rc.d.bbclass b/meta/classes/update-rc.d.bbclass
index 265c4be..1366fee 100644
--- a/meta/classes/update-rc.d.bbclass
+++ b/meta/classes/update-rc.d.bbclass
@@ -20,28 +20,14 @@  def use_updatercd(d):
         return '[ -n "$D" -o ! -d /run/systemd/system ]'
     return 'true'
 
-updatercd_preinst() {
-if ${@use_updatercd(d)} && [ -z "$D" -a -f "${INIT_D_DIR}/${INITSCRIPT_NAME}" ]; then
-	${INIT_D_DIR}/${INITSCRIPT_NAME} stop || :
-fi
-if ${@use_updatercd(d)} && type update-rc.d >/dev/null 2>/dev/null; then
-	if [ -n "$D" ]; then
-		OPT="-f -r $D"
-	else
-		OPT="-f"
-	fi
-	update-rc.d $OPT ${INITSCRIPT_NAME} remove
-fi
-}
-
 PACKAGE_WRITE_DEPS += "update-rc.d-native"
 
 updatercd_postinst() {
 if ${@use_updatercd(d)} && type update-rc.d >/dev/null 2>/dev/null; then
 	if [ -n "$D" ]; then
-		OPT="-f -r $D"
+		OPT="-r $D"
 	else
-		OPT="-f -s"
+		OPT="-s"
 	fi
 	update-rc.d $OPT ${INITSCRIPT_NAME} ${INITSCRIPT_PARAMS}
 fi
@@ -79,7 +65,7 @@  python __anonymous() {
 PACKAGESPLITFUNCS_prepend = "${@bb.utils.contains('DISTRO_FEATURES', 'sysvinit', 'populate_packages_updatercd ', '', d)}"
 PACKAGESPLITFUNCS_remove_class-nativesdk = "populate_packages_updatercd "
 
-populate_packages_updatercd[vardeps] += "updatercd_prerm updatercd_postrm updatercd_preinst updatercd_postinst"
+populate_packages_updatercd[vardeps] += "updatercd_prerm updatercd_postrm updatercd_postinst"
 populate_packages_updatercd[vardepsexclude] += "OVERRIDES"
 
 python populate_packages_updatercd () {
@@ -95,7 +81,7 @@  python populate_packages_updatercd () {
             d.appendVar('RDEPENDS_' + pkg, ' %sinitd-functions' % (mlprefix))
 
     def update_rcd_package(pkg):
-        bb.debug(1, 'adding update-rc.d calls to preinst/postinst/prerm/postrm for %s' % pkg)
+        bb.debug(1, 'adding update-rc.d calls to postinst/prerm/postrm for %s' % pkg)
 
         localdata = bb.data.createCopy(d)
         overrides = localdata.getVar("OVERRIDES")
@@ -103,12 +89,6 @@  python populate_packages_updatercd () {
 
         update_rcd_auto_depend(pkg)
 
-        preinst = d.getVar('pkg_preinst_%s' % pkg)
-        if not preinst:
-            preinst = '#!/bin/sh\n'
-        preinst += localdata.getVar('updatercd_preinst')
-        d.setVar('pkg_preinst_%s' % pkg, preinst)
-
         postinst = d.getVar('pkg_postinst_%s' % pkg)
         if not postinst:
             postinst = '#!/bin/sh\n'

Comments

changqing.li@windriver.com June 20, 2019, 2:07 a.m.
Please also help to review this patch, Thanks.

On 4/30/19 2:56 PM, changqing.li@windriver.com wrote:
> From: Changqing Li <changqing.li@windriver.com>
>
> * update-rc.d has added support of enable/disable options, which are
>    expected to keep the previous configuration even after upgrade the packages.
>    With support for these options, it will only create start/stop link
>    when there are none, or it will keep the previous configuration.
>
>    Our preinst uses "-f remove" to remove any links under the /etc/rcrunlevel.d
>    which is conflicting behavior with disable/enable options, so remove it.
>
>    For example, if a user disabled one service before upgrade,
>    then after upgrade the service could be started. This happens because during preinst,
>    all links have been deleted, then postinst may create the link to start service.
>
>    With this change, we remove preinst and therefore keep the previous links
>    so that after upgrade, if a link existed for the package, then the postinst
>    will not create new start/stop links.
>
> * remove '-f' for postinst. Previously, the keepalived recipe used 'remove'
>    during postinst, so we needed the -f, but now the keepalived recipe has fixed
>    this problem, so it's safe to remove '-f'.
>
> [Yocto #12955]
>
> Signed-off-by: Changqing Li <changqing.li@windriver.com>
> ---
>   documentation/ref-manual/ref-variables.xml |  2 +-
>   meta/classes/update-rc.d.bbclass           | 28 ++++------------------------
>   2 files changed, 5 insertions(+), 25 deletions(-)
>
> diff --git a/documentation/ref-manual/ref-variables.xml b/documentation/ref-manual/ref-variables.xml
> index 536bd15..8fc48c3 100644
> --- a/documentation/ref-manual/ref-variables.xml
> +++ b/documentation/ref-manual/ref-variables.xml
> @@ -7243,7 +7243,7 @@
>                       to the <filename>update-rc.d</filename> command.
>                       For more information on valid parameters, please see the
>                       <filename>update-rc.d</filename> manual page at
> -                    <ulink url='http://www.tin.org/bin/man.cgi?section=8&amp;topic=update-rc.d'></ulink>.
> +                    <ulink url='https://manpages.debian.org/wheezy/sysv-rc/update-rc.d.8.en.html'></ulink>.
>                   </para>
>               </glossdef>
>           </glossentry>
> diff --git a/meta/classes/update-rc.d.bbclass b/meta/classes/update-rc.d.bbclass
> index 265c4be..1366fee 100644
> --- a/meta/classes/update-rc.d.bbclass
> +++ b/meta/classes/update-rc.d.bbclass
> @@ -20,28 +20,14 @@ def use_updatercd(d):
>           return '[ -n "$D" -o ! -d /run/systemd/system ]'
>       return 'true'
>   
> -updatercd_preinst() {
> -if ${@use_updatercd(d)} && [ -z "$D" -a -f "${INIT_D_DIR}/${INITSCRIPT_NAME}" ]; then
> -	${INIT_D_DIR}/${INITSCRIPT_NAME} stop || :
> -fi
> -if ${@use_updatercd(d)} && type update-rc.d >/dev/null 2>/dev/null; then
> -	if [ -n "$D" ]; then
> -		OPT="-f -r $D"
> -	else
> -		OPT="-f"
> -	fi
> -	update-rc.d $OPT ${INITSCRIPT_NAME} remove
> -fi
> -}
> -
>   PACKAGE_WRITE_DEPS += "update-rc.d-native"
>   
>   updatercd_postinst() {
>   if ${@use_updatercd(d)} && type update-rc.d >/dev/null 2>/dev/null; then
>   	if [ -n "$D" ]; then
> -		OPT="-f -r $D"
> +		OPT="-r $D"
>   	else
> -		OPT="-f -s"
> +		OPT="-s"
>   	fi
>   	update-rc.d $OPT ${INITSCRIPT_NAME} ${INITSCRIPT_PARAMS}
>   fi
> @@ -79,7 +65,7 @@ python __anonymous() {
>   PACKAGESPLITFUNCS_prepend = "${@bb.utils.contains('DISTRO_FEATURES', 'sysvinit', 'populate_packages_updatercd ', '', d)}"
>   PACKAGESPLITFUNCS_remove_class-nativesdk = "populate_packages_updatercd "
>   
> -populate_packages_updatercd[vardeps] += "updatercd_prerm updatercd_postrm updatercd_preinst updatercd_postinst"
> +populate_packages_updatercd[vardeps] += "updatercd_prerm updatercd_postrm updatercd_postinst"
>   populate_packages_updatercd[vardepsexclude] += "OVERRIDES"
>   
>   python populate_packages_updatercd () {
> @@ -95,7 +81,7 @@ python populate_packages_updatercd () {
>               d.appendVar('RDEPENDS_' + pkg, ' %sinitd-functions' % (mlprefix))
>   
>       def update_rcd_package(pkg):
> -        bb.debug(1, 'adding update-rc.d calls to preinst/postinst/prerm/postrm for %s' % pkg)
> +        bb.debug(1, 'adding update-rc.d calls to postinst/prerm/postrm for %s' % pkg)
>   
>           localdata = bb.data.createCopy(d)
>           overrides = localdata.getVar("OVERRIDES")
> @@ -103,12 +89,6 @@ python populate_packages_updatercd () {
>   
>           update_rcd_auto_depend(pkg)
>   
> -        preinst = d.getVar('pkg_preinst_%s' % pkg)
> -        if not preinst:
> -            preinst = '#!/bin/sh\n'
> -        preinst += localdata.getVar('updatercd_preinst')
> -        d.setVar('pkg_preinst_%s' % pkg, preinst)
> -
>           postinst = d.getVar('pkg_postinst_%s' % pkg)
>           if not postinst:
>               postinst = '#!/bin/sh\n'
Richard Purdie June 20, 2019, 7:28 a.m.
On Thu, 2019-06-20 at 10:07 +0800, Changqing Li wrote:
> Please also help to review this patch, Thanks.

This patch needs to go to OE-Core for the bbclass part. The
documentation piece needs to be separated out as its a different 
repository. Also doesn't it depend on the change I just made to the
update-rc.d repo so there will be a SRCREV that needs to be updated
that this depends on?

Thanks,

Richard
changqing.li@windriver.com June 20, 2019, 9:09 a.m.
On 6/20/19 3:28 PM, richard.purdie@linuxfoundation.org wrote:
> On Thu, 2019-06-20 at 10:07 +0800, Changqing Li wrote:
>> Please also help to review this patch, Thanks.
> This patch needs to go to OE-Core for the bbclass part. The
> documentation piece needs to be separated out as its a different
> repository. Also doesn't it depend on the change I just made to the
> update-rc.d repo so there will be a SRCREV that needs to be updated
> that this depends on?

I will  split   the oe-core and document part.

without change in update-rc.d.bbclass, new function add in update-rc.d 
repo will not work.

since preinst in update-rc.d.bbclass will delete all the links under  
rcrunlevel.d, this behavior

conflicts with enable/disable options.

and you're right,  I also need to update SRCREV of update-rc.d.bb in 
oe-core repo.

Thanks.

I will send it later.

> Thanks,
>
> Richard
>
>
>