Re: [PATCH] biosdisk / open_device() messing up offsets

[Bean]

On Wed, Jun 11, 2008 at 6:58 AM, Pavel Roskin <address@hidden> wrote:
> On Wed, 2008-06-11 at 02:51 +0800, Bean wrote:
>> Hi,
>>
>> I spot another bug, please try the new patch.
>
> You probably meant to use (LOG2_EXT2_BLOCK_SIZE (data) + 9) in your
> patch because the block size is in 512 bytes units, unlike sizeof:
>
> diff --git a/fs/ext2.c b/fs/ext2.c
> index e4ce47b..fb13a80 100644
> --- a/fs/ext2.c
> +++ b/fs/ext2.c
> @@ -260,8 +260,8 @@ grub_ext2_blockgroup (struct grub_ext2_data *data, int 
> group,
>   int blkno, blkoff;
>
>   group *= sizeof (struct grub_ext2_block_group);
> -  blkno = group >> LOG2_EXT2_BLOCK_SIZE (data);
> -  blkoff = group  - (blkno << LOG2_EXT2_BLOCK_SIZE (data));
> +  blkno = group >> (LOG2_EXT2_BLOCK_SIZE (data) + 9);
> +  blkoff = group - (blkno << (LOG2_EXT2_BLOCK_SIZE (data) + 9));
>
>   return grub_disk_read (data->disk,
>                         (grub_fshelp_map_block (data->journal,
>
> That appears to fix everything!!!  grub-fstest is loading, GRUB is
> working in qemu on the live filesystem, and the normal boot is still
> working.
>

Oh, nice catch, I make a silly mistake again, :)

> I think we should consider having a journaling layer above
> grub_disk_read() that would do the substitution and avoid such bugs in
> the future.

This should be all right, but we need to move the journal support from
fshelp.c to disk.c, anyone objects ?

-- 
Bean