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

[Eric Wong <e@80x24.org>]

Dominique Martinet <asmadeus@codewreck.org> wrote:
> 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 ?

Actually, it's not endianess, it's architecture specific :x
The nasty patch below fixes it.  I really wish there are better
options for scripting languages to use C headers :<

> 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 :)

Thanks, just added the Noticed-by: as an attribution to the
below patch.

> this isn't really a problem, I've only tried because I'm a monkey :)

Yeah.  Setting BTRFS_TESTDIR to a non-btrfs dir isn't going to
be supported :>

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

You're welcome, and will do.

-----8<-----
Subject: [PATCH] syscall: FS_IOC_*FLAGS: define on per-architecture basis

It turns out these Linux ioctls are unfortunately
architecture-dependent, and not endian-dependent.
Fixup some warning messages while we're at it, too.

Fixes: 14fa0abdcc7b6513 ("rewrite Linux nodatacow use in pure Perl w/o system")
Link: https://public-inbox.org/meta/YfdYqLhDVQRQ9NGT@codewreck.org/
Noticed-by: Dominique Martinet <asmadeus@codewreck.org>
---
 lib/PublicInbox/Syscall.pm | 35 +++++++++++++++++++++++------------
 1 file changed, 23 insertions(+), 12 deletions(-)

diff --git a/lib/PublicInbox/Syscall.pm b/lib/PublicInbox/Syscall.pm
index bcfae2cb..31c67a14 100644
--- a/lib/PublicInbox/Syscall.pm
+++ b/lib/PublicInbox/Syscall.pm
@@ -69,6 +69,7 @@ our (
      );
 
 my $SYS_fstatfs; # don't need fstatfs64, just statfs.f_type
+my ($FS_IOC_GETFLAGS, $FS_IOC_SETFLAGS);
 my $SFD_CLOEXEC = 02000000; # Perl does not expose O_CLOEXEC
 our $no_deprecated = 0;
 
