Post by Roland MainzPost by Roland MainzPost by Glenn FowlerPost by Roland MainzPost by Roland MainzThe following code from one of my test scripts no longer work in
-- snip --
[Invalid argument]
-- snip --
This fails because of the following code tries to open the parent dir
-- snip --
227 #if O_XATTR
228 if ((f = openat(dev.fd, ".",
O_INTERCEPT|O_RDONLY|O_XATTR)) >= 0)
229 {
230 fd = openat(f, "..",
oflags|O_INTERCEPT, mode);
231 close(f);
232 return fd;
233 }
234 #endif
-- snip --
... erm.. I don't understand this... what is this code trying to do ?
-- snip --
diff -r -u original/src/lib/libast/path/pathopen.c
build_i386_64bit_debug/src/lib/libast/path/pathopen.c
--- src/lib/libast/path/pathopen.c 2013-08-29 07:17:52.000000000 +0200
+++ src/lib/libast/path/pathopen.c 2013-09-27 04:06:38.362136396 +0200
@@ -222,16 +222,12 @@
/* preserve open() semantics if possible */
- if (oflags & (O_DIRECTORY|O_SEARCH))
- return openat(dev.fd, ".",
oflags|O_INTERCEPT, mode);
-#if O_XATTR
- if ((f = openat(dev.fd, ".",
O_INTERCEPT|O_RDONLY|O_XATTR)) >= 0)
- {
- fd = openat(f, "..", oflags|O_INTERCEPT, mode);
- close(f);
- return fd;
- }
+ if (oflags & (O_DIRECTORY|O_SEARCH
+#ifdef O_XATTR
+ |O_XATTR
#endif
+ ))
+ return openat(dev.fd, ".",
oflags|O_INTERCEPT, mode);
this patch is wrong -- don't apply it
see my previous post
Why is it wrong ? The |fd = openat(f, "..", oflags|O_INTERCEPT,
mode);| in the original code could've never worked in any possible way
because ".." in an xattr directory is doomed to fail anyway...
Actually I'm wrong... it should work... but the performance
implications are horrible because if the filesystem doesn't have the
xattr _data_ yet it may create them from the "default set"... as
result we trigger a write operation on the first time of access (of
course this doesn't happen if the XATTR or SATTR data are already in
use.. for example on UFS SATTR data may be used to store the ACL data
etc.) ... |O_XATTR| wasn't designed with this usage in mind...
Looking back at the original failure:
$ ksh -c 'redirect {n}<. ; redirect
{m}</dev/file/flags at xattr@/dev/fd/$n ; cd -f $m ; true' # creates this
truss output:
-- snip --
[snip]
open(".", O_RDONLY) = 3
fstat(3, 0xFFFFFD7FFFDFE810) = 0
fcntl(3, F_SETFD, 0x00000001) = 0
fcntl(3, F_DUPFD, 0x0000000A) = 11
close(3) = 0
ioctl(11, TCGETS, 0xFFFFFD7FFFDFF0EC) Err#25 ENOTTY
lseek(11, 0, SEEK_CUR) = 0
fstat(11, 0xFFFFFD7FFFDFF160) = 0
fcntl(11, F_SETFD, 0x00000001) = 0
fcntl(11, F_GETFL) = 8192
openat(11, ".", O_RDONLY|O_XATTR) = 3
fstat(3, 0xFFFFFD7FFFDFE810) = 0
openat(3, "..", O_RDONLY|O_XATTR) Err#22 EINVAL
close(3) = 0
open("/usr/lib/locale/en_US.UTF-8/LC_MESSAGES/SUNW_OST_OSLIB.mo",
O_RDONLY) Err#2 ENOENT
./arch/sol11.i386-64/bin/ksh: /dev/file/flags at xattr@/dev/fd/11: cannot
open [Invalid argument]
[snip]
-- snip --
The |openat(3, "..", O_RDONLY|O_XATTR)| ruines the whole thing.
However if I filter out the |O_XATTR| flag the whole "io.sh" testsuite
triggers mass failures... and if I disable the whole codeblock within
|#if O_XATTR ... #endif| I can't enter the XATTR dir anymore.
... and the following code doesn't work because it always passes the
whole path to |openat()| instead of just the /dev/fd/-portion:
-- snip --
236 /* see if the filesystem groks
.../[<pid>]/<fd>/... paths */
237
238 if ((!(oflags & O_CREAT) || !access(b,
F_OK)) && (fd = openat(AT_FDCWD, b, oflags|O_INTERCEPT, mode)) >= 0)
239 return fd;
-- snip --
That should work... but... when I tried it with this patch...
-- snip --
diff -r -u original/src/lib/libast/path/pathopen.c
build_i386_64bit_debug/src/lib/libast/path/pathopen.c
--- src/lib/libast/path/pathopen.c 2013-08-29 07:17:52.000000000 +0200
+++ src/lib/libast/path/pathopen.c 2013-09-27 07:41:55.847283054 +0200
@@ -224,18 +224,11 @@
if (oflags & (O_DIRECTORY|O_SEARCH))
return openat(dev.fd, ".",
oflags|O_INTERCEPT, mode);
-#if O_XATTR
- if ((f = openat(dev.fd, ".",
O_INTERCEPT|O_RDONLY|O_XATTR)) >= 0)
- {
- fd = openat(f, "..", oflags|O_INTERCEPT, mode);
- close(f);
- return fd;
- }
-#endif
/* see if the filesystem groks
.../[<pid>]/<fd>/... paths */
- if ((!(oflags & O_CREAT) || !access(b, F_OK))
&& (fd = openat(AT_FDCWD, b, oflags|O_INTERCEPT, mode)) >= 0)
+ const char *b_devfd = strstr(b, "/dev/fd/");
+ if ((!(oflags & O_CREAT) || !access(b, F_OK))
&& (fd = openat(AT_FDCWD, b_devfd, oflags|O_INTERCEPT, mode)) >= 0)
return fd;
/* stuck with dup semantics -- the best we can
do at this point */
-- snip --
... I found out that the resulting |open("/dev/fd/11",
O_RDONLY|O_XATTR)| will NOT give me a XATTR directory (which is AFAIK
a bug...).
Short: xx@@@!!!!<censored>@@@!!!!<censored>@@@!!!!<censored>!!xx!!
BTW: Using /proc/self/fd/ seems to work:
-- snip --
$ ./arch/sol11.i386\-64/bin/ksh -c 'redirect {n}<. ; redirect
{m}</dev/file/flags at xattr@/proc/self/fd/$n ; cd -f $m ; ls;true'
SUNWattr_ro SUNWattr_rw
-- snip --
So basically we're back the point where the whole code within |#if
O_XATTR| doesn't work... and shouldn't be used because the
implementations will perform horribly performance-wise.
----
Bye,
Roland
--
__ . . __
(o.\ \/ /.o) roland.mainz at nrubsig.org
\__\/\/__/ MPEG specialist, C&&JAVA&&Sun&&Unix programmer
/O /==\ O\ TEL +49 641 3992797
(;O/ \/ \O;)