Re: [PATCH 0/4] add killattr inode operation to allow filesystems to interpret ATTR_KILL_S*ID bits

2007-08-28 Thread Josef Sipek
On Tue, Aug 28, 2007 at 08:11:14PM +0100, Christoph Hellwig wrote:
> 
> Sorry for not replying to the previsious revisions, but I've been out
> for on vacation.
> 
> I can't say I like this version.  Now we've got callouts at two rather close
> levels which is not very nice from the interface POV.
> 
> Maybe preference is for the first scheme where we simply move interpreation
> of the ATTR_KILL_SUID/ATTR_KILL_SGID into the setattr routine and provide
> a nice helper for the normal filesystem to use.
> 
> If people are really concerned about adding two lines of code to the
> handfull of setattr operation there's a variant of this scheme that can
> avoid it:
 
It's not about adding 2 lines of code - it's about adding the requirement
for the fs to call a function.

>  - notify_change is modified to not clear the ATTR_KILL_SUID/ATTR_KILL_SGID
>but update ia_mode and the ia_valid flag to include ATTR_MODE.
>  - disk filesystems stay unchanged and never look at
>ATTR_KILL_SUID/ATTR_KILL_SGID, but nfs can check for it and ignore
>the ATTR_MODE flags and ia_valid in this case and do the right thing
>on the server side.

Sounds reasonable.

Josef 'Jeff' Sipek.

-- 
I abhor a system designed for the "user", if that word is a coded pejorative
meaning "stupid and unsophisticated."
- Ken Thompson
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/4] Fix mainline filesystems to handle ATTR_KILL_ bits correctly

2007-08-21 Thread Josef Sipek
On Tue, Aug 21, 2007 at 07:35:51AM -0400, Jeff Layton wrote:
> On Tue, 21 Aug 2007 15:35:08 +1000
> Timothy Shimmin <[EMAIL PROTECTED]> wrote:
> 
> > Jeff Layton wrote:
> > > This should fix all of the filesystems in the mainline kernels to handle
> > > ATTR_KILL_SUID and ATTR_KILL_SGID correctly. For most of them, this is
> > > just a matter of making sure that they call generic_attrkill early in
> > > the setattr inode op.
> > > 
> > > Signed-off-by: Jeff Layton <[EMAIL PROTECTED]>
> > > ---
> > >  fs/xfs/linux-2.6/xfs_iops.c   |5 -
> > > --- a/fs/xfs/linux-2.6/xfs_iops.c
> > > +++ b/fs/xfs/linux-2.6/xfs_iops.c
> > > @@ -651,12 +651,15 @@ xfs_vn_setattr(
> > >   struct iattr*attr)
> > >  {
> > >   struct inode*inode = dentry->d_inode;
> > > - unsigned intia_valid = attr->ia_valid;
> > > + unsigned intia_valid;
> > >   bhv_vnode_t *vp = vn_from_inode(inode);
> > >   bhv_vattr_t vattr = { 0 };
> > >   int flags = 0;
> > >   int error;
> > >  
> > > + generic_attrkill(inode->i_mode, attr);
> > > + ia_valid = attr->ia_valid;
> > > +
> > >   if (ia_valid & ATTR_UID) {
> > >   vattr.va_mask |= XFS_AT_UID;
> > >   vattr.va_uid = attr->ia_uid;
> > 
> > Looks reasonable to me for XFS.
> > Acked-by: Tim Shimmin <[EMAIL PROTECTED]>
> > 
> > So before, this clearing would happen directly in notify_change()
> > and now this won't happen until notify_change() calls i_op->setattr
> > which for a particular fs it can call generic_attrkill() to do it.
> > So I guess for the cases where i_op->setattr is called outside of
> > via notify_change, we don't normally have ATTR_KILL_SUID/SGID
> > set so that nothing will happen there?
> 
> Right. If neither ATTR_KILL bit is set then generic_attrkill is a
> noop.
> 
> > I guess just wondering the effect with having the code on all
> > setattr's. (I'm not familiar with the code path)
> > 
> 
> These bits are referenced in very few places in the current kernel
> tree -- mostly in the VFS layer. The *only* place I see that they
> actually get interpreted into a mode change is in notify_change. So
> places that call setattr ops w/o going through notify_change are
> not likely to have those bits set.
> 
> But hypothetically, if a fs did set ATTR_KILL_* and call setattr
> directly, then the setattr would now include a mode change that
> clears setuid or setgid bits where it may not have before.

It almost sounds like an argument for a new inode op (NULL would use
generic_attr_kill).

Josef 'Jeff' Sipek.

-- 
A CRAY is the only computer that runs an endless loop in just 4 hours...
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC 12/26] ext2 white-out support

2007-08-01 Thread Josef Sipek
On Wed, Aug 01, 2007 at 02:10:31PM -0500, Dave Kleikamp wrote:
> On Wed, 2007-08-01 at 14:44 -0400, Josef Sipek wrote:
> > Alright not the greatest of examples, there is something to be said about
> > symmetry, so...let me try again :)
> > 
> > /a/
> > /b/bar  (whiteout for bar)
> > /c/foo/qwerty
> > 
> > Now, let's mount a union of {a,b,c}, and we'll see:
> > 
> > $ find /u
> > /u
> > /u/foo
> > /u/foo/qwerty
> > $ mv /u/foo /u/bar
> > 
> > Now what? How do you rename? Do you rename in the same branch (assuming it
> > is rw)?
> 
> Er, no.  According to Documentation/filesystems/union-mounts.txt, "only
> the topmost layer of the mount stack can be altered".
 
This brings up an very interesting (but painful) question...which makes more
sense? Allowing the modifications in only the top-most branch, or any branch
(given the user allows it at mount-time)?

This is really question to the community at large, not just you, Dave :)

> > 1) "cp -r" the entire subtree being renamed to highest-priority branch, and
> > rename there (you might have to recreate a series of directories to have a
> > place to "cp" to...so you got "cp -r" _AND_ "mkdir -p"-like code in the VFS!
> > 1/2 a :) )
> 
> I think this is the only alternative, given the design.
 
Right. Doing something like this at the filesystem level (as we do in
unionfs) seems less painful - filesystems are places full of all sorts of
nefarious activities to begin with. Having it in the VFS seems...even
uglier.

Josef 'Jeff' Sipek.

-- 
*NOTE: This message is ROT-13 encrypted twice for extra protection*
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC 12/26] ext2 white-out support

2007-08-01 Thread Josef Sipek
On Wed, Aug 01, 2007 at 10:23:29AM -0500, Dave Kleikamp wrote:
> On Tue, 2007-07-31 at 13:11 -0400, Josef Sipek wrote:
> > On Tue, Jul 31, 2007 at 07:00:12PM +0200, Jan Blunck wrote:
> > > On Tue, Jul 31, Josef Sipek wrote:
> > > 
> > > > On Mon, Jul 30, 2007 at 06:13:35PM +0200, Jan Blunck wrote:
> > > > > Introduce white-out support to ext2.
> > > > 
> > > > I think storing whiteouts on the branches is wrong. It creates all sort 
> > > > of
> > > > nasty cases when people actually try to use unioning. Imagine a (no-so
> > > > unlikely) scenario where you have 2 unions, and they share a branch. If 
> > > > you
> > > > create a whiteout in one union on that shared branch, the whiteout 
> > > > magically
> > > > affects the other union as well! Whiteouts are a union-level construct, 
> > > > and
> > > > therefore storing them at the branch level is wrong.
> > > 
> > > So you think that just because you mounted the filesystem somewhere else 
> > > it
> > > should look different? This is what sharing is all about. If you share a
> > > filesystem you also share the removal of objects.
> > 
> > The removal happens at the union level, not the branch level. Say you have:
> > 
> > /a/
> > /b/foo
> > /c/foo
> > 
> > And you mount /u1 as a union of {a,b}, and /u2 as union of {a,c}.
> 
> Who does this?  I'm assuming that a is the "top" layer.  Aren't union
> mounts typically about sharing lower layers and having a separate rw
> layer for each union mount?
 
Alright not the greatest of examples, there is something to be said about
symmetry, so...let me try again :)

/a/
/b/bar  (whiteout for bar)
/c/foo/qwerty

Now, let's mount a union of {a,b,c}, and we'll see:

$ find /u
/u
/u/foo
/u/foo/qwerty
$ mv /u/foo /u/bar

Now what? How do you rename? Do you rename in the same branch (assuming it
is rw)? If you do, you'll get:

$ find /u
/u

Oops! There's a whiteout in /b that hides the directory in /c -- rename(2)
shouldn't make directory subtrees disappear.

There are two ways to solve this:

1) "cp -r" the entire subtree being renamed to highest-priority branch, and
rename there (you might have to recreate a series of directories to have a
place to "cp" to...so you got "cp -r" _AND_ "mkdir -p"-like code in the VFS!
1/2 a :) )

2) Don't store whiteouts within branches. This makes it really easy to
rename and remove the whiteout.

Sure, you could try to rename in-place and remove the whiteout, but what if
you have:

/a/
/b/bar  (whiteout)
/c/bar/blah
/d/foo/qwerty

$ mv /u/foo /u/bar

You can't just remove the whiteout, because that'd uncover the whited-out
directory bar in /c.

