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*
>> + /*
>> + * MD wants sector size set == block size to avoid switching.
>> + * Otherwise, if not specfied via command, use device sectorsize
>> + */
>> + if (ft.sectoralign || !ssflag) {
>> + if (ft.sectoralign)
>> + sectorsize = blocksize;
>> + else
>> + sectorsize = ft.sectorsize;
>
> This still confuses the heck out of me. What do you think about the
> incremental patch at the end of the mail?
>
>> if (slflag || ssflag)
>> xi.setblksize = sectorsize;
>> - else
>> - xi.setblksize = 1;
>
> So for the defaul case we now never set the sector size in the libxfs
> init. This seems safe to me, but why did we do it before? Could
> a previous user have left it set to a wrong value?
ok so I read this wrong on my previous reply I guess.
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 ...
> 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.
Thanks,
-Eric
|