Re: [PATCH V2] mkfs: handle 4k sector devices more cleanly

[Christoph Hellwig <hch@infradead.org>]

On Fri, Jan 08, 2010 at 08:24:00PM -0600, Eric Sandeen wrote:
> Christoph Hellwig wrote:
> > On Fri, Jan 08, 2010 at 10:46:41AM -0600, Eric Sandeen wrote:
> >> +extern void platform_findsizes (char *path, int fd, long long *sz, int *bsz);
> > 
> > Can you move the prototype from libxfs/init.h to include/libxfs.h
> > instead of adding it to the .c file?
> 
> actually it seems a little out of place in libxfs/init.h; that probably
> works but there are no other platform_* functions there...
> 
> Should this be more like an xfs_findsizes in libxfs, which
> calls platform_findsizes internally just for consistency? *shrug*

I'd just move the header around to keep it simple for now.

> The only way this is used is: it's sent to libxfs_init and then from there
> only to libxfs_open which does:
> 
>         if (!readonly && setblksize && (statb.st_mode & S_IFMT) == S_IFBLK) {
>                 if (setblksize == 1)
>                         /* use the default blocksize */
>                         (void)platform_set_blocksize(fd, path, statb.st_rdev, XFS_MIN_SECTORSIZE, 0);
>                 else {
>                         /* given an explicit blocksize to use */
>                         if (platform_set_blocksize(fd, path, statb.st_rdev, setblksize, 1))
>                             exit(1);
>                 }
>         }
> 
> so "1" seems to have the special meaning of "use XFS_MIN_SECTORSIZE"
> 
> Hm, ok but in my patch setblksize is 0, so it's not getting set.
> I suppose this -might- mess up other utils ...

Yes, now we do not set any size, while previously we always set
XFS_MIN_SECTORSIZE.  I don't think this actually changes anything - the
kernel doesn't make use of this blocksize information outside of
filesystems, and we already reset it on mount.

> > Maye we should just do the xi.setblksize = sectorsize unconditionally?
> 
> I think that's best.  It's already defaulted to XFS_MIN_SECTORSIZE
> so should be no functional change if it doesn't get otherwise set - although
> I think it -does- get set in all cases now - either from ft.sectoralign/blocksize,
> from the explicit -s setting, or the ft.sectorsize by default.
> 
> What do you think about removing the "1" magic if so?  At that point I think
> nothing relies on it.

repair uses it in a weird way.

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs