* Re: [PATCH 1/1] busybox: fix the on-target upgrade problem
2013-07-06 10:27 ` Martin Jansa
@ 2013-07-08 8:20 ` ChenQi
0 siblings, 0 replies; 3+ messages in thread
From: ChenQi @ 2013-07-08 8:20 UTC (permalink / raw)
To: Martin Jansa; +Cc: qingtao.cao, openembedded-core
[-- Attachment #1: Type: text/plain, Size: 7305 bytes --]
On 07/06/2013 06:27 PM, Martin Jansa wrote:
> On Wed, Jul 03, 2013 at 12:48:12PM +0800, Qi.Chen@windriver.com wrote:
>> From: Chen Qi <Qi.Chen@windriver.com>
>>
>> We now can have a 'one-binary' version of busybox, or 'two-binary'
>> version of busybox, controlled by the 'BUSYBOX_SPLIT_SUID' variable.
>> This makes on-target upgrade a problem, as we have to support the
>> following four upgrading paths.
>>
>> For convenience, in the following context, A is used to denote a
>> 'two-binary' version of busybox while B is used to denote a 'one-binary'
>> version of busybox.
>>
>> A --(upgrade)--> B
>> B --(upgrade)--> A
>> A --(upgrade)--> A
>> B --(upgrade)--> B
>>
>> This patch makes effort to support the above four situations.
> Thanks for looking into it (it's more complicated then I've first
> expected) and it looks like there is more issues probably caused
> by applet path moved in latest busybox upgrade (but ip looks weird) :/
>
> update-alternatives: Error: cannot register alternative addgroup to
> /usr/sbin/addgroup since it is already registered to /bin/addgroup
> update-alternatives: Error: cannot register alternative adduser to
> /usr/sbin/adduser since it is already registered to /bin/adduser
>
> update-alternatives: Error: cannot register alternative delgroup to
> /usr/sbin/delgroup since it is already registered to /bin/delgroup
> update-alternatives: Error: cannot register alternative deluser to
> /usr/sbin/deluser since it is already registered to /bin/deluser
>
> update-alternatives: Error: cannot register alternative ip to /sbin/ip
> since it is already registered to /bin/ip
>
> SHR root@gjama ~ $ cat /var/lib/opkg/alternatives/ip
> /bin/ip
> busybox 50
> /bin/busybox 50
> /bin/busybox.nosuid 50
> SHR root@gjama ~ $ cat /var/lib/opkg/alternatives/adduser
> /bin/adduser
> /bin/busybox.nosuid 50
> SHR root@gjama ~ $ cat /var/lib/opkg/alternatives/addgroup
> /bin/addgroup
> /bin/busybox.nosuid 50
> SHR root@gjama ~ $ cat /var/lib/opkg/alternatives/delgroup
> /bin/delgroup
> /bin/busybox.nosuid 50
> SHR root@gjama ~ $ cat /var/lib/opkg/alternatives/deluser
> /bin/deluser
> /bin/busybox.nosuid 50
Hi Martin,
The problem roots in opkg's update-alternatives.
opkg's update-alternatives cannot handle following situations.
/bin/<cmd> --> /bin/<cmd>.<pkgA>
/usr/bin/<cmd> --> /usr/bin/<cmd>.<pkgB>.
[Let's denote this situation as situationA, we have different links and
different targets in situationA.]
It has a simple assumption in its design, that is, the links for the
same command should be the same.
In case of our busybox upgrade, we have:
/bin/deluser --> /bin/busybox.nosuid
/usr/sbin/deluser --> /bin/busybox.nosuid
[Let's denote this situation as situationB, we have different links but
the same target in situationB.]
The links are different, /bin/deluser and /usr/sbin/deluser, so opkg's
update-alternatives rendered an error while registering the alternative.
According to its assumption, opkg's update-alternatives should not
support situationA and situationB.
But I think it should at least be able to deal with situationB. Because
it's not unusual in Yocto/OE to create symlinks via update-alternatives
and situationB might indicate an on-device upgrade.
I've just made a patch to support this situationB. I'm still doing some
tests on this patch. Please see attachment for more details.
Any thoughts on this problem?
Best Regards,
Chen Qi
>> [YOCTO #4802]
>>
>> Signed-off-by: Chen Qi <Qi.Chen@windriver.com>
>> ---
>> meta/recipes-core/busybox/busybox.inc | 48 +++++++++++++++++++++++++++++++++
>> 1 file changed, 48 insertions(+)
>>
>> diff --git a/meta/recipes-core/busybox/busybox.inc b/meta/recipes-core/busybox/busybox.inc
>> index 8567d64..acd2bfb 100644
>> --- a/meta/recipes-core/busybox/busybox.inc
>> +++ b/meta/recipes-core/busybox/busybox.inc
>> @@ -190,6 +190,10 @@ do_install () {
>> install -m 0644 ${S}/busybox.links.suid ${D}${sysconfdir}
>> install -m 0644 ${S}/busybox.links.nosuid ${D}${sysconfdir}
>> ln -sf busybox.nosuid ${D}${base_bindir}/sh
>> + # Keep a default busybox for people who want to invoke busybox directly.
>> + # This is also useful for the on device upgrade. Because we want
>> + # to use the busybox command in postinst.
>> + ln -sf busybox.nosuid ${D}${base_bindir}/busybox
>> else
>> if grep -q "CONFIG_FEATURE_SUID=y" ${B}/.config; then
>> install -m 4755 ${B}/busybox ${D}${base_bindir}
>> @@ -198,6 +202,12 @@ do_install () {
>> fi
>> install -m 0644 ${S}/busybox.links ${D}${sysconfdir}
>> ln -sf busybox ${D}${base_bindir}/sh
>> + # We make this symlink here to eliminate the error when upgrading together
>> + # with busybox-syslog. Without this symlink, the opkg may think of the
>> + # busybox.nosuid as obsolete and remove it, resulting in dead links like
>> + # /bin/sed -> /bin/busybox.nosuid. This will make upgrading busybox-syslog fail.
>> + # This symlink will be safely deleted in postinst, thus no negative effect.
>> + ln -sf busybox ${D}${base_bindir}/busybox.nosuid
>> fi
>> else
>> install -d ${D}${base_bindir} ${D}${base_sbindir}
>> @@ -306,6 +316,44 @@ python do_package_prepend () {
>> set_alternative_vars("/etc/busybox.links.suid", "/bin/busybox.suid")
>> }
>>
>> +pkg_postinst_${PN} () {
>> + # This part of code is dedicated to the on target upgrade problem.
>> + # It's known that if we don't make appropriate symlinks before update-alternatives calls,
>> + # there will be errors indicating missing commands such as 'sed'.
>> + # These symlinks will later be updated by update-alternatives calls.
>> + test -n 2 > /dev/null || alias test='busybox test'
>> + if test "x$D" = "x"; then
>> + # Remove busybox.nosuid if it's a symlink, because this situation indicates
>> + # that we're installing or upgrading to a one-binary busybox.
>> + if test -h /bin/busybox.nosuid; then
>> + rm -f /bin/busybox.nosuid
>> + fi
>> + for suffix in "" ".nosuid" ".suid"; do
>> + if test -e /etc/busybox.links$suffix; then
>> + while read link; do
>> + if test ! -e "$link"; then
>> + case "$link" in
>> + /*/*/*)
>> + to="../../bin/busybox$suffix"
>> + ;;
>> + /bin/*)
>> + to="busybox$suffix"
>> + ;;
>> + /*/*)
>> + to="../bin/busybox$suffix"
>> + ;;
>> + esac
>> + # we can use busybox here because even if we are using splitted busybox
>> + # we've made a symlink from /bin/busybox to /bin/busybox.nosuid.
>> + busybox rm -f $link
>> + busybox ln -s $to $link
>> + fi
>> + done < /etc/busybox.links$suffix
>> + fi
>> + done
>> + fi
>> +}
>> +
>> pkg_prerm_${PN} () {
>> # This is so you can make busybox commit suicide - removing busybox with no other packages
>> # providing its files, this will make update-alternatives work, but the update-rc.d part
>> --
>> 1.7.9.5
>>
>> _______________________________________________
>> Openembedded-core mailing list
>> Openembedded-core@lists.openembedded.org
>> http://lists.openembedded.org/mailman/listinfo/openembedded-core
[-- Attachment #2: 0001-opkg-fix-update-alternatives-to-handle-on-device-upg.patch --]
[-- Type: text/x-patch, Size: 3661 bytes --]
From 02f2bba4d3d92daf870f86ae02a206af51a53b1d Mon Sep 17 00:00:00 2001
From: Chen Qi <qi.chen@windriver.com>
Date: Mon, 8 Jul 2013 04:13:14 -0400
Subject: [PATCH] opkg: fix update-alternatives to handle on-device upgrade
---
...ate-alternatives-fix-register_alt-process.patch | 61 ++++++++++++++++++++
meta/recipes-devtools/opkg/opkg_svn.bb | 3 +-
2 files changed, 63 insertions(+), 1 deletion(-)
create mode 100644 meta/recipes-devtools/opkg/opkg/0001-update-alternatives-fix-register_alt-process.patch
diff --git a/meta/recipes-devtools/opkg/opkg/0001-update-alternatives-fix-register_alt-process.patch b/meta/recipes-devtools/opkg/opkg/0001-update-alternatives-fix-register_alt-process.patch
new file mode 100644
index 0000000..2903dfd
--- /dev/null
+++ b/meta/recipes-devtools/opkg/opkg/0001-update-alternatives-fix-register_alt-process.patch
@@ -0,0 +1,61 @@
+From da31772150f764ee2cb11de38a4e7b6278105423 Mon Sep 17 00:00:00 2001
+From: Chen Qi <qi.chen@windriver.com>
+Date: Mon, 8 Jul 2013 03:32:42 -0400
+Subject: [PATCH] update-alternatives: fix register_alt process
+
+---
+ utils/update-alternatives.in | 24 +++++++++++++++++++++---
+ 1 file changed, 21 insertions(+), 3 deletions(-)
+
+diff --git a/utils/update-alternatives.in b/utils/update-alternatives.in
+index 2a6f6cc..3d592b2 100644
+--- a/utils/update-alternatives.in
++++ b/utils/update-alternatives.in
+@@ -46,6 +46,7 @@ register_alt() {
+ [ $# -lt 2 ] && return 1
+ local name="$1"
+ local link="$2"
++ local path="$3"
+
+ if [ ! -d $ad ]; then
+ mkdir -p $ad
+@@ -54,8 +55,25 @@ register_alt() {
+ if [ -e "$ad/$name" ]; then
+ local olink=`head -n 1 $ad/$name`
+ if [ "$link" != "$olink" ]; then
+- echo "update-alternatives: Error: cannot register alternative $name to $link since it is already registered to $olink" >&2
+- return 1
++ local target_conflict="no"
++ if [ `wc -l $ad/$name | cut -d' ' -f1` != "2" ]; then
++ target_conflict="yes"
++ elif [ `tail -n 1 $ad/$name | cut -d' ' -f1` != "$path" ]; then
++ target_conflict="yes"
++ else
++ target_conflict="no"
++ fi
++ if [ "$target_conflict" = "yes" ]; then
++ echo "update-alternatives: Error: cannot register alternative $name to $link since it is already registered to $olink and there are more than one target" >&2
++ return 1
++ else
++ # Only one target for name and the link has changed.
++ echo "$link" > $ad/$name.new
++ tail -n 1 $ad/$name >> $ad/$name.new
++ mv $ad/$name.new $ad/$name
++ ln -snf $path $link
++ rm -f $olink
++ fi
+ fi
+ else
+ echo "$link" > "$ad/$name"
+@@ -137,7 +155,7 @@ do_install() {
+ # This is a bad hack, but I haven't thought of a cleaner solution yet...
+ [ -n "$OPKG_OFFLINE_ROOT" ] && path=`echo $path | sed "s|^$OPKG_OFFLINE_ROOT/*|/|"`
+
+- register_alt $name $link
++ register_alt $name $link $path
+ add_alt $name $path $priority
+ find_best_alt $name
+ }
+--
+1.7.9.5
+
diff --git a/meta/recipes-devtools/opkg/opkg_svn.bb b/meta/recipes-devtools/opkg/opkg_svn.bb
index 9f3512d..6ba6195 100644
--- a/meta/recipes-devtools/opkg/opkg_svn.bb
+++ b/meta/recipes-devtools/opkg/opkg_svn.bb
@@ -2,7 +2,8 @@ require opkg.inc
SRC_URI = "svn://opkg.googlecode.com/svn;module=trunk;protocol=http \
file://obsolete_automake_macros.patch \
- file://0001-Fix-libopkg-header-installation.patch \
+ file://0001-Fix-libopkg-header-installation.patch \
+ file://0001-update-alternatives-fix-register_alt-process.patch \
"
S = "${WORKDIR}/trunk"
--
1.7.9.5
^ permalink raw reply related [flat|nested] 3+ messages in thread