On Mon, Jun 10, 2019 at 9:37 AM Amir Goldstein <amir73il@xxxxxxxxx> wrote: > > On Mon, Jun 10, 2019 at 6:58 AM Theodore Ts'o <tytso@xxxxxxx> wrote: > > > > On Sun, Jun 02, 2019 at 03:41:11PM +0300, Amir Goldstein wrote: > > > This test case was split out of Dave Chinner's copy_file_range bounds > > > check test to reduce the requirements for running the bounds check. > > > > > > Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx> > > > > I've just updated to the latest fstests, where this has landed as > > generic/554. This test is failing on ext4, and should fail on all > > file systems which do not support copy_file_range (ext4, nfsv3, etc.), > > since xfs_io will fall back to emulating this via reading and writing > > Why do you think this is xfs_io fall back and not kernel fall back to > do_splice_direct()? Anyway, both cases allow read from swapfile > on upstream. > > > the file, and this causes a test failure because: > > > > > +echo swap files return ETXTBUSY > > > +_format_swapfile $testdir/swapfile 16m > > > +swapon $testdir/swapfile > > > +$XFS_IO_PROG -f -c "copy_range -l 32k $testdir/file" $testdir/swapfile > > > +$XFS_IO_PROG -f -c "copy_range -l 32k $testdir/swapfile" $testdir/copy > > > +swapoff $testdir/swapfile > > > > Currently, the VFS doesn't prevent us from reading a swap file. > > Perhaps it shouldn't, for security (theatre) reasons, but root can > > read the raw block device anyway, so it's really kind of pointless. > > > > Hmm, my intention with the copy_file_range() behavior was that > it mostly follows user copy limitations/semantics. > I guess preventing copy *from* swapfile doesn't make much sense > and we should relax this check in the new c_f_r bounds check in VFS. > > > I'm not sure what's the best way fix this, but I'm going to exclude > > this test in my test appliance builds for now. > > > > Trying to understand the desired flow of tests and fixes. > I agree that generic/554 failure may be a test/interface bug that > we should fix in a way that current upstream passes the test for > ext4. Unless there is objection, I will send a patch to fix the test > to only test copy *to* swapfile. I made this change and test still failed on upstream ext4, because kernel performs copy_file_range() to swapfile. To me that seems like a real kernel bug, which is addressed by vfs c_f_r patches, so I don't know if you should be excluding the test from test appliance after all ? Thanks, Amir. > > generic/553, OTOH, is expected to fail on upstream kernel. > Are you leaving 553 in appliance build in anticipation to upstream fix? > I guess the answer is in the ext4 IS_IMMUTABLE patch that you > posted and plan to push to upstream/stable sooner than VFS patches. > > In any case, the VFS c_f_r patches are aiming for v5.3 and > I will make sure to promote them for stable as well. > > Do you think that should there be a different policy w.r.t timing of > merging xfstests tests that fail on upstream kernel? > > Thanks, > Amir.
