On Fri, 27 Sep 2013 07:24:31 +0200 Roland Mainz 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... it does work xattr gave the rope and I used it and its a beautiful application of an unintened consequence so at this point its a performance hit to get open() semantics or no performance hit with dup() semantics you want to debug all the cases that will fail with dup() semantics? and are you sure readonly access to a synthesized default xattr causes a write? now *that* would be a performance hit find / of a small program that did openat(open(file,0),O_XATTR) would really fill up the nameless space with physical default xattrs? and I could do that for files I could read but don't own? if one were to set up an experiment, what program lists xattr usage, du -@? df -@? fsck -@ ...? _______________________________________________ ast-developers mailing list [email protected] http://lists.research.att.com/mailman/listinfo/ast-developers