Josef 'Jeff' Sipek.

-- 
Bad pun of the week: The formula 1 control computer suffered from a race
condition
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC 12/26] ext2 white-out support

2007-08-01 Thread Josef Sipek
On Wed, Aug 01, 2007 at 07:58:49PM +0200, Jan Engelhardt wrote:
> 
> On Jul 31 2007 12:36, Josef Sipek wrote:
> >[2] http://www.filesystems.org/unionfs-odf.txt
> 
> >Instead, the new ODF code stores whiteouts as hardlinks to a special
> >(regular) zero-length file in odf (/odf/whiteout), and it stores opaqueness
> >information for directories in the inode GID bits in an ODF file system
> >(e.g., ext2, XFS, etc.) on the local machine.  This avoids the name-space
> >pollution and avoids races with network file systems, while minimizing inode
> >consummation in /odf.
> 
> Inode GID bits - are you reducing my 32 bits of gid_t to 31 bits?
> That does not work out either.

No. The ODF code just uses the GID bits to store extra info. The GID is
_NOT_ used to store the GID of the file. The GID of the file is still coming
from the branches.

Josef 'Jeff' Sipek.

-- 
I abhor a system designed for the "user", if that word is a coded pejorative
meaning "stupid and unsophisticated."
- Ken Thompson
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC 12/26] ext2 white-out support

2007-08-01 Thread Josef Sipek
On Wed, Aug 01, 2007 at 12:00:42PM +0200, Hans-Peter Jansen wrote:
> Am Dienstag, 31. Juli 2007 19:00 schrieb Jan Blunck:
> > On Tue, Jul 31, Josef Sipek wrote:
> > > On Mon, Jul 30, 2007 at 06:13:35PM +0200, Jan Blunck wrote:
> > > > Introduce white-out support to ext2.
> > >
> > > I think storing whiteouts on the branches is wrong. It creates all sort
> > > of nasty cases when people actually try to use unioning. Imagine a
> > > (no-so unlikely) scenario where you have 2 unions, and they share a
> > > branch. If you create a whiteout in one union on that shared branch,
> > > the whiteout magically affects the other union as well! Whiteouts are a
> > > union-level construct, and therefore storing them at the branch level
> > > is wrong.
> >
> > So you think that just because you mounted the filesystem somewhere else
> > it should look different? This is what sharing is all about. If you share
> > a filesystem you also share the removal of objects.
> 
> No. At least I don't. 
> 
> Usage case: I heavily depend on using union mounts in diskless nfs setups, 
> since it drops the amount of administration of many systems _near_ one. It 
> boils down on installing the distribution of your choice in a directory, 
> union mount it ro, overlayed with a node private one (doing this in initrd 
> on the client for several reasons),

You're not sharing the rw layer so it's a different scenario, and will not
have the problem I'm talking about. See my other post [1] for exact scenario
where storing whiteouts on a branch would cause problems.

> add a little boot and automatic setup 
> machinery and be done. Since all changes are persistant, any system can be 
> set up individually, and still mostly only one tree is needed to keep up to 
> date.. Being in production in an office environment since two years without 
> major hassle (*).

Unionfs is used by many people in this way.

Josef 'Jeff' Sipek.

[1] http://lkml.org/lkml/2007/7/31/365

-- 
Intellectuals solve problems; geniuses prevent them
- Albert Einstein
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC 12/26] ext2 white-out support

2007-07-31 Thread Josef Sipek
On Tue, Jul 31, 2007 at 06:03:06PM +0100, Mark Williamson wrote:
> > Really the only sane way of keeping track of whiteouts seems some external
> > store. We did an experiment with Unionfs, and moving the whiteout handling
> > to effectively a "library" that did all the dirty work cleaned up the code
> > considerably [2,3].
> 
> What about keeping track of whiteouts in a special file (or files) in the top 
> level filesystem of the union?  For instance, having a /.whiteouts file at 
> the root of the top FS in the stack, instead of storing union-specific data 
> in the flags / inode numbers of the lower levels.
 
What is needed is a "filesystem" that has all the directory bits only. For
ODF, we opted to "abuse" existing filesystems to see if it actually helped
Unionfs, and I think it did help. Really, now what we (unionfs) need is a
cleanup of the ODF code, with a bit better defined interface.

...
> This might avoid requiring a store external to the stack of filesystems and I 
> believe it would solve the problem with shared branches and arbitrary 
> stacking that you described?

We generally did a loopback mount on a file. Very similar to your idea.

> I guess a rather similar effect could be had by somehow storing loopback 
> mountable ODF filesystems in the top layer of a union somewhere (e.g. with 
> the default path /.odf) and allowing the user to specify an alternate 
> location at mount time if necessary.  So maybe these approaches are quite 
> similar after all...

Very :) We forced the user to mount the fs in the odf loopback manually, but
there's no reason why we couldn't do an in-kernel mount on unionfs mount
time.

Josef 'Jeff' Sipek.

-- 
Once you have their hardware. Never give it back.
(The First Rule of Hardware Acquisition)
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC 12/26] ext2 white-out support

2007-07-31 Thread Josef Sipek
On Tue, Jul 31, 2007 at 07:00:12PM +0200, Jan Blunck wrote:
> On Tue, Jul 31, Josef Sipek wrote:
> 
> > On Mon, Jul 30, 2007 at 06:13:35PM +0200, Jan Blunck wrote:
> > > Introduce white-out support to ext2.
> > 
> > I think storing whiteouts on the branches is wrong. It creates all sort of
> > nasty cases when people actually try to use unioning. Imagine a (no-so
> > unlikely) scenario where you have 2 unions, and they share a branch. If you
> > create a whiteout in one union on that shared branch, the whiteout magically
> > affects the other union as well! Whiteouts are a union-level construct, and
> > therefore storing them at the branch level is wrong.
> 
> So you think that just because you mounted the filesystem somewhere else it
> should look different? This is what sharing is all about. If you share a
> filesystem you also share the removal of objects.

The removal happens at the union level, not the branch level. Say you have:

/a/
/b/foo
/c/foo

And you mount /u1 as a union of {a,b}, and /u2 as union of {a,c}.

$ find /u*
/u1
/u1/foo
/u2
/u2/foo
$ rm /u1/foo # this creates whiteout for "foo" in /a
$ find /u*
/u1
/u2

Is that what you'd expect as a user? I don't think so.

...
> > Really the only sane way of keeping track of whiteouts seems some external
> > store. We did an experiment with Unionfs, and moving the whiteout handling
> > to effectively a "library" that did all the dirty work cleaned up the code
> > considerably [2,3].
> 
> Haven't checked if you could use ODF for a generic store for filesystems that
> couldn't support whiteouts. This might be an interesting idea.
 
Yes, since the ODF is completely separate, you can use _any_ filesystem and
regardless of whether or not they support whiteouts.

Josef 'Jeff' Sipek.

-- 
Once you have their hardware. Never give it back.
(The First Rule of Hardware Acquisition)
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC 12/26] ext2 white-out support

2007-07-31 Thread Josef Sipek
On Mon, Jul 30, 2007 at 06:13:35PM +0200, Jan Blunck wrote:
> Introduce white-out support to ext2.

I think storing whiteouts on the branches is wrong. It creates all sort of
nasty cases when people actually try to use unioning. Imagine a (no-so
unlikely) scenario where you have 2 unions, and they share a branch. If you
create a whiteout in one union on that shared branch, the whiteout magically
affects the other union as well! Whiteouts are a union-level construct, and
therefore storing them at the branch level is wrong.

If you store whiteouts on the branches, you'll probably want readdir to not
include them. That's relatively cheap if you have a whiteout bit in the
inode, but I don't think filesystems should be forced to use up rather
prescious inode bits for whiteouts/opaqueness [1].

Really the only sane way of keeping track of whiteouts seems some external
store. We did an experiment with Unionfs, and moving the whiteout handling
to effectively a "library" that did all the dirty work cleaned up the code
considerably [2,3].

> Known Bugs:
> - Needs a reserved inode number for white-outs
> - S_OPAQUE isn't persistently stored

Out of curiosity, how do you keep track of opaqueness while the fs is
mounted?

Josef 'Jeff' Sipek.

[1] http://www.mail-archive.com/linux-fsdevel@vger.kernel.org/msg02904.html
[2] http://www.filesystems.org/unionfs-odf.txt
[3] 
http://download.filesystems.org/unionfs/unionfs-2.0-odf/linux-2.6.20-rc6-odf1.diff.gz

-- 
UNIX is user-friendly ... it's just selective about who it's friends are
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC PATCH 2/4] Mount changes to support union mount.

