Page MenuHomeFreeBSD

Port HAMMER2 from DragonFly (read-only support)
AbandonedPublic

Authored by gbe on Nov 11 2022, 6:22 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, May 4, 12:38 AM
Unknown Object (File)
Sat, May 4, 12:38 AM
Unknown Object (File)
Sat, May 4, 12:25 AM
Unknown Object (File)
Mon, Apr 22, 10:17 AM
Unknown Object (File)
Sat, Apr 13, 12:45 AM
Unknown Object (File)
Feb 23 2024, 2:34 PM
Unknown Object (File)
Jan 11 2024, 5:26 AM
Unknown Object (File)
Dec 23 2023, 5:41 AM

Details

Reviewers
emaste
markj
mckusick
bcr
Group Reviewers
manpages
Summary

See the design document below for details of HAMMER2.
https://gitweb.dragonflybsd.org/dragonfly.git/blob/HEAD:/sys/vfs/hammer2/DESIGN

Note that clustering support mentioned in the design document is
unimplemented even in DragonFly, so this patch doesn't either. This
patch provides read-only local mount.

The patch consists of new code only, and it doesn't make any change
to the existing subsystems. New files are under BSDL except for Zlib.
It's been compiled and tested on amd64 and i386.

  • sys/fs/hammer2 - HAMMER2 file system. The code is rewritten based on DragonFly. Some files are mostly portable as they don't directly talk to VFS or GEOM, but others are not hence rewritten. Things only used by write support are mostly removed. Rewrite includes conforming to style(9) where possible.
  • sys/fs/hammer2/zlib - Zlib implementation used by HAMMER2. HAMMER2 can use Zlib for compression. FreeBSD has sys/contrib/zlib with seemingly newer version, but it can't simply be used as drop-in replacement due to symbol conflict with other part of the kernel.
  • sbin/hammer2 - HAMMER2 file system utility. Implements directives many of which invoke HAMMER2 ioctls. The code is basically same as DragonFly, except that write (creation) related directives, clustering related directives, and directives with assumption on DragonFly specific device name are all removed.
  • sbin/newfs_hammer2 - HAMMER2 newfs command. The code is basically same as DragonFly, but this command could be dropped if that's preferred. This command enables you to at least test mounting empty file system without using existing images.
  • sbin/mount_hammer2 - HAMMER2 mount command. All it does is invoke mount syscall, but the code is rewritten to use FreeBSD specific nmount(2) syscall.
  • sbin/fsck_hammer2 - A simple ondisk (hashed radix tree) checker. The code is basically same as DragonFly.

PR: 267687
Submitted by: kusumi.tomohiro@gmail.com

Test Plan

make buildworld buildkernel

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 48282
Build 45168: arc lint + arc unit

Event Timeline

gbe requested review of this revision.Nov 11 2022, 6:22 AM

A clarification request and a few grammar fixes. (Since this doesn't look like a vendor import, are local changes OK?)

sbin/fsck_hammer2/fsck_hammer2.8
56
sbin/hammer2/hammer2.8
68

Redundant with "At least one PFS on a HAMMER2 filesystem..." above, if I understand both correctly.

sbin/newfs_hammer2/newfs_hammer2.8
79

Is that also true of the .Fx port?

118
126
151
173
185

No mention of bringing it into FreeBSD?

While I appreciate the effort that went into this port, I have to ask, why is it useful? Is there a group of users wanting read-only HAMMER2 support? Is there some plan to implement write support in the future? A new filesystem imposes extra maintenance burden on VFS developers and on kernel developers generally, who have to respond to bug reports, static analysis reports, security bugs, etc.. And, some quality time in the bug tracker suggests that we are already stretched too thinly. So, there should be some commensurate benefit from having this driver in the base system. I'd be happy to have my skepticism proven wrong, but for that some evidence must be provided.

On a more mundane note, I believe @delphij recently did the work of consolidating the different copies of zlib we have in the tree into contrib/zlib; this adds a new copy. We should avoid adding new copies of zlib to the base system. There is a note about "symbol conflicts" in the base system but there's not enough detail to justify a new copy of zlib.

sbin/newfs_hammer2/newfs_hammer2.8
Is that also true of the .Fx port?

I didn't write or designed this, but it's likely "no larger than 64K" for both D and F now.

(I tried to comment inline, but can't get rid of "Unsubmitted" mark.)

sbin/newfs_hammer2/newfs_hammer2.8
79

I didn't write this, but it's probably now "no larger than 64K" for both D and F.

@markj wrote:

I believe @delphij recently did the work of consolidating the different copies of zlib we have in the tree into contrib/zlib; this adds a new copy. We should avoid adding new copies of zlib to the base system.

I concur. Adding alternative implementations of existing APIs should be a red flag, actually. For example, this kind of assertion could've probably prevented the first, substandard WireGuard implementation in D26137 from hitting the tree.

I generally agree with the comments by markj. As someone who helps to maintain VFS infrastructure, the addition of a new filesystem means one more place that changes need to be propagated. I do not see a lot of use cases for a read-only HAMMER filesystem. I would be much more willing to see it added if it has write ability added as FreeBSD users could then use it without need of having a Dragonfly system running. Another alternative would be to make this code available as a port which could be tried out by those wishing to use it. More importantly it would push off tracking changes to the VFS infrastructure to the port maintainer.

I would be much more willing to see it added if it has write ability added

Write support being hard requirement is understandable, but I don't have a plan to do it right now.

Another alternative would be to make this code available as a port which could be tried out by those wishing to use it.

If this is an option, could you elaborate ?

I'm not familiar with a ports port with kernel code involved.
I'm assuming "make install" builds a kernel module, but is there an example of that within the existing ports ?

I'm not familiar with a ports port with kernel code involved.
I'm assuming "make install" builds a kernel module, but is there an example of that within the existing ports ?

you may want to check some ports ending with -kmod for example sysutils/openzfs-kmod

(I tried to comment inline, but can't get rid of "Unsubmitted" mark.)

Inline comments are only submitted when (after submitting each) you also press the Submit button at the bottom.

I'm not familiar with a ports port with kernel code involved.
I'm assuming "make install" builds a kernel module, but is there an example of that within the existing ports ?

you may want to check some ports ending with -kmod for example sysutils/openzfs-kmod

https://docs.freebsd.org/en/books/porters-handbook/special/#requiring-kernel-sources is also relevant.

Another alternative would be to make this code available as a port which could be tried out by those wishing to use it. More importantly it would push off tracking changes to the VFS infrastructure to the port maintainer.

Thanks for suggestions for those who replied regarding ports.

Created a ports port (sysutils/hammer2) PR.
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=267982

bcr added a subscriber: bcr.

There is now a port committed as sysutils/hammer2, based on work done in https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=267982 .
I think we can safely close this review.

There is now a port sysutils/hammer2 available.