Patchwork ofpath bugfix powerpc g5 dual sata

login
register
mail settings
Submitter peter cros
Date Sept. 27, 2008, 2:57 p.m.
Message ID <48f46e890809270757i24409c08lf54e830549167937@mail.gmail.com>
Download mbox | patch
Permalink /patch/22976/
State Under Review
Headers show

Comments

peter cros - Sept. 27, 2008, 2:57 p.m.
Hi,

I am submitting this small patch to fix a residual bug in ofpath, for Apple
powerpc64 g5 with dual sata drives, ubuntu and possibly other installations
which use dash as their default shell.

When ybin runs ofpath in the target root system with the default shell (#!
/bin/sh ), in ubuntu this is /bin/sh -> dash.

The "let" statement used to increment the ID for multiple k2-sata drives, is
not found by the dash shell as used by ubuntu, in my g5 for example.

Failure to increment K2_DEVICE_ID returns the same k2-sata path for each of
multiple sata drives, ybin fails to find correct boot drive.

This patch just avoids using "let".

Tested on powerpc g5 Openfirmware V4, dual sata drives, ubuntu 804.

 -----------------------------------
~/src/gits$ diff -pu yaboot/ybin/ofpath test/ybin/ofpath
 --------------------------------------------------------

:$PARTITION"

-----------------------------
peter cros
peter cros - Nov. 18, 2008, 3:12 p.m.
This one still needs fixing.