2007-06-21 Thread Josef Sipek
On Wed, Jun 20, 2007 at 02:23:30PM +0530, Bharata B Rao wrote:
> (replying from a different ID as you didn't copy me on reply)
> 
> On 6/20/07, Jan Blunck <[EMAIL PROTECTED]> wrote:
> >On Wed, 20 Jun 2007 11:22:41 +0530, Bharata B Rao wrote:
> >
> >> +/*
> >> + * When propagating mount events to peer group, this is called under
> >> + * vfsmount_lock. Hence using GFP_ATOMIC for kmalloc here.
> >> + * TODO: Can we use a separate kmem cache for union_mount ?
> >> + */
> >> +struct union_mount *alloc_union_mount(struct vfsmount *src_mnt,
> >> + struct dentry *src_dentry, struct vfsmount *dst_mnt,
> >> + struct dentry *dst_dentry)
> >> +{
> >> + struct union_mount *u;
> >> + u = kmalloc(sizeof(struct union_mount), GFP_ATOMIC);
> >> + if (!u)
> >> + return u;
> >> + u->dst_mnt = mntget(dst_mnt);
> >> + u->dst_dentry = dget(dst_dentry);
> >> + u->src_mnt = src_mnt;
> >> + u->src_dentry = dget(src_dentry);
> >> + INIT_LIST_HEAD(&u->hash);
> >> + INIT_LIST_HEAD(&u->list);
> >> + return u;
> >> +}
> >
> >Hmm, you pin the dentries in memory until umount. This isn't good. Besides
> >that this doesn't work with file systems that do invalidate their
> >dentries. The file system must have a chance to replace the dentry in the
> >union structure.
> 
> Yes, both top level and next level dentries are pinned until umount of
> the upper layer. I was thinking if we could prune these from
> prune_dcache(). What do you think ?
> 
> Ok, I haven't thought about filesystem invalidating the dentries. Yet
> to understand the dentry invalidation, but would filesystem invalidate
> an inuse dentry ?
 
NFS, and quite possibly other network/cluster filesystems can encounter all
sort of odd conditions where the dentry might change, no?

Josef "Jeff" Sipek.

-- 
Hegh QaQ law'
quvHa'ghach QaQ puS
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] unionfs section mismatch

2007-06-06 Thread Josef Sipek
On Wed, Jun 06, 2007 at 02:51:02PM -0700, Randy Dunlap wrote:
> From: Randy Dunlap <[EMAIL PROTECTED]>
 
Applied.

Thanks.

Josef "Jeff" Sipek.

-- 
Don't drink and derive. Alcohol and algebra don't mix.
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/2] fs: Fix indentation in do_path_lookup

2007-05-06 Thread Josef Sipek
On Sun, May 06, 2007 at 12:11:24PM +0100, Christoph Hellwig wrote:
> On Sat, May 05, 2007 at 06:59:39PM -0400, Josef 'Jeff' Sipek wrote:
> > Signed-off-by: Josef 'Jeff' Sipek <[EMAIL PROTECTED]>
> > ---
> >  fs/namei.c |2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> > 
> > diff --git a/fs/namei.c b/fs/namei.c
> > index 0262594..600a4e7 100644
> > --- a/fs/namei.c
> > +++ b/fs/namei.c
> > @@ -1159,7 +1159,7 @@ out:
> > if (likely(retval == 0)) {
> > if (unlikely(!audit_dummy_context() && nd && nd->dentry &&
> > nd->dentry->d_inode))
> > -   audit_inode(name, nd->dentry->d_inode);
> > +   audit_inode(name, nd->dentry->d_inode);
> > }
...
> nd is guaranteed to be zero.
 
Should that say non-zero?


Josef "Jeff" Sipek.

-- 
Humans were created by water to transport it upward.
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 1/1] fs: add 4th case to do_path_lookup

2007-04-29 Thread Josef Sipek
Signed-off-by: Josef 'Jeff' Sipek <[EMAIL PROTECTED]>

diff --git a/fs/namei.c b/fs/namei.c
index 2995fba..1516a9b 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -1125,6 +1125,10 @@ static int fastcall do_path_lookup(int dfd, const char 
*name,
nd->mnt = mntget(fs->rootmnt);
nd->dentry = dget(fs->root);
read_unlock(&fs->lock);
+   } else if (flags & LOOKUP_ONE) {
+   /* nd->mnt and nd->dentry already set, just grab references */
+   mntget(nd->mnt);
+   dget(nd->dentry);
} else if (dfd == AT_FDCWD) {
read_lock(&fs->lock);
nd->mnt = mntget(fs->pwdmnt);
diff --git a/include/linux/namei.h b/include/linux/namei.h
index 92b422b..aa89d97 100644
--- a/include/linux/namei.h
+++ b/include/linux/namei.h
@@ -48,6 +48,7 @@ enum {LAST_NORM, LAST_ROOT, LAST_DOT, LAST_DOTDOT, LAST_BIND};
  *  - internal "there are more path compnents" flag
  *  - locked when lookup done with dcache_lock held
  *  - dentry cache is untrusted; force a real lookup
+ *  - lookup path from given dentry/vfsmount pair
  */
 #define LOOKUP_FOLLOW   1
 #define LOOKUP_DIRECTORY2
@@ -55,6 +56,7 @@ enum {LAST_NORM, LAST_ROOT, LAST_DOT, LAST_DOTDOT, LAST_BIND};
 #define LOOKUP_PARENT  16
 #define LOOKUP_NOALT   32
 #define LOOKUP_REVAL   64
+#define LOOKUP_ONE128
 /*
  * Intent data
  */
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 0/1] [RFC] New mode for path_lookup (V1)

2007-04-29 Thread Josef Sipek
Stackable file systems frequently need to lookup paths or path components
starting from an arbitrary point in the namespace (identified by a dentry
and a vfsmount).  Currently, such file systems use lookup_one_len, which is
frowned upon [1] as it does not pass the lookup intent along; not passing a
lookup intent, for example, can trigger BUG_ON's when stacking on top of
NFSv4.

The following patch introduces a new mode to path_lookup to allow lookup to
start from an arbitrary point in the namespace.  This approach has been
suggested by Christoph Hellwig at the Linux Storage & Filesystem workshop in
February of this year.

One indicates that the lookup should be relative to a dentry-vfsmnt pair by
using the LOOKUP_ONE flag.  For example, the following snippet of code,
looks up "pathcomponent" in a directory pointed to by
parent_{dentry,vfsmnt}:

nd.dentry = parent_dentry;
nd.mnt = parent_vfsmnt;

err = path_lookup("pathcomponent", LOOKUP_ONE, &nd);
if (!err) {
/* exits */

...

/* once done, release the references */
path_release(&nd);
} else if (err == -ENOENT) {
/* doesn't exits */
} else {
/* other error */
}

VFS functions such as lookup_create can be used on the nameidata structure
to pass the create intent to the file system.

Currently, there is no easy way to pass the LOOKUP_OPEN intent.  The proper
way would be to call open_namei.

We'd like to get comments about what's necessary to make stackable file
systems do lookups right - this includes potential changes to open_namei. 

Josef 'Jeff' Sipek.

[1] http://marc.info/?l=linux-kernel&m=117343337823760&w=2
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [GIT PULL -mm] Unionfs branch management code

2007-04-10 Thread Josef Sipek
On Tue, Apr 10, 2007 at 01:22:52PM -0400, Shaya Potter wrote:
> Andrew Morton wrote:
> >On Mon,  9 Apr 2007 10:53:51 -0400 "Josef 'Jeff' Sipek" 
> ><[EMAIL PROTECTED]> wrote:
> >
> >>The following patches introduce new branch-management code into Unionfs as
> >>well as fix a number of stability issues and resource leaks.
> >
> >I have a mental note that unionfs is in the "stuck" state, due to general
> >agreement that we should implement this functionality at the VFS level, one
> >reason for which is unionfs's upper-vs-lower coherency problems.
> 
> How can a union file system with a decent set of useful semantics be 
> fully implemented at the VFS layer in a "clean" manner?

Unioning is quite odd. It uses concepts, some of which do indeed belong in
the VFS (like actual merging of the lower directories), but others that most
definitely do not (like whiteouts).

> For instance, a major use of unionfs is live CDs, namely unionfs w/ a 
> read-only and read-write layer.  Unionfs enables files to be copied up 
> from the read-only layer to the read-write layer.
> 
> Does one really want to implement "copyup" in the VFS?
 
I don't think that copyup is the problem, but whiteouts...oh boy.
Whiteouts/some kind of persistent storage is most definitely a filesystem
construct, and it does not belong in the VFS.

Josef "Jeff" Sipek.

-- 
If I have trouble installing Linux, something is wrong. Very wrong.
- Linus Torvalds
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [GIT PULL -mm] Unionfs branch management code

2007-04-09 Thread Josef Sipek
On Mon, Apr 09, 2007 at 10:49:48AM -0700, Andrew Morton wrote:
> On Mon,  9 Apr 2007 10:53:51 -0400 "Josef 'Jeff' Sipek" <[EMAIL PROTECTED]> 
> wrote:
> 
> > The following patches introduce new branch-management code into Unionfs as
> > well as fix a number of stability issues and resource leaks.

First, a quick note...Unionfs used to have branch management code, but the
code was so crufty that we decided to spare the eyes (and brains) of kernel
developers at large by ripping it out. This series of patches just
reintroduces the functionality in a sane way. With that said...

> I have a mental note that unionfs is in the "stuck" state, due to general
> agreement that we should implement this functionality at the VFS level, one
> reason for which is unionfs's upper-vs-lower coherency problems.