@@ -98,6 +99,8 @@ if ($^O eq "linux") {
         $SYS_signalfd4 = 327;
         $SYS_renameat2 //= 353;
 	$SYS_fstatfs = 100;
+	$FS_IOC_GETFLAGS = 0x80046601;
+	$FS_IOC_SETFLAGS = 0x40046602;
     } elsif ($machine eq "x86_64") {
         $SYS_epoll_create = 213;
         $SYS_epoll_ctl    = 233;
@@ -105,6 +108,8 @@ if ($^O eq "linux") {
         $SYS_signalfd4 = 289;
 	$SYS_renameat2 //= 316;
 	$SYS_fstatfs = 138;
+	$FS_IOC_GETFLAGS = 0x80086601;
+	$FS_IOC_SETFLAGS = 0x40086602;
     } elsif ($machine eq 'x32') {
         $SYS_epoll_create = 1073742037;
         $SYS_epoll_ctl = 1073742057;
@@ -112,6 +117,8 @@ if ($^O eq "linux") {
         $SYS_signalfd4 = 1073742113;
 	$SYS_renameat2 //= 0x40000000 + 316;
 	$SYS_fstatfs = 138;
+	$FS_IOC_GETFLAGS = 0x80046601;
+	$FS_IOC_SETFLAGS = 0x40046602;
     } elsif ($machine eq 'sparc64') {
 	$SYS_epoll_create = 193;
 	$SYS_epoll_ctl = 194;
@@ -121,6 +128,8 @@ if ($^O eq "linux") {
 	$SYS_renameat2 //= 345;
 	$SFD_CLOEXEC = 020000000;
 	$SYS_fstatfs = 158;
+	$FS_IOC_GETFLAGS = 0x40086601;
+	$FS_IOC_SETFLAGS = 0x80086602;
     } elsif ($machine =~ m/^parisc/) {
         $SYS_epoll_create = 224;
         $SYS_epoll_ctl    = 225;
@@ -135,6 +144,8 @@ if ($^O eq "linux") {
         $SYS_signalfd4 = 313;
 	$SYS_renameat2 //= 357;
 	$SYS_fstatfs = 100;
+	$FS_IOC_GETFLAGS = 0x40086601;
+	$FS_IOC_SETFLAGS = 0x80086602;
     } elsif ($machine eq "ppc") {
         $SYS_epoll_create = 236;
         $SYS_epoll_ctl    = 237;
@@ -143,6 +154,8 @@ if ($^O eq "linux") {
         $SYS_signalfd4 = 313;
 	$SYS_renameat2 //= 357;
 	$SYS_fstatfs = 100;
+	$FS_IOC_GETFLAGS = 0x40086601;
+	$FS_IOC_SETFLAGS = 0x80086602;
     } elsif ($machine =~ m/^s390/) {
         $SYS_epoll_create = 249;
         $SYS_epoll_ctl    = 250;
@@ -174,6 +187,8 @@ if ($^O eq "linux") {
         $SYS_signalfd4 = 74;
 	$SYS_renameat2 //= 276;
 	$SYS_fstatfs = 44;
+	$FS_IOC_GETFLAGS = 0x80086601;
+	$FS_IOC_SETFLAGS = 0x40086602;
     } elsif ($machine =~ m/arm(v\d+)?.*l/) {
         # ARM OABI
         $SYS_epoll_create = 250;
@@ -191,6 +206,8 @@ if ($^O eq "linux") {
         $SYS_signalfd4 = 5283;
 	$SYS_renameat2 //= 5311;
 	$SYS_fstatfs = 5135;
+	$FS_IOC_GETFLAGS = 0x40046601;
+	$FS_IOC_SETFLAGS = 0x80046602;
     } elsif ($machine =~ m/^mips/) {
         $SYS_epoll_create = 4248;
         $SYS_epoll_ctl    = 4249;
@@ -199,6 +216,8 @@ if ($^O eq "linux") {
         $SYS_signalfd4 = 4324;
 	$SYS_renameat2 //= 4351;
 	$SYS_fstatfs = 4100;
+	$FS_IOC_GETFLAGS = 0x40046601;
+	$FS_IOC_SETFLAGS = 0x80046602;
     } else {
         # as a last resort, try using the *.ph files which may not
         # exist or may be wrong
@@ -345,22 +364,14 @@ sub nodatacow_fh {
 	my $f_type = unpack('l!', $buf); # statfs.f_type is a signed word
 	return if $f_type != 0x9123683E; # BTRFS_SUPER_MAGIC
 
-	state ($FS_IOC_GETFLAGS, $FS_IOC_SETFLAGS);
-	unless (defined $FS_IOC_GETFLAGS) {
-		if (substr($Config{byteorder}, 0, 4) eq '1234') {
-			$FS_IOC_GETFLAGS = 0x80086601;
-			$FS_IOC_SETFLAGS = 0x40086602;
-		} else { # Big endian
-			$FS_IOC_GETFLAGS = 0x40086601;
-			$FS_IOC_SETFLAGS = 0x80086602;
-		}
-	}
+	$FS_IOC_GETFLAGS //
+		return warn('FS_IOC_GETFLAGS undefined for platform');
 	ioctl($fh, $FS_IOC_GETFLAGS, $buf) //
-		return warn("FS_IOC_GET_FLAGS: $!\n");
+		return warn("FS_IOC_GETFLAGS: $!\n");
 	my $attr = unpack('l!', $buf);
 	return if ($attr & 0x00800000); # FS_NOCOW_FL;
 	ioctl($fh, $FS_IOC_SETFLAGS, pack('l', $attr | 0x00800000)) //
-		warn("FS_IOC_SET_FLAGS: $!\n");
+		warn("FS_IOC_SETFLAGS: $!\n");
 }
 
 sub nodatacow_dir {