* [PATCH 0/1] useradd.bbclass: use locking of bb.utils to avoid lock race issue of useradd/groupadd
@ 2012-07-22 6:53 jackie.huang
2012-07-22 6:53 ` [PATCH 1/1] " jackie.huang
0 siblings, 1 reply; 9+ messages in thread
From: jackie.huang @ 2012-07-22 6:53 UTC (permalink / raw)
To: openembedded-core
From: Jackie Huang <jackie.huang@windriver.com>
A race condition can occur when adding users and groups to the
passwd and group files, in [YOCTO #1794], 10 times retry added
but it is not fixed completely.
This fix re-writes the useradd_preinst and useradd_sysroot with
python and use locking of bb.utils to lock the passwd and group
files before executing useradd/groupadd commands to avoid the
lock race themselves.
[YOCTO #2779]
Signed-off-by: Jackie Huang <jackie.huang@windriver.com>
---
The following changes since commit 4bce3417917a3e88ba6529db394525fba82e0699:
EFI: Make installer EFI aware (2012-07-19 17:48:53 +0100)
are available in the git repository at:
git://git.pokylinux.org/poky-contrib jhuang0/yocto_2779_groupadd_0722
http://git.pokylinux.org/cgit.cgi/poky-contrib/log/?h=jhuang0/yocto_2779_groupadd_0722
Jackie Huang (1):
useradd.bbclass: use locking of bb.utils to avoid lock race issue of
useradd/groupadd
meta/classes/useradd.bbclass | 284 ++++++++++++++++++------------------------
1 files changed, 124 insertions(+), 160 deletions(-)
--
1.7.4
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH 1/1] useradd.bbclass: use locking of bb.utils to avoid lock race issue of useradd/groupadd 2012-07-22 6:53 [PATCH 0/1] useradd.bbclass: use locking of bb.utils to avoid lock race issue of useradd/groupadd jackie.huang @ 2012-07-22 6:53 ` jackie.huang 2012-07-22 8:36 ` Richard Purdie 2012-07-23 10:09 ` Richard Purdie 0 siblings, 2 replies; 9+ messages in thread From: jackie.huang @ 2012-07-22 6:53 UTC (permalink / raw) To: openembedded-core From: Jackie Huang <jackie.huang@windriver.com> A race condition can occur when adding users and groups to the passwd and group files, in [YOCTO #1794], 10 times retry added but it is not fixed completely. This fix re-writes the useradd_preinst and useradd_sysroot with python and use locking of bb.utils to lock the passwd and group files before executing useradd/groupadd commands to avoid the lock race themselves. [YOCTO #2779] Signed-off-by: Jackie Huang <jackie.huang@windriver.com> --- meta/classes/useradd.bbclass | 284 ++++++++++++++++++------------------------ 1 files changed, 124 insertions(+), 160 deletions(-) diff --git a/meta/classes/useradd.bbclass b/meta/classes/useradd.bbclass index bb8f42b..ed5ed69 100644 --- a/meta/classes/useradd.bbclass +++ b/meta/classes/useradd.bbclass @@ -14,126 +14,90 @@ USERADDDEPENDS_virtclass-nativesdk = "" # c) As the preinst script in the target package at do_rootfs time # d) As the preinst script in the target package on device as a package upgrade # -useradd_preinst () { -OPT="" -SYSROOT="" - -if test "x$D" != "x"; then - # Installing into a sysroot - SYSROOT="$D" - OPT="--root $D" - - # Add groups and users defined for all recipe packages - GROUPADD_PARAM="${@get_all_cmd_params(d, 'group')}" - USERADD_PARAM="${@get_all_cmd_params(d, 'user')}" -else - # Installing onto a target - # Add groups and users defined only for this package - GROUPADD_PARAM="${GROUPADD_PARAM}" - USERADD_PARAM="${USERADD_PARAM}" -fi - -# Perform group additions first, since user additions may depend -# on these groups existing -if test "x$GROUPADD_PARAM" != "x"; then - echo "Running groupadd commands..." - # Invoke multiple instances of groupadd for parameter lists - # separated by ';' - opts=`echo "$GROUPADD_PARAM" | cut -d ';' -f 1` - remaining=`echo "$GROUPADD_PARAM" | cut -d ';' -f 2-` - while test "x$opts" != "x"; do - groupname=`echo "$opts" | awk '{ print $NF }'` - group_exists=`grep "^$groupname:" $SYSROOT/etc/group || true` - if test "x$group_exists" = "x"; then - count=1 - while true; do - eval $PSEUDO groupadd $OPT $opts || true - group_exists=`grep "^$groupname:" $SYSROOT/etc/group || true` - if test "x$group_exists" = "x"; then - # File locking issues can require us to retry the command - echo "WARNING: groupadd command did not succeed. Retrying..." - sleep 1 - else - break - fi - count=`expr $count + 1` - if test $count = 11; then - echo "ERROR: tried running groupadd command 10 times without success, giving up" - exit 1 - fi - done - else - echo "Note: group $groupname already exists, not re-creating it" - fi - - if test "x$opts" = "x$remaining"; then - break - fi - opts=`echo "$remaining" | cut -d ';' -f 1` - remaining=`echo "$remaining" | cut -d ';' -f 2-` - done -fi - -if test "x$USERADD_PARAM" != "x"; then - echo "Running useradd commands..." - # Invoke multiple instances of useradd for parameter lists - # separated by ';' - opts=`echo "$USERADD_PARAM" | cut -d ';' -f 1` - remaining=`echo "$USERADD_PARAM" | cut -d ';' -f 2-` - while test "x$opts" != "x"; do - # useradd does not have a -f option, so we have to check if the - # username already exists manually - username=`echo "$opts" | awk '{ print $NF }'` - user_exists=`grep "^$username:" $SYSROOT/etc/passwd || true` - if test "x$user_exists" = "x"; then - count=1 - while true; do - eval $PSEUDO useradd $OPT $opts || true - user_exists=`grep "^$username:" $SYSROOT/etc/passwd || true` - if test "x$user_exists" = "x"; then - # File locking issues can require us to retry the command - echo "WARNING: useradd command did not succeed. Retrying..." - sleep 1 - else - break - fi - count=`expr $count + 1` - if test $count = 11; then - echo "ERROR: tried running useradd command 10 times without success, giving up" - exit 1 - fi - done - else - echo "Note: username $username already exists, not re-creating it" - fi - - if test "x$opts" = "x$remaining"; then - break - fi - opts=`echo "$remaining" | cut -d ';' -f 1` - remaining=`echo "$remaining" | cut -d ';' -f 2-` - done -fi +def useradd_preinst(d): + import re + + sysroot = "" + opt = "" + + dir = d.getVar('STAGING_DIR_TARGET', True) + if dir: + # Installing into a sysroot + sysroot = dir + opt = "--root %s" % dir + + # Add groups and users defined for all recipe packages + groupadd_param = get_all_cmd_params(d, 'group') + useradd_param = get_all_cmd_params(d, 'user') + else: + # Installing onto a target + # Add groups and users defined only for this package + groupadd_param = d.getVar('GROUPADD_PARAM', True) + useradd_param = d.getVar('GROUPADD_PARAM', True) + + group_file = '%s/etc/group' % sysroot + user_file = '%s/etc/passwd' % sysroot + + # Use the locking of bb to the group/passwd file to avoid the + # locking issue of groupadd/useradd + group_lock = '%s.locked' % group_file + user_lock = '%s.locked' % user_file + lockfiles = [group_lock, user_lock] + + with bb.utils.fileslocked(lockfiles): + # Perform group additions first, since user additions may depend + # on these groups existing + if groupadd_param and sysroot: + bb.debug(1, "Running groupadd commands ...") + # Invoke multiple instances of groupadd for parameter lists + # separated by ';' + param_list = groupadd_param.split(';') + for opts in param_list: + groupname = opts.split()[-1] + with open(group_file, 'r') as f: + passwd_lines = f.read() + group_re = re.compile('\n%s' % groupname) + if group_re.search(passwd_lines): + bb.note("Note: groupname %s already exists, not re-creating it" % groupname) + continue + try: + output, err = bb.process.run('groupadd %s %s' % (opt, opts)) + except bb.process.CmdError as exc: + bb.error("Failed to add group: %s" % exc) + else: + bb.note("Successful to add group %s" % groupname) + + if useradd_param: + bb.debug(1, "Running useradd commands ...") + # Invoke multiple instances of useradd for parameter lists + # separated by ';' + param_list = useradd_param.split(';') + for opts in param_list: + # useradd does not have a -f option, so we have to check if the + # username already exists manually + username = opts.split()[-1] + with open(user_file, 'r') as f: + passwd_lines = f.read() + user_re = re.compile('\n%s' % username) + if user_re.search(passwd_lines): + bb.note("Note: username %s already exists, not re-creating it" % username) + continue + try: + output, err = bb.process.run('useradd %s %s' % (opt, opts)) + except bb.process.CmdError as exc: + bb.error("Failed to add user: %s" % exc) + else: + bb.note("Successful to add user %s" % username) + +fakeroot python useradd_sysroot () { + useradd_preinst(d) } -useradd_sysroot () { - # Pseudo may (do_install) or may not (do_populate_sysroot_setscene) be running - # at this point so we're explicit about the environment so pseudo can load if - # not already present. - export PSEUDO="${FAKEROOTENV} PSEUDO_LOCALSTATEDIR=${STAGING_DIR_TARGET}${localstatedir}/pseudo ${STAGING_DIR_NATIVE}${bindir}/pseudo" - - # Explicitly set $D since it isn't set to anything - # before do_install - D=${STAGING_DIR_TARGET} - useradd_preinst +fakeroot python useradd_sysroot_sstate () { + if d.getVar("BB_CURRENTTASK", True) == "package_setscene": + useradd_preinst(d) } -useradd_sysroot_sstate () { - if [ "${BB_CURRENTTASK}" = "package_setscene" ] - then - useradd_sysroot - fi -} do_install[prefuncs] += "${SYSROOTFUNC}" SYSROOTFUNC = "useradd_sysroot" @@ -146,7 +110,7 @@ SYSROOTPOSTFUNC_virtclass-cross = "" SYSROOTPOSTFUNC_virtclass-native = "" SYSROOTPOSTFUNC_virtclass-nativesdk = "" -USERADDSETSCENEDEPS = "${MLPREFIX}base-passwd:do_populate_sysroot_setscene shadow-native:do_populate_sysroot_setscene ${MLPREFIX}shadow-sysroot:do_populate_sysroot_setscene" +USERADDSETSCENEDEPS = "base-passwd:do_populate_sysroot_setscene shadow-native:do_populate_sysroot_setscene ${MLPREFIX}shadow-sysroot:do_populate_sysroot_setscene" USERADDSETSCENEDEPS_virtclass-cross = "" USERADDSETSCENEDEPS_virtclass-native = "" USERADDSETSCENEDEPS_virtclass-nativesdk = "" @@ -154,61 +118,61 @@ do_package_setscene[depends] = "${USERADDSETSCENEDEPS}" # Recipe parse-time sanity checks def update_useradd_after_parse(d): - useradd_packages = d.getVar('USERADD_PACKAGES', True) + useradd_packages = d.getVar('USERADD_PACKAGES', True) - if not useradd_packages: - raise bb.build.FuncFailed, "%s inherits useradd but doesn't set USERADD_PACKAGES" % d.getVar('FILE') + if not useradd_packages: + raise bb.build.FuncFailed, "%s inherits useradd but doesn't set USERADD_PACKAGES" % d.getVar('FILE') - for pkg in useradd_packages.split(): - if not d.getVar('USERADD_PARAM_%s' % pkg, True) and not d.getVar('GROUPADD_PARAM_%s' % pkg, True): - raise bb.build.FuncFailed, "%s inherits useradd but doesn't set USERADD_PARAM or GROUPADD_PARAM for package %s" % (d.getVar('FILE'), pkg) + for pkg in useradd_packages.split(): + if not d.getVar('USERADD_PARAM_%s' % pkg, True) and not d.getVar('GROUPADD_PARAM_%s' % pkg, True): + raise bb.build.FuncFailed, "%s inherits useradd but doesn't set USERADD_PARAM or GROUPADD_PARAM for package %s" % (d.getVar('FILE'), pkg) python __anonymous() { - update_useradd_after_parse(d) + update_useradd_after_parse(d) } # Return a single [GROUP|USER]ADD_PARAM formatted string which includes the # [group|user]add parameters for all USERADD_PACKAGES in this recipe def get_all_cmd_params(d, cmd_type): - import string - - param_type = cmd_type.upper() + "ADD_PARAM_%s" - params = [] + import string + + param_type = cmd_type.upper() + "ADD_PARAM_%s" + params = [] - useradd_packages = d.getVar('USERADD_PACKAGES', True) or "" - for pkg in useradd_packages.split(): - param = d.getVar(param_type % pkg, True) - if param: - params.append(param) + useradd_packages = d.getVar('USERADD_PACKAGES', True) or "" + for pkg in useradd_packages.split(): + param = d.getVar(param_type % pkg, True) + if param: + params.append(param) - return string.join(params, "; ") + return string.join(params, "; ") # Adds the preinst script into generated packages fakeroot python populate_packages_prepend () { - def update_useradd_package(pkg): - bb.debug(1, 'adding user/group calls to preinst for %s' % pkg) - - """ - useradd preinst is appended here because pkg_preinst may be - required to execute on the target. Not doing so may cause - useradd preinst to be invoked twice, causing unwanted warnings. - """ - preinst = d.getVar('pkg_preinst_%s' % pkg, True) or d.getVar('pkg_preinst', True) - if not preinst: - preinst = '#!/bin/sh\n' - preinst += d.getVar('useradd_preinst', True) - d.setVar('pkg_preinst_%s' % pkg, preinst) - - # RDEPENDS setup - rdepends = d.getVar("RDEPENDS_%s" % pkg, True) or "" - rdepends += ' ' + d.getVar('MLPREFIX') + 'base-passwd' - rdepends += ' ' + d.getVar('MLPREFIX') + 'shadow' - d.setVar("RDEPENDS_%s" % pkg, rdepends) - - # Add the user/group preinstall scripts and RDEPENDS requirements - # to packages specified by USERADD_PACKAGES - if not bb.data.inherits_class('nativesdk', d): - useradd_packages = d.getVar('USERADD_PACKAGES', True) or "" - for pkg in useradd_packages.split(): - update_useradd_package(pkg) + def update_useradd_package(pkg): + bb.debug(1, 'adding user/group calls to preinst for %s' % pkg) + + """ + useradd preinst is appended here because pkg_preinst may be + required to execute on the target. Not doing so may cause + useradd preinst to be invoked twice, causing unwanted warnings. + """ + preinst = d.getVar('pkg_preinst_%s' % pkg, True) or d.getVar('pkg_preinst', True) + if not preinst: + preinst = '#!/bin/sh\n' + preinst += d.getVar('useradd_preinst', True) + d.setVar('pkg_preinst_%s' % pkg, preinst) + + # RDEPENDS setup + rdepends = d.getVar("RDEPENDS_%s" % pkg, True) or "" + rdepends += ' ' + d.getVar('MLPREFIX') + 'base-passwd' + rdepends += ' ' + d.getVar('MLPREFIX') + 'shadow' + d.setVar("RDEPENDS_%s" % pkg, rdepends) + + # Add the user/group preinstall scripts and RDEPENDS requirements + # to packages specified by USERADD_PACKAGES + if not bb.data.inherits_class('nativesdk', d): + useradd_packages = d.getVar('USERADD_PACKAGES', True) or "" + for pkg in useradd_packages.split(): + update_useradd_package(pkg) } -- 1.7.4 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/1] useradd.bbclass: use locking of bb.utils to avoid lock race issue of useradd/groupadd 2012-07-22 6:53 ` [PATCH 1/1] " jackie.huang @ 2012-07-22 8:36 ` Richard Purdie 2012-07-22 12:10 ` Huang, Jie (Jackie) 2012-07-23 10:09 ` Richard Purdie 1 sibling, 1 reply; 9+ messages in thread From: Richard Purdie @ 2012-07-22 8:36 UTC (permalink / raw) To: Patches and discussions about the oe-core layer On Sun, 2012-07-22 at 14:53 +0800, jackie.huang@windriver.com wrote: > From: Jackie Huang <jackie.huang@windriver.com> > > A race condition can occur when adding users and groups to the > passwd and group files, in [YOCTO #1794], 10 times retry added > but it is not fixed completely. > > This fix re-writes the useradd_preinst and useradd_sysroot with > python and use locking of bb.utils to lock the passwd and group > files before executing useradd/groupadd commands to avoid the > lock race themselves. > > [YOCTO #2779] > > Signed-off-by: Jackie Huang <jackie.huang@windriver.com> > --- > meta/classes/useradd.bbclass | 284 ++++++++++++++++++------------------------ > 1 files changed, 124 insertions(+), 160 deletions(-) Please resend this with the whitespace issues resolved. Its near impossible to review as it stands :( Cheers, Richard ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/1] useradd.bbclass: use locking of bb.utils to avoid lock race issue of useradd/groupadd 2012-07-22 8:36 ` Richard Purdie @ 2012-07-22 12:10 ` Huang, Jie (Jackie) 2012-07-23 1:56 ` Randy MacLeod 0 siblings, 1 reply; 9+ messages in thread From: Huang, Jie (Jackie) @ 2012-07-22 12:10 UTC (permalink / raw) To: Patches and discussions about the oe-core layer >On Sun, 2012-07-22 at 14:53 +0800, jackie.huang@windriver.com wrote: >> From: Jackie Huang <jackie.huang@windriver.com> >> >> A race condition can occur when adding users and groups to the >>passwd and group files, in [YOCTO #1794], 10 times retry added >> but it is not fixed completely. >> >> This fix re-writes the useradd_preinst and useradd_sysroot with >> python and use locking of bb.utils to lock the passwd and group >> files before executing useradd/groupadd commands to avoid the >> lock race themselves. >> >> [YOCTO #2779] >> >> Signed-off-by: Jackie Huang <jackie.huang@windriver.com> >> --- >> meta/classes/useradd.bbclass | 284 ++++++++++++++++++------------------------ >> 1 files changed, 124 insertions(+), 160 deletions(-) > >Please resend this with the whitespace issues resolved. Its near >impossible to review as it stands :( Re-sent, sorry about that. Thanks, Jackie > >Cheers, > >Richard _______________________________________________ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/1] useradd.bbclass: use locking of bb.utils to avoid lock race issue of useradd/groupadd 2012-07-22 12:10 ` Huang, Jie (Jackie) @ 2012-07-23 1:56 ` Randy MacLeod 2012-07-23 3:24 ` Randy MacLeod 0 siblings, 1 reply; 9+ messages in thread From: Randy MacLeod @ 2012-07-23 1:56 UTC (permalink / raw) To: Patches and discussions about the oe-core layer, Robert Yang On 12-07-22 08:10 AM, Huang, j (Jackie) wrote: > > >> On Sun, 2012-07-22 at 14:53 +0800, jackie.huang@windriver.com wrote: >>> From: Jackie Huang <jackie.huang@windriver.com> >>> >>> A race condition can occur when adding users and groups to the >>> passwd and group files, in [YOCTO #1794], 10 times retry added >>> but it is not fixed completely. >>> >>> This fix re-writes the useradd_preinst and useradd_sysroot with >>> python and use locking of bb.utils to lock the passwd and group >>> files before executing useradd/groupadd commands to avoid the >>> lock race themselves. >>> >>> [YOCTO #2779] >>> >>> Signed-off-by: Jackie Huang <jackie.huang@windriver.com> >>> --- >>> meta/classes/useradd.bbclass | 284 ++++++++++++++++++------------------------ >>> 1 files changed, 124 insertions(+), 160 deletions(-) >> >> Please resend this with the whitespace issues resolved. Its near >> impossible to review as it stands :( > > Re-sent, sorry about that. Hi Jackie, I don't see that your new version sent at 8:01 is any better but I could be wrong... Robert, Please take a look and help Jackie with formatting and perhaps small single purpose commits as needed - i.e. separate the whitespace changes from the functional changes. This problem is happening frequently so I'd like to get this worked out upstream. Thanks, // Randy > > Thanks, > Jackie > >> >> Cheers, >> >> Richard > > > _______________________________________________ > Openembedded-core mailing list > Openembedded-core@lists.openembedded.org > http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core > > _______________________________________________ > Openembedded-core mailing list > Openembedded-core@lists.openembedded.org > http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core > -- # Randy MacLeod. MTS, Linux, Wind River Direct: 613.963.1350 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/1] useradd.bbclass: use locking of bb.utils to avoid lock race issue of useradd/groupadd 2012-07-23 1:56 ` Randy MacLeod @ 2012-07-23 3:24 ` Randy MacLeod 0 siblings, 0 replies; 9+ messages in thread From: Randy MacLeod @ 2012-07-23 3:24 UTC (permalink / raw) To: openembedded-core, Huang, Jie (Jackie) On 12-07-22 09:56 PM, Randy MacLeod wrote: > On 12-07-22 08:10 AM, Huang, j (Jackie) wrote: >> >> >>> On Sun, 2012-07-22 at 14:53 +0800, jackie.huang@windriver.com wrote: >>>> From: Jackie Huang <jackie.huang@windriver.com> >>>> >>>> A race condition can occur when adding users and groups to the >>>> passwd and group files, in [YOCTO #1794], 10 times retry added >>>> but it is not fixed completely. >>>> >>>> This fix re-writes the useradd_preinst and useradd_sysroot with >>>> python and use locking of bb.utils to lock the passwd and group >>>> files before executing useradd/groupadd commands to avoid the >>>> lock race themselves. >>>> >>>> [YOCTO #2779] >>>> >>>> Signed-off-by: Jackie Huang <jackie.huang@windriver.com> >>>> --- >>>> meta/classes/useradd.bbclass | 284 >>>> ++++++++++++++++++------------------------ >>>> 1 files changed, 124 insertions(+), 160 deletions(-) >>> >>> Please resend this with the whitespace issues resolved. Its near >>> impossible to review as it stands :( >> >> Re-sent, sorry about that. > > Hi Jackie, > > I don't see that your new version sent at 8:01 is any better > but I could be wrong... > > Robert, > Please take a look and help Jackie with formatting > and perhaps small single purpose commits as needed - > i.e. separate the whitespace changes from the functional changes. Summary: Jackie did the right thing; I need to read more carefully. ;-) // Randy Details: I've looked at the two emails more closely now and see the white space problem lines and saw that Jackie did fix those up in the new patch. Specifically: -USERADDSETSCENEDEPS = "${MLPREFIX}base-passwd:do_populate_sy +USERADDSETSCENEDEPS = "base-passwd:do_populate_sysroot_setsc USERADDSETSCENEDEPS_virtclass-cross = "" USERADDSETSCENEDEPS_virtclass-native = "" USERADDSETSCENEDEPS_virtclass-nativesdk = "" @@ -154,61 +118,61 @@ do_package_setscene[depends] = "${USERA # Recipe parse-time sanity checks def update_useradd_after_parse(d): - useradd_packages = d.getVar('USERADD_PACKAGES', True) + useradd_packages = d.getVar('USERADD_PACKAGES', True) - if not useradd_packages: - raise bb.build.FuncFailed, "%s inherits useradd but + if not useradd_packages: + raise bb.build.FuncFailed, "%s inherits usera - for pkg in useradd_packages.split(): - if not d.getVar('USERADD_PARAM_%s' % pkg, True) and - raise bb.build.FuncFailed, "%s inherits useradd + for pkg in useradd_packages.split(): + if not d.getVar('USERADD_PARAM_%s' % pkg, Tru + raise bb.build.FuncFailed, "%s inheri python __anonymous() { - update_useradd_after_parse(d) + update_useradd_after_parse(d) } Sorry for the noise. // Randy > > This problem is happening frequently so I'd like to get this > worked out upstream. > > Thanks, > // Randy > > >> >> Thanks, >> Jackie >> >>> >>> Cheers, >>> >>> Richard >> >> >> _______________________________________________ >> Openembedded-core mailing list >> Openembedded-core@lists.openembedded.org >> http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core >> >> _______________________________________________ >> Openembedded-core mailing list >> Openembedded-core@lists.openembedded.org >> http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core >> > > -- # Randy MacLeod. MTS, Linux, Wind River Direct: 613.963.1350 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/1] useradd.bbclass: use locking of bb.utils to avoid lock race issue of useradd/groupadd 2012-07-22 6:53 ` [PATCH 1/1] " jackie.huang 2012-07-22 8:36 ` Richard Purdie @ 2012-07-23 10:09 ` Richard Purdie 2012-07-24 1:59 ` jhuang0 1 sibling, 1 reply; 9+ messages in thread From: Richard Purdie @ 2012-07-23 10:09 UTC (permalink / raw) To: Patches and discussions about the oe-core layer On Sun, 2012-07-22 at 14:53 +0800, jackie.huang@windriver.com wrote: > From: Jackie Huang <jackie.huang@windriver.com> > > A race condition can occur when adding users and groups to the > passwd and group files, in [YOCTO #1794], 10 times retry added > but it is not fixed completely. > > This fix re-writes the useradd_preinst and useradd_sysroot with > python and use locking of bb.utils to lock the passwd and group > files before executing useradd/groupadd commands to avoid the > lock race themselves. > > [YOCTO #2779] > > Signed-off-by: Jackie Huang <jackie.huang@windriver.com> > --- > meta/classes/useradd.bbclass | 284 ++++++++++++++++++------------------------ > 1 files changed, 124 insertions(+), 160 deletions(-) > > diff --git a/meta/classes/useradd.bbclass b/meta/classes/useradd.bbclass > index bb8f42b..ed5ed69 100644 > --- a/meta/classes/useradd.bbclass > +++ b/meta/classes/useradd.bbclass > @@ -14,126 +14,90 @@ USERADDDEPENDS_virtclass-nativesdk = "" > # c) As the preinst script in the target package at do_rootfs time > # d) As the preinst script in the target package on device as a package upgrade > # > -useradd_preinst () { > -OPT="" > -SYSROOT="" > - > -if test "x$D" != "x"; then [...] > - done > -fi > +def useradd_preinst(d): > + import re > + [...] > - # Add the user/group preinstall scripts and RDEPENDS requirements > - # to packages specified by USERADD_PACKAGES > - if not bb.data.inherits_class('nativesdk', d): > - useradd_packages = d.getVar('USERADD_PACKAGES', True) or "" > - for pkg in useradd_packages.split(): > - update_useradd_package(pkg) > + def update_useradd_package(pkg): > + bb.debug(1, 'adding user/group calls to preinst for %s' % pkg) > + > + """ > + useradd preinst is appended here because pkg_preinst may be > + required to execute on the target. Not doing so may cause > + useradd preinst to be invoked twice, causing unwanted warnings. > + """ > + preinst = d.getVar('pkg_preinst_%s' % pkg, True) or d.getVar('pkg_preinst', True) > + if not preinst: > + preinst = '#!/bin/sh\n' > + preinst += d.getVar('useradd_preinst', True) This looks like we're adding the contents of the useradd_preinst function (changed from shell to python) to a script headed with "#!/bin/sh"? Python script with a /bin/sh header isn't a good idea. We can't really depend on python being on the target device to add users in the postinstall script either. So in summary, this patch needs a lot more thought and hasn't been well tested. python functions should also be 4 space indented. Cheers, Richard ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/1] useradd.bbclass: use locking of bb.utils to avoid lock race issue of useradd/groupadd 2012-07-23 10:09 ` Richard Purdie @ 2012-07-24 1:59 ` jhuang0 0 siblings, 0 replies; 9+ messages in thread From: jhuang0 @ 2012-07-24 1:59 UTC (permalink / raw) To: Patches and discussions about the oe-core layer, Richard Purdie On 7/23/2012 6:09 PM, Richard Purdie wrote: > On Sun, 2012-07-22 at 14:53 +0800, jackie.huang@windriver.com wrote: >> From: Jackie Huang<jackie.huang@windriver.com> >> >> A race condition can occur when adding users and groups to the >> passwd and group files, in [YOCTO #1794], 10 times retry added >> but it is not fixed completely. >> >> This fix re-writes the useradd_preinst and useradd_sysroot with >> python and use locking of bb.utils to lock the passwd and group >> files before executing useradd/groupadd commands to avoid the >> lock race themselves. >> >> [YOCTO #2779] >> >> Signed-off-by: Jackie Huang<jackie.huang@windriver.com> >> --- >> meta/classes/useradd.bbclass | 284 ++++++++++++++++++------------------------ >> 1 files changed, 124 insertions(+), 160 deletions(-) >> >> diff --git a/meta/classes/useradd.bbclass b/meta/classes/useradd.bbclass >> index bb8f42b..ed5ed69 100644 >> --- a/meta/classes/useradd.bbclass >> +++ b/meta/classes/useradd.bbclass >> @@ -14,126 +14,90 @@ USERADDDEPENDS_virtclass-nativesdk = "" >> # c) As the preinst script in the target package at do_rootfs time >> # d) As the preinst script in the target package on device as a package upgrade >> # >> -useradd_preinst () { >> -OPT="" >> -SYSROOT="" >> - >> -if test "x$D" != "x"; then > > [...] > >> - done >> -fi >> +def useradd_preinst(d): >> + import re >> + > > [...] > > >> - # Add the user/group preinstall scripts and RDEPENDS requirements >> - # to packages specified by USERADD_PACKAGES >> - if not bb.data.inherits_class('nativesdk', d): >> - useradd_packages = d.getVar('USERADD_PACKAGES', True) or "" >> - for pkg in useradd_packages.split(): >> - update_useradd_package(pkg) >> + def update_useradd_package(pkg): >> + bb.debug(1, 'adding user/group calls to preinst for %s' % pkg) >> + >> + """ >> + useradd preinst is appended here because pkg_preinst may be >> + required to execute on the target. Not doing so may cause >> + useradd preinst to be invoked twice, causing unwanted warnings. >> + """ >> + preinst = d.getVar('pkg_preinst_%s' % pkg, True) or d.getVar('pkg_preinst', True) >> + if not preinst: >> + preinst = '#!/bin/sh\n' >> + preinst += d.getVar('useradd_preinst', True) > > > This looks like we're adding the contents of the useradd_preinst > function (changed from shell to python) to a script headed with > "#!/bin/sh"? Python script with a /bin/sh header isn't a good idea. Hi Richard, This patch you replied is the first one I sent with whitespace issues, I didn't change the function update_useradd_package. > > We can't really depend on python being on the target device to add users > in the postinstall script either. > > So in summary, this patch needs a lot more thought and hasn't been well > tested. You're right, I didn't think about that it will be script running on the target device and I just tested for tow different contexts: 1) Before do_install 2) At do_populate_sysroot_setscene when installing from sstate packages > > python functions should also be 4 space indented. > I will change with 4 space indent. Thanks, Jackie > Cheers, > > Richard > > > > > _______________________________________________ > Openembedded-core mailing list > Openembedded-core@lists.openembedded.org > http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core > > -- Jackie Huang WIND RIVER | China Development Center MSN:jackielily@hotmail.com Tel: +86 8477 8594 Mobile: +86 138 1027 4745 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 0/1] useradd.bbclass: use locking of bb.utils to avoid lock race issue of useradd/groupadd
@ 2012-07-22 12:01 jackie.huang
0 siblings, 0 replies; 9+ messages in thread
From: jackie.huang @ 2012-07-22 12:01 UTC (permalink / raw)
To: openembedded-core
From: Jackie Huang <jackie.huang@windriver.com>
A race condition can occur when adding users and groups to the
passwd and group files, in [YOCTO #1794], 10 times retry added
but it is not fixed completely.
This fix re-writes the useradd_preinst and useradd_sysroot with
python and use locking of bb.utils to lock the passwd and group
files before executing useradd/groupadd commands to avoid the
lock race themselves.
[YOCTO #2779]
Signed-off-by: Jackie Huang <jackie.huang@windriver.com>
---
The following changes since commit 1eae5b6c87cd4f825dba9d2526b34231d33b1e92:
python-native: Use append instead of += so the lsb override for EXTRA_OECONF works as expected (2012-07-22 12:07:53 +0100)
are available in the git repository at:
git://git.pokylinux.org/poky-contrib jhuang0/yocto_2779_groupadd_0722_v2
http://git.pokylinux.org/cgit.cgi/poky-contrib/log/?h=jhuang0/yocto_2779_groupadd_0722_v2
Jackie Huang (1):
useradd.bbclass: use locking of bb.utils to avoid lock race issue of
useradd/groupadd
meta/classes/useradd.bbclass | 200 +++++++++++++++++------------------------
1 files changed, 83 insertions(+), 117 deletions(-)
--
1.7.4
^ permalink raw reply [flat|nested] 9+ messages in threadend of thread, other threads:[~2012-07-24 2:11 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-07-22 6:53 [PATCH 0/1] useradd.bbclass: use locking of bb.utils to avoid lock race issue of useradd/groupadd jackie.huang 2012-07-22 6:53 ` [PATCH 1/1] " jackie.huang 2012-07-22 8:36 ` Richard Purdie 2012-07-22 12:10 ` Huang, Jie (Jackie) 2012-07-23 1:56 ` Randy MacLeod 2012-07-23 3:24 ` Randy MacLeod 2012-07-23 10:09 ` Richard Purdie 2012-07-24 1:59 ` jhuang0 -- strict thread matches above, loose matches on Subject: below -- 2012-07-22 12:01 [PATCH 0/1] " jackie.huang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox