[CFT] BeFS filesystem 0.6

[CFT] BeFS filesystem 0.6

[Will Dyson]

Hi,

For some months now, I have been working on a read-only implementation 
of the BeOS's native filesystem (BeFS). I just released version 0.6, and 
I think it works pretty darn well. <http://befs-driver.sourceforge.net/>

I've had some reports of it working well on other peoples' machines as 
well. So what I'd really like now is for more people to try and break 
it. If you have a Be filesystem partition, please try it out. If you 
don't (but still feel like testing anyway), the download page has a 
filesystem image you can try it out on.

I'd also appreciate anyone who wants to critique the code itself. Thanks!

-- 
Will Dyson

Re: [CFT] BeFS filesystem 0.6

[Alexander Viro]

On Mon, 24 Dec 2001, Will Dyson wrote:

> Hi,
> 
> For some months now, I have been working on a read-only implementation 
> of the BeOS's native filesystem (BeFS). I just released version 0.6, and 
> I think it works pretty darn well. <http://befs-driver.sourceforge.net/>
> 
> I've had some reports of it working well on other peoples' machines as 
> well. So what I'd really like now is for more people to try and break 
> it. If you have a Be filesystem partition, please try it out. If you 
> don't (but still feel like testing anyway), the download page has a 
> filesystem image you can try it out on.
> 
> I'd also appreciate anyone who wants to critique the code itself. Thanks!

Umm...  Obvious comments:

a) typedef struct super_block vfs_sb;  Please don't.

b) in inode.c:
	inode->i_mode = (umode_t) raw_inode->mode;
is wrong.  It's guaranteed bug either on big- or little-endian boxen.
Same for mtime, uid and gid.  befs_count_blocks() also needs cleanup.

c) befs_read_block()...  Erm.  Why bother with extra layer of abstraction?

d) befs_readdir().  You _are_ guaranteed that inode is non-NULL, you put
pointer to befs_dir_operations only is S_ISDIR is true and S_ISDIR is
mutually exclusive with S_ISLNK.

e) are there sparse files on BeFS?  If yes - you want to make BH_Mapped
conditional in get_block() (set it if block is present, don't if it's a
hole in file).

f) befs_arch().  You probably want to make that an option...

g) endianness problems in read_super()...

h) TODO: use line breaks ;-)

Re: [CFT] BeFS filesystem 0.6

[Will Dyson]

Alexander Viro wrote:

 
> Umm...  Obvious comments:
> 
> a) typedef struct super_block vfs_sb;  Please don't.
> 
> b) in inode.c:
> 	inode->i_mode = (umode_t) raw_inode->mode;
> is wrong.  It's guaranteed bug either on big- or little-endian boxen.
> Same for mtime, uid and gid.  befs_count_blocks() also needs cleanup.
> 
> c) befs_read_block()...  Erm.  Why bother with extra layer of abstraction?
> 
> d) befs_readdir().  You _are_ guaranteed that inode is non-NULL, you put
> pointer to befs_dir_operations only is S_ISDIR is true and S_ISDIR is
> mutually exclusive with S_ISLNK.
> 
> e) are there sparse files on BeFS?  If yes - you want to make BH_Mapped
> conditional in get_block() (set it if block is present, don't if it's a
> hole in file).
> 
> f) befs_arch().  You probably want to make that an option...
> 
> g) endianness problems in read_super()...
> 
> h) TODO: use line breaks ;-)



Hi Al. Thanks for taking the time to respond.


A & B) I originaly had plans to make the driver work under a variety of 
different VFSs (maybe freebsd and atheos) and to have user-space tools 
(like the old mtools for DOS) for people on other OSs. That was the 
motivation behind those extra abstractions, but I've actually kinda lost 
interest in the idea. I'll just take 'em out if it really offends 
people's asthetic sense. Perhaps kernel drivers just weren't meant to be 
portable.

B) The PPC and x86 versions of BeOS each write their filesystem metadata 
in native byte-order. So this would only be a problem with disks that 
were formatted on a big-endian system and read on a little-endian (or 
the other way around, of course). I've been meaning to handle that case 
"someday".

D) Ok. I killed that test.

E) To my knowledge, there are not sparse files on BeFS.

F) Ideally, I would detect both the endianness of the filesystem and the 
host, and refuse to mount if they differ (or byte-swap the metadata). 
Can I rely on #ifdef __LITTLE_ENDIAN to detect the endianness I am 
running on?

G) Same thing as B, I think.

H) Yeah. I despise editors that won't line-wrap for you. But this is 
unix, so I guess I gotta accomodate them. Is makeing sure they don't 
overflow 80 columns enough? Or should that be 78?

 > befs_count_blocks() also needs cleanup.

How so, please?

-- 
Will Dyson

Re: [CFT] BeFS filesystem 0.6

[Alexander Viro]

On Tue, 25 Dec 2001, Will Dyson wrote:

> A & B) I originaly had plans to make the driver work under a variety of 
> different VFSs (maybe freebsd and atheos) and to have user-space tools 
> (like the old mtools for DOS) for people on other OSs. That was the 
> motivation behind those extra abstractions, but I've actually kinda lost 
> interest in the idea. I'll just take 'em out if it really offends 
> people's asthetic sense. Perhaps kernel drivers just weren't meant to be 
> portable.

Well, if you want portable - do it in userland and use CODA...
 
> B) The PPC and x86 versions of BeOS each write their filesystem metadata 
> in native byte-order. So this would only be a problem with disks that 
> were formatted on a big-endian system and read on a little-endian (or 
> the other way around, of course). I've been meaning to handle that case 
> "someday".

Better do it early, while amount of code is not too large.  Endian-clean
code is easy.  The rule is: never treat any "integer" types from disk as
assignment-compatible with int.  Always use something like fs32_to_cpu(),
etc. - it's cleaner that way and inlined functions actually work fine.

> F) Ideally, I would detect both the endianness of the filesystem and the 
> host, and refuse to mount if they differ (or byte-swap the metadata). 
> Can I rely on #ifdef __LITTLE_ENDIAN to detect the endianness I am 
> running on?

See above (and check how sysvfs does it - there it's even more obvious,
since you have to deal with _3_ variants of on-disk bytesex (big-endian,
little-endian and NUXI).

>  > befs_count_blocks() also needs cleanup.
> 
> How so, please?

Same thing - endianness issues.