Great, I would like to sponsor this request.  I will proceed...

Thanks,
Lin

Juergen Keil wrote:
> Lin,
>
> Ok, so I'm sending a copy of this reply to request-sponsor at opensolaris.org,
> to request a sponsor for 6541114 :-)
>
> My contributor agreement # : OS0003
>
>
>
>   
>> Date: Tue, 17 Jul 2007 09:19:14 -0700
>> From: Lin Ling <lin.ling at sun.com>
>> To: Juergen Keil <jk at tools.de>
>> Subject: Re: 6541114: GRUB/ZFS fails to load files from a default compressed 
>> (lzjb) 
>>     
> root
>   
>> Hi Juergen,
>>
>> This is great!  I have not had a chance to find the root cause for this 
>> bug yet. Thanks!
>>
>> So let's proceed with the contributor/sponsor process so that I can put your
>> fix into the gate, sounds ok with you?
>>
>> Thanks,
>> Lin
>>
>> Juergen Keil wrote:
>>     
>>> Hi,
>>>
>>> In the opensolaris bug database I saw that you're the responsible engineer
>>> for bug 6541114: GRUB/ZFS fails to load files from a default
>>> compressed (lzjb) root
>>>
>>> Did you already work on that bug?
>>>
>>> In the last couple of days I've been experimenting with zfs boot / zfs root,
>>> and I've also run into the grub "Error 16: Inconsistent filesystem 
>>> structure"
>>> problem, and I think I understand the problem (zfs files with holes - not 
>>> yet
>>> supported by grub's zfs support) and I do have a fix (see the attachment).
>>>
>>>   
>>> ------------------------------------------------------------------------
>>>
>>> Synopsis: zfs boot needs support for loading files containing holes
>>>
>>>
>>> zfs boot does not work for me; while loading the boot_archive from 
>>> an lzjb compressed zfs bootfs, grub fails with the following message:
>>>
>>>   Error 16: Inconsistent filesystem structure
>>>
>>>
>>> (I found a bug-id that mentioned this error when default compressed zfs
>>> filesystem are used, so at some point I made experiments with uncompressed
>>> zfs but that failed with the same error message)
>>>
>>>
>>> Adding printf debugging to grub's code, I found out that it fails
>>> in fsys_zfs.c, function zio_read(), with these parameters:
>>>
>>>     bp 0x1a0000, buf 0x180000, stack 0x1a4080
>>>     psize 512 lsize 0 comp 0 cksum 0
>>>
>>> The loop inside zio_read will use 3x continue, because
>>> "bp->blk_dva[i].dva_word[0] == 0 && bp->blk_dva[i].dva_word[1] == 0":
>>>
>>>     /* pick a good dva from the block pointer */
>>>     for (i = 0; i < SPA_DVAS_PER_BP; i++) {
>>>
>>>             if (bp->blk_dva[i].dva_word[0] == 0 &&
>>>                 bp->blk_dva[i].dva_word[1] == 0)
>>>                     continue;
>>>
>>>             /* read in a block */
>>>                 ...
>>>         }
>>>     return (ERR_FSYS_CORRUPT);
>>>
>>>
>>> Why is lsize 0? And comp 0 (=ZIO_COMPRESS_INHERIT)?  And cksum 0
>>> (=ZIO_CHECKSUM_INHERIT)? There are a lot of successful zio_read()
>>> calls before the failed one, any they all use lsize > 0, comp == 3
>>> (ZIO_COMPRESS_LZJB) and cksum 6 or 7 (ZIO_CHECKSUM_FLETCHER_2 /
>>> ZIO_CHECKSUM_FLETCHER_4).
>>>
>>> Is this a "read" asking for a zero sized buffer?
>>>
>>>     ------------------------------------------------------------------------
>>>
>>> WRONG IDEA:
>>> ===========
>>>
>>> Adding more printfs to grub... This reveals that we're apparently
>>> failing inside a zfs_read(), called with parameter "len == 101895168".
>>> This length looks suspicous.  Might be byte swapped?
>>>
>>> % adb
>>> 0t101895168=X
>>>                 612cc00         
>>> cc1206=D
>>>                 13373958    
>>>
>>> Byte swapping the length gives us 13373958 bytes, which might be a
>>> reasonable size of the compressed or uncompressed boot_archive file...
>>>
>>> (Same issue with the 32-bit boot archive: we get a filemax of
>>> 62959616; which is 11583491 when we bswap it).
>>>
>>>
>>> So, where does that len parameter come from?  Adding prints to zfs_open
>>> shows that "DN_BONUS(DNODE))->zp_size" gives us a filemax size of
>>> 101895168 for file /platform/i86pc/amd64/boot_archive:
>>>
>>>
>>>     /* get the file size and set the file position to 0 */
>>>     filemax = ((znode_phys_t *)DN_BONUS(DNODE))->zp_size;
>>>     filepos = 0;
>>>     grub_printf("in %s, filemax for %s: %d\n",
>>>         __func__, filename, filemax);
>>>
>>>
>>> Checking the real boot_archive sizes on the compressed zfs bootfs:
>>> /platform/i86pc/boot_archive:  62959616 bytes, du -sh 15M
>>> /platform/i86pc/amd64/boot_archive: 101895168 bytes, du -sh 17M.
>>>
>>> So, the sizes weren't suspicious;  I was experimenting with a 
>>> Solaris "core install", which doesn't have /bin/mkisofs installed.
>>> In this case create_ramdisk creates an ufs format boot_archive, which
>>> is *much* bigger than an hsfs format boot_archive.
>>>
>>>     ------------------------------------------------------------------------
>>>
>>> Next idea:  we're failing because the boot_archive image file has holes?
>>>
>>> That appears to be the case.
>>>
>>> Suggested fix:
>>> ==============
>>>
>>> zfs boot needs support for files with holes.
>>>
>>> diff -r 2b2bb89445af usr/src/grub/grub-0.95/stage2/fsys_zfs.c
>>> --- a/usr/src/grub/grub-0.95/stage2/fsys_zfs.c      Fri Jul 13 09:59:54 
>>> 2007 -0700
>>> +++ b/usr/src/grub/grub-0.95/stage2/fsys_zfs.c      Sat Jul 14 16:07:12 
>>> 2007 +0200
>>> @@ -380,8 +380,15 @@ dmu_read(dnode_phys_t *dn, uint64_t blki
>>>             *bp = bp_array[idx];
>>>             if (level == 0)
>>>                     tmpbuf = buf;
>>> -           if (errnum = zio_read(bp, tmpbuf, stack))
>>> -                   return (errnum);
>>> +           if (BP_IS_HOLE(bp)) {
>>> +                   int blocksize =
>>> +                       level ? 1 << dn->dn_indblkshift
>>> +                       : dn->dn_datablkszsec << SPA_MINBLOCKSHIFT;
>>> +                   grub_memset(tmpbuf, 0, blocksize);
>>> +           } else {
>>> +                   if (errnum = zio_read(bp, tmpbuf, stack))
>>> +                           return (errnum);
>>> +           }
>>>  
>>>             bp_array = tmpbuf;
>>>     }
>>>       
>
>   

Reply via email to