Created attachment 16722 [details] Patch that reworks the logic in btrfs_fget_compression Dear Sir/Madam, We have discovered that with commit a55a2bfb9143002161ef119298891725c6d3c075 from 2020-10-13, the function btrfs_fget_compression is permanently broken. That commit is the rewrite from SMB_VFS_GET_COMPRESSION to SMB_VFS_FGET_COMPRESSION. It is the last commit that touches the btrfs_fget_compression function. There is none after it. The issue is that if the branch in source3/modules/vfs_btrfs.c:466 is taken, the call to ioctl() will always fail. That is because the fd is initialized to -1 and then never touched again. Code from master: ------------------------------- static NTSTATUS btrfs_fget_compression(struct vfs_handle_struct *handle, TALLOC_CTX *mem_ctx, struct files_struct *fsp, uint16_t *_compression_fmt) { char buf[PATH_MAX]; const char *p = NULL; int ret; long flags = 0; > int fd = -1; NTSTATUS status; if (!fsp->fsp_flags.is_pathref) { > ret = ioctl(fd, FS_IOC_GETFLAGS, &flags); if (ret < 0) { DBG_WARNING("FS_IOC_GETFLAGS failed: %s, fd %lld\n", strerror(errno), (long long)fd); return map_nt_error_from_unix(errno); } [...] ------------------------------- This happpens always for any share using btrfs as filesystem. This causes a logging of the error message in line 469 for every file request, which means it happens ever couple of milliseconds. This effectively breaks btrfs support. This pertains any release in the 4.14 series. We have developed a small patch that reworks the logic to avoid calling ioctl on fd=-1, but the compression remains broken. The patch is attached. It applies onto 4.14.5. Kind regards Noel Kuntze
Created attachment 16723 [details] Patch that reworks the logic in btrfs_fget_compression (right patch)
Hm, look like I broke it, let me check...
The patch effectively silences the bug, but all written files are not compressed. So I guess set_compression is broken, too.
Correction: Not all files are written compressed anymore. Some are compressed, some aren't.
Created attachment 16725 [details] Possible patch for master Can you please check this version?
Thank you. The patch works.
Patch works here as well.
When can we expect this patch to be merged into master?
(In reply to Noel Maximilian Kuntze from comment #8) I've put it on my todo list.
Is it normal that it takes so long to process a patch for a regression (Priority 5)?
(In reply to Noel Maximilian Kuntze from comment #10) Usually not, no. But as "amount of work to do / available development resources = time it takes" sometimes specific bugs or feautures sit a wee bit longer in the queue. Sorry!
But it's a regression on a complete feature in use, not a new feature? So what exactly happened here? It makes no sense to me.
(In reply to Noel Maximilian Kuntze from comment #12) Noel, if you need a guaranteed response time for a bug, the only way to get that is to purchase a support contract. Without that, all bug fixes are done on a best effort basis. That's the norm in FLOSS projects.
This bug was referenced in samba master: ed35fce4fe48b1fa26854a7b4bb151b5c5fb6fc6
Created attachment 16803 [details] git-am fix for 4.15.next, 4.14.next. Cherry-picked from master.
Reassigning to Jule for inclusion in 4.14 and 4.15.
Pushed to autobuild-v4-{15,14}-test.
This bug was referenced in samba v4-15-test: 35d474c303004c1a2a82fcd9f8f87a7b28a85713
This bug was referenced in samba v4-14-test: e9cbf386be77230e3c3d51b878953bf4afbf93ff
Closing out bug report. Thanks!
This bug was referenced in samba v4-15-stable (Release samba-4.15.0rc7): 35d474c303004c1a2a82fcd9f8f87a7b28a85713
This bug was referenced in samba v4-14-stable (Release samba-4.14.8): e9cbf386be77230e3c3d51b878953bf4afbf93ff