Merge lp:~vicamo/systemd/systemd into lp:ubuntu/vivid/systemd

[You-Sheng Yang]

Please see https://bugs.launchpad.net/ubuntu/+source/systemd/+bug/1435687

This patch tries to reuse device names that is already written in tmp rules and has exactly the same match expr with the one that is going to be written out.

Per Didier Roche's comment, I'm not going to request for review until it's destiny has been decided in upstream Debian bug. And maybe until then, we can use PredictableNetworkInterfaceNames feature from systemd directly and do not even need this at all. Just for a backup for now.

[Martin Pitt (pitti)]

Structurally, please don't use a debian/patches/*.patch against files in debian/, just apply them inline. But I don't think we should do something like this at all. I'll follow up in the bug report, where we can discuss the solution. Thanks!

review: Disapprove

Unmerged revisions

75. By You-Sheng Yang on 2015-03-24

* Add Avoid-duplicated-udev-match-rules.patch to avoid duplicated
  udev match entries for the same network interface on devices with
  read-only rootfs. (LP: #1435687)

=== added directory '.pc/Avoid-duplicated-udev-match-rules.patch'
=== added file '.pc/Avoid-duplicated-udev-match-rules.patch/.timestamp'
=== added directory '.pc/Avoid-duplicated-udev-match-rules.patch/debian'
=== added directory '.pc/Avoid-duplicated-udev-match-rules.patch/debian/extra'
=== added file '.pc/Avoid-duplicated-udev-match-rules.patch/debian/extra/rule_generator.functions'
--- .pc/Avoid-duplicated-udev-match-rules.patch/debian/extra/rule_generator.functions	1970-01-01 00:00:00 +0000
+++ .pc/Avoid-duplicated-udev-match-rules.patch/debian/extra/rule_generator.functions	2015-03-24 07:42:49 +0000
@@ -0,0 +1,116 @@
+# functions used by the udev rule generator
+
+# Copyright (C) 2006 Marco d'Itri <[e-mail]>
+
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 2 of the License, or
+# (at your option) any later version.
+
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+PATH='/sbin:/bin'
+#
+
+PATH='/sbin:/bin'
+
+# Read a single line from file $1 in the $DEVPATH directory.
+# The function must not return an error even if the file does not exist.
+sysread() {
+	local file="$1"
+	[ -e "/sys$DEVPATH/$file" ] || return 0
+	local value
+	read value < "/sys$DEVPATH/$file" || return 0
+	echo "$value"
+}
+
+sysreadlink() {
+	local file="$1"
+	[ -e "/sys$DEVPATH/$file" ] || return 0
+	readlink -f /sys$DEVPATH/$file 2> /dev/null || true
+}
+
+# Return true if a directory is writeable.
+writeable() {
+	if ln -s test-link $1/.is-writeable 2> /dev/null; then
+		rm -f $1/.is-writeable
+		return 0
+	else
+		return 1
+	fi
+}
+
+# Create a lock file for the current rules file.
+lock_rules_file() {
+	RUNDIR=/run/udev
+	[ -e "$RUNDIR" ] || return 0
+
+	RULES_LOCK="$RUNDIR/.lock-${RULES_FILE##*/}"
+
+	retry=30
+	while ! mkdir $RULES_LOCK 2> /dev/null; do
+		if [ $retry -eq 0 ]; then
+			 echo "Cannot lock $RULES_FILE!" >&2
+			 exit 2
+		fi
+		sleep 1
+		retry=$(($retry - 1))
+	done
+}
+
+unlock_rules_file() {
+	[ "$RULES_LOCK" ] || return 0
+	rmdir $RULES_LOCK || true
+}
+
+# Choose the real rules file if it is writeable or a temporary file if not.
+# Both files should be checked later when looking for existing rules.
+choose_rules_file() {
+	RUNDIR=/run/udev
+	local tmp_rules_file="$RUNDIR/tmp-rules--${RULES_FILE##*/}"
+	[ -e "$RULES_FILE" -o -e "$tmp_rules_file" ] || PRINT_HEADER=1
+
+	if writeable ${RULES_FILE%/*}; then
+		RO_RULES_FILE='/dev/null'
+	else
+		RO_RULES_FILE=$RULES_FILE
+		RULES_FILE=$tmp_rules_file
+	fi
+}
+
+# Return the name of the first free device.
+raw_find_next_available() {
+	local links="$1"
+
+	local basename=${links%%[ 0-9]*}
+	local max=-1
+	for name in $links; do
+		local num=${name#$basename}
+		[ "$num" ] || num=0
+		[ $num -gt $max ] && max=$num
+	done
+
+	local max=$(($max + 1))
+	# "name0" actually is just "name"
+	[ $max -eq 0 ] && return
+	echo "$max"
+}
+
+# Find all rules matching a key (with action) and a pattern.
+find_all_rules() {
+	local key="$1"
+	local linkre="$2"
+	local match="$3"
+
+	local search='.*[[:space:],]'"$key"'"('"$linkre"')".*'
+	echo $(sed -n -r -e 's/^#.*//' -e "${match}s/${search}/\1/p" \
+		$RO_RULES_FILE \
+		$([ -e $RULES_FILE ] && echo $RULES_FILE) \
+		2>/dev/null)
+}
 
=== added file '.pc/Avoid-duplicated-udev-match-rules.patch/debian/extra/write_net_rules'
--- .pc/Avoid-duplicated-udev-match-rules.patch/debian/extra/write_net_rules	1970-01-01 00:00:00 +0000
+++ .pc/Avoid-duplicated-udev-match-rules.patch/debian/extra/write_net_rules	2015-03-24 07:42:49 +0000
@@ -0,0 +1,141 @@
+#!/bin/sh -e
+
+# This script is run to create persistent network device naming rules
+# based on properties of the device.
+# If the interface needs to be renamed, INTERFACE_NEW=<name> will be printed
+# on stdout to allow udev to IMPORT it.
+
+# variables used to communicate:
+#   MATCHADDR             MAC address used for the match
+#   MATCHID               bus_id used for the match
+#   MATCHDEVID            dev_id used for the match
+#   MATCHDRV              driver name used for the match
+#   MATCHIFTYPE           interface type match
+#   COMMENT               comment to add to the generated rule
+#   INTERFACE_NAME        requested name supplied by external tool
+#   INTERFACE_NEW         new interface name returned by rule writer
+
+# Copyright (C) 2006 Marco d'Itri <[e-mail]>
+# Copyright (C) 2007 Kay Sievers <[e-mail]>
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# debug, if UDEV_LOG=<debug>
+if [ -n "$UDEV_LOG" ]; then
+	if [ "$UDEV_LOG" -ge 7 ]; then
+		set -x
+	fi
+fi
+
+RULES_FILE='/etc/udev/rules.d/70-persistent-net.rules'
+
+. /lib/udev/rule_generator.functions
+
+interface_name_taken() {
+	local value="$(find_all_rules 'NAME=' $INTERFACE)"
+	if [ "$value" ]; then
+		return 0
+	else
+		return 1
+	fi
+}
+
+find_next_available() {
+	raw_find_next_available "$(find_all_rules 'NAME=' "$1")"
+}
+
+write_rule() {
+	local match="$1"
+	local name="$2"
+	local comment="$3"
+
+	{
+	if [ "$PRINT_HEADER" ]; then
+		PRINT_HEADER=
+		echo "# This file was automatically generated by the $0"
+		echo "# program, run by the persistent-net-generator.rules rules file."
+		echo "#"
+		echo "# You can modify it, as long as you keep each rule on a single"
+		echo "# line, and change only the value of the NAME= key."
+	fi
+
+	echo ""
+	[ "$comment" ] && echo "# $comment"
+	echo "SUBSYSTEM==\"net\", ACTION==\"add\"$match, NAME=\"$name\""
+	} >> $RULES_FILE
+}
+
+if [ -z "$INTERFACE" ]; then
+	echo "missing \$INTERFACE" >&2
+	exit 1
+fi
+
+# Prevent concurrent processes from modifying the file at the same time.
+lock_rules_file
+
+# Check if the rules file is writeable.
+choose_rules_file
+
+# the DRIVERS key is needed to not match bridges and VLAN sub-interfaces
+if [ "$MATCHADDR" ]; then
+	match="$match, DRIVERS==\"?*\", ATTR{address}==\"$MATCHADDR\""
+fi
+
+if [ "$MATCHDRV" ]; then
+	match="$match, DRIVERS==\"$MATCHDRV\""
+fi
+
+if [ "$MATCHDEVID" ]; then
+	match="$match, ATTR{dev_id}==\"$MATCHDEVID\""
+fi
+
+if [ "$MATCHID" ]; then
+	match="$match, KERNELS==\"$MATCHID\""
+fi
+
+if [ "$MATCHIFTYPE" ]; then
+	match="$match, ATTR{type}==\"$MATCHIFTYPE\""
+fi
+
+if [ -z "$match" ]; then
+	echo "missing valid match" >&2
+	unlock_rules_file
+	exit 1
+fi
+
+basename=${INTERFACE%%[0-9]*}
+match="$match, KERNEL==\"$basename*\""
+
+if [ "$INTERFACE_NAME" ]; then
+	# external tools may request a custom name
+	COMMENT="$COMMENT (custom name provided by external tool)"
+	if [ "$INTERFACE_NAME" != "$INTERFACE" ]; then
+		INTERFACE=$INTERFACE_NAME;
+		echo "INTERFACE_NEW=$INTERFACE"
+	fi
+else
+	# if a rule using the current name already exists, find a new name
+	if interface_name_taken; then
+		INTERFACE="$basename$(find_next_available "$basename[0-9]*")"
+		# prevent INTERFACE from being "eth" instead of "eth0"
+		[ "$INTERFACE" = "${INTERFACE%%[ \[\]0-9]*}" ] && INTERFACE=${INTERFACE}0
+		echo "INTERFACE_NEW=$INTERFACE"
+	fi
+fi
+
+write_rule "$match" "$INTERFACE" "$COMMENT"
+
+unlock_rules_file
+
+exit 0
 
=== modified file '.pc/applied-patches'
--- .pc/applied-patches	2014-04-27 13:21:11 +0000
+++ .pc/applied-patches	2015-03-24 07:42:49 +0000
@@ -64,3 +64,4 @@
 Fix-a-race-condition-at-boot-with-serio-devices.patch
 Create-disk-by-partlabel-links-for-mmcblk-partitions.patch
 Support-system-image-read-only-etc.patch
+Avoid-duplicated-udev-match-rules.patch
 
=== modified file 'debian/extra/rule_generator.functions'
--- debian/extra/rule_generator.functions	2013-11-06 14:01:26 +0000
+++ debian/extra/rule_generator.functions	2015-03-24 07:42:49 +0000
@@ -108,7 +108,7 @@
 	local linkre="$2"
 	local match="$3"
 
-	local search='.*[[:space:],]'"$key"'"('"$linkre"')".*'
+	local search='.*'"$key"'"('"$linkre"')".*'
 	echo $(sed -n -r -e 's/^#.*//' -e "${match}s/${search}/\1/p" \
 		$RO_RULES_FILE \
 		$([ -e $RULES_FILE ] && echo $RULES_FILE) \
 
=== modified file 'debian/extra/write_net_rules'
--- debian/extra/write_net_rules	2014-04-26 17:29:00 +0000
+++ debian/extra/write_net_rules	2015-03-24 07:42:49 +0000
@@ -43,7 +43,7 @@
 . /lib/udev/rule_generator.functions
 
 interface_name_taken() {
-	local value="$(find_all_rules 'NAME=' $INTERFACE)"
+	local value="$(find_all_rules '[[:space:],]NAME=' $INTERFACE)"
 	if [ "$value" ]; then
 		return 0
 	else
@@ -52,7 +52,7 @@
 }
 
 find_next_available() {
-	raw_find_next_available "$(find_all_rules 'NAME=' "$1")"
+	raw_find_next_available "$(find_all_rules '[[:space:],]NAME=' "$1")"
 }
 
 write_rule() {
@@ -127,10 +127,20 @@
 else
 	# if a rule using the current name already exists, find a new name
 	if interface_name_taken; then
-		INTERFACE="$basename$(find_next_available "$basename[0-9]*")"
-		# prevent INTERFACE from being "eth" instead of "eth0"
-		[ "$INTERFACE" = "${INTERFACE%%[ \[\]0-9]*}" ] && INTERFACE=${INTERFACE}0
-		echo "INTERFACE_NEW=$INTERFACE"
+		match_exp="$(echo "$match" | sed -e s/{/\\\\{/g -e s/}/\\\\}/g -e s/*/\\\\*/g -e s/?/\\\\?/g)"
+		INTERFACE_NAME="$(find_all_rules 'SUBSYSTEM=="net", ACTION=="add"'"${match_exp}"', NAME=' "$basename[0-9]*")"
+		if [ -z "$INTERFACE_NAME" ]; then
+			INTERFACE="$basename$(find_next_available "$basename[0-9]*")"
+			# prevent INTERFACE from being "eth" instead of "eth0"
+			[ "$INTERFACE" = "${INTERFACE%%[ \[\]0-9]*}" ] && INTERFACE=${INTERFACE}0
+			echo "INTERFACE_NEW=$INTERFACE"
+		elif [ "$INTERFACE_NAME" != "$INTERFACE" ]; then
+			INTERFACE=$INTERFACE_NAME
+			echo "INTERFACE_NEW=$INTERFACE"
+		else
+			unlock_rules_file
+			exit 0
+		fi
 	fi
 fi
 
 
=== added file 'debian/patches/Avoid-duplicated-udev-match-rules.patch'
--- debian/patches/Avoid-duplicated-udev-match-rules.patch	1970-01-01 00:00:00 +0000
+++ debian/patches/Avoid-duplicated-udev-match-rules.patch	2015-03-24 07:42:49 +0000
@@ -0,0 +1,133 @@
+From ca74f1efff06983160fe28bc3f931de73438de0a Mon Sep 17 00:00:00 2001
+From: You-Sheng Yang <[e-mail]>
+Date: Wed, 18 Mar 2015 11:33:32 +0800
+Subject: [PATCH] Avoid duplicated udev match rules
+
+On devices with read-only rootfs, e.g. mobile phones, nic device number
+(wlan<N>) may increase every time disabled and re-enabled. To be more
+precisely, this happens only on devices when disabling a nic removes the
+corresponding driver.
+
+"/lib/udev/rules.d/75-persistent-net-generator.rules" checks whether
+NAME attribute has been assigned to wlan<N> device: if yes, skip all the
+followed steps, or, call to "/lib/udev/write_net_rules" to generate a
+persistent device name rule file. That persistent file should be created
+under "/etc/udev/rules.d" and named "70-persistent-net.rules", so it
+guarantees NAME attribute should be assigned if available before being
+read. However, when rootfs was previously mounted as read-only, a file
+"/run/udev/tmp-rules--70-persistent-net.rules" is created instead. This
+temporary file is supposed to be moved back into "/etc/udev/rules.d" by
+a systemd service udev-finish right after the system finishes start-up
+chaos. Again, if rootfs is still mounted as read-only, this move will
+certainly fail. One last important thing, /run/udev is _NOT_ included in
+udev rules inclusion paths, so any rules written here will not be taken
+into account when processing uevents.
+
+So, when wlan0 is probed for the first time on a device with read-only
+rootfs, udev creates "/run/udev/tmp-rules--70-persistent-net.rules" and
+inserts one rule for it. When wlan0 is disabled and re-enabled, since
+"/run/udev/tmp-rules--70-persistent-net.rules" is not taken into
+account, its NAME attribute will not be set, and udev recognize it as a
+new nic and tries to write another rule for it again. However, in this
+time, "wlan0" has been taken in the previously written temporary rules
+file, so "wlan1" is chosen instead, and an exactly the same matching
+rule (except for NAME= part) is appended to
+"/run/udev/tmp-rules--70-persistent-net.rules". When the device is
+again disabled and re-enabled, "wlan2" will be assigned. And so on ....
+
+This patch add an additional step to search if current match rule had
+been previously written and reuse that interface name if available.
+
+Signed-off-by: You-Sheng Yang <[e-mail]>
+
+diff --git a/debian/extra/rule_generator.functions b/debian/extra/rule_generator.functions
+index 925193e..a676ff7 100644
+--- a/debian/extra/rule_generator.functions
++++ b/debian/extra/rule_generator.functions
+@@ -100,14 +100,14 @@ raw_find_next_available() {
+ }
+ 
+ # Find all rules matching a key (with action) and a pattern.
+ find_all_rules() {
+ 	local key="$1"
+ 	local linkre="$2"
+ 	local match="$3"
+ 
+-	local search='.*[[:space:],]'"$key"'"('"$linkre"')".*'
++	local search='.*'"$key"'"('"$linkre"')".*'
+ 	echo $(sed -n -r -e 's/^#.*//' -e "${match}s/${search}/\1/p" \
+ 		$RO_RULES_FILE \
+ 		$([ -e $RULES_FILE ] && echo $RULES_FILE) \
+ 		2>/dev/null)
+ }
+diff --git a/debian/extra/write_net_rules b/debian/extra/write_net_rules
+index 4379792..b05ed43 100644
+--- a/debian/extra/write_net_rules
++++ b/debian/extra/write_net_rules
+@@ -38,26 +38,26 @@ if [ -n "$UDEV_LOG" ]; then
+ 	fi
+ fi
+ 
+ RULES_FILE='/etc/udev/rules.d/70-persistent-net.rules'
+ 
+ . /lib/udev/rule_generator.functions
+ 
+ interface_name_taken() {
+-	local value="$(find_all_rules 'NAME=' $INTERFACE)"
++	local value="$(find_all_rules '[[:space:],]NAME=' $INTERFACE)"
+ 	if [ "$value" ]; then
+ 		return 0
+ 	else
+ 		return 1
+ 	fi
+ }
+ 
+ find_next_available() {
+-	raw_find_next_available "$(find_all_rules 'NAME=' "$1")"
++	raw_find_next_available "$(find_all_rules '[[:space:],]NAME=' "$1")"
+ }
+ 
+ write_rule() {
+ 	local match="$1"
+ 	local name="$2"
+ 	local comment="$3"
+ 
+ 	{
+@@ -122,20 +122,30 @@ if [ "$INTERFACE_NAME" ]; then
+ 	COMMENT="$COMMENT (custom name provided by external tool)"
+ 	if [ "$INTERFACE_NAME" != "$INTERFACE" ]; then
+ 		INTERFACE=$INTERFACE_NAME;
+ 		echo "INTERFACE_NEW=$INTERFACE"
+ 	fi
+ else
+ 	# if a rule using the current name already exists, find a new name
+ 	if interface_name_taken; then
+-		INTERFACE="$basename$(find_next_available "$basename[0-9]*")"
+-		# prevent INTERFACE from being "eth" instead of "eth0"
+-		[ "$INTERFACE" = "${INTERFACE%%[ \[\]0-9]*}" ] && INTERFACE=${INTERFACE}0
+-		echo "INTERFACE_NEW=$INTERFACE"
++		match_exp="$(echo "$match" | sed -e s/{/\\\\{/g -e s/}/\\\\}/g -e s/*/\\\\*/g -e s/?/\\\\?/g)"
++		INTERFACE_NAME="$(find_all_rules 'SUBSYSTEM=="net", ACTION=="add"'"${match_exp}"', NAME=' "$basename[0-9]*")"
++		if [ -z "$INTERFACE_NAME" ]; then
++			INTERFACE="$basename$(find_next_available "$basename[0-9]*")"
++			# prevent INTERFACE from being "eth" instead of "eth0"
++			[ "$INTERFACE" = "${INTERFACE%%[ \[\]0-9]*}" ] && INTERFACE=${INTERFACE}0
++			echo "INTERFACE_NEW=$INTERFACE"
++		elif [ "$INTERFACE_NAME" != "$INTERFACE" ]; then
++			INTERFACE=$INTERFACE_NAME
++			echo "INTERFACE_NEW=$INTERFACE"
++		else
++			unlock_rules_file
++			exit 0
++		fi
+ 	fi
+ fi
+ 
+ write_rule "$match" "$INTERFACE" "$COMMENT"
+ 
+ unlock_rules_file
+ 
+ exit 0
+-- 
+2.1.4
+
 
=== modified file 'debian/patches/series'
--- debian/patches/series	2014-04-27 13:21:11 +0000
+++ debian/patches/series	2015-03-24 07:42:49 +0000
@@ -64,3 +64,4 @@
 Fix-a-race-condition-at-boot-with-serio-devices.patch
 Create-disk-by-partlabel-links-for-mmcblk-partitions.patch
 Support-system-image-read-only-etc.patch
+Avoid-duplicated-udev-match-rules.patch