Right. The upper-vs-lower coherency problem is indeed a problem, but it is
not a _Unionfs_ problem, but rather a _stackable filesystems_ problem
(eCrypfs suffers from the same issue people are just less likely to trip
over it as no one in their right mind modifies encrypted data by hand). If
we hope to have Linux do stacking (which, I think makes sense), we need to
make few changes to the kernel to allow stackable filesystems to work
better, and safer. We're working on an OLS paper which discusses some of
these ideas (some of which we got at LSF back in February) - for example, do
we want to have strong or weak cache coherency? (When the lower pages
change, do we want to have the VM enforce coherency, or can we use more of
an NFS-like coherency model - checking {a,c,m}time.)

Josef "Jeff" Sipek.

-- 
Intellectuals solve problems; geniuses prevent them
- Albert Einstein
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] unionfs: sioq not __exit

2007-03-19 Thread Josef Sipek
On Mon, Mar 19, 2007 at 03:22:26PM -0700, Randy Dunlap wrote:
> From: Randy Dunlap <[EMAIL PROTECTED]>
> 
> stop_sioq() is called from both __init and __exit functions, so it
> shouldn't be marked __exit.
> 
> Reported on the kernelnewbies mailing list, but no patch offered there.
> 
> Signed-off-by: Randy Dunlap <[EMAIL PROTECTED]>

Applied.

Thanks.

Josef "Jeff" Sipek.

-- 
Debugging is twice as hard as writing the code in the first place.
Therefore, if you write the code as cleverly as possible, you are, by
definition, not smart enough to debug it.
- Brian W. Kernighan 
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/2] eCryptfs: convert lookup_one_len() to lookup_one_len_nd()

2007-03-09 Thread Josef Sipek
On Fri, Mar 09, 2007 at 09:40:00AM +, Christoph Hellwig wrote:
> On Sat, Feb 17, 2007 at 03:56:55AM -0500, Josef 'Jeff' Sipek wrote:
> > From: Michael Halcrow <[EMAIL PROTECTED]>
> > 
> > Call the new lookup_one_len_nd() rather than lookup_one_len().  This fixes 
> > an
> > oops when stacked on NFS.
> > 
> > Note that there are still some issues with eCryptfs on NFS having to do with
> > directory deletion (I'm not getting an oops, just an -EBUSY).
> 
> Biug NACK here.  This is just working around the broken lookup intents
> code.  lookup_one_len still is a hack for some network filesystems that
> unfortunately grew a few too many users.
 
I'm working on the 4-th case to do_path_lookup. The only problem left, is
the negative dentries. do_path_lookup returns -ENOENT. Which would means
that we need either a wrapper that returns the negative dentry instead of 
 
> Implementing this might be a good idea anyway to clean up the mess
> do_path_lookup is currently.

Clean up? There are 4 (including the new case) "setup" cases, and then the
actual lookup happens. You'll need the crazy if conditions unless the mening
of the arguments changes.

Josef "Jeff" Sipek.

-- 
UNIX is user-friendly ... it's just selective about who it's friends are
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 4/4] fs/unionfs/: Don't duplicate the struct nameidata

2007-01-30 Thread Josef Sipek
On Tue, Jan 30, 2007 at 09:42:33AM +, Christoph Hellwig wrote:
> On Mon, Jan 29, 2007 at 03:37:42PM -0500, Josef 'Jeff' Sipek wrote:
> > The only fields that we have to watch out for are the dentry and vfsmount.
> > Additionally, this makes Unionfs gentler on the stack as nameidata is rather
> > large.
> 
> That's onviously not true at all.  To handle any filesystems using intents
> (e.g. NFSv4) you need to do much more.  Then again doing things correctly
> doesn't seem to be interesting to the stackable filesystems crowd an this
> problem has been constantly ignored over the last year, including merging
> ecryptfs which has been broken in the same way.
> 
> Folks, if you want your stackable filesystem toys taken seriously you
> need to fix these kind of issues instead of talking them away.  And yes,
> this will involve some surgery to the VFS.

Indeed. I asked around (#linuxfs) and it seemed that restoring the
dentry/vfsmount was sufficient for the purpose of passing intents down. If
this is not the case, I'll revert the patch to do the full namei allocation.

Josef "Jeff" Sipek.

-- 
Computer Science is no more about computers than astronomy is about
telescopes.
- Edsger Dijkstra
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 4/4] fs/unionfs/: Don't duplicate the struct nameidata

2007-01-29 Thread Josef Sipek
On Mon, Jan 29, 2007 at 03:37:43PM -0500, Josef 'Jeff' Sipek wrote:
> Josef 'Jeff' Sipek (3):
>   fs/unionfs/: Remove stale_inode.c
>   fs/unionfs/: Andrew Morton's comments
>   fs/unionfs/: Don't duplicate the struct nameidata
> 
>  fs/unionfs/branchman.c   |4 +-
>  fs/unionfs/commonfops.c  |   54 +++---
>  fs/unionfs/copyup.c  |   67 +++
>  fs/unionfs/dentry.c  |   19 +++-
>  fs/unionfs/fanout.h  |   51 +
>  fs/unionfs/file.c|   17 +++-
>  fs/unionfs/inode.c   |   69 +++-
>  fs/unionfs/lookup.c  |  113 
> +++---
>  fs/unionfs/main.c|   32 +++---
>  fs/unionfs/rdstate.c |2 +-
>  fs/unionfs/rename.c  |8 ++--
>  fs/unionfs/sioq.c|   19 
>  fs/unionfs/sioq.h|1 -
>  fs/unionfs/stale_inode.c |  112 -
>  fs/unionfs/subr.c|   65 +-
>  fs/unionfs/super.c   |7 +--
>  fs/unionfs/union.h   |   84 +++---
>  fs/unionfs/unlink.c  |8 ++--
>  fs/unionfs/xattr.c   |   16 +++---
>  19 files changed, 330 insertions(+), 418 deletions(-)

Not sure why this was sent. The other 4/4 email is the correct one.

Josef "Jeff" Sipek.

-- 
Bad pun of the week: The formula 1 control computer suffered from a race
condition
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH: 2.6.20-rc4-mm1] JFS: Avoid deadlock introduced by explicit I/O plugging

