User Details
- User Since
- May 9 2014, 11:04 PM (596 w, 1 d)
Wed, Oct 8
Could you please give an example of the before and after output?
Could you give an example of what the output looks like before and after?
Tue, Oct 7
The MD_LEN variable is now badly named. I think it's ok to leave its value the same. Most test cases don't require 1 MB of I/O. But you should at least change that name.
Mon, Oct 6
Sun, Oct 5
Fri, Oct 3
After seeing this code in practice it seems quite complicated. I do think that the kernel based approach would be simpler. But an important question is: what file systems already use this option on Linux? How many of them would work with either this implementation, or the hypothetical kernel-based one?
Thu, Oct 2
Wed, Oct 1
So is -1 being cast to true? Thank you GCC for telling us that. If so, I think we should replace ATF_REQUIRE_MSG with ATF_REQUIRE_EQ_MSG. I think that would be more clear.
What's the problem? Under what conditions is the current code incorrect?
Sat, Sep 27
Fri, Sep 26
Thu, Sep 25
Wed, Sep 24
Sat, Sep 20
I have a few thoughts:
Fri, Sep 19
- Also add a test case for writes to a file that lies in a sockbuf
Mon, Sep 15
Fri, Sep 12
Sep 9 2025
Sep 7 2025
What bug exactly does this "fix"? Did you encounter a bug in your work, or did you just discover this problem by inspection? If the former, we should add a test case to tests/sys/fs/fusefs.
Aug 29 2025
Aug 21 2025
@arrowd I agree with your change. The only thing I wanted different is a more detailed commit message. Of course, Mark might disagree.
Aug 12 2025
Aug 8 2025
Aug 7 2025
Ok. I'm sure that you'll get the NFS client part done. The rest of it looks fine to me.
Here's a stupid question. Is it possible for nfsd to reexport an NFS mountpoint? If so, then nfs_copy_file_range will need to check the flag, too.
Shouldn't there also be some ZFS code that returns ENOSYS if the cloning requirement cannot be satisfied? And fuse_vnop_copy_file_range cannot ever satisfy that requirement, so it should return ENOSYS just like in vn_generic_copy_file_range
Instead of this, I think a better solution would be for ctld to ignore EEXIST errors. That would not only fix your situation, but would also go some way towards recovering from a ctld that crashed (currently, if ctld crashes the only way to recover is to reboot or to manually remove every target with ctladm). The port::kernel_add() method would need to be be adjusted to return an errno, of course. But the ioctls already do. I'm not sure what error code they currently return; that might need to be adjusted.
Is there a way to reproduce this bug using unpatched FreeBSD?
Aug 6 2025
Aug 5 2025
Aug 4 2025
Aug 3 2025
Aug 2 2025
Jul 30 2025
Jul 23 2025
Jul 22 2025
Jul 20 2025
The commit message says "jail". But kern.vm_guest doesn't change in a jail. Did you mean to say "in a VM"?
Jul 18 2025
Jul 16 2025
Jul 15 2025
You need to add "LIBADD= xo" to usr.bin/fstat/Makefile. Also, a lot of fields are missing in the html output, for example file_type. Is that intentional?
Jul 14 2025
The regression test is at D51316 .
I have tested this patch and confirm that it works. I've also written a minimal reproduction case, which I'll post as a separate review. And while I understand Mark's concern, I tend to agree with arrowd's approach. BMAP should be advisory, and readahead should be optional, so errors in neither should cause the main read to fail.
But I would like to see a better commit message. It should contain enough information to describe the problem without resorting to Bugzilla, since Bugzilla might not be around forever.
Jul 6 2025
I have no idea why Arcanist decided to include all of those extra revisions when I updated the diff. I did not rebase. Only the most recent three revisions are intended to be part of the review. I'll abandon and reopen.
- Abandon the custom kernel module. Rely on existing syscalls instead.
- move the test into the kern directory
Jul 5 2025
Jul 4 2025
The fusefs part LGTM. I didn't read the rest.
Jul 3 2025
Jun 28 2025
This is ok. It should normally be unnecessary, though, because ctladm should always be installed. Are you using pkgbase or something? I guess it might be optional in a pkgbase world.