Re: [PATCH] nodatacow: quiet chattr errors [was: Test failures with 1.7.0]

[Dominique Martinet <asmadeus@codewreck.org>]

Eric Wong wrote on Mon, Jan 31, 2022 at 02:03:11AM +0000:
> Ah, intentionally setting BTRFS_TESTDIR to something that isn't
> btrfs will break, yes.

Ok, so this is specific to the test.
Checking now nodatacow_fh skips files with non-btrfs magic, so it looks
good to me!

I've taken a look at the code now, just one more question: I don't
understand why you've made the ioctl value depend on endianness ?
get and set are respectively
_IOR('f', 1, long) and _IOW('f', 2, long)
which translate to
(2 << 30) | (8 << 16) | ('f' << 8) | 1
(1 << 30) | (8 << 16) | ('f' << 8) | 2
so should be
0x80086601
0x40086602
regardless of endianness ? as far as I understand the memory
representation might change but values themselves are handled the same,
and I don't see anything in the defines that'd swap just the R/W bits
around, but then again I don't have any big endian machine around and
never really played with one.


That aside the code looks good to me, if you do Reviewed-by tags feel
free to add mine (Dominique Martinet <asmadeus@codewreck.org>) once that
question is answered.
If you don't care, I don't care either :)


> I suppose an explicit BAIL_OUT is in order, here:

Well, tmpfs does not have attrs so lsattr fails and this triggers this
bailout (this would also "work" for nixos tests which likely don't have
lsattr available), but for example ext4 and xfs have working lsattr so
we get another failure:

=========
$ BTRFS_TESTDIR=/boot0/test strace -o /tmp/strace -f prove -bvw t/nodatacow.t 
t/nodatacow.t .. 
ok 1 - use PublicInbox::Syscall;
not ok 2 - `C' attribute set on fd with pure Perl

#   Failed test '`C' attribute set on fd with pure Perl'
#   at t/nodatacow.t line 33.
#                   '--------------e------- /boot0/test/nodatacow-80SC/pp.f
# '
#     doesn't match '(?^:C.*\/boot0\/test\/nodatacow\-80SC\/pp\.f)'
not ok 3 - `C' attribute set on dir with pure Perl
=========


But anyway, the test itself is skipped unless BTRFS_TESTDIR is set so
this isn't really a problem, I've only tried because I'm a monkey :)

Thanks again for the support, don't hesitate to ask if you need further
info or tests for the zfs problems.
-- 
Dominique