Repository navigation
Unique inodes - #455
Unique inodes#455
Conversation
|
Confirmed that the issues I saw with 452 were exclusively on very old 32 bit systems. Only happened on buster/stretch/jessie. Bullseye and newer were fine. |
d09c46a to
e6784a8
Compare
| l->fh = realloc(tmp_fh, tmp_fh->handle_bytes + sizeof(*l->fh)); | ||
| if (!l->fh) |
There was a problem hiding this comment.
do we need to prevent overriding l->fh here with NULL on errors?
There was a problem hiding this comment.
I think this is correct. If the realloc fails then we free the old pointer (tmp_fh) and return -1. The -1 then gets assigned to l->nfs_filehandles in direct_load_data_source and then we will either abort in main.c if nfs filehandle support is required due to xino=on or we'll set lo.nfs_filehandles to zero which means we won't try to use them. (around line 5880 of main.c)
I'd tend to assume that if realloc fails here though, things are going to die pretty quickly afterwards!
But I've realised that when we return 0 due to nfs filehandles not supported l->fh may or may not be a valid handle for making requests, so I'm adding code at line 228 to free l->fh and reset to NULL so it's consistent - that might have been what you were alluding to.
Another thing that I meant to bring up is whether I need to bump the version in plugin_manager.c? I've added a new member to data_source but I couldn't see exactly how this was supposed to work and how plugins know what version they should expose. Feels like, at the very least, there's a #define PLUGIN_VERSION_NUMBER missing from fuse-overlayfs.h. I haven't been able to find any plugins, not even an example.
There was a problem hiding this comment.
Oh, I've found a plugin here: https://gist.github.com/giuseppe/a669ed7248de557a9b5fd272ffe2a4f4 although that seems a bit out of date.
Having a look now
There was a problem hiding this comment.
https://github.com/tjwoodall/fuse-overlayfs/pull/new/plugin-test
It needs some cleanup but I've resurrected it and written a quick test.
And technically, the version doesn't need a bump, because l->nfs_filehandles will be zero due to the calloc where we allocate the ovl_layer which means that lo.nfs_filehandles will be zero and hence we will never call (or attempt to access) ds->get_nfs_filehandle.
There was a problem hiding this comment.
Pull request overview
This PR implements support for unique inode numbers across multiple filesystem layers in fuse-overlayfs. When layers span multiple devices, the PR introduces a new xino option that allows stable inode numbers across mounts using NFS file handle hashing.
Changes:
- Added new
xinooption (off/auto/on) to control inode number generation strategy - Implemented NFS filehandle-based inode hashing for stable inodes across mounts
- Added
ino32_toption for 32-bit compatibility - Added comprehensive test suites for passthrough and mmap functionality
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test-passthrough.sh | New test script verifying inode passthrough behavior and uniqueness across layers |
| tests/test-mmap.sh | New test script for memory mapping with duplicate inodes across layers |
| main.c | Core implementation of inode generation strategies with new get_st_ino() function and xino option parsing |
| fuse-overlayfs.h | Added struct fields for inode passthrough mode, NFS filehandles support, and 32-bit compatibility |
| direct.c | Implemented NFS filehandle support with FNV-1a hashing and detection logic |
| fuse-overlayfs.1.md | Documentation for new xino and ino32_t options |
| fuse-overlayfs.1 | Generated man page with formatting updates |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
e6784a8 to
c2ac5d2
Compare
|
could you please run |
. remove unused limits.h include . cleanup whitespace/continuation in macro definition . correct printf to report the values being compared. . correct pthread_mutex_init call take NULL rather than assume (enum) PTHREAD_MUTEX_DEFAULT == 0 Signed-off-by: Tim Woodall <builder@woodall.me.uk>
c2ac5d2 to
1604dd4
Compare
mmap depends on st_ino being unique for all files on a device.
When all the layers are on the same device we pass the st_ino from the
underlying device through to stat/readdir. This is identical to previous
behaviour.
If they are not on the same device then, by default, we pass through the
nodeid instead. This is guaranteed to be unique but is not stable across
mounts. For almost all usecases this is sufficient (and as fast as
passing through st_dev)
Additionally we add --xino={on,auto,off}
xino=off is the same as not passing it above, use st_dev or nodeid as
above.
xino=on requires that nfs-filehandles are available on all the layers
and we use a hash of the filehandle as st_dev where we would use nodeid
otherwise. The mount will fail if nfs-filehandles are not supported,
even if all the files are on the same layer.
xino=auto will use nfs-filehandles in place of nodeid if they are
available but will fall back to nodeid if they are not available.
Note that we always use passthrough st_ino if all the layers are on the
same device regardless of the xino setting.
One other change, where FUSE_CAP_NO_EXPORT_SUPPORT is supported by fuse,
if the fuse-overlayfs inodes are not stable across mounts then we
prevent the fuse-overlayfs mount from being used with nfs filehandles.
N.B. If all the layers are on the same underlying device then we do NOT
check that that supports NFS filehandles/has stable inodes across
mounts. We are only considering un-mounting and re-mounting the
fuse-overlayfs device with regards to stable inodes, not re-mounts of
the underlying layers.
Signed-off-by: Tim Woodall <builder@woodall.me.uk>
Signed-off-by: Tim Woodall <builder@woodall.me.uk>
1604dd4 to
e58fb12
Compare
Done. One more thing I've noticed l->st_dev will be zero in all plugins (that don't explicitly set this), that means they'll be deemed to be all on the same device. I've added code to set st_dev to the hash of the text configuring the plugin (the plugin can then override this) |
No description provided.