Page MenuHomeFreeBSD

add virtiofs kernel module
Needs ReviewPublic

Authored by emil_etsalapatis.com on Aug 14 2024, 3:55 PM.
Tags
None
Referenced Files
F163065796: D46296.id156287.diff
Sun, Jul 19, 6:48 PM
F163003959: D46296.diff
Sun, Jul 19, 2:33 AM
F163001433: D46296.id142083.diff
Sun, Jul 19, 1:58 AM
Unknown Object (File)
Sat, Jul 18, 7:45 PM
Unknown Object (File)
Sat, Jul 18, 4:54 PM
Unknown Object (File)
Sat, Jul 18, 3:09 AM
Unknown Object (File)
Thu, Jul 16, 6:52 PM
Unknown Object (File)
Mon, Jul 13, 3:41 PM

Details

Summary

Add a virtiofs file system based on FUSE to interface with the virtio fs device. The file system communicates to a FUSE server on the host through this device.

Diff Detail

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

Event Timeline

I'm still a little shaky on the details of virtio-fs, but I can't find anything obviously wrong here.

sys/fs/fuse/virtiofs_vfsops.c
5

I think we're not supposed to use "All rights reserved" in new files.

This revision is now accepted and ready to land.Aug 21 2024, 9:01 PM

Thanks a lot for working on this!

sys/fs/fuse/virtiofs_vfsops.c
419

Can we use the "from" for the tag like p9fs does? It's a bit strange to have an unused arbitrary string as the "from", and I haven't managed to get root mount to work even though it seemingly is passing the options…

sys/fs/fuse/virtiofs_vfsops.c
393

The "strict access policy" is only breaking things for virtiofs, i.e. if you mount a home directory for a user, that user wouldn't be able to access it.

sys/fs/fuse/virtiofs_vfsops.c
419

ah, re: mountroot, virtiofs needs to be added to vfs_mountroot_wait_if_neccessary (ugh why don't we have proper registered flags for this)… And then, remount support is expected because it mounts as RO first.

sys/fs/fuse/virtiofs_vfsops.c
419

Actually that whole routine is bogus, as is waiting for the dev under the mountpoint. I've written it locally, but it was on a laptop that decided to die suddenly for reasons I've not been able to diagnose.

sys/fs/fuse/virtiofs_vfsops.c
375

You also have to *set* the size here with fiov_adjust like the local FUSE transport does when it copies data from userspace in fticket_aw_pull_uio right after fuse_body_audit.

Took me a few hours to debug why getdirentries was completely busted (: It relies on the buffer size to stop iteration; without fiov_adjust the buffer size was always 4096 so it would iterate past the actual message size, hitting a wall of zeroes, interpreting that as an entry with a 0-length name and bailing with EINVAL.

With that fixed, virtiofs is already looking a lot better than p9fs. Namely, I could install a package with pkg under a virtiofs root, while under p9fs I'm hitting rather strange errors. Reusing the FUSE protocol was a really clever idea!

Thanks for the feedback! I will update the diff accordingly.

sys/fs/fuse/virtiofs_vfsops.c
375

Great catch! I think this also fixes a bug I had been trying to triage for write back mode when heavily loading the VM. I will update and fix.

419

Sounds good, just to make sure I've gotten it right: Changing the VFS option from "tag" to "from" should bring virtiofs mount options in line w/ other file systems.

419

Re: remount, I can add it to this diff if it helps with testing. I omitted it to keep this diff set as small as possible, because it is already pretty large.

sys/fs/fuse/virtiofs_vfsops.c
124

This just causes all buffers to leak.

A couple more notes from testing:

There's some other place where FUSE layer can pick up zero-padded-to-power-of-two things, now with 1024 instead of 4096:

WARNING: FUSE protocol violation for server mounted at /: Returned an embedded NUL from FUSE_READLINK.  This warning will not be repeated.
Len 1024
0000   62 61 73 68 00 00 00 00 00 00 00 00 00 00 00 00  |bash............|
0010   00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  |................|
…

And trying to just find something in a src tree checkout, there are sporadic errors like

find: ./contrib/llvm-project/llvm/lib/DebugInfo/Symbolize: Bad file descriptor
find: ./sys/dev/qat/qat_hw: Invalid argument
find: ./sys/dev/qat/qat_api/common/stubs: Invalid argument
// etc…

that seem completely random, on one invocation there are none, on another there's a couple…

sys/fs/fuse/virtiofs_vfsops.c
419

yes, the "from" is the 'magic' one that's the source argument in the mount command that you have to pass anyway

472

"virtiofs.virtio" (the last fs gets cut off) is kinda redundant, so this isn't necessary

A couple more notes from testing:

There's some other place where FUSE layer can pick up zero-padded-to-power-of-two things, now with 1024 instead of 4096:

WARNING: FUSE protocol violation for server mounted at /: Returned an embedded NUL from FUSE_READLINK.  This warning will not be repeated.
Len 1024
0000   62 61 73 68 00 00 00 00 00 00 00 00 00 00 00 00  |bash............|
0010   00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  |................|
…

And trying to just find something in a src tree checkout, there are sporadic errors like

find: ./contrib/llvm-project/llvm/lib/DebugInfo/Symbolize: Bad file descriptor
find: ./sys/dev/qat/qat_hw: Invalid argument
find: ./sys/dev/qat/qat_api/common/stubs: Invalid argument
// etc…

that seem completely random, on one invocation there are none, on another there's a couple…

Thank you for the testing! Are you using writethrough or writeback mode for FUSE? Also, what command line arguments are you using for virtiofsd on the host side?

A couple more notes from testing:

There's some other place where FUSE layer can pick up zero-padded-to-power-of-two things, now with 1024 instead of 4096:

WARNING: FUSE protocol violation for server mounted at /: Returned an embedded NUL from FUSE_READLINK.  This warning will not be repeated.
Len 1024
0000   62 61 73 68 00 00 00 00 00 00 00 00 00 00 00 00  |bash............|
0010   00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  |................|
…

And trying to just find something in a src tree checkout, there are sporadic errors like

find: ./contrib/llvm-project/llvm/lib/DebugInfo/Symbolize: Bad file descriptor
find: ./sys/dev/qat/qat_hw: Invalid argument
find: ./sys/dev/qat/qat_api/common/stubs: Invalid argument
// etc…

that seem completely random, on one invocation there are none, on another there's a couple…

Thank you for the testing! Are you using writethrough or writeback mode for FUSE? Also, what command line arguments are you using for virtiofsd on the host side?

Never adjusted any parameters on the client side. Adding --writeback to virtiofsd results in panic: FUSE: ticket still on answer delivery list.
I was using --inode-file-handles=prefer --allow-direct-io --allow-mmap mostly. But changing flags doesn't make the random errors go away.

(not surprising as write caching stuff shouldn't affect running find a bunch of times)

Could someone provide some simple scripts for bringing up and testing a share?

Could someone provide some simple scripts for bringing up and testing a share?

Will do, I will also update the diff with Val's feedback. Sorry about that, I have had no downtime since September to work on this.

I assume a script that spins up a VM and does reads/writes should be enough? Running the FUSE tests with this as originally planned would require D45370, but that feature is still very much a WIP.

  • virtiofs: rename 'tag' VFS option to 'from'
  • address feedback
This revision now requires review to proceed.Jan 13 2025, 4:41 AM

There are two outstanding bugs that need fixing before this can be merged:

  • Handling FUSE_DESTROY on unmount is incorrect. The virtiofsd client returns -ENOSYS and hangs the unmount operation.
  • Reference counting for the FUSE messages is incorrect. The refcount_acquire() in line 123 does cause buffers to leak, but removing it causes read/write errors so I need to check the reference counting throughout the code.
  • virtiofs: turn off FUSE_DESTROY for virtiofs
  • virtiofs: rename 'tag' VFS option to 'from'
  • fs/fuse/virtiofs: adjust FUSE flags and disallow fsync
  • sys/fs/fuse: disallow FUSE_INTERRUPT for virtiofs

Just a note that KDE rolls out a new CI infrastructure and they would greatly benefit from the virtiofs support.

adjust ms queue usage, ticket response, and error handling/style bugfixes

upload actual diff (previous update accidentally added different diff)

fix copy-paste job for SPDX License

Hi Emil — thanks for this, I'm keen to get virtio-fs working on FreeBSD!! Anything I can do to help, please let me know!

I've been trying to build and test D46296 on an unusual target: a FreeBSD/arm64 guest under Apple's Virtualization.framework (macOS host), which shares host directories exclusively over virtio-fs — so this driver is exactly what I'm very keen to see working here! I rebased D46296 onto current main and tried to build the virtiofs module, and ran into a couple of things that I think currently block building/testing it standalone:

  1. The virtio-fs transport isn't included, and the glue needs a newer one than what's public. virtiofs_vfsops.c does #include <dev/virtio/fs/virtio_fs.h>, but sys/dev/virtio/fs/ isn't part of this revision, and it isn't in main either (only sys/dev/virtio/p9fs is). I couldn't find a companion review that adds it. I tried grafting the transport from your virtiofs-head branch, but the interface has moved on since then — virtiofs_vfsops.c calls:

    vtfs_register_cb(vtfs, forget_ticket, complete_ticket, cancel_ticket, teardown, data); /* which is 6 args */

while the published vtfs_register_cb() takes 5 (no cancel_ticket callback). So the matching transport seems to be newer than anything that's public.

Is sys/dev/virtio/fs/ going up as a separate review? Is it available somewhere and I just missed it? (apologies if that's the case) If the version that matches this revision was pushed, I think it'd let reviewers (and (being selfish) me) actually build and exercise the whole stack.

  1. One other minor trouble, on rebase to current main: FSESS_VIRTIOFS 0x20000000 now collides — main added FSESS_WARN_LSEXTATTR_NUL 0x20000000 since this diff's base, so it needs the next free bit (0x40000000). Maybe this is all related.

One small build-infra note in case it might help someone else: building the module standalone needs SRCS+= vnode_if.h in sys/modules/virtiofs/Makefile (otherwise vnode.h's #include "vnode_if.h" isn't generated).

Once the transport is available I would be very happy to build and exercise the full stack on arm64 under Apple VZ and report back — it might be some platform coverage that's a bit different. Thanks again for the work on this!

Hi Emil — thanks for this, I'm keen to get virtio-fs working on FreeBSD!! Anything I can do to help, please let me know!

I've been trying to build and test D46296 on an unusual target: a FreeBSD/arm64 guest under Apple's Virtualization.framework (macOS host), which shares host directories exclusively over virtio-fs — so this driver is exactly what I'm very keen to see working here! I rebased D46296 onto current main and tried to build the virtiofs module, and ran into a couple of things that I think currently block building/testing it standalone:

Hi Stefan! Testing the patchset would be greatly helpful, thanks you for trying it out. I have been testing exclusively on an x86 Linux host so a macOS host is a great smoke test for it.

For testing, can you try out cherry-picking the patches from my tree here ? The tree has four patches that correspond to Diffs D46295, D46296, D55047, and D55046. The diffs add the virtio device and file system, and add two fixes to the FUSE code. D55046 is abandoned because it wasn't addressing the problem to its full extent. Still, please include it to avoid cache inconsistencies.

  1. The virtio-fs transport isn't included, and the glue needs a newer one than what's public. virtiofs_vfsops.c does #include <dev/virtio/fs/virtio_fs.h>, but sys/dev/virtio/fs/ isn't part of this revision, and it isn't in main either (only sys/dev/virtio/p9fs is). I couldn't find a companion review that adds it. I tried grafting the transport from your virtiofs-head branch, but the interface has moved on since then — virtiofs_vfsops.c calls:

    vtfs_register_cb(vtfs, forget_ticket, complete_ticket, cancel_ticket, teardown, data); /* which is 6 args */

while the published vtfs_register_cb() takes 5 (no cancel_ticket callback). So the matching transport seems to be newer than anything that's public.

I think the missing diff is D46295. The virtiofs-head branch is badly named and is behind virtiofs. The virtiofs branch should have the most up-to-date code.

Is sys/dev/virtio/fs/ going up as a separate review? Is it available somewhere and I just missed it? (apologies if that's the case) If the version that matches this revision was pushed, I think it'd let reviewers (and (being selfish) me) actually build and exercise the whole stack.

  1. One other minor trouble, on rebase to current main: FSESS_VIRTIOFS 0x20000000 now collides — main added FSESS_WARN_LSEXTATTR_NUL 0x20000000 since this diff's base, so it needs the next free bit (0x40000000). Maybe this is all related.

Thank you for the heads up! I will rebase to avoid the collision.

One small build-infra note in case it might help someone else: building the module standalone needs SRCS+= vnode_if.h in sys/modules/virtiofs/Makefile (otherwise vnode.h's #include "vnode_if.h" isn't generated).

Once the transport is available I would be very happy to build and exercise the full stack on arm64 under Apple VZ and report back — it might be some platform coverage that's a bit different. Thanks again for the work on this!

Thank you! I appreciate the kind words, if I can clarify anything further or there is any other issue please let me know, I'd be happy to help.

Thanks Emil — this is what I was looking for. I just built from the virtiofs branch and got the full stack compiling on main: fusefs.ko, the vtfs transport, and virtiofs.ko all build cleanly. The only needed fix is SRCS+= vnode_if.h in sys/modules/virtiofs/Makefile, so the standalone module build emits the generated header before compiling virtiofs_vfsops.c.

I also confirmed the target itself works: a FreeBSD 16.0-CURRENT/arm64 snapshot boots fine under Apple's Virtualization.framework, and VZ exposes both the virtio-fs device (0x105a, sitting unclaimed) and a vsock device — so everything's in place for a runtime test.

Where I got stuck is the branch's age. virtiofs is based on a main that's roughly 3,700 commits behind current -CURRENT, so the modules are KBI-incompatible with a current kernel — kldload panics. I tried rebasing the driver onto a current snapshot to get a matched build, but it conflicts in fuse_internal_cache_attrs(): main now takes CACHED_ATTR_LOCK(vp) there, which overlaps your "ensure called with the exclusive vnode lock" change (0aeffd6). That's a locking-semantics call I didn't want to start taking guesses at.

Just for the sake of clarity, this is a separate (and rather more demanding) request than the FSESS_VIRTIOFS bit you offered to fix in your post above. The blocker for testing is the branch's base being about 3700 commits behind today's -CURRENT: nothing built from it will load on a current kernel. Would it be possible for you to bring the whole virtiofs branch up onto a recent main? I think I could then finish off an arm64/VZ test fairly quickly — I've already got a snapshot that boots under VZ and the build working, so it'd just be loading the updated modules and mounting a share. Happy to report back what I find. Thanks again for the pointers!

Hi Emil, I've been pondering this a bit in the last few weeks, trying to learn a bit as I'm going along, and as is probably fairly obvious, with the help of a LLM. I feel like I've made a bit of progress:

I was able to rebase the virtiofs branch (base + 4 commits: D46295 transport, the virtiofs module, and the two fuse fixes) onto current main (snapshot 20260616, 3b69c7201fd0) on arm64. The rebased tree is here if it's useful to pull: https://codeberg.org/brechanbech/freebsd-src/src/branch/virtiofs-rebase-20260616 All three modules — patched fusefs, vtfs (transport), and virtiofs (glue) — build clean against current main. A few things came out of the rebase that I will try and re-visit, unless you (or someone) can provide me with some guidance:

  1. fuse: Ensure fuse_internal_cache_attrs() is called with exclusive lock (0aeffd6) is superseded by upstream. main reworked exactly this area in 7e68af7ce2c1 "fusefs: redo vnode attribute locking", which introduces a dedicated CACHED_ATTR_LOCK (fuse_internal_cache_attrs() now asserts ASSERT_CACHED_ATTRS_LOCKED, not the vnode lock). Carrying 0aeffd6 on top would re-introduce the exclusive-vnode-lock approach the rework removed. Since the commit touches only fuse_internal.c and the virtiofs driver never calls fuse_internal_cache_attrs() directly (it reaches attr-caching only via the shared fuse_internal_do_getattr, which 7e68af7 already locks), I dropped 0aeffd6 in the rebase with no effect on virtiofs. I would tentatively suggest dropping it upstream too.
  1. fuse: Do not unconditionally overwrite local vnode size with remote (79d7a88) still looks necessary — unlike 0aeffd6 I kept this commit in the rebase, because the bug it fixes appears unfixed in current main. In current fuse_internal_cache_attrs() the setsize block (if (vnode_isreg(vp) && attr->size != fvdat->cached_attrs.va_size)) is still unguarded and vp_cache_at->va_size = attr->size is unconditional, so a FUSE_GETATTR racing a local write with FN_SIZECHANGE set can still clobber the local size and truncate data — and this affects fusefs in general, not just virtiofs. I re-applied its two substantive hunks on top of the new locking (dropping its dependence on 0aeffd6's elock_vnode/ASSERT_VOP_ELOCKED).
  1. FSESS_VIRTIOFS bit collides on current main. The branch defines FSESS_VIRTIOFS 0x20000000, but main now uses 0x20000000 for FSESS_WARN_LSEXTATTR_NUL (fuse_ipc.h). 0x40000000 is just the next free bit.
  1. Standalone module builds on current main need vnode_if.h. Both sys/modules/virtiofs/Makefile and sys/modules/virtio/fs/Makefile need SRCS+= vnode_if.h to build with cd sys/modules/<m> && make — the sources pull in sys/vnode.h (via sys/mount.h), which on current main #includes the generated header. (The transport newly needs this after moving to current main, where vnode.h began including vnode_if.h.)

Runtime status — arm64 under Apple Virtualization.framework: not yet completed, this is where I stopped because I couldn't get this to work. With the rebase built, I couldn't get a clean runtime test, but I'm confident the blocker is the base system on the -CURRENT snapshot under VZ, not virtiofs — it never reaches module load or mount:

  • The boot deadlocks in the IPv6 router-advertisement path: a lock-order reversal, _sx_xlock (sleepable) while holding the vtnet0-rx0 mutex (non-sleepable), in nd6_ra_input -> in6_ifadd -> in6_joingroup (in6_mcast.c). On a debug (WITNESS/INVARIANTS) GENERIC kernel this wedges the box rather than just warning.
  • Disabling IPv6 on the interface removes that reversal, but boot then still hangs after the network stage, before /etc/rc.local.

I'm really not sure what the issue is here, but I think I'm right in saying that both are independent of this change (stock net stack + VZ virtio-net); so the arm64/VZ runtime validation is still open on my end. If it's useful I can file the IPv6-RA LOR separately with the full trace. Happy to retry the virtiofs runtime test once I have a -CURRENT image that boots cleanly under VZ (or on a release kernel).

Note the transport (D46295) is required to build any of this so the two reviews need to kind of jibe with each other.

Just following up on the comment above. It looks like any issue with running was fixed by getting the latest snapshot, and doesn’t look like it was any issue with virtiofs — the driver runs fine on current main.

On a current snapshot (2026-07-13, main-n287406-60382b4a04fa) it’s possible to re-base the driver and run the full path on arm64 under VZ: all three modules load (patched fusefs, the vtfs transport, and virtiofs), the device attaches, mount -t virtiofs <tag> /mnt returns 0, and bidirectional file I/O works — it’s now possible to read a file the host had placed on the share and write one back that shows up host-side. So it looks like the driver builds and runs on current main / arm64 under Apple VZ.

The driver rebased cleanly onto current main (your commits preserved) and it’s all at this URL, if it's useful to pull: https://codeberg.org/brechanbech/freebsd-src/src/branch/virtiofs-rebase-20260713

The four items from above still stand and are worth a bit of a review:

(1) 0aeffd6 looks superseded by 7e68af7 "fusefs: redo vnode attribute locking" — I dropped it;

(2) 79d7a88 fixes an FN_SIZECHANGE size-clobber that still seems live in main, so I kept it;

(3) FSESS_VIRTIOFS collides at 0x20000000 on current main (FSESS_WARN_LSEXTATTR_NUL), 0x40000000 is free; but I think we discussed this before.

(4) both sys/modules/virtiofs/Makefile and sys/modules/virtio/fs/Makefile need SRCS+= vnode_if.h to build standalone against current main.

Hope all this makes sense.