Re: [PATCH] ext2fs: Update to upstream Hurd-reserved xattr index for "gnu.*".

2020-05-13 Thread Samuel Thibault
Hello,

Jan Nieuwenhuizen, le mer. 13 mai 2020 14:04:04 +0200, a ecrit:
> So WDYT, is my patch for the Hurd (the first message in this thread)

Applied, thanks!

> Any ideas or suggestions on my Linux patch?

I would say not bother introducing a configuration variable, and just
include xattr_hurd.o in ext4-y. This is probably a matter of a few
hundred bytes only, it's not worth extending the already-long list of
options.

Samuel



Re: [PATCH] ext2fs: Update to upstream Hurd-reserved xattr index for "gnu.*".

2020-05-13 Thread Jan Nieuwenhuizen
Samuel Thibault writes:

Hello Samuel,

> Jan Nieuwenhuizen, le mar. 12 mai 2020 16:12:34 +0200, a ecrit:
>> setfattr --name=gnu.translator --value='/hurd/pflocal\0' 
>> /mnt/servers/socket/1
>
> man setfattr says
>
> If the given string is enclosed in double quotes, the inner string is
> treated as text.  In that case, backslashes and double quotes have
> special meanings and need to be escaped by a preceding backslash.
>
> so I guess it needs 
>
> setfattr --name=gnu.translator --value='"/hurd/pflocal\0"' 
> /mnt/servers/socket/1
>
> to actually interpret \0

Yes, that's it; thank you!

So I've now managed to create a vm-image using Guix that boots the Hurd
straight into our Guile RC script, without the need to use Bash for
that.  See


https://gitlab.com/janneke/guix/-/commit/3b4c0dcda783e8c866be154ab46ec95fb581f08f
./pre-inst-env guix system vm-image --target=i586-pc-gnu 
gnu/system/examples/bare-hurd.tmpl

and

wget https://lilypond.org/janneke/hurd/hurd-xattr.img.gz
gunzip hurd-xattr.img.gz
guix environment --ad-hoc qemu -- qemu-system-i386 -enable-kvm \
 -snapshot -hda hurd-xattr.img -m 2G \
 -device rtl8139,netdev=net0 -netdev 
user,id=net0,hostfwd=tcp:127.0.0.1:10022-:

So WDYT, is my patch for the Hurd (the first message in this thread)

https://lists.gnu.org/archive/html/bug-hurd/2020-05/msg00016.html

OK to apply now?  Any ideas or suggestions on my Linux patch?

>> --8<---cut here---start->8---
>> fsck --yes --force /
>> fsysopts / --writable
>> mv /servers/socket/1 /servers/socket/1-linux
>> touch /servers/socket/1
>> setfattr --name=gnu.translator --value='/hurd/pflocal\0' /servers/socket/1
>
> Note that glibc's setxattr, i.e. _hurd_xattr_set, translates that
> into a __file_set_translator() RPC call. Did you pass the proper option
> to make ext2fs record the translator as xattr instead of passive record?

Yes...

>> And I guess there must be an incompatibility between Linux and the Hurd
>> in how setfattr embeds the xattr attributes into the file system.
>> 
>> How to best "diff" this aspect in the file system; how to proceed?
>
> debugfs can be used for that.

Right, thanks.  I was looking for something like that.

>> Inspired by Shengyu's GSoC code that simply seemed to use fprintf for
>> debbugging, I tried adding some debug printing in inode.c
>> 
>> fprintf (stderr, "gnu.translator[%d,%d]=%s\n", datalen, strlen (*namep), 
>> *namep);
>
> Printf is the simplest way to make sure things are happening the way one
> wants, yes. Note however that in the case of translators even the output on
> stderr is buffered, so you need to flush it with fflush(stderr). Even
> safer is to use snprintf + mach_print().
>
>> but that does not seem to work,
>
> More precisely?
> I'll assume "does not show up".

(that was more precisely exactly what I meant to ask)

> If your print doesn't show up, try to put a print in other places which
> are definitely to be called such as in diskfs_user_read_node(). If those
> come up, then it means the code you put your prints is not even called,
> so put prints in code you thought was calling it etc. up to the RPC that
> you thought would be called, then jump to libc which was supposed to be
> making the RPC call, etc.

Thank you!  It took me a while to find the right setfattr curse so I
dabbled in here some more and can confirm that combining your
instructions, e.g., like so

--8<---cut here---start->8---
diff --git a/ext2fs/inode.c b/ext2fs/inode.c
index a2e804b9..f4e29eb5 100644
--- a/ext2fs/inode.c
+++ b/ext2fs/inode.c
@@ -700,6 +700,9 @@ diskfs_get_translator (struct node *np, char **namep, 
unsigned *namelen)
   void *transloc;
   struct ext2_inode *di;
 
+  fprintf (stderr, "diskfs_get_translator\n");
+  fflush (stderr);
+
   if (sblock->s_creator_os != EXT2_OS_HURD)
 return EOPNOTSUPP;
--8<---cut here---end--->8---
 
"just work".  That's helpful knowledge to have anyway.

Greetings,
janneke

-- 
Jan Nieuwenhuizen  | GNU LilyPond http://lilypond.org
Freelance IT http://JoyofSource.com | AvatarĀ® http://AvatarAcademy.com



Re: [PATCH] ext2fs: Update to upstream Hurd-reserved xattr index for "gnu.*".

2020-05-12 Thread Samuel Thibault
Jan Nieuwenhuizen, le mar. 12 mai 2020 16:12:34 +0200, a ecrit:
> setfattr --name=gnu.translator --value='/hurd/pflocal\0' /mnt/servers/socket/1

man setfattr says

If the given string is enclosed in double quotes, the inner string is
treated as text.  In that case, backslashes and double quotes have
special meanings and need to be escaped by a preceding backslash.

so I guess it needs 

setfattr --name=gnu.translator --value='"/hurd/pflocal\0"' /mnt/servers/socket/1

to actually interpret \0

> --8<---cut here---start->8---
> fsck --yes --force /
> fsysopts / --writable
> mv /servers/socket/1 /servers/socket/1-linux
> touch /servers/socket/1
> setfattr --name=gnu.translator --value='/hurd/pflocal\0' /servers/socket/1

Note that glibc's setxattr, i.e. _hurd_xattr_set, translates that
into a __file_set_translator() RPC call. Did you pass the proper option
to make ext2fs record the translator as xattr instead of passive record?

> And I guess there must be an incompatibility between Linux and the Hurd
> in how setfattr embeds the xattr attributes into the file system.
> 
> How to best "diff" this aspect in the file system; how to proceed?

debugfs can be used for that.

> Inspired by Shengyu's GSoC code that simply seemed to use fprintf for
> debbugging, I tried adding some debug printing in inode.c
> 
> fprintf (stderr, "gnu.translator[%d,%d]=%s\n", datalen, strlen (*namep), 
> *namep);

Printf is the simplest way to make sure things are happening the way one
wants, yes. Note however that in the case of translators even the output on
stderr is buffered, so you need to flush it with fflush(stderr). Even
safer is to use snprintf + mach_print().

> but that does not seem to work,

More precisely?
I'll assume "does not show up".

If your print doesn't show up, try to put a print in other places which
are definitely to be called such as in diskfs_user_read_node(). If those
come up, then it means the code you put your prints is not even called,
so put prints in code you thought was calling it etc. up to the RPC that
you thought would be called, then jump to libc which was supposed to be
making the RPC call, etc.

Samuel



[PATCH] ext2fs: Update to upstream Hurd-reserved xattr index for "gnu.*".

2020-05-12 Thread Jan Nieuwenhuizen
Hello,

I have looked a bit further into upstream xattr support and and have
been testing that using the two attached patches.  As you can see,
upstream reserved INDEX 10 for the Hurd, but the Hurd has been using
INDEX 7.

The (trivial) patch for the Hurd to change this should be OK, but until
this all fully works I'm just a bit less certain about the Linux patch.

Anyway, using the patched Linux we can add gnu.translator attributes to
files that are recognized by Linux (on a Hurd file system) as well as by
the Hurd.

So now we can actually do this

--8<---cut here---start->8---
dd if=/dev/zero of=file bs=1k count=1000
losetup /dev/loop0 file
mke2fs -t ext2 -o hurd -O ext_attr /dev/loop0
mount -t ext2 -o x-xattr-translator-records /dev/loop0 /mnt
mkdir -p /mnt/servers/socket
touch /mnt/servers/socket/1
setfattr --name=gnu.translator --value='/hurd/pflocal\0' /mnt/servers/socket/1
getfattr --name=gnu.translator /mnt/servers/socket/1
# file: 1
gnu.translator="/hurd/pflocal"
--8<---cut here---end--->8---

(on GNU/Linux), and I am using that now in Guix to cross build a
vm-image for the Hurd.

So far, so good.  However, combining these does not work yet.  When I
reduce libexec/runsystem to something like this

