Patchwork [3/3] runqemu-ifup, runqemu-internal: be wiser when locating the network tools

login
register
mail settings
Submitter Dexuan Cui
Date Dec. 1, 2011, 9:21 a.m.
Message ID <7c75cf6a7d4a3c102f1f44eca16eb5c43c530391.1322731182.git.dexuan.cui@intel.com>
Download mbox | patch
Permalink /patch/15957/
State New
Headers show

Comments

Dexuan Cui - Dec. 1, 2011, 9:21 a.m.
When working on the self-hosted-image work, I found the PATH variable in the
Level-1 target doesn't have /sbin and /usr/sbin, so "runqemu" can't
run properly since the tools are installeld at
/sbin/ifconfig
/sbin/route
/usr/sbin/iptables

The patch is used to fix the issue by setting a temp PATH when running which.

Signed-off-by: Dexuan Cui <dexuan.cui@intel.com>
---
 scripts/runqemu-ifup     |    8 +++++---
 scripts/runqemu-internal |    3 ++-
 2 files changed, 7 insertions(+), 4 deletions(-)
Richard Purdie - Dec. 1, 2011, 10:48 a.m.
On Thu, 2011-12-01 at 17:21 +0800, Dexuan Cui wrote:
> When working on the self-hosted-image work, I found the PATH variable in the
> Level-1 target doesn't have /sbin and /usr/sbin, so "runqemu" can't
> run properly since the tools are installeld at
> /sbin/ifconfig
> /sbin/route
> /usr/sbin/iptables
> 
> The patch is used to fix the issue by setting a temp PATH when running which.
> 
> Signed-off-by: Dexuan Cui <dexuan.cui@intel.com>
> ---
>  scripts/runqemu-ifup     |    8 +++++---
>  scripts/runqemu-internal |    3 ++-
>  2 files changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/scripts/runqemu-ifup b/scripts/runqemu-ifup
> index 870cb6b..9e697a8 100755
> --- a/scripts/runqemu-ifup
> +++ b/scripts/runqemu-ifup
> @@ -64,7 +64,9 @@ if [ $STATUS -ne 0 ]; then
>  	exit 1
>  fi
>  
> -IFCONFIG=`which ifconfig 2> /dev/null`
> +PATH_TMP="/sbin:/bin:/usr/sbin:/usr/bin:/usr/local/sbin:/usr/local/bin"
> +
> +IFCONFIG=`{ PATH=$PATH:$PATH_TMP; which ifconfig 2> /dev/null; }`
>  if [ "x$IFCONFIG" = "x" ]; then
>  	# better than nothing...
>  	IFCONFIG=/sbin/ifconfig

I don't really like this, its getting hard to understand whats going on.
Can we abstract this to a function which tries PATH, then tries our own
PATH_TMP? This would reduce code duplication and makes it clearer what
the code is doing...

Cheers,

Richard
Dexuan Cui - Dec. 2, 2011, 2:10 a.m.
Richard Purdie wrote on 2011-12-01:
> On Thu, 2011-12-01 at 17:21 +0800, Dexuan Cui wrote:
>> When working on the self-hosted-image work, I found the PATH
>> variable in the
>> Level-1 target doesn't have /sbin and /usr/sbin, so "runqemu" can't
>> run properly since the tools are installeld at /sbin/ifconfig
>> /sbin/route /usr/sbin/iptables
>> 
>> The patch is used to fix the issue by setting a temp PATH when running
>> which.
>> 
>> Signed-off-by: Dexuan Cui <dexuan.cui@intel.com>
>> ---
>>  scripts/runqemu-ifup     |    8 +++++---
>>  scripts/runqemu-internal |    3 ++-
>>  2 files changed, 7 insertions(+), 4 deletions(-)
>> diff --git a/scripts/runqemu-ifup b/scripts/runqemu-ifup index
>> 870cb6b..9e697a8 100755
>> --- a/scripts/runqemu-ifup
>> +++ b/scripts/runqemu-ifup
>> @@ -64,7 +64,9 @@ if [ $STATUS -ne 0 ]; then
>>  	exit 1
>>  fi
>> -IFCONFIG=`which ifconfig 2> /dev/null`
>> +PATH_TMP="/sbin:/bin:/usr/sbin:/usr/bin:/usr/local/sbin:/usr/local/bin"
>> +
>> +IFCONFIG=`{ PATH=$PATH:$PATH_TMP; which ifconfig 2> /dev/null; }`
>>  if [ "x$IFCONFIG" = "x" ]; then
>>  	# better than nothing...
>>  	IFCONFIG=/sbin/ifconfig
> 
> I don't really like this, its getting hard to understand whats going on.
> Can we abstract this to a function which tries PATH, then tries our
> own PATH_TMP? This would reduce code duplication and makes it clearer
> what the code is doing...
Good suggestion! 
I'll re-make the patch and re-send it out.
BTW, since I'll be on leave later today, I might not be able to finish this today.
I discussed with Saul and he could kindly help me to finish this. :-)

Thanks,
-- Dexuan

Patch

diff --git a/scripts/runqemu-ifup b/scripts/runqemu-ifup
index 870cb6b..9e697a8 100755
--- a/scripts/runqemu-ifup
+++ b/scripts/runqemu-ifup
@@ -64,7 +64,9 @@  if [ $STATUS -ne 0 ]; then
 	exit 1
 fi
 
-IFCONFIG=`which ifconfig 2> /dev/null`
+PATH_TMP="/sbin:/bin:/usr/sbin:/usr/bin:/usr/local/sbin:/usr/local/bin"
+
+IFCONFIG=`{ PATH=$PATH:$PATH_TMP; which ifconfig 2> /dev/null; }`
 if [ "x$IFCONFIG" = "x" ]; then
 	# better than nothing...
 	IFCONFIG=/sbin/ifconfig
@@ -74,7 +76,7 @@  if [ ! -x "$IFCONFIG" ]; then
 	exit 1
 fi
 
-ROUTE=`which route`
+ROUTE=`{ PATH=$PATH:$PATH_TMP; which route 2>/dev/null; }`
 if [ "x$ROUTE" = "x" ]; then
 	# better than nothing...
 	ROUTE=/sbin/route
@@ -84,7 +86,7 @@  if [ ! -x "$ROUTE" ]; then
 	exit 1
 fi
 
-IPTABLES=`which iptables 2> /dev/null`
+IPTABLES=`{ PATH=$PATH:$PATH_TMP; which iptables 2> /dev/null; }`
 if [ "x$IPTABLES" = "x" ]; then
 	IPTABLES=/sbin/iptables
 fi
diff --git a/scripts/runqemu-internal b/scripts/runqemu-internal
index 2968ed9..3214fde 100755
--- a/scripts/runqemu-internal
+++ b/scripts/runqemu-internal
@@ -141,7 +141,8 @@  if [ ! -d "$LOCKDIR" ]; then
     chmod 777 $LOCKDIR
 fi
 
-IFCONFIG=`which ifconfig 2> /dev/null`
+PATH_TMP="/sbin:/bin:/usr/sbin:/usr/bin:/usr/local/sbin:/usr/local/bin"
+IFCONFIG=`{ PATH=$PATH:$PATH_TMP; which ifconfig 2> /dev/null; }`
 if [ -z "$IFCONFIG" ]; then
     IFCONFIG=/sbin/ifconfig
 fi