2007-01-17 Thread Josef Sipek
On Wed, Jan 17, 2007 at 04:55:49PM -0600, Dave Kleikamp wrote:
...
> diff -Nurp linux-2.6.20-rc4-mm1/fs/jfs/jfs_lock.h linux/fs/jfs/jfs_lock.h
> --- linux-2.6.20-rc4-mm1/fs/jfs/jfs_lock.h2006-11-29 15:57:37.0 
> -0600
> +++ linux/fs/jfs/jfs_lock.h   2007-01-17 15:30:19.0 -0600
> @@ -22,6 +22,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  /*
>   *   jfs_lock.h
> @@ -42,6 +43,7 @@ do {
> \
>   if (cond)   \
>   break;  \
>   unlock_cmd; \
> + blk_replug_current_nested();\
>   schedule(); \
>   lock_cmd;   \
>   }   \

Is {,un}lock_cmd a macro? ...

Jeff.

-- 
Keyboard not found!
Press F1 to enter Setup
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Unionfs in -mm

2007-01-12 Thread Josef Sipek
On Tue, Jan 09, 2007 at 03:08:57PM -0500, Jason Lunz wrote:
...
> But I have a bug to report. I'm trying out yesterday's 24-patch series
> on 2.6.20-rc4 (http://thread.gmane.org/gmane.linux.kernel/481661).
> 
> The root filesystem is a union of a ro squashfs and a rw tmpfs.
> The initramfs sets it up something like this:
> 
> mkdir /os
> mount -r -t squashfs /dev/ram0 /os
> 
> mkdir /cow
> mount -t tmpfs -o mode=0755 tmpfs /cow
> 
> mount -w -o dirs=/cow=rw:/os=ro -t unionfs unionfs /root
 
>From the names of the mountpoint I assume you chroot, pivot_root, etc.
Right?
 
Does the problem manifest itself when you mount proc on /root/proc?
 
Thanks for trying the code,

Josef "Jeff" Sipek.

-- 
Research, n.:
  Consider Columbus:
He didn't know where he was going.
When he got there he didn't know where he was.
When he got back he didn't know where he had been.
And he did it all on someone else's money.
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 01/24] Unionfs: Documentation

2007-01-10 Thread Josef Sipek
On Wed, Jan 10, 2007 at 05:12:15PM +0100, Jan Kara wrote:
>   I see :). To me it just sounds as if you want to do remount-read-only
> for source filesystems, which is operation we support perfectly fine,
> and after that create union mount. But I agree you cannot do quite that
> since you need to have write access later from your union mount. So
> maybe it's not so easy as I thought.
>   On the other hand, there was some effort to support read-only bind-mounts of
> read-write filesystems (there were even some patches floating around but
> I don't think they got merged) and that should be even closer to what
> you'd need...

Since the RO flag is per-mount point, how do you guarantee that no one is
messing with the fs? (I haven't looked at the patches that do per mount
ro flag, but this would require some over-arching ro flag - in the
superblock most likely.)

Josef "Jeff" Sipek.

-- 
I think there is a world market for maybe five computers.
- Thomas Watson, chairman of IBM, 1943.
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 01/24] Unionfs: Documentation

2007-01-09 Thread Josef Sipek
On Tue, Jan 09, 2007 at 09:53:45AM +, Christoph Hellwig wrote:
> On Mon, Jan 08, 2007 at 07:03:35PM -0500, Erez Zadok wrote:
> > However, I must caution that a file system like ecryptfs is very different
> > from Unionfs, the latter being a fan-out file system---and both have very
> > different goals.  The common code between the two file systems, at this
> > stage, is not much (and we've already extracted some of it into the "stackfs
> > layer").
> 
> I think that's an very important point.  We have a chance to get that
> non-fanout filesystems right quite easily - something I wished that would
> have been done before the ecryptfs merge - while getting fan-out stackable
> filesystems is a really hard task.

Hard or harder?

> In addition to that I know exactly
> one fan-out stackable filesystem that is posisbly useful, which is unionfs.

RAIF is another fan-out stackable fs with much more complex logic. (Just the
other day, I saw an announcement for a new version on fsdevel.)

Josef "Jeff" Sipek.

-- 
Evolution, n.:
  A hypothetical process whereby infinitely improbable events occur with
  alarming frequency, order arises from chaos, and no one is given credit.
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 01/24] Unionfs: Documentation

2007-01-09 Thread Josef Sipek
On Tue, Jan 09, 2007 at 09:49:35AM +, Christoph Hellwig wrote:
> On Mon, Jan 08, 2007 at 06:25:16PM -0500, Josef Sipek wrote:
> > > There's no such problem with bind mounts.  It's surprising to see such a
> > > restriction with union mounts.
> > 
> > Bind mounts are a purely VFS level construct. Unionfs is, as the name
> > implies, a filesystem. Last year at OLS, it seemed that a lot of people
> > agreed that unioning is neither purely a fs construct, nor purely a vfs
> > construct.
> > 
> > I'm using Unionfs (and ecryptfs) as guinea pigs to make linux fs stacking
> > friendly - a topic to be discussed at LSF in about a month.
> 
> And unionfs is the wrong thing do use for this.  Unioning is a complex
> namespace operation and needs to be implemented in the VFS or at least
> needs a lot of help from the VFS.  Getting namespace cache coherency
> and especially locking right is imposisble with out that.

What I meant was that I use them as an example for a linear and fanout
stacking examples. While unioning itself is a complex operation, the general
idea of one set of vfs objects (dentry, inode, etc.) pointing to several
lower ones is very generic and applies to all fan-out stackable fs.

Josef "Jeff" Sipek.

-- 
Linux, n.:
  Generous programmers from around the world all join forces to help
  you shoot yourself in the foot for free. 
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 01/24] Unionfs: Documentation

2007-01-08 Thread Josef Sipek
On Tue, Jan 09, 2007 at 01:19:48AM +0100, Giuseppe Bilotta wrote:
> As a simple user without much knowledge of kernel internals, much less
> so filesystems, couldn't something based on the same principle of
> lsof+fam be used to handle these situations?

Using inotify has been suggested before. That let the upper filesystem
know when something changed on the lower filesystem.

I think that, while it would work, it is not the right solution.

Josef "Jeff" Sipek.

-- 
Intellectuals solve problems; geniuses prevent them
- Albert Einstein
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 01/24] Unionfs: Documentation

2007-01-08 Thread Josef Sipek
On Mon, Jan 08, 2007 at 05:00:18PM -0600, Michael Halcrow wrote:
> On Mon, Jan 08, 2007 at 03:51:31PM -0500, Erez Zadok wrote:
> > BTW, this is a problem with all stackable file systems, including
> > ecryptfs.  To be fair, our Unionfs users have come up against this
> > problem, usually for the first time they use Unionfs :-).
> 
> I suspect that the only reason why this has not yet surfaced as a
> major issue in eCryptfs is because nobody is bothering to manipulate
> the eCryptfs-encrypted lower files. The only code out there right now
> that can make sense of the files is in the eCryptfs kernel module.
 
Yeah, you got lucky :)
 
> > Detecting such processes may not be easy.  What to do with them,
> > once detected, is also unclear.  We welcome suggestions.
> 
> My first instinct is to say that stacked filesystem should not even
> begin to open the file if it is already opened by something other than
> the stacked filesystem (-EPERM with a message in the syslog about the
> problem).

That seems a rather bit too drastic, no?

> In the case when a stacked filesystem wants to open a file
> that is already opened by something other than the stacked filesystem,
> the stacked filesystem loses. Once the process closes the file, the
> process is hitherto prevented from accessing the file again (via the
> before-mentioned mechanism of hiding the lower namespace).
 
I'd much prefer to somehow link the related pages and invalidate other
"copies" (even after transformations such as encryption). Limiting when
files can be open just sounds nasty.
 
> Unionfs and eCryptfs share almost exactly the same namespace
> issues. Unionfs happens to be impacted by them more than eCryptfs
> because of the differences in how people actually access the files
> under the two filesystems.
 
Yep.
 
> > Jeff, I don't think it's acceptable to OOPS.
> 
> For now, stacked filesystems just need to stay on their toes. There
> are several places where assumptions need to be checked.

I think those places can eliminated by modifying the VFS a bit. Heck, it
might even make the NFS folks' lives easier :)

Josef "Jeff" Sipek.

-- 
Bad pun of the week: The formula 1 control computer suffered from a race
condition
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 01/24] Unionfs: Documentation

2007-01-08 Thread Josef Sipek
On Mon, Jan 08, 2007 at 02:02:24PM -0800, Andrew Morton wrote:
> On Mon, 08 Jan 2007 16:30:48 -0500
> Shaya Potter <[EMAIL PROTECTED]> wrote:
> 
> > On Mon, 2007-01-08 at 13:19 -0800, Andrew Morton wrote:
> > > On Mon, 8 Jan 2007 14:43:39 -0500 (EST) Shaya Potter <[EMAIL PROTECTED]> 
> > > wrote:
> > > >  It's the same thing as modifying a block 
> > > > device while a file system is using it.  Now, when unionfs gets 
> > > > confused, 
> > > > it shouldn't oops, but would one expect ext3 to allow one to modify its 
> > > > backing store while its using it?
> > > 
> > > There's no such problem with bind mounts.  It's surprising to see such a
> > > restriction with union mounts.
> > 
> > the difference is bind mounts are a vfs construct, while unionfs is a
> > file system.
> 
> Well yes.  So the top-level question is "is this the correct way of doing
> unionisation?".
 
Namespace unification doesn't seem to fit into neither vfs-only nor fs-only
category. My guess is that some of the code that's currently in unionfs
could be replaced by some vfs-level magic.
 
> I suspect not, in which case unionfs is at best a stopgap until someone
> comes along and implements unionisation at the VFS level, at which time
> unionfs goes away.
> 
> That could take a long time.  The questions we're left to ponder over are
> things like
> 
> a) is unionfs a sufficiently useful stopgap to justify a merge and

We (unionfs team) think so :)

> b) would an interim merge of unionfs increase or decrease the motivation
>for someone to do a VFS implementation?

And is a VFS implementation the right way to do it?

> I suspect the answer to b) is "increase": if unionfs proves to be useful
> then people will be motivated to produce more robust implementations of the
> same functionality.

I think it would "increase" the chance of people doing the right thing -
whatever it may be.

> Is there vendor interest in unionfs?

Many people are currently using the full - lkml-unsafe :) version of the
code which has a considerable amount of bells and whistles - at least
compared to the minimal version submitted. I want to take the good things
"port" them over and make sure everything is good.

Josef "Jeff" Sipek.

-- 
The box said "Windows XP or better required". So I installed Linux.
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 01/24] Unionfs: Documentation

2007-01-08 Thread Josef Sipek
On Mon, Jan 08, 2007 at 01:19:57PM -0800, Andrew Morton wrote:
... 
> If it's not in the changelog or the documentation, it doesn't exist.

Good point. I'll add it for next time.

> >  It's the same thing as modifying a block 
> > device while a file system is using it.  Now, when unionfs gets confused, 
> > it shouldn't oops, but would one expect ext3 to allow one to modify its 
> > backing store while its using it?
> 
> There's no such problem with bind mounts.  It's surprising to see such a
> restriction with union mounts.

Bind mounts are a purely VFS level construct. Unionfs is, as the name
implies, a filesystem. Last year at OLS, it seemed that a lot of people
agreed that unioning is neither purely a fs construct, nor purely a vfs
construct.

I'm using Unionfs (and ecryptfs) as guinea pigs to make linux fs stacking
friendly - a topic to be discussed at LSF in about a month.

Josef "Jeff" Sipek.

-- 
Computer Science is no more about computers than astronomy is about
telescopes.
- Edsger Dijkstra
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 01/24] Unionfs: Documentation

2007-01-08 Thread Josef Sipek
On Mon, Jan 08, 2007 at 11:18:52AM -0800, Andrew Morton wrote:
> On Sun,  7 Jan 2007 23:12:53 -0500
> "Josef 'Jeff' Sipek" <[EMAIL PROTECTED]> wrote:
> 
> > +Modifying a Unionfs branch directly, while the union is mounted, is
> > +currently unsupported.
> 
> Does this mean that if I have /a/b/ and /c/d/ unionised under /mnt/union, I
> am not allowed to alter anything under /a/b/ and /c/d/?  That I may only
> alter stuff under /mnt/union?
 
That's correct.
 
> If so, that sounds like a significant limitation.

It is significant, and I am trying to figure out a nice clean way to allow
_all_ stackable filesystems (yes, ecryptfs which is in vanilla has the same
problem too) to have data modified under them without any cache incoherence.

> > Any such change can cause Unionfs to oops, or stay
> > silent and even RESULT IN DATA LOSS.
> 
> With a rather rough user interface ;)

That statement is meant to scare people away from modifying the lower fs :)
I tortured unionfs quite a bit, and it can oops but it takes some effort.

> Also, is it possible to add new branches to an existing union mount?  And
> to take old ones away?

Not in the code in the 20-odd patches, it is a minimal version of the code,
maade to be as clean as possible. I hope to   port the  more complex
features into this code when things like lower branch manipulation are fixed
in a real way - there have been suggestions of (ab)using inotify to keep
track of the lower files, but I would prefer to find a real solution.

Josef "Jeff" Sipek.

-- 
Research, n.:
  Consider Columbus:
He didn't know where he was going.
When he got there he didn't know where he was.
When he got back he didn't know where he had been.
And he did it all on someone else's money.
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Is a NULL check missing in nfs_lookup?

2007-01-04 Thread Josef Sipek
On Thu, Jan 04, 2007 at 07:47:34PM -0500, Shaya Potter wrote:
> yes, you're writing a stackable file system (the cs.sunysb gives that
> away) and have run a lookup_one_len() on a nfs mounted file system and
> that means nd is null.
> 
> Erez's group is trying to fix that situation so the intents can be
> passed correctly.

If you look for the last Unionfs submission to lkml/fsdevel, one of the
patches creates lookup_one_len_nd which allows you to pass a nameidata
structure down to the lower filesystem.

Beware, NFSv4 is cranky when it comes to lookup intents. In Unionfs, we have
some additional lookup_one_len's in a number of places that have no
nameidata that we can work off of, and that's causing quite a number of
problems as NFSv4 gets confused when it doesn't see an intent.

Josef "Jeff" Sipek.

-- 
You measure democracy by the freedom it gives its dissidents, not the
freedom it gives its assimilated conformists.
- Abbie Hoffman
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 26/35] Unionfs: Privileged operations workqueue

2006-12-08 Thread Josef Sipek
On Fri, Dec 08, 2006 at 07:03:00PM +0100, Jan Engelhardt wrote:
> On Dec 8 2006 12:43, Josef Sipek wrote:
...
> >args->err is modified. If args is declared const, gcc complains.
> 
> I never said making "args" const, but
> rather [-> http://lkml.org/lkml/2006/12/5/210 ] I said:
> 
>   "Care to make that: const struct mknod_args *m = &args->mknod;?"

Eeek. Sorry.

Josef "Jeff" Sipek.

-- 
NT is to UNIX what a doughnut is to a particle accelerator.
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 26/35] Unionfs: Privileged operations workqueue

2006-12-08 Thread Josef Sipek
On Fri, Dec 08, 2006 at 06:02:28PM +0100, Jan Engelhardt wrote:
> 
> On Dec 8 2006 11:00, Josef Sipek wrote:
> 
> +void __unionfs_mkdir(void *data)
> +{
> + struct sioq_args *args = data;
> + struct mkdir_args *m = &args->mkdir;
> +
> + args->err = vfs_mkdir(m->parent, m->dentry, m->mode);
> + complete(&args->comp);
> +}
> 
> >> The members of m (i.e. m->*) are not modified as for as __unionfs_mknod 
> >> goes.
> >> vfs_mknod may only modify the members of m->parent (i.e. m->parent->*)
> > 
> >Yes they are. The return value is passed through a member of m.
> 
> Where - where do you see that m->parent, m->dentry or m->mode are modified?
> (The original submission is above.)

args->err is modified. If args is declared const, gcc complains. Sorry, the
example code I sent was a bit mis-leading (different from the actual code
above), but it triggered the same check in gcc.

Josef "Jeff" Sipek.

-- 
It used to be said [...] that AIX looks like one space alien discovered
Unix, and described it to another different space alien who then implemented
AIX. But their universal translators were broken and they'd had to gesture a
lot.
- Paul Tomblin 
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/3] ensure unique i_ino in filesystems without permanent inode numbers (libfs superblock cleanup)

2006-12-08 Thread Josef Sipek
On Fri, Dec 08, 2006 at 08:08:03AM -0500, Jeff Layton wrote:
> Josef Sipek wrote:
> >> -  ret = simple_fill_super(sb, IPATHFS_MAGIC, files);
> >> +  ret = simple_fill_super(sb, IPATHFS_MAGIC, files, 1);
> >
> > I don't know...the magic looking 1 and 0 (later in the patch) seem a bit
> > arbitrary. Maybe a #define is in order?
> 
> Yeah, I'm not fond of that, though the comments on simple_fill_super should
> explain it. Basically, I need simple_fill_super to operate in two different
> "modes", and I was using the extra flag to key this. I'm not clear on what
> sort of #define would make sense here. Can you suggest something?
 
First I was thinking about defining 2 constats, but maybe the better thing
to do would be to

1) rename simple_fill_super to __simple_fill_super
2) #define simple_fill_super_foo(...) to __siple_fill_super(., 0)
3) #define simple_fill_super_bar(...) to __siple_fill_super(., 1)

(Or equivalent thing using inline functions.)
 
I can't really think of any good name for #define'd flag.

Beware, I'm pretty much just thinking out loud.. :)
 
> >> @@ -399,7 +407,10 @@ int simple_fill_super(struct super_block
> >>inode->i_blocks = 0;
> >>inode->i_atime = inode->i_mtime = inode->i_ctime =
> >>CURRENT_TIME;
> >
> > I'd indent CURRENT_TIME a bit.
> 
> I wasn't planning on touching those parts of the code that don't need to be
> changed, since formatting deltas can make it harder to see the "actual"
> changes in the patch. That should probably be addressed in a follow-on
> patch if you think it needs to be changed.

Oh, sorry, that wasn't your code. You're right about it not being the the
right thing to fix in your patch.

Actually, it looks like another problem created by line-wrapping.

Josef "Jeff" Sipek.

-- 
NT is to UNIX what a doughnut is to a particle accelerator.
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 26/35] Unionfs: Privileged operations workqueue

2006-12-08 Thread Josef Sipek
On Fri, Dec 08, 2006 at 11:38:13AM +0100, Jan Engelhardt wrote:
> 
> On Dec 7 2006 21:17, Josef Sipek wrote:
> >> >> > >+void __unionfs_mknod(void *data)
> >> >> > >+{
> >> >> > >+struct sioq_args *args = data;
> >> >> > >+struct mknod_args *m = &args->mknod;
> 
> ...
> | vfs_mknod(m->parent, m->dentry, m->mode, m->dev);
> 
> >> >If I make the *args = data line const, then gcc (4.1) yells about 
> >> >modifying
> >> >a const variable 3 lines down..
> >> >
> >> >args->err = vfs_mknod(m->parent, m->dentry, m->mode, m->dev);
> >> >
> >> >Sure, I could cast, but that seems like adding cruft for no good reason.
> >> 
> >> No I despise casts more than missing consts. Why would gcc throw a warning?
> >> Let's take this super simple program
> >
> >No, this program doesn't tickle the problem.. Try to compile this one:
> 
> The members of m (i.e. m->*) are not modified as for as __unionfs_mknod goes.
> vfs_mknod may only modify the members of m->parent (i.e. m->parent->*)
 
Yes they are. The return value is passed through a member of m.
 
Josef "Jeff" Sipek.

-- 
The box said "Windows XP or better required". So I installed Linux.
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/3] ensure unique i_ino in filesystems without permanent inode numbers (libfs superblock cleanup)

2006-12-07 Thread Josef Sipek
On Thu, Dec 07, 2006 at 05:13:08PM -0500, Jeff Layton wrote:
> This patch ensures that the inodes allocated by the functions get_sb_pseudo
> and simple_fill_super are unique, provided of course, that the filesystems
> calling them play by the rules. Currently that isn't the case, but will be
> as I get to each of the filesystems.
> 
> The patch itself is pretty simple, but I also had to fix up the callers of
> simple_fill_super to make sure they passed in the seq flag. For this first
> pass, I set it on any filesystem with an empty "files" struct to 0. That may
> need to be revised as I review each filesystem, but should be OK for now.
> 
> Signed-off-by: Jeff Layton <[EMAIL PROTECTED]>
> 
> diff --git a/drivers/infiniband/hw/ipath/ipath_fs.c 
> b/drivers/infiniband/hw/ipath/ipath_fs.c
> index d9ff283..c127995 100644
> --- a/drivers/infiniband/hw/ipath/ipath_fs.c
> +++ b/drivers/infiniband/hw/ipath/ipath_fs.c
> @@ -514,7 +514,7 @@ static int ipathfs_fill_super(struct sup
>   {""},
>   };
> 
> - ret = simple_fill_super(sb, IPATHFS_MAGIC, files);
> + ret = simple_fill_super(sb, IPATHFS_MAGIC, files, 1);

I don't know...the magic looking 1 and 0 (later in the patch) seem a bit
arbitrary. Maybe a #define is in order?

> -int simple_fill_super(struct super_block *s, int magic, struct tree_descr 
> *files)
> +/*
> + * Some filesystems require that particular entries have particular i_ino 
> values. Those
> + * callers need to set the "seq" flag to make sure that i_ino is assigned 
> sequentially
> + * to the files starting with 0.
> + */
> +int simple_fill_super(struct super_block *s, int magic, struct tree_descr 
> *files, int seq)

