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 <[email protected]> <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: [email protected] Lihovarská 1060/12 tel: +420 284 028 964 190 00 Prague 9 fax: +420 284 028 951 Czech Republic http://www.suse.cz _______________________________________________ parted-devel mailing list [email protected] http://lists.alioth.debian.org/mailman/listinfo/parted-devel