--8<---cut here---start->8---
#! /gnu/store/s8pcby4hjxf7d4pfzrwd3ngd813i8skw-bash-minimal-5.0.16/bin/bash

# XXX Guile needs pipe support for its finalizer thread to start
PATH=/gnu/store/6is7b5xjdfdwzym5cfhjf7jpa3824h42-hurd-0.9-1.91a5167/bin:/gnu/store/6is7b5xjdfdwzym5cfhjf7jpa3824h42-hurd-0.9-1.91a5167/sbin:/gnu/store/a4vdhbfflmbpc346lsvl3v0plplmg5ma-attr-2.4.48/bin:/gnu/store/y9vicb9spdy9lfsipv75yy5aavwf5xyn-coreutils-8.32/bin:/gnu/store/s5kx9ybvdkxcyg1243rl4fdq139b-sed-4.8/bin:/gnu/store/wy7k8v4iik6kzh9vw1fjzcnj7jhsh5fv-util-linux-2.35.1/sbin

echo foo | sed s/o/O/
echo Starting 
/gnu/store/6is7b5xjdfdwzym5cfhjf7jpa3824h42-hurd-0.9-1.91a5167/libexec/rc ...
exec /gnu/store/6is7b5xjdfdwzym5cfhjf7jpa3824h42-hurd-0.9-1.91a5167/libexec/rc
--8<---cut here---end--->8---

the Hurd says

/gnu/store/6is7b5xjdfdwzym5cfhjf7jpa3824h42-hurd-0.9-1.91a5167/runsystem: 
pipe error: Translator died

When I leave out the echo | sed pipe, starting Guile just hangs.

When I insert this

--8<---cut here---start->8---
fsck --yes --force /
fsysopts / --writable
mv /servers/socket/1 /servers/socket/1-linux
touch /servers/socket/1
setfattr --name=gnu.translator --value='/hurd/pflocal\0' /servers/socket/1
--8<---cut here---end--->8---

it works: So, we're getting real close! \o/

And I guess there must be an incompatibility between Linux and the Hurd
in how setfattr embeds the xattr attributes into the file system.

How to best "diff" this aspect in the file system; how to proceed?
Inspired by Shengyu's GSoC code that simply seemed to use fprintf for
debbugging, I tried adding some debug printing in inode.c

fprintf (stderr, "gnu.translator[%d,%d]=%s\n", datalen, strlen (*namep), 
*namep);

but that does not seem to work, or I am looking in the wrong place.  Ideas?

Greetings,
janneke

>From 75cb948c575fca3962c4cce115d31dd178bc389f Mon Sep 17 00:00:00 2001
From: "Jan (janneke) Nieuwenhuizen" 
Date: Tue, 12 May 2020 07:39:59 +0200
Subject: [PATCH] ext2fs: Update to upstream Hurd-reserved xattr index for
 "gnu.*".

See
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=3980bd3b406addb327d858aebd19e229ea340b9a

This supports setting (and reading) of passive trasnlators from
GNU/Linux, e.g.

dd if=/dev/zero of=file bs=1k count=1000
losetup /dev/loop0 file
mke2fs -t ext2 -o hurd -O ext_attr /dev/loop0
mount -t ext2 -o x-xattr-translator-records /dev/loop0 /mnt
mkdir -p /mnt/servers/socket
touch /mnt/servers/socket/1
setfattr --name=gnu.translator --value='/hurd/pflocal\0' /mnt/servers/socket/1
getfattr --name=gnu.translator /mnt/servers/socket/1
# file: 1
gnu.translator="/hurd/pflocal"

* ext2fs/xattr.c (struct _xattr_prefix): For "gnu.*", use index for
the Hurd (10).
---
 ext2fs/xattr.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ext2fs/xattr.c b/ext2fs/xattr.c
index f6ea0f39..78458214 100644
--- a/ext2fs/xattr.c
+++ b/ext2fs/xattr.c
@@ -1,6 +1,6 @@
  /* Ext2 support for extended attributes
 
-   Copyright (C) 2006, 2016 Free Software Foundation, Inc.
+   Copyright (C) 2006, 2016, 2020 Free Software Foundation, Inc.
 
Written by Thadeu Lima de Souza Cascardo 
and Shengyu Zhang 
@@ -39,7 +39,7 @@ xattr_prefixes[] =
   {
   1, "user.", sizeof "user." - 1},
   {
-  7, "gnu.", sizeof "gnu." - 1},
+  10, "gnu.", sizeof "gnu." - 1},
   {