Line wraped.

> @@ -399,7 +407,10 @@ int simple_fill_super(struct super_block
>   inode->i_blocks = 0;
>   inode->i_atime = inode->i_mtime = inode->i_ctime = 
>   CURRENT_TIME;

I'd indent CURRENT_TIME a bit.

Josef "Jeff" Sipek.

-- 
If I have trouble installing Linux, something is wrong. Very wrong.
- Linus Torvalds
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 35/35] Unionfs: Extended Attributes support

2006-12-07 Thread Josef Sipek
On Thu, Dec 07, 2006 at 12:04:43PM +0100, Jan Engelhardt wrote:
> 
> On Dec 4 2006 07:31, Josef 'Jeff' Sipek wrote:
> 
> If the makefile contains
> 
> >--- a/fs/unionfs/Makefile
> >+++ b/fs/unionfs/Makefile
> >@@ -3,3 +3,5 @@ obj-$(CONFIG_UNION_FS) += unionfs.o
> > unionfs-y := subr.o dentry.o file.o inode.o main.o super.o \
> > stale_inode.o branchman.o rdstate.o copyup.o dirhelper.o \
> > rename.o unlink.o lookup.o commonfops.o dirfops.o sioq.o
> >+
> >+unionfs-$(CONFIG_UNION_FS_XATTR) += xattr.o
> 
> then you don't need
> 
> >--- a/fs/unionfs/copyup.c
> >+++ b/fs/unionfs/copyup.c
> >@@ -18,6 +18,75 @@
> > 
> > #include "union.h"
> > 
> >+#ifdef CONFIG_UNION_FS_XATTR
> 
> ^^ this, do you?.
 
