On Fri, Sep 27, 2013 at 7:24 AM, Roland Mainz <[email protected]> wrote:
> On Fri, Sep 27, 2013 at 6:48 AM, Roland Mainz <[email protected]>
> wrote:
>> On Fri, Sep 27, 2013 at 6:17 AM, Glenn Fowler <[email protected]> wrote:
>>> On Fri, 27 Sep 2013 04:17:31 +0200 Roland Mainz wrote:
>>>> On Fri, Sep 27, 2013 at 3:50 AM, Roland Mainz <[email protected]>
>>>> wrote:
>>>> > The following code from one of my test scripts no longer work in
>>>> > ast-ksh.2013-09-26 on Solaris 11/AMD64/64bit:
>>>> > -- snip --
>>>> > $ ksh -c 'redirect {n}<. ; redirect {m}</dev/file/flags@xattr@/dev/fd/$n
>>>> > '
>>>> > /home/test001/bin/ksh: /dev/file/flags@xattr@/dev/fd/11: cannot open
>>>> > [Invalid argument]
>>>> > -- snip --
>>>> >
>>>> > This fails because of the following code tries to open the parent dir
>>>> > of an XATTR dir via:
>>>> > -- 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 ?
>>>
>>>> This patch fixes the problem:
>>>> -- 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@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@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@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) [email protected]
\__\/\/__/ MPEG specialist, C&&JAVA&&Sun&&Unix programmer
/O /==\ O\ TEL +49 641 3992797
(;O/ \/ \O;)
_______________________________________________
ast-developers mailing list
[email protected]
http://lists.research.att.com/mailman/listinfo/ast-developers
