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

[Pavel Roskin]

On Mon, 2008-06-09 at 02:57 +0800, Bean wrote:

> Thanks, can you also give me the output of dumpe2fs ?
> 
> dumpe2fs /dev/sda1

I'll send it privately, as it won't compress much.

I was trying to find out which change breaks the ext3 support.  It turns
out grub-fstest starts working if I comment out this line in fs/ext2.c:

log->mapping[num++] = grub_be_to_cpu32 (tag->block);

Alternatively, I could replace it with

log->mapping[num++] = GRUB_FSHELP_JOURNAL_UNUSED_MAPPING;

and grub-fstest would list /boot properly.  I added some debug prints,
to see which states it goes through, and here's the end of the output:

descriptor
commit
descriptor
commit
descriptor
commit
descriptor
commit
descriptor
commit
descriptor
commit
descriptor
commit
revoke
descriptor
commit
num = 15628, last_num = 15627
grub/ System.map vmlinuz-2.6.26-rc4-wl.old vmlinuz
System.map-2.6.26-rc4-wl.old vmlinuz-2.6.26-rc4-wl
System.map-2.6.26-rc4-wl initrd-2.6.26-rc4-wl.img memtest86+-2.01 

The number of the blocks is quite large, but we are never hitting the
"default" case.  "revoke" wasn't present in subsequent runs.  The
numbers (num and last_num) keep growing every time by about 20.

I also tried to see what happens on the fshelp.c side.  So I applied
this patch:

diff --git a/fs/fshelp.c b/fs/fshelp.c
index faec1f7..027b579 100644
--- a/fs/fshelp.c
+++ b/fs/fshelp.c
@@ -329,12 +329,18 @@ grub_fshelp_map_block (grub_fshelp_journal_t log, 
grub_disk_addr_t block)
   if (map_block < 0)
     return block;
 
+  grub_printf("map_block = %ld, start_block = %ld, first_block = %ld, 
last_block = %d\n",
+             map_block, log->start_block, log->first_block, log->last_block);
   map_block += log->start_block;
   if (map_block >= log->last_block)
     map_block -= log->last_block - log->first_block;
 
   if (log->type == GRUB_FSHELP_JOURNAL_TYPE_BLOCK)
     return log->blkno + map_block;
-  else
-    return log->get_block (log->node, map_block);
+  else {
+    grub_disk_addr_t ret;
+    ret = log->get_block (log->node, map_block);
+    grub_printf("map_block = %ld, block = %ld, ret = %ld\n", map_block, block, 
ret);
+    return ret;
+  }
 }

That's what I get:

# ./grub-fstest /dev/sda1 ls /boot
map_block = 18988, start_block = 23124, first_block = 1, last_block = 32768
map_block = 9345, block = 1, ret = 10901
map_block = 18780, start_block = 23124, first_block = 1, last_block = 32768
map_block = 9137, block = 1027, ret = 10692
map_block = 18988, start_block = 23124, first_block = 1, last_block = 32768
map_block = 9345, block = 1, ret = 10901
map_block = 18780, start_block = 23124, first_block = 1, last_block = 32768
map_block = 9137, block = 1027, ret = 10692
map_block = 18988, start_block = 23124, first_block = 1, last_block = 32768
map_block = 9345, block = 1, ret = 10901

But if I change "return ret;" to "return block;" to ignore the lookup, I
get this:

# ./grub-fstest /dev/sda1 ls /boot
map_block = 19373, start_block = 23124, first_block = 1, last_block = 32768
map_block = 9730, block = 1, ret = 11286
map_block = 19209, start_block = 23124, first_block = 1, last_block = 32768
map_block = 9566, block = 1027, ret = 11122
map_block = 19373, start_block = 23124, first_block = 1, last_block = 32768
map_block = 9730, block = 1, ret = 11286
map_block = 19209, start_block = 23124, first_block = 1, last_block = 32768
map_block = 9566, block = 1027, ret = 11122
map_block = 19373, start_block = 23124, first_block = 1, last_block = 32768
map_block = 9730, block = 1, ret = 11286
map_block = 12763, start_block = 23124, first_block = 1, last_block = 32768
map_block = 3120, block = 15237122, ret = 4670
grub/ System.map vmlinuz-2.6.26-rc4-wl.old vmlinuz
System.map-2.6.26-rc4-wl.old vmlinuz-2.6.26-rc4-wl
System.map-2.6.26-rc4-wl initrd-2.6.26-rc4-wl.img memtest86+-2.01 

One thing that surprises me is that the value we are supposed to return
is always below 12000.

Then I created file /boot/foo, ran sync several times, and that's what I
get if "block" is returned:

# ./grub-fstest /dev/sda1 ls /boot
map_block = 19438, start_block = 23124, first_block = 1, last_block = 32768
map_block = 9795, block = 1, ret = 11351
map_block = 19630, start_block = 23124, first_block = 1, last_block = 32768
map_block = 9987, block = 1027, ret = 11543
map_block = 19438, start_block = 23124, first_block = 1, last_block = 32768
map_block = 9795, block = 1, ret = 11351
map_block = 19630, start_block = 23124, first_block = 1, last_block = 32768
map_block = 9987, block = 1027, ret = 11543
map_block = 19438, start_block = 23124, first_block = 1, last_block = 32768
map_block = 9795, block = 1, ret = 11351
map_block = 19627, start_block = 23124, first_block = 1, last_block = 32768
map_block = 9984, block = 15237122, ret = 11540
map_block = 19628, start_block = 23124, first_block = 1, last_block = 32768
map_block = 9985, block = 15239168, ret = 11541
map_block = 19628, start_block = 23124, first_block = 1, last_block = 32768
map_block = 9985, block = 15239168, ret = 11541
map_block = 19628, start_block = 23124, first_block = 1, last_block = 32768
map_block = 9985, block = 15239168, ret = 11541
map_block = 19628, start_block = 23124, first_block = 1, last_block = 32768
map_block = 9985, block = 15239168, ret = 11541
map_block = 19628, start_block = 23124, first_block = 1, last_block = 32768
map_block = 9985, block = 15239168, ret = 11541
map_block = 19628, start_block = 23124, first_block = 1, last_block = 32768
map_block = 9985, block = 15239168, ret = 11541
grub/ map_block = 19628, start_block = 23124, first_block = 1, last_block = 
32768
map_block = 9985, block = 15239168, ret = 11541
map_block = 19628, start_block = 23124, first_block = 1, last_block = 32768
map_block = 9985, block = 15239168, ret = 11541
System.map map_block = 19628, start_block = 23124, first_block = 1, last_block 
= 32768
map_block = 9985, block = 15239168, ret = 11541
map_block = 19628, start_block = 23124, first_block = 1, last_block = 32768
map_block = 9985, block = 15239168, ret = 11541
vmlinuz-2.6.26-rc4-wl.old map_block = 19628, start_block = 23124, first_block = 
1, last_block = 
32768
map_block = 9985, block = 15239168, ret = 11541
map_block = 19628, start_block = 23124, first_block = 1, last_block = 32768
map_block = 9985, block = 15239168, ret = 11541
vmlinuz map_block = 19628, start_block = 23124, first_block = 1, last_block = 
32768
map_block = 9985, block = 15239168, ret = 11541
map_block = 19628, start_block = 23124, first_block = 1, last_block = 32768
map_block = 9985, block = 15239168, ret = 11541
System.map-2.6.26-rc4-wl.old map_block = 19628, start_block = 23124, 
first_block = 1, last_block
 = 32768
map_block = 9985, block = 15239168, ret = 11541
map_block = 19628, start_block = 23124, first_block = 1, last_block = 32768
map_block = 9985, block = 15239168, ret = 11541
vmlinuz-2.6.26-rc4-wl map_block = 19628, start_block = 23124, first_block = 1, 
last_block = 3276
8
map_block = 9985, block = 15239168, ret = 11541
map_block = 19628, start_block = 23124, first_block = 1, last_block = 32768
map_block = 9985, block = 15239168, ret = 11541
System.map-2.6.26-rc4-wl map_block = 19628, start_block = 23124, first_block = 
1, last_block = 3
2768
map_block = 9985, block = 15239168, ret = 11541
map_block = 19628, start_block = 23124, first_block = 1, last_block = 32768
map_block = 9985, block = 15239168, ret = 11541
foo map_block = 19628, start_block = 23124, first_block = 1, last_block = 32768
map_block = 9985, block = 15239168, ret = 11541
map_block = 19628, start_block = 23124, first_block = 1, last_block = 32768
map_block = 9985, block = 15239168, ret = 11541
initrd-2.6.26-rc4-wl.img map_block = 19628, start_block = 23124, first_block = 
1, last_block = 3
2768
map_block = 9985, block = 15239168, ret = 11541
map_block = 19628, start_block = 23124, first_block = 1, last_block = 32768
map_block = 9985, block = 15239168, ret = 11541
memtest86+-2.01 

But I get this if "ret" is returned:

# ./grub-fstest /dev/sda1 ls /boot
map_block = 19438, start_block = 23124, first_block = 1, last_block = 32768
map_block = 9795, block = 1, ret = 11351
map_block = 19656, start_block = 23124, first_block = 1, last_block = 32768
map_block = 10013, block = 1027, ret = 11569
map_block = 19438, start_block = 23124, first_block = 1, last_block = 32768
map_block = 9795, block = 1, ret = 11351
map_block = 19656, start_block = 23124, first_block = 1, last_block = 32768
map_block = 10013, block = 1027, ret = 11569
map_block = 19438, start_block = 23124, first_block = 1, last_block = 32768
map_block = 9795, block = 1, ret = 11351

"ret" is still in the same region.

-- 
Regards,
Pavel Roskin