Beware, copyup.c gets compiled all the time even when you don't have xattrs
enabled.
 
> >+#include "union.h"
> >+
> >+/* This is lifted from fs/xattr.c */
> >+void *xattr_alloc(size_t size, size_t limit)
> 
> Same as for init_inode_cache, possibly prefix with unionfs.

Right.

Josef "Jeff" Sipek.

-- 
A computer without Microsoft is like chocolate cake without mustard.
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 16/35] Unionfs: Copyup Functionality

2006-12-07 Thread Josef Sipek
On Tue, Dec 05, 2006 at 10:09:19PM +0100, Jan Engelhardt wrote:
> 
> On Dec 4 2006 07:30, Josef 'Jeff' Sipek wrote:
> >+/* Determine the mode based on the copyup flags, and the existing dentry. */
> >+static int copyup_permissions(struct super_block *sb,
> >+  struct dentry *old_hidden_dentry,
> >+  struct dentry *new_hidden_dentry)
> >+{
> >+struct inode *i = old_hidden_dentry->d_inode;
> 
> Screams for constification. (Or rather I do.)
 
I'm not following.
 
> >+{
> >+int err = 0;
> >+umode_t old_mode = old_hidden_dentry->d_inode->i_mode;
> 
> Generel question for everybody: Why do we have two same (at least on i386
> that's the case) types, umode_t and mode_t?

No idea.

> >+} else if (S_ISBLK(old_mode)
> >+   || S_ISCHR(old_mode)
> >+   || S_ISFIFO(old_mode)
> >+   || S_ISSOCK(old_mode)) {
> >+args.mknod.parent = new_hidden_parent_dentry->d_inode;
> >+args.mknod.dentry = new_hidden_dentry;
> >+args.mknod.mode = old_mode;
> 
> I'd say the indent got screwed up, and it's not a mailer thing.

Right.

> >+input_file = dentry_open(old_hidden_dentry,
> >+unionfs_lower_mnt_idx(dentry, old_bindex), O_RDONLY | 
> >O_LARGEFILE);
> >+if (IS_ERR(input_file)) {
...
> >+output_file = dentry_open(new_hidden_dentry,
> >+unionfs_lower_mnt_idx(dentry, new_bindex), O_WRONLY | 
> >O_LARGEFILE);
> 
> Here we got an 80-column buster.

/me whistles innocently

> >+/* TODO: should we reset the error to something like -EIO? */
> 
> Handle it :)  - if it does not take a paper.

I'm not even sure if we want to reset it to begin with.

If we don't reset, the user may get some non-sensical errors, but on the
other hand, if we reset to EIO, we guarantee that the user will get a
"confusing" error message.

Josef "Jeff" Sipek.

-- 
All science is either physics or stamp collecting.
- Ernest Rutherford
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 15/35] Unionfs: Common file operations

2006-12-07 Thread Josef Sipek
On Tue, Dec 05, 2006 at 10:02:10PM +0100, Jan Engelhardt wrote:
> On Dec 4 2006 07:30, Josef 'Jeff' Sipek wrote:
> >+long unionfs_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
> >+{
> >+long err;
> >+
> >+if ((err = unionfs_file_revalidate(file, 1)))
> >+goto out;
> >+
> >+/* check if asked for local commands */
> >+switch (cmd) {
> >+case UNIONFS_IOCTL_INCGEN:
> >+/* Increment the superblock generation count */
> >+err = -EACCES;
> >+if (!capable(CAP_SYS_ADMIN))
> >+goto out;
> >+err = unionfs_ioctl_incgen(file, cmd, arg);
> >+break;
> >+
> >+case UNIONFS_IOCTL_QUERYFILE:
> >+/* Return list of branches containing the given file */
> >+err = unionfs_ioctl_queryfile(file, cmd, arg);
> >+break;
> >+
> >+default:
> >+/* pass the ioctl down */
> >+err = do_ioctl(file, cmd, arg);
> >+break;
> >+}
> >+
> >+out:
> >+return err;
> >+}
> 
> 
> I think there was an ioctl for files to find out where a particular
> file lives on disk.

That's the UNIONFS_IOCTL_QUERYFILE case.

> Do you think unionfs should handle it and return
> something more or less meaningful?

It is a very useful functionality for any stackable filesystem which may
store files on one of several branches. Do I think that it should be
factored out of Unionfs into fsstack or similar? Not at the moment. If there
seems to be a need for it, then I'd gladly move it out of Unionfs, but until
then I don't see a reason to do anything special.

Josef "Jeff" Sipek.

-- 
Debugging is twice as hard as writing the code in the first place.
Therefore, if you write the code as cleverly as possible, you are, by
definition, not smart enough to debug it.
- Brian W. Kernighan 
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 26/35] Unionfs: Privileged operations workqueue

2006-12-07 Thread Josef Sipek
On Wed, Dec 06, 2006 at 07:46:50PM +0100, Jan Engelhardt wrote:
> I smell a big conspiracy! So yet again it's mixed mixed
> 
> fs$ grep __init */*.c | grep -v ' init_'
> sysfs/mount.c:int __init sysfs_init(void)
> sysv/inode.c:int __init sysv_init_icache(void)
> proc/vmcore.c:static int __init vmcore_init(void)
> proc/nommu.c:static int __init proc_nommu_init(void)
> proc/proc_misc.c:void __init proc_misc_init(void)
> proc/proc_tty.c:void __init proc_tty_init(void)
> proc/root.c:void __init proc_root_init(void)
 
Yep.
 
> >> > >+void __unionfs_mknod(void *data)
> >> > >+{
> >> > >+   struct sioq_args *args = data;
> >> > >+   struct mknod_args *m = &args->mknod;
> >> > 
> >> > Care to make that: const struct mknod_args *m = &args->mknod;?
> >> > (Same for other places)
> >>  
> >> Right.
> > 
> >If I make the *args = data line const, then gcc (4.1) yells about modifying
> >a const variable 3 lines down..
> >
> >args->err = vfs_mknod(m->parent, m->dentry, m->mode, m->dev);
> >
> >Sure, I could cast, but that seems like adding cruft for no good reason.
> 
> No I despise casts more than missing consts. Why would gcc throw a warning?
> Let's take this super simple program

No, this program doesn't tickle the problem.. Try to compile this one:

<<<
struct mknod_args {
int mode;
int dev;
};

void  __mknod(const void *data)
{
const struct mknod_args *args = data;
args->mode = 0;
}

int main(void) {
const struct mknod_args *m;
__mknod(m);
return 0;
}
>>>

$ gcc -Wall -c test.c
test.c: In function âmknodâtest.c:10: error: assignment of read-only location


Josef "Jeff" Sipek.

-- 
Reality is merely an illusion, albeit a very persistent one.
- Albert Einstein
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC][PATCH] Secure Deletion and Trash-Bin Support for Ext4

2006-12-06 Thread Josef Sipek
On Wed, Dec 06, 2006 at 07:14:58PM -0800, Nicholas Miell wrote:
> On Thu, 2006-12-07 at 13:49 +1100, David Chinner wrote:
> > On Wed, Dec 06, 2006 at 09:35:30PM -0500, Josef Sipek wrote:
> > > On Thu, Dec 07, 2006 at 12:44:27PM +1100, David Chinner wrote:
> > > > Maybe we should be using EAs for this sort of thing instead of flags
> > > > on the inode? If we keep adding inode flags for generic features
> > > > then we are going to force more than just XFS into inode format
> > > > changes eventually
> > > 
> > > Aren't EAs slow? Maybe not on XFS but on other filesystems...
> > 
> > Only when they don't fit in the inode itself and extra
> > disk seeks are needed to retrieve them.
> > 
> > Cheers,
> > 
> > Dave.
> 
> Also keep in mind that the EA doesn't actually have to have a physical
> representation on disk (or, rather, it does, but it doesn't need to be
> the same representation used by EAs in the user namespace).
> 
> This means that if one of those slow EA filesystems still has room for
> flags in the inode, it can synthesize the EA on demand.
 
Interesting point. Filesystems such as XFS would be unaffected (at least
with attr v1, not sure how attr2 does things in XFS wrt EAs.)

Another question is, suppose that filesystem x is one of the slow EA
filesystems, exposing this whiteout inode flag as EA would require fs x to
be compiled with EA support. As a matter of fact, it would require all the
filesystems to have EA support. Sure, overall it is less work for me, but I
am more interested in doing the Right Thing(tm) - which may be something
completely different from EA or inode flag (e.g., what Ted Tso suggested at
OLS - effectively a metadata-only ondisk format for unionfs.)
 
> This is even preferable to ioctls for the interface to new filesystem
> metadata -- if a backup or archive program knows how to store EAs, it
> will be able to save and restore any new exotic metadata without any
> extra effort.

Agreed.

Josef "Jeff" Sipek.

-- 
UNIX is user-friendly ... it's just selective about who it's friends are
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC][PATCH] Secure Deletion and Trash-Bin Support for Ext4

2006-12-06 Thread Josef Sipek
On Thu, Dec 07, 2006 at 12:44:27PM +1100, David Chinner wrote:
> Maybe we should be using EAs for this sort of thing instead of flags
> on the inode? If we keep adding inode flags for generic features
> then we are going to force more than just XFS into inode format
> changes eventually

Aren't EAs slow? Maybe not on XFS but on other filesystems...

Josef "Jeff" Sipek.

-- 
Linux, n.:
  Generous programmers from around the world all join forces to help
  you shoot yourself in the foot for free. 
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC][PATCH] Secure Deletion and Trash-Bin Support for Ext4

2006-12-06 Thread Josef Sipek
On Wed, Dec 06, 2006 at 08:11:00PM +1100, David Chinner wrote:
> They are defined but unused in 2.6.19, right? I can't see anywhere
> in the 2.6.19 ext2/3/4/reiser trees that actually those flags,
> including setting and retrieving them from disk. JFS i can see
> sets, clears and retreives them, but not the fielsystems you
> mention. Though I might just be blind. ;)
> 
> If all we need to add to XFS is support for those flags, then XFS
> support would be trivial to add.
> 
> Oh, damn. I take that back. We're almost out of flag space in the on
> disk inode - these two flags would use the last 2 flag bits so this
> may require an on disk inode format change in XFS. This will be
> a little more complex than I first thought, but not impossible
> as we already support two on-disk inode format versions.
 
Hrm. I was toying around with the idea of using a flag to mark inodes as
whiteouts (similar to what BSD does) for Unionfs. I remember that Jan Blunck
tried similar thing in his implementation of VFS unionfs mounts.

I am not entirely convinced that whiteout inode flag is the right way to do
things, but I'm just raising this now as I wouldn't want to wait for new
ondisk format for XFS to say that Unionfs supports XFS. (Assuming that it is
the right approach.) ;-)
 
Josef "Jeff" Sipek.

-- 
I already backed up the box once, I can do it again!
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 26/35] Unionfs: Privileged operations workqueue

2006-12-06 Thread Josef Sipek
On Tue, Dec 05, 2006 at 02:50:13PM -0500, Josef Sipek wrote:
> On Tue, Dec 05, 2006 at 08:27:51PM +0100, Jan Engelhardt wrote:
> > 
> > On Dec 4 2006 07:30, Josef 'Jeff' Sipek wrote:
> > >+#include "union.h"
> > >+
> > >+struct workqueue_struct *sioq;
> > >+
> > >+int __init init_sioq(void)
> > 
> > Although it's just me, I'd prefer sioq_init(), sioq_exit(),
> > sioq_run(), etc. to go in hand with what most drivers use as naming
> > (i.e.  "_" ).
> 
> That makes sense.
 
Hrm. Looking at the code, I noticed that the opposite is true:

destroy_filldir_cache();
destroy_inode_cache();
destroy_dentry_cache();
unregister_filesystem(&unionfs_fs_type);

The last one in particular...
 
> > >+void __unionfs_mknod(void *data)
> > >+{
> > >+  struct sioq_args *args = data;
> > >+  struct mknod_args *m = &args->mknod;
> > 
> > Care to make that: const struct mknod_args *m = &args->mknod;?
> > (Same for other places)
>  
> Right.
 
If I make the *args = data line const, then gcc (4.1) yells about modifying
a const variable 3 lines down..

args->err = vfs_mknod(m->parent, m->dentry, m->mode, m->dev);

Sure, I could cast, but that seems like adding cruft for no good reason.

Josef "Jeff" Sipek.

-- 
Only two things are infinite, the universe and human stupidity, and I'm not
sure about the former.
- Albert Einstein
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 26/35] Unionfs: Privileged operations workqueue

2006-12-05 Thread Josef Sipek
On Tue, Dec 05, 2006 at 08:27:51PM +0100, Jan Engelhardt wrote:
> 
> On Dec 4 2006 07:30, Josef 'Jeff' Sipek wrote:
> >+#include "union.h"
> >+
> >+struct workqueue_struct *sioq;
> >+
> >+int __init init_sioq(void)
> 
> Although it's just me, I'd prefer sioq_init(), sioq_exit(),
> sioq_run(), etc. to go in hand with what most drivers use as naming
> (i.e.  "_" ).

That makes sense.

> >+sioq = create_workqueue("unionfs_siod");
> 
> Beat me: what does SIO stand for?

Super-user IO - sometimes we need to perform actions which would fail due to
the unix permissions on the parent directory (e.g., rmdir a directory which
appears empty, but in reality contains whiteouts).

> >+void fin_sioq(void)
> 
> No __exit tag?

Good catch.

> >+void __unionfs_mknod(void *data)
> >+{
> >+struct sioq_args *args = data;
> >+struct mknod_args *m = &args->mknod;
> 
> Care to make that: const struct mknod_args *m = &args->mknod;?
> (Same for other places)
 
Right.
 
> >+
> >+args->err = vfs_mknod(m->parent, m->dentry, m->mode, m->dev);
> >+complete(&args->comp);
> >+}
> >+void __unionfs_symlink(void *data)
> >+{
> 
> Hah, missing free line :^)

:)

> >+struct sioq_args *args = data;
> >+struct symlink_args *s = &args->symlink;
> >+
> >+args->err = vfs_symlink(s->parent, s->dentry, s->symbuf, s->mode);
> >+complete(&args->comp);
> >+}
> >+
> 
> >+++ b/fs/unionfs/sioq.h
> >+struct isopaque_args {
> >+struct dentry *dentry;
> >+};
> 
> This name puzzled me at first. iso - paque - "same (o)paque"?
> Try is_opaque_args.

Will do.

> >+/* Extern definitions for our privledge escalation helpers */
> 
> -> privilege

Why do I keep misspelling that?

Thanks for the comments.

Josef "Jeff" Sipek.

-- 
Bad pun of the week: The formula 1 control computer suffered from a race
condition
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Unionfs: Stackable namespace unification filesystem

2006-12-05 Thread Josef Sipek
On Mon, Dec 04, 2006 at 07:30:33AM -0500, Josef 'Jeff' Sipek wrote:
> The following patches are in a git repo at:
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/jsipek/unionfs.git
> 
> (master.kernel.org:/pub/scm/linux/kernel/git/jsipek/unionfs.git)
> 
> The repository contains the following 35 commits (also available as patches
> in replies to this email).
 
Wow, I completely forgot to say what changed since the previous posting...

- Added more comments (akpm)
- Cleaned up inode/dentry/file/sb private data structure member names
- Moved struct inode from unionfs_inode_container into unionfs_inode_info
(following ext2's example)
- Renamed {d,i,f,s}topd to UNIONFS_{D,I,F,S} (following almost any fs's
example)
- Removed *_ptr and *_lhs macros, open-coding the assignments (Pekka Enberg)
- Kill wrappers (e.g., unionfs_kill_block_super) (Pekka Enberg)
- Few tiny coding style fixes
- Removed some unnecessary complexity (no need to pass struct file and a
struct dentry for that file to a function - just pass a struct file
and dereference in the function, etc.)
- Removed some unnecessary checks (if (foo) kfree(foo); is completely
redundant as kfree has a null check)
- Changed C++-style comments to C-style comments (Pekka Enberg)
- Use struct kmem_cache instead of kmem_cache_t (Pekka Enberg)
- Use anonymous unions for sioq (Jan Engelhardt)
- Moved some functionality into fsstack (most of it is in -mm already)

All in all about 1/8th of the code was touched in one way or another.
 
Josef "Jeff" Sipek.
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html