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