[parted-devel] [PATCH 2/2] test for the bootcode-in-extended-partition fix

Fri May 22 14:42:48 UTC 2009

Hi!

On Fri, May 22, 2009 at 01:57:31PM +0200, Joel Granados wrote:
> 
> This is great!!! my comments bellow.
> On Fri, May 22, 2009 at 11:11:32AM +0200, Petr Uzel wrote:
> > * tests/t2300-dos-label-extended-bootcode.sh: New file.
> > * tests/Makefile.am (TESTS): Add t2300-dos-label-extended-bootcode.sh.
> > 
> > Signed-off-by: Petr Uzel <petr.uzel at suse.cz>
<SNIP>
> > +
> > +test_description='Make sure that parted preserves bootcode in extended partition.'
> > +
> > +: ${srcdir=.}
> > +. $srcdir/test-lib.sh
> > +
> > +
> > +N=10M
> 
> Why so large?  I made some tests and 100K was enough for these types of
> tests.  Seems a waist of effort to me.

TBH, the size comes from /dev/random :)

> 
> > +prim_part_size=5M
> 
> Why so large?

N/2 (so still from /dev/random)

> 
> > +dev=loop-file
> > +bootcode=bootcode
> > +bootcode_size=440
> > +bootcode_before=bootcode_before
> > +bootcode_after=bootcode_after
> > +
> > +
> > +test_expect_success \
> > +	'Create the test file' \
> > +	'dd if=/dev/null of=$dev bs=1 seek=$N 2>/dev/null'
> > +
> > +test_expect_success \
> > +	'Create msdos label' \
> > +	'parted -s $dev mklabel msdos > out 2>&1'
> > +test_expect_success 'Expect no output' 'compare out /dev/null'
> > +
> > +test_expect_success \
> > +	'Create primary partition' \
> > +	'parted -s $dev mkpart primary 0 $prim_part_size > out 2>&1'
> > +test_expect_success 'Expect no output' 'compare out /dev/null'
> 
> Why is the primary partition created?  The test also works without the
> primary partition.  You can create an extended and a logical, then
> install and save the boot code, then erase the logical partition, then
> test to see if the boot code is still there.

Well, I've tried to reproduce the original issue. But I agree that the
primary partition is not needed for this test.

> 
> > +
> > +test_expect_success \
> > +	'Create extended partition' \
> > +	'parted -s $dev mkpart extended $prim_part_size $N > out 2>&1'
> > +test_expect_success 'Expect no output' 'compare out /dev/null'
> > +
> > +test_expect_success \
> > +	'Create logical partition' \
> > +	'parted -s $dev mkpart logical $prim_part_size $N > out 2>&1'
> > +test_expect_success 'Expect no output' 'compare out /dev/null'
> > +
> 
> I think you can install the boot code with the following code.  I tested
> this and it works pretty well.  You would have to apply it to your
> test.
> 
> <snip>
> dd if=file of=file2 bs=512c count=32 &&
> dd if=/dev/urandom of=file2 seek=16384c bs=1c count=446 &&
> dd if=file of=file2 skip=16830 seek=16830 bs=1c count=85570
> </snip>

OK, this can make the test simpler. But when we know the position of
extended partition, we could even install the bootcode directly with
on call to dd:

[*] dd if=/dev/urandom of=file bs=1c count=440 seek=16384 conv=notrunc

> 
> Note that those three lines install a randomised boot section (I used
> 446 but 440 is better).  Also note that the calculations expect the
> partitions to be in certain places.  With this in mind the previous code
> must be preceeded by the following partition creation (I think the
> parted lines need the -s arg)
> 
> <snip>
> dd if=/dev/zero of=file bs=1024c count=100
> parted file mklabel msdos
> parted file unit s mkpart extended 32s 127s
> parted file unit s mkpart logical 64s 127s
> </snip>
> 
> I have attached a bash script of what I think the test should look like.
> IMO its much cleaner than what you propose.  Pls have a look and tell me
> what you think.

Yes, let's keep the test simple. So if you agree, I will rework the
test according to your script, but with [*].

Thanks a lot for your feedback!

> > +test_expect_success \
> > +	'Get start of extended partition' \
> > +	'ep_start=`parted -s $dev unit B print | grep extended | sed -e "s/^ *[[:digit:]]* *\([[:digit:]]*\)B.*$/\1/"`'
> > +
> > +test_expect_success \
> > +	'Prepare fake bootcode' \
> > +	'rm -rf $bootcode && for char in `seq 1 440`; do echo -n "X" >> $bootcode; done'
> > +
> > +test_expect_success \
> > +	'Install fake bootcode' \
> > +	'dd if=$bootcode of=$dev bs=1 seek=$ep_start count=$bootcode_size conv=notrunc 2> /dev/null'
> > +
> > +test_expect_success \
> > +	'Save fake bootcode for later comparison' \
> > +	'dd if=$dev of=$bootcode_before bs=1 skip=$ep_start count=$bootcode_size 2>/dev/null'
> > +
> > +test_expect_success \
> > +	'Unset bootflag for logical partition' \
> > +	'parted -s $dev set 5 boot of > out 2>&1'
> > +test_expect_success 'Expect no output' 'compare out /dev/null'
> > +
> > +test_expect_success \
> > +	'Retrieve bootcode after parted operation' \
> > +	'dd if=$dev of=$bootcode_after bs=1 skip=$ep_start count=$bootcode_size 2>/dev/null'
> > +
> > +test_expect_success \
> > +	'Expect bootcode has not changed' \
> > +	'compare $bootcode_before $bootcode_after'
> > +
> > +test_done
> > -- 
> > 1.6.3
> > 
> > 

> #!/bin/bash
> 
> parted="sudo /usr/local/sbin/parted"
> # Create the test label
> dd if=/dev/zero of=file bs=1024c count=100
> $parted -s file mklabel msdos
> $parted -s file unit s mkpart extended 32s 127s
> $parted -s file unit s mkpart logical 64s 127s
> 
> # Install the boot code
> dd if=file of=file2 bs=512c count=32
> dd if=/dev/urandom of=file2 seek=16384c bs=1c count=440
> dd if=file of=file2 skip=16824 seek=16824 bs=1c count=85576
> mv file2 file
> 
> # Print so we can see its still valid (not in the real test)
> $parted -s file unit s print free
> 
> # extract the boot code for comparison.
> dd if=file of=before skip=16384 bs=1c count=440
> 
> # do something to the label
> $parted -s file rm 5
> 
> # extract the boot code for comparison.
> dd if=file of=after skip=16384 bs=1c count=440
> 
> #compare
> diff before after
> 
> # Print so we can see its still valid (not in the real test)
> $parted -s file unit s print free
> 
> rm file before after

-- 
Best regards / s pozdravem

Petr Uzel, Packages maintainer
---------------------------------------------------------------------
SUSE LINUX, s.r.o.                          e-mail: puzel at suse.cz
Lihovarská 1060/12                          tel: +420 284 028 964
190 00 Prague 9                             fax: +420 284 028 951
Czech Republic                              http://www.suse.cz