[PATCH] unbreak booting with virtio

[PATCH] unbreak booting with virtio

[Glauber Costa]

Since commit 89e671e3, extboot is broken due to wrong checksum

The problem is that printf "\\$sum" syntax will require an octal
representation, so the fix I'm proposing is to convert it first.

Signed-off-by: Glauber Costa <glommer@redhat.com>
CC: Alexander Graf <agraf@suse.de>
---
 pc-bios/optionrom/signrom.sh |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/pc-bios/optionrom/signrom.sh b/pc-bios/optionrom/signrom.sh
index 4322811..3512cc4 100755
--- a/pc-bios/optionrom/signrom.sh
+++ b/pc-bios/optionrom/signrom.sh
@@ -42,4 +42,5 @@ sum=$(( 256 - $sum ))
 
 # and write the output file
 cp "$1" "$2"
+sum=$(echo "obase=8; $sum" | bc)
 printf "\\$sum" | dd of="$2" bs=1 count=1 seek=$size conv=notrunc 2>/dev/null
-- 
1.6.2.2

Re: [PATCH] unbreak booting with virtio

[Alexander Graf]

On 31.07.2009, at 20:19, Glauber Costa <glommer@redhat.com> wrote:

> Since commit 89e671e3, extboot is broken due to wrong checksum
>
> The problem is that printf "\\$sum" syntax will require an octal
> representation, so the fix I'm proposing is to convert it first.

Is there no easy way to tell printf we're on decimal? I don't have a  
Linux system handy atm, but I thought \90 was in fact a 90.

Either way, my only complaint would be to introduce a dependency on bc.


Alex

>
> Signed-off-by: Glauber Costa <glommer@redhat.com>
> CC: Alexander Graf <agraf@suse.de>
> ---
> pc-bios/optionrom/signrom.sh |    1 +
> 1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/pc-bios/optionrom/signrom.sh b/pc-bios/optionrom/ 
> signrom.sh
> index 4322811..3512cc4 100755
> --- a/pc-bios/optionrom/signrom.sh
> +++ b/pc-bios/optionrom/signrom.sh
> @@ -42,4 +42,5 @@ sum=$(( 256 - $sum ))
>
> # and write the output file
> cp "$1" "$2"
> +sum=$(echo "obase=8; $sum" | bc)
> printf "\\$sum" | dd of="$2" bs=1 count=1 seek=$size conv=notrunc 2>/ 
> dev/null
> -- 
> 1.6.2.2
>

Re: [PATCH] unbreak booting with virtio

[Glauber Costa]

On Fri, Jul 31, 2009 at 09:55:39PM +0200, Alexander Graf wrote:
>
> On 31.07.2009, at 20:19, Glauber Costa <glommer@redhat.com> wrote:
>
>> Since commit 89e671e3, extboot is broken due to wrong checksum
>>
>> The problem is that printf "\\$sum" syntax will require an octal
>> representation, so the fix I'm proposing is to convert it first.
>
> Is there no easy way to tell printf we're on decimal? I don't have a  
> Linux system handy atm, but I thought \90 was in fact a 90.
>
> Either way, my only complaint would be to introduce a dependency on bc.
Not that I'm aware of.
But would be happy to know, too.

Re: [PATCH] unbreak booting with virtio

[Ján ONDREJ (SAL)]

On Fri, Jul 31, 2009 at 09:55:39PM +0200, Alexander Graf wrote:
>
> On 31.07.2009, at 20:19, Glauber Costa <glommer@redhat.com> wrote:
>
>> Since commit 89e671e3, extboot is broken due to wrong checksum
>>
>> The problem is that printf "\\$sum" syntax will require an octal
>> representation, so the fix I'm proposing is to convert it first.
>
> Is there no easy way to tell printf we're on decimal? I don't have a  
> Linux system handy atm, but I thought \90 was in fact a 90.
>
> Either way, my only complaint would be to introduce a dependency on bc.

>> cp "$1" "$2"
>> +sum=$(echo "obase=8; $sum" | bc)
>> printf "\\$sum" | dd of="$2" bs=1 count=1 seek=$size conv=notrunc 2>/ 
>> dev/null

May be it's better to use awk:

echo $sum | awk '{ printf("%c", $1) }' \
  | dd of="$2" bs=1 count=1 seek=$size conv=notrunc 2>/dev/null

This version accepts decimal $sum, does not need conversion to octal.

			SAL

Re: [PATCH] unbreak booting with virtio

[Ján ONDREJ (SAL)]

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

On Sat, Aug 01, 2009 at 09:04:55AM +0200, Ján ONDREJ (SAL) wrote:
> On Fri, Jul 31, 2009 at 09:55:39PM +0200, Alexander Graf wrote:
> >
> > On 31.07.2009, at 20:19, Glauber Costa <glommer@redhat.com> wrote:
> >
> >> cp "$1" "$2"
> >> +sum=$(echo "obase=8; $sum" | bc)
> >> printf "\\$sum" | dd of="$2" bs=1 count=1 seek=$size conv=notrunc 2>/ 
> >> dev/null
> 
> May be it's better to use awk:
> 
> echo $sum | awk '{ printf("%c", $1) }' \
>   | dd of="$2" bs=1 count=1 seek=$size conv=notrunc 2>/dev/null
> 
> This version accepts decimal $sum, does not need conversion to octal.

Ane may be it's better to write whole checksum in awk, it's faster and
cleaner. See attached patch (I am sorry, I don't know how to use git-email).

		SAL

[-- Attachment #2: signrom-awk.patch --]
[-- Type: text/plain, Size: 1342 bytes --]

diff --git a/pc-bios/optionrom/signrom.sh b/pc-bios/optionrom/signrom.sh
index 4273d1f..065b6a9 100755
--- a/pc-bios/optionrom/signrom.sh
+++ b/pc-bios/optionrom/signrom.sh
@@ -18,28 +18,31 @@
 #
 # Copyright Novell Inc, 2009
 #   Authors: Alexander Graf <agraf@suse.de>
+#            Jan Ondrej (SAL) <ondrejj(at)salstar.sk>
 #
 # Syntax: signrom.sh <input> <output>
 
 # did we get proper arguments?
 test "$1" -a "$2" || exit 1
 
-sum=0
-
-# find out the file size
-x=`dd if="$1" bs=1 count=1 skip=2 2>/dev/null | od -t u1 -A n`
-#size=`expr $x \* 512 - 1`
-size=$(( $x * 512 - 1 ))
-
-# now get the checksum
-for i in `od -A n -t u1 -v "$1"`; do
-    # add each byte's value to sum
-    sum=$(( $sum + $i ))
-done
-
-sum=$(( $sum % 256 ))
-sum=$(( 256 - $sum ))
-
-# and write the output file
-cp "$1" "$2"
-printf "\\$sum" | dd of="$2" bs=1 count=1 seek=$size conv=notrunc 2>/dev/null
+od -A n -t u1 -v "$1" | awk '
+    BEGIN {
+        checksum=0
+        # last byte will be replaced by checksum
+        last=-1
+    }
+    {
+        # go over all record in row
+        for(i=1; i<=NF; i++) {
+            checksum+=$i
+            if (last>=0) {
+                printf "%c", last
+            }
+            last=$i
+        }
+    }
+    END {
+        # compute checksum
+        printf "%c", 256-(checksum%256)
+    }
+' > $2

Re: [PATCH] unbreak booting with virtio

[Alexander Graf]

On 01.08.2009, at 10:01, Ján ONDREJ (SAL) wrote:

> On Sat, Aug 01, 2009 at 09:04:55AM +0200, Ján ONDREJ (SAL) wrote:
>> On Fri, Jul 31, 2009 at 09:55:39PM +0200, Alexander Graf wrote:
>>>
>>> On 31.07.2009, at 20:19, Glauber Costa <glommer@redhat.com> wrote:
>>>
>>>> cp "$1" "$2"
>>>> +sum=$(echo "obase=8; $sum" | bc)
>>>> printf "\\$sum" | dd of="$2" bs=1 count=1 seek=$size conv=notrunc  
>>>> 2>/
>>>> dev/null
>>
>> May be it's better to use awk:
>>
>> echo $sum | awk '{ printf("%c", $1) }' \
>>  | dd of="$2" bs=1 count=1 seek=$size conv=notrunc 2>/dev/null
>>
>> This version accepts decimal $sum, does not need conversion to octal.
>
> Ane may be it's better to write whole checksum in awk, it's faster and
> cleaner. See attached patch (I am sorry, I don't know how to use git- 
> email).

My awk magic is pretty bad, but I think this version is missing the  
padding, right?

Alex

Re: [PATCH] unbreak booting with virtio

[Paolo Bonzini]

On 07/31/2009 09:55 PM, Alexander Graf wrote:
>
> On 31.07.2009, at 20:19, Glauber Costa <glommer@redhat.com> wrote:
>
>> Since commit 89e671e3, extboot is broken due to wrong checksum
>>
>> The problem is that printf "\\$sum" syntax will require an octal
>> representation, so the fix I'm proposing is to convert it first.
>
> Is there no easy way to tell printf we're on decimal? I don't have a
> Linux system handy atm, but I thought \90 was in fact a 90.
>
> Either way, my only complaint would be to introduce a dependency on bc.

printf `printf '\\%o' $sum`

Paolo

Re: [PATCH] unbreak booting with virtio

[Alexander Graf]

On 01.08.2009, at 01:39, Glauber Costa wrote:

> On Fri, Jul 31, 2009 at 09:55:39PM +0200, Alexander Graf wrote:
>>
>> On 31.07.2009, at 20:19, Glauber Costa <glommer@redhat.com> wrote:
>>
>>> Since commit 89e671e3, extboot is broken due to wrong checksum
>>>
>>> The problem is that printf "\\$sum" syntax will require an octal
>>> representation, so the fix I'm proposing is to convert it first.
>>
>> Is there no easy way to tell printf we're on decimal? I don't have a
>> Linux system handy atm, but I thought \90 was in fact a 90.
>>
>> Either way, my only complaint would be to introduce a dependency on  
>> bc.
> Not that I'm aware of.
> But would be happy to know, too.

Hum, reading the documentation again it in fact needs an octal number.
But we could use printf for the conversion as well!

$ printf "%o" 65
101

That wouldn't introduce a new dependency that might be missing on  
random OSs and looks rather clean to me.

Alex

Re: [PATCH] unbreak booting with virtio

[Ján ONDREJ (SAL)]

On Sat, Aug 01, 2009 at 10:19:20AM +0200, Alexander Graf wrote:
>
> On 01.08.2009, at 10:01, Ján ONDREJ (SAL) wrote:
>
>> On Sat, Aug 01, 2009 at 09:04:55AM +0200, Ján ONDREJ (SAL) wrote:
>>> On Fri, Jul 31, 2009 at 09:55:39PM +0200, Alexander Graf wrote:
>>>>
>>>> On 31.07.2009, at 20:19, Glauber Costa <glommer@redhat.com> wrote:
>>>>
>>>>> cp "$1" "$2"
>>>>> +sum=$(echo "obase=8; $sum" | bc)
>>>>> printf "\\$sum" | dd of="$2" bs=1 count=1 seek=$size conv=notrunc 
>>>>> 2>/
>>>>> dev/null
>>>
>>> May be it's better to use awk:
>>>
>>> echo $sum | awk '{ printf("%c", $1) }' \
>>>  | dd of="$2" bs=1 count=1 seek=$size conv=notrunc 2>/dev/null
>>>
>>> This version accepts decimal $sum, does not need conversion to octal.
>>
>> Ane may be it's better to write whole checksum in awk, it's faster and
>> cleaner. See attached patch (I am sorry, I don't know how to use git- 
>> email).
>
> My awk magic is pretty bad, but I think this version is missing the  
> padding, right?

Right. If you prefer this version agains Paolos double printf, I can
update it, if not, just use Pauols solution.

			SAL

Re: [PATCH] unbreak booting with virtio

[Avi Kivity]

On 07/31/2009 09:19 PM, Glauber Costa wrote:
> Since commit 89e671e3, extboot is broken due to wrong checksum
>
> The problem is that printf "\\$sum" syntax will require an octal
> representation, so the fix I'm proposing is to convert it first.
>
>    

Whichever way this gets resolved, please send it upstream since the bug 
exists there.

-- 
error compiling committee.c: too many arguments to function