[PATCH 0/1] Replace retry logic in useradd with flock

[PATCH 0/1] Replace retry logic in useradd with flock

[kai.kang]

From: Kai Kang <kai.kang@windriver.com>

Hi Ross,

We met problems that useradd fails to add users. I found it a simultaneous issue, so created this patch.

As a comparison of build time:

Without flock:
1
$ time bitbake core-image-minimal

real    38m8.625s
user    318m43.262s
sys     62m47.710s


2
$ time bitbake core-image-sato

real    54m13.065s
user    587m1.386s
sys     122m16.949s



With flock:
1
$ time bitbake core-image-minimal

real    40m55.178s
user    316m26.576s
sys     58m53.322s


2
$ time bitbake core-image-sato

real    55m11.312s
user    580m16.787s
sys     118m16.385s


The following changes since commit 36b43b233c841b3c09fc17e04686b4b758780963:

  lib/oe/terminal: set workdir for konsole terminal (2016-02-14 11:42:19 +0000)

are available in the git repository at:

  git://git.pokylinux.org/poky-contrib kangkai/flock
  http://git.pokylinux.org/cgit.cgi/poky-contrib/log/?h=kangkai/flock

Kai Kang (1):
  useradd_base.bbclass: fix simultaneous with flock

 meta/classes/useradd.bbclass      |   8 +-
 meta/classes/useradd_base.bbclass | 165 ++++++++++----------------------------
 2 files changed, 48 insertions(+), 125 deletions(-)

-- 
2.6.1

[PATCH 1/1] useradd_base.bbclass: fix simultaneous with flock

[kai.kang]

From: Kai Kang <kai.kang@windriver.com>

When perform useradd during populate sysroot, it locks files passwd.lock
and group.lock at same time. And then it meets a dead lock issue.

Use flock to reslove it by using an universal lock file for all the
user and group related operations. The lock file is put in image dir
/tmp. /tmp will be mounted as a tmpfs, so the lock file will not be
visible when boot the target.

[YOCTO #9022]

Signed-off-by: Kai Kang <kai.kang@windriver.com>
---
 meta/classes/useradd.bbclass      |   8 +-
 meta/classes/useradd_base.bbclass | 165 ++++++++++----------------------------
 2 files changed, 48 insertions(+), 125 deletions(-)

diff --git a/meta/classes/useradd.bbclass b/meta/classes/useradd.bbclass
index c960656..db74123 100644
--- a/meta/classes/useradd.bbclass
+++ b/meta/classes/useradd.bbclass
@@ -48,6 +48,8 @@ if test "x$UA_SYSROOT" = "x"; then
 	GROUPMEMS_PARAM="${GROUPMEMS_PARAM}"
 fi
 
+mkdir -p $SYSROOT/tmp
+
 # Perform group additions first, since user additions may depend
 # on these groups existing
 if test "x`echo $GROUPADD_PARAM | tr -d '[:space:]'`" != "x"; then
@@ -57,7 +59,7 @@ if test "x`echo $GROUPADD_PARAM | tr -d '[:space:]'`" != "x"; then
 	opts=`echo "$GROUPADD_PARAM" | cut -d ';' -f 1`
 	remaining=`echo "$GROUPADD_PARAM" | cut -d ';' -f 2-`
 	while test "x$opts" != "x"; do
-		perform_groupadd "$SYSROOT" "$OPT $opts" 10
+		perform_groupadd "$SYSROOT" "$OPT $opts"
 		if test "x$opts" = "x$remaining"; then
 			break
 		fi
@@ -73,7 +75,7 @@ if test "x`echo $USERADD_PARAM | tr -d '[:space:]'`" != "x"; then
 	opts=`echo "$USERADD_PARAM" | cut -d ';' -f 1`
 	remaining=`echo "$USERADD_PARAM" | cut -d ';' -f 2-`
 	while test "x$opts" != "x"; do
-		perform_useradd "$SYSROOT" "$OPT $opts" 10
+		perform_useradd "$SYSROOT" "$OPT $opts"
 		if test "x$opts" = "x$remaining"; then
 			break
 		fi
@@ -89,7 +91,7 @@ if test "x`echo $GROUPMEMS_PARAM | tr -d '[:space:]'`" != "x"; then
 	opts=`echo "$GROUPMEMS_PARAM" | cut -d ';' -f 1`
 	remaining=`echo "$GROUPMEMS_PARAM" | cut -d ';' -f 2-`
 	while test "x$opts" != "x"; do
-		perform_groupmems "$SYSROOT" "$OPT $opts" 10
+		perform_groupmems "$SYSROOT" "$OPT $opts"
 		if test "x$opts" = "x$remaining"; then
 			break
 		fi
diff --git a/meta/classes/useradd_base.bbclass b/meta/classes/useradd_base.bbclass
index ab3cd35..a5baceb 100644
--- a/meta/classes/useradd_base.bbclass
+++ b/meta/classes/useradd_base.bbclass
@@ -4,7 +4,7 @@
 
 # The following functions basically have similar logic.
 # *) Perform necessary checks before invoking the actual command
-# *) Invoke the actual command, make retries if necessary
+# *) Invoke the actual command with flock
 # *) Error out if an error occurs.
 
 # Note that before invoking these functions, make sure the global variable
@@ -13,26 +13,16 @@
 perform_groupadd () {
 	local rootdir="$1"
 	local opts="$2"
-	local retries="$3"
-	bbnote "${PN}: Performing groupadd with [$opts] and $retries times of retry"
+	bbnote "${PN}: Performing groupadd with [$opts]"
 	local groupname=`echo "$opts" | awk '{ print $NF }'`
 	local group_exists="`grep "^$groupname:" $rootdir/etc/group || true`"
 	if test "x$group_exists" = "x"; then
-		local count=0
-		while true; do
-			eval $PSEUDO groupadd $opts || true
-			group_exists="`grep "^$groupname:" $rootdir/etc/group || true`"
-			if test "x$group_exists" = "x"; then
-				bbwarn "${PN}: groupadd command did not succeed. Retrying..."
-			else
-				break
-			fi
-			count=`expr $count + 1`
-			if test $count = $retries; then
-				bbfatal "${PN}: Tried running groupadd command $retries times without success, giving up"
-			fi
-                        sleep $count
-		done
+		opts=`echo $opts | sed s/\'/\"/g`
+		eval flock -x -w 100 $rootdir/tmp/user-and-group-ops.flock -c \'$PSEUDO groupadd $opts\' || true
+		group_exists="`grep "^$groupname:" $rootdir/etc/group || true`"
+		if test "x$group_exists" = "x"; then
+			bbfatal "${PN}: groupadd command did not succeed."
+		fi
 	else
 		bbnote "${PN}: group $groupname already exists, not re-creating it"
 	fi
@@ -41,26 +31,16 @@ perform_groupadd () {
 perform_useradd () {
 	local rootdir="$1"
 	local opts="$2"
-	local retries="$3"
-	bbnote "${PN}: Performing useradd with [$opts] and $retries times of retry"
+	bbnote "${PN}: Performing useradd with [$opts]"
 	local username=`echo "$opts" | awk '{ print $NF }'`
 	local user_exists="`grep "^$username:" $rootdir/etc/passwd || true`"
 	if test "x$user_exists" = "x"; then
-	       local count=0
-	       while true; do
-		       eval $PSEUDO useradd $opts || true
-		       user_exists="`grep "^$username:" $rootdir/etc/passwd || true`"
-		       if test "x$user_exists" = "x"; then
-			       bbwarn "${PN}: useradd command did not succeed. Retrying..."
-		       else
-			       break
-		       fi
-		       count=`expr $count + 1`
-		       if test $count = $retries; then
-				bbfatal "${PN}: Tried running useradd command $retries times without success, giving up"
-		       fi
-		       sleep $count
-	       done
+		opts=`echo $opts | sed s/\'/\"/g`
+		eval flock -x -w 100 $rootdir/tmp/user-and-group-ops.flock -c  \'$PSEUDO useradd $opts\' || true
+		user_exists="`grep "^$username:" $rootdir/etc/passwd || true`"
+		if test "x$user_exists" = "x"; then
+			bbfatal "${PN}: useradd command did not succeed."
+		fi
 	else
 		bbnote "${PN}: user $username already exists, not re-creating it"
 	fi
@@ -69,8 +49,7 @@ perform_useradd () {
 perform_groupmems () {
 	local rootdir="$1"
 	local opts="$2"
-	local retries="$3"
-	bbnote "${PN}: Performing groupmems with [$opts] and $retries times of retry"
+	bbnote "${PN}: Performing groupmems with [$opts]"
 	local groupname=`echo "$opts" | awk '{ for (i = 1; i < NF; i++) if ($i == "-g" || $i == "--group") print $(i+1) }'`
 	local username=`echo "$opts" | awk '{ for (i = 1; i < NF; i++) if ($i == "-a" || $i == "--add") print $(i+1) }'`
 	bbnote "${PN}: Running groupmems command with group $groupname and user $username"
@@ -84,25 +63,11 @@ perform_groupmems () {
 	fi
 	local mem_exists="`grep "^$groupname:[^:]*:[^:]*:\([^,]*,\)*$username\(,[^,]*\)*" $rootdir/etc/group || true`"
 	if test "x$mem_exists" = "x"; then
-		local count=0
-		while true; do
-			eval $PSEUDO groupmems $opts || true
-			mem_exists="`grep "^$groupname:[^:]*:[^:]*:\([^,]*,\)*$username\(,[^,]*\)*" $rootdir/etc/group || true`"
-			if test "x$mem_exists" = "x"; then
-				bbwarn "${PN}: groupmems command did not succeed. Retrying..."
-			else
-				break
-			fi
-			count=`expr $count + 1`
-			if test $count = $retries; then
-				if test "x$gshadow" = "xno"; then
-					rm -f $rootdir${sysconfdir}/gshadow
-					rm -f $rootdir${sysconfdir}/gshadow-
-				fi
-				bbfatal "${PN}: Tried running groupmems command $retries times without success, giving up"
-			fi
-			sleep $count
-		done
+		eval flock -x -w 100 $rootdir/tmp/user-and-group-ops.flock -c \'$PSEUDO groupmems $opts\' || true
+		mem_exists="`grep "^$groupname:[^:]*:[^:]*:\([^,]*,\)*$username\(,[^,]*\)*" $rootdir/etc/group || true`"
+		if test "x$mem_exists" = "x"; then
+			bbfatal "${PN}: groupmems command did not succeed."
+		fi
 	else
 		bbnote "${PN}: group $groupname already contains $username, not re-adding it"
 	fi
@@ -115,26 +80,15 @@ perform_groupmems () {
 perform_groupdel () {
 	local rootdir="$1"
 	local opts="$2"
-	local retries="$3"
-	bbnote "${PN}: Performing groupdel with [$opts] and $retries times of retry"
+	bbnote "${PN}: Performing groupdel with [$opts]"
 	local groupname=`echo "$opts" | awk '{ print $NF }'`
 	local group_exists="`grep "^$groupname:" $rootdir/etc/group || true`"
 	if test "x$group_exists" != "x"; then
-		local count=0
-		while true; do
-			eval $PSEUDO groupdel $opts || true
-			group_exists="`grep "^$groupname:" $rootdir/etc/group || true`"
-			if test "x$group_exists" != "x"; then
-				bbwarn "${PN}: groupdel command did not succeed. Retrying..."
-			else
-				break
-			fi
-			count=`expr $count + 1`
-			if test $count = $retries; then
-				bbfatal "${PN}: Tried running groupdel command $retries times without success, giving up"
-			fi
-			sleep $count
-		done
+		eval flock -x -w 100 $rootdir/tmp/user-and-group-ops.flock -c \'$PSEUDO groupdel $opts\' || true
+		group_exists="`grep "^$groupname:" $rootdir/etc/group || true`"
+		if test "x$group_exists" != "x"; then
+			bbfatal "${PN}: groupdel command did not succeed."
+		fi
 	else
 		bbnote "${PN}: group $groupname doesn't exist, not removing it"
 	fi
@@ -143,26 +97,15 @@ perform_groupdel () {
 perform_userdel () {
 	local rootdir="$1"
 	local opts="$2"
-	local retries="$3"
-	bbnote "${PN}: Performing userdel with [$opts] and $retries times of retry"
+	bbnote "${PN}: Performing userdel with [$opts]"
 	local username=`echo "$opts" | awk '{ print $NF }'`
 	local user_exists="`grep "^$username:" $rootdir/etc/passwd || true`"
 	if test "x$user_exists" != "x"; then
-	       local count=0
-	       while true; do
-		       eval $PSEUDO userdel $opts || true
-		       user_exists="`grep "^$username:" $rootdir/etc/passwd || true`"
-		       if test "x$user_exists" != "x"; then
-			       bbwarn "${PN}: userdel command did not succeed. Retrying..."
-		       else
-			       break
-		       fi
-		       count=`expr $count + 1`
-		       if test $count = $retries; then
-				bbfatal "${PN}: Tried running userdel command $retries times without success, giving up"
-		       fi
-		       sleep $count
-	       done
+		eval flock -x -w 100 $rootdir/tmp/user-and-group-ops.flock -c \'$PSEUDO userdel $opts\' || true
+		user_exists="`grep "^$username:" $rootdir/etc/passwd || true`"
+		if test "x$user_exists" != "x"; then
+			bbfatal "${PN}: userdel command did not succeed."
+		fi
 	else
 		bbnote "${PN}: user $username doesn't exist, not removing it"
 	fi
@@ -174,25 +117,14 @@ perform_groupmod () {
 	set +e
 	local rootdir="$1"
 	local opts="$2"
-	local retries="$3"
-	bbnote "${PN}: Performing groupmod with [$opts] and $retries times of retry"
+	bbnote "${PN}: Performing groupmod with [$opts]"
 	local groupname=`echo "$opts" | awk '{ print $NF }'`
 	local group_exists="`grep "^$groupname:" $rootdir/etc/group || true`"
 	if test "x$group_exists" != "x"; then
-		local count=0
-		while true; do
-			eval $PSEUDO groupmod $opts
-			if test $? != 0; then
-				bbwarn "${PN}: groupmod command did not succeed. Retrying..."
-			else
-				break
-			fi
-			count=`expr $count + 1`
-			if test $count = $retries; then
-				bbfatal "${PN}: Tried running groupmod command $retries times without success, giving up"
-			fi
-			sleep $count
-		done
+		eval flock -x -w 100 $rootdir/tmp/user-and-group-ops.flock -c \'$PSEUDO groupmod $opts\'
+		if test $? != 0; then
+			bbwarn "${PN}: groupmod command did not succeed."
+		fi
 	else
 		bbwarn "${PN}: group $groupname doesn't exist, unable to modify it"
 	fi
@@ -204,25 +136,14 @@ perform_usermod () {
 	set +e
 	local rootdir="$1"
 	local opts="$2"
-	local retries="$3"
-	bbnote "${PN}: Performing usermod with [$opts] and $retries times of retry"
+	bbnote "${PN}: Performing usermod with [$opts]"
 	local username=`echo "$opts" | awk '{ print $NF }'`
 	local user_exists="`grep "^$username:" $rootdir/etc/passwd || true`"
 	if test "x$user_exists" != "x"; then
-	       local count=0
-	       while true; do
-		       eval $PSEUDO usermod $opts
-		       if test $? != 0; then
-			       bbwarn "${PN}: usermod command did not succeed. Retrying..."
-		       else
-			       break
-		       fi
-		       count=`expr $count + 1`
-		       if test $count = $retries; then
-				bbfatal "${PN}: Tried running usermod command $retries times without success, giving up"
-		       fi
-		       sleep $count
-	       done
+		eval flock -x -w 100 $rootdir/tmp/user-and-group-ops.flock -c \'$PSEUDO usermod $opts\'
+		if test $? != 0; then
+			bbfatal "${PN}: usermod command did not succeed."
+		fi
 	else
 		bbwarn "${PN}: user $username doesn't exist, unable to modify it"
 	fi
-- 
2.6.1

Re: [PATCH 1/1] useradd_base.bbclass: fix simultaneous with flock

[Burton, Ross]

[-- Attachment #1: Type: text/plain, Size: 369 bytes --]

On 15 February 2016 at 01:59, <kai.kang@windriver.com> wrote:

> +               eval flock -x -w 100 $rootdir/tmp/user-and-group-ops.flock
> -c  \'$PSEUDO useradd $opts\' || true
>

This is a great patch, but why not just flock $rootdir/etc instead of new
file in tmp?    Surely images end up being created with
/tmp/user-and-group-ops.flock in them?

Ross

[-- Attachment #2: Type: text/html, Size: 793 bytes --]

Re: [PATCH 1/1] useradd_base.bbclass: fix simultaneous with flock

[Kang Kai]

[-- Attachment #1: Type: text/plain, Size: 722 bytes --]

On 2016年02月15日 20:31, Burton, Ross wrote:
>
> On 15 February 2016 at 01:59, <kai.kang@windriver.com 
> <mailto:kai.kang@windriver.com>> wrote:
>
>     +          eval flock -x -w 100
>     $rootdir/tmp/user-and-group-ops.flock -c  \'$PSEUDO useradd
>     $opts\' || true
>
>
> This is a great patch, but why not just flock $rootdir/etc instead of 
> new file in tmp?    Surely images end up being created with 
> /tmp/user-and-group-ops.flock in them?

I didn't try to think take a directory as a lock and I'll try it. For 
the lock file in $rootdir/tmp/, it is ok that /tmp will be remounted and 
user won't see the lock file.

Thanks,
Kai



>
> Ross


-- 
Regards,
Neil | Kai Kang


[-- Attachment #2: Type: text/html, Size: 2127 bytes --]