On Sun, Sep 28, 2008 at 1:57 AM, peter cros <pxwpxw8@gmail.com> wrote:
> Hi,
>
> I am submitting this small patch to fix a residual bug in ofpath, for Apple
> powerpc64 g5 with dual sata drives, ubuntu and possibly other installations
> which use dash as their default shell.
>
> When ybin runs ofpath in the target root system with the default shell (#!
> /bin/sh ), in ubuntu this is /bin/sh -> dash.
>
> The "let" statement used to increment the ID for multiple k2-sata drives, is
> not found by the dash shell as used by ubuntu, in my g5 for example.
>
> Failure to increment K2_DEVICE_ID returns the same k2-sata path for each of
> multiple sata drives, ybin fails to find correct boot drive.
>
> This patch just avoids using "let".
>
> Tested on powerpc g5 Openfirmware V4, dual sata drives, ubuntu 804.
>
>  -----------------------------------
> ~/src/gits$ diff -pu yaboot/ybin/ofpath test/ybin/ofpath
>  --------------------------------------------------------
>
> --- yaboot/ybin/ofpath    2008-09-26 21:36:38.000000000 +1000
> +++ test/ybin/ofpath    2008-09-26 16:41:00.000000000 +1000
> @@ -306,7 +306,8 @@ scsi_ofpath()
>          K2_DEVICE_ID=0
>          while [ "$DEVICE_PATH" = "" ] ; do
>              SCSI_HOSTNUMBER=`expr $SCSI_HOSTNUMBER - 1`
> -            let "K2_DEVICE_ID += 1"
> +# let not found in dash     #       let "K2_DEVICE_ID += 1"
> +        K2_DEVICE_ID=$((K2_DEVICE_ID+1))
>              DEVICE_PATH="$(printhost $SCSI_HOSTNUMBER $HOST_LIST)"
>          done
>          echo
> "${DEVICE_PATH##*device-tree}/k2-sata@$K2_DEVICE_ID/disk@0:$PARTITION"
>
> -----------------------------
> peter cros
>
>
Roman Rakus - Nov. 18, 2008, 3:40 p.m.
peter cros wrote:
> This one still needs fixing.
>
> On Sun, Sep 28, 2008 at 1:57 AM, peter cros <pxwpxw8@gmail.com> wrote:
>   
>> Hi,
>>
>> I am submitting this small patch to fix a residual bug in ofpath, for Apple
>> powerpc64 g5 with dual sata drives, ubuntu and possibly other installations
>> which use dash as their default shell.
>>
>> When ybin runs ofpath in the target root system with the default shell (#!
>> /bin/sh ), in ubuntu this is /bin/sh -> dash.
>>
>> The "let" statement used to increment the ID for multiple k2-sata drives, is
>> not found by the dash shell as used by ubuntu, in my g5 for example.
>>
>> Failure to increment K2_DEVICE_ID returns the same k2-sata path for each of
>> multiple sata drives, ybin fails to find correct boot drive.
>>
>> This patch just avoids using "let".
>>
>> Tested on powerpc g5 Openfirmware V4, dual sata drives, ubuntu 804.
>>
>>  -----------------------------------
>> ~/src/gits$ diff -pu yaboot/ybin/ofpath test/ybin/ofpath
>>  --------------------------------------------------------
>>
>> --- yaboot/ybin/ofpath    2008-09-26 21:36:38.000000000 +1000
>> +++ test/ybin/ofpath    2008-09-26 16:41:00.000000000 +1000
>> @@ -306,7 +306,8 @@ scsi_ofpath()
>>          K2_DEVICE_ID=0
>>          while [ "$DEVICE_PATH" = "" ] ; do
>>              SCSI_HOSTNUMBER=`expr $SCSI_HOSTNUMBER - 1`
>> -            let "K2_DEVICE_ID += 1"
>> +# let not found in dash     #       let "K2_DEVICE_ID += 1"
>> +        K2_DEVICE_ID=$((K2_DEVICE_ID+1))
>>              DEVICE_PATH="$(printhost $SCSI_HOSTNUMBER $HOST_LIST)"
>>          done
>>          echo
>> "${DEVICE_PATH##*device-tree}/k2-sata@$K2_DEVICE_ID/disk@0:$PARTITION"
>>
>> -----------------------------
>> peter cros
>>
>>
>>     
> _______________________________________________
> Yaboot-devel mailing list
> Yaboot-devel@ozlabs.org
> https://ozlabs.org/mailman/listinfo/yaboot-devel
>   
I can confirm that `let' is bashism (bash builtin) and is better to 
avoid using it. Patch you proposed looks well. Using $(()) is posixly 
correct arithmetic expansion. What do you think Paul? There are no other 
let statements.
RR
Przemyslaw Iskra - Nov. 18, 2008, 5:20 p.m.
On Tue, Nov 18, 2008 at 04:40:30PM +0100, Roman Rakus wrote:
> peter cros wrote:
>> This one still needs fixing.

I think this file requires unification.

>>> --- yaboot/ybin/ofpath    2008-09-26 21:36:38.000000000 +1000
>>> +++ test/ybin/ofpath    2008-09-26 16:41:00.000000000 +1000
>>> @@ -306,7 +306,8 @@ scsi_ofpath()
>>>          K2_DEVICE_ID=0
>>>          while [ "$DEVICE_PATH" = "" ] ; do
>>>              SCSI_HOSTNUMBER=`expr $SCSI_HOSTNUMBER - 1`

backticks `` for execution + expr for math

>>> -            let "K2_DEVICE_ID += 1"
>>> +# let not found in dash     #       let "K2_DEVICE_ID += 1"
>>> +        K2_DEVICE_ID=$((K2_DEVICE_ID+1))

let / $(()) for math

>>>              DEVICE_PATH="$(printhost $SCSI_HOSTNUMBER $HOST_LIST)"

$() for execution


Would be nice to stick to one form, seems like older versions preffer
$(()) for math, it spawns less children; and mostly $() is used for
execution, but there are many exceptions.


> I can confirm that `let' is bashism (bash builtin) and is better to  
> avoid using it. Patch you proposed looks well. Using $(()) is posixly  
> correct arithmetic expansion. What do you think Paul? There are no other  
> let statements.

some other shells (ksh) also allow use of "let", but it isn't POSIX
compliant so it should be replaced.

Patch

--- yaboot/ybin/ofpath    2008-09-26 21:36:38.000000000 +1000
+++ test/ybin/ofpath    2008-09-26 16:41:00.000000000 +1000
@@ -306,7 +306,8 @@  scsi_ofpath()
         K2_DEVICE_ID=0
         while [ "$DEVICE_PATH" = "" ] ; do
             SCSI_HOSTNUMBER=`expr $SCSI_HOSTNUMBER - 1`
-            let "K2_DEVICE_ID += 1"
+# let not found in dash     #       let "K2_DEVICE_ID += 1"
+        K2_DEVICE_ID=$((K2_DEVICE_ID+1))
             DEVICE_PATH="$(printhost $SCSI_HOSTNUMBER $HOST_LIST)"
         done
         echo "${DEVICE_PATH##*device-tree}/k2-sata@$K2_DEVICE_ID/disk@0