Re: [PATCH v7 07/10] fsmonitor: implement filesystem change listener for Linux
- From
- Paul Tarjan <paul@paultarjan.com>
- Date
- Mar 4, 2026, 18:17 UTC
- Message-ID
- <20260304181750.25730-1-github@paulisageek.com>
- In-Reply-To
- <aafii97SoHS96rkA@pks.im>
On Tue, Mar 4, 2026, Patrick Steinhardt wrote:
> I saw that this define is only wired up for CMake. I guess we should > also add it to our Makefile (probably via config.mak.uname) and Meson > (probably via compiler.has_header()).
It's already wired up in all three:
- config.mak.uname line 78: BASIC_CFLAGS += -DHAVE_LINUX_MAGIC_H
- meson.build: compiler.has_header('linux/magic.h') check
- CMakeLists.txt: add_compile_definitions(HAVE_LINUX_MAGIC_H)While looking at CMakeLists.txt I noticed the GIT-BUILD-OPTIONS string replacements were hardcoded to "win32" for both FSMONITOR_DAEMON_BACKEND and FSMONITOR_OS_SETTINGS. Fixed that to use the CMake variables so Linux/macOS builds get the right values.
> This selection looks rather interesting to me. Why wouldn't we include > common filesystems like ext4 and the like? Certainly hints that the > function needs better documentation, and potentially a better name.
It's only used as a fallback when /proc/mounts isn't available. Since we only care about naming the remote/special filesystems that is_remote_fs() flags as incompatible, there's no need for ext4 etc. Updated the comment in v8 to make this clearer. Also removed V9FS_MAGIC from this function since 9p is used for local VM/container host mounts where fsmonitor works fine, and it's not in is_remote_fs().
> Sorry, but I still don't quite understand what we're doing here. Isn't > the longest matching mount point always the one that statfs(3p) gave us? > Why do we have to scan "/proc/mounts"?
statfs(2) gives us f_type (magic number) and f_fsid but not the human-readable filesystem type string. We need the type name (e.g. "nfs", "cifs") for the fs_info.typename field, which check_uds_volume() compares against strings like "msdos" and "ntfs". /proc/mounts has that string, and we match on f_fsid to find the right entry.
Also fixed a redundant statfs() call: find_mount() was calling statfs(path, ...) again even though the caller already had the result. Changed it to take a const pointer to the caller's statfs instead.