Re: [patch 1/2] VFS: new fgetattr() file operation

2007-09-24 Thread Miklos Szeredi
> > But it's has various dawbacks, like rmdir doesn't work if there are
> > open files within an otherwise empty directory.
> > 
> > I'd happily accept suggestions on how to deal with this differenty.
> 
> NFS has that problem because it really has to sillyrename into the same
> directory. I don't see that ssh/sftp needs to do that. Instead it can
> sillyrename anywhere in the filesystem.

I don't think it can.  How can we find in a reliable way another
directory, which is writable by the user?

Miklos
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch 1/2] VFS: new fgetattr() file operation

2007-09-24 Thread Alan Cox
> But it's has various dawbacks, like rmdir doesn't work if there are
> open files within an otherwise empty directory.
> 
> I'd happily accept suggestions on how to deal with this differenty.

NFS has that problem because it really has to sillyrename into the same
directory. I don't see that ssh/sftp needs to do that. Instead it can
sillyrename anywhere in the filesystem.

Alan
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch 1/2] VFS: new fgetattr() file operation

2007-09-24 Thread Miklos Szeredi
> On Mon, Sep 24, 2007 at 03:18:10PM +0200, Miklos Szeredi wrote:
> > > Or not support such a broken protocol at all.
> > 
> > Wonder what people would say if we removed support for NFSv[23].
> > 
> > Just because a protocol does not support "perfect" UNIX semantics, it
> > doesn't mean it's broken.  By that standard almost all network
> > filesystem protocols are severely broken.
> 
> Well, they are broken by these and other standards.  At least nfs and
> cifs maintainers do the workarounds for this brokeness where they belong.

And my patch is not working around a problem, rather solving a problem
in a correct way.

Let me summarise it:

There are valid reasons we have fstat() in addition to stat/lstat.
For example we want to protect agains races involving unlink/rename on
an open file.

Say I want to implement a network filesystem with a pure unprivileged
userspace sever (this is basically what sshfs does).

I want my filesystem client implementation to keep all these
advantages of fstat().  So how can I do that?  There's a simple way:
implement this operation with fstat() on the server.  I get all the
advantages on the remote system automatically.

But for that the filesystem needs to have the open file that the
fstat() on the client was performed on.

It's that simple.  There's really no ugly hacks going on behind the
scenes.  It's just that we do want to delegate some properties of this
operation onto the server, and the simplest and best implementation is
to just let the filesystem have the information it needs.

Why is that such a big problem?

Miklos
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch 1/2] VFS: new fgetattr() file operation

2007-09-24 Thread Christoph Hellwig
On Mon, Sep 24, 2007 at 03:18:10PM +0200, Miklos Szeredi wrote:
> > Or not support such a broken protocol at all.
> 
> Wonder what people would say if we removed support for NFSv[23].
> 
> Just because a protocol does not support "perfect" UNIX semantics, it
> doesn't mean it's broken.  By that standard almost all network
> filesystem protocols are severely broken.

Well, they are broken by these and other standards.  At least nfs and
cifs maintainers do the workarounds for this brokeness where they belong.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch 1/2] VFS: new fgetattr() file operation

2007-09-24 Thread Miklos Szeredi
> On Mon, Sep 24, 2007 at 03:06:06PM +0200, Miklos Szeredi wrote:
> > A file isn't deleted while there are still links or open files
> > refering to it.  So getting the attributes for a file with nlink==0 is
> > perfectly valid while the file is still open.
> 
> Is it?  Why not just pretend that the attributes are wiped when the file
> is deleted.

You mean "when finally unlinked"?  Delete happens when the file is
closed.

> Effectively, they are, since they can't affect anything.

Sure it can.  It may be open on the server as well.

> > If a network filesystem protocol can't handle operations (be it data
> > or metadata) on an unlinked file, we must do sillirenaming, so that
> > the file is not actually unlinked.
> 
> Or you could call getattr right before you unlink and cache the result
> in the client.

The file can still be modified after being unlinked.

And even if we did this caching thing and modify the attributes when
the file is modified, it would not deal with access on the remote end,
and would be much more complex than the other alternatives.

Miklos
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch 1/2] VFS: new fgetattr() file operation

2007-09-24 Thread Miklos Szeredi
> > If a network filesystem protocol can't handle operations (be it data
> > or metadata) on an unlinked file, we must do sillirenaming, so that
> > the file is not actually unlinked.
> 
> Or not support such a broken protocol at all.

Wonder what people would say if we removed support for NFSv[23].

Just because a protocol does not support "perfect" UNIX semantics, it
doesn't mean it's broken.  By that standard almost all network
filesystem protocols are severely broken.

Miklos
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch 1/2] VFS: new fgetattr() file operation

2007-09-24 Thread Matthew Wilcox
On Mon, Sep 24, 2007 at 03:06:06PM +0200, Miklos Szeredi wrote:
> A file isn't deleted while there are still links or open files
> refering to it.  So getting the attributes for a file with nlink==0 is
> perfectly valid while the file is still open.

Is it?  Why not just pretend that the attributes are wiped when the file
is deleted.  Effectively, they are, since they can't affect anything.

> If a network filesystem protocol can't handle operations (be it data
> or metadata) on an unlinked file, we must do sillirenaming, so that
> the file is not actually unlinked.

Or you could call getattr right before you unlink and cache the result
in the client.

-- 
Intel are signing my paycheques ... these opinions are still mine
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch 1/2] VFS: new fgetattr() file operation

2007-09-24 Thread Christoph Hellwig
On Mon, Sep 24, 2007 at 03:06:06PM +0200, Miklos Szeredi wrote:
> If a network filesystem protocol can't handle operations (be it data
> or metadata) on an unlinked file, we must do sillirenaming, so that
> the file is not actually unlinked.

Or not support such a broken protocol at all.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch 1/2] VFS: new fgetattr() file operation

2007-09-24 Thread Miklos Szeredi
> > > and if that means adding silly rename support so be it.
> > 
> > That's what is done currently.
> > 
> > But it's has various dawbacks, like rmdir doesn't work if there are
> > open files within an otherwise empty directory.
> > 
> > I'd happily accept suggestions on how to deal with this differenty.
> 
> Only sillyrename files with nlink > 1?  I don't see how attributes can
> change anything for a deleted file.

I don't quite understand your suggestion.

A file isn't deleted while there are still links or open files
refering to it.  So getting the attributes for a file with nlink==0 is
perfectly valid while the file is still open.

If a network filesystem protocol can't handle operations (be it data
or metadata) on an unlinked file, we must do sillirenaming, so that
the file is not actually unlinked.

Miklos
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch 1/2] VFS: new fgetattr() file operation

2007-09-24 Thread Matthew Wilcox
On Mon, Sep 24, 2007 at 02:48:08PM +0200, Miklos Szeredi wrote:
> > and if that means adding silly rename support so be it.
> 
> That's what is done currently.
> 
> But it's has various dawbacks, like rmdir doesn't work if there are
> open files within an otherwise empty directory.
> 
> I'd happily accept suggestions on how to deal with this differenty.

Only sillyrename files with nlink > 1?  I don't see how attributes can
change anything for a deleted file.

-- 
Intel are signing my paycheques ... these opinions are still mine
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch 1/2] VFS: new fgetattr() file operation

2007-09-24 Thread Miklos Szeredi
> On Mon, Sep 24, 2007 at 02:24:54PM +0200, Miklos Szeredi wrote:
> > Thanks to everyone for the feedback.  Here's two of the VFS patches
> > reworked according to comments.  I also plan to rework the setattr()
> > patch accordingly and perhaps the xattr patch, altough that is the
> > lowest priority.
> > 
> > Christoph, are these OK with you in this form?
> 
> Not at all.  Attribute operations like this have no business at all
> looking at the struct file.  Please fix your dreaded filesystem to
> implement proper unix semantics intead,

It's a fixed protocol, with servers installed on millions of servers.
The protocol is called SFTP and the server is part of the OpenSSH
package.  There's nothing I can change there.

And even if I could change the protocol, it's impossible to implement
full UNIX semantics with a userspace server.  Please think a bit about
it.

> and if that means adding silly rename support so be it.

That's what is done currently.

But it's has various dawbacks, like rmdir doesn't work if there are
open files within an otherwise empty directory.

I'd happily accept suggestions on how to deal with this differenty.

Thanks,
Miklos
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch 1/2] VFS: new fgetattr() file operation

2007-09-24 Thread Christoph Hellwig
On Mon, Sep 24, 2007 at 02:24:54PM +0200, Miklos Szeredi wrote:
> Thanks to everyone for the feedback.  Here's two of the VFS patches
> reworked according to comments.  I also plan to rework the setattr()
> patch accordingly and perhaps the xattr patch, altough that is the
> lowest priority.
> 
> Christoph, are these OK with you in this form?

Not at all.  Attribute operations like this have no business at all
looking at the struct file.  Please fix your dreaded filesystem to
implement proper unix semantics intead, and if that means adding
silly rename support so be it.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch 1/2] VFS: new fgetattr() file operation

2007-09-24 Thread Christoph Hellwig
On Mon, Sep 24, 2007 at 02:24:54PM +0200, Miklos Szeredi wrote:
 Thanks to everyone for the feedback.  Here's two of the VFS patches
 reworked according to comments.  I also plan to rework the setattr()
 patch accordingly and perhaps the xattr patch, altough that is the
 lowest priority.
 
 Christoph, are these OK with you in this form?

Not at all.  Attribute operations like this have no business at all
looking at the struct file.  Please fix your dreaded filesystem to
implement proper unix semantics intead, and if that means adding
silly rename support so be it.

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch 1/2] VFS: new fgetattr() file operation

2007-09-24 Thread Miklos Szeredi
 On Mon, Sep 24, 2007 at 02:24:54PM +0200, Miklos Szeredi wrote:
  Thanks to everyone for the feedback.  Here's two of the VFS patches
  reworked according to comments.  I also plan to rework the setattr()
  patch accordingly and perhaps the xattr patch, altough that is the
  lowest priority.
  
  Christoph, are these OK with you in this form?
 
 Not at all.  Attribute operations like this have no business at all
 looking at the struct file.  Please fix your dreaded filesystem to
 implement proper unix semantics intead,

It's a fixed protocol, with servers installed on millions of servers.
The protocol is called SFTP and the server is part of the OpenSSH
package.  There's nothing I can change there.

And even if I could change the protocol, it's impossible to implement
full UNIX semantics with a userspace server.  Please think a bit about
it.

 and if that means adding silly rename support so be it.

That's what is done currently.

But it's has various dawbacks, like rmdir doesn't work if there are
open files within an otherwise empty directory.

I'd happily accept suggestions on how to deal with this differenty.

Thanks,
Miklos
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch 1/2] VFS: new fgetattr() file operation

2007-09-24 Thread Matthew Wilcox
On Mon, Sep 24, 2007 at 02:48:08PM +0200, Miklos Szeredi wrote:
  and if that means adding silly rename support so be it.
 
 That's what is done currently.
 
 But it's has various dawbacks, like rmdir doesn't work if there are
 open files within an otherwise empty directory.
 
 I'd happily accept suggestions on how to deal with this differenty.

Only sillyrename files with nlink  1?  I don't see how attributes can
change anything for a deleted file.

-- 
Intel are signing my paycheques ... these opinions are still mine
Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step.
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch 1/2] VFS: new fgetattr() file operation

2007-09-24 Thread Miklos Szeredi
   and if that means adding silly rename support so be it.
  
  That's what is done currently.
  
  But it's has various dawbacks, like rmdir doesn't work if there are
  open files within an otherwise empty directory.
  
  I'd happily accept suggestions on how to deal with this differenty.
 
 Only sillyrename files with nlink  1?  I don't see how attributes can
 change anything for a deleted file.

I don't quite understand your suggestion.

A file isn't deleted while there are still links or open files
refering to it.  So getting the attributes for a file with nlink==0 is
perfectly valid while the file is still open.

If a network filesystem protocol can't handle operations (be it data
or metadata) on an unlinked file, we must do sillirenaming, so that
the file is not actually unlinked.

Miklos
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch 1/2] VFS: new fgetattr() file operation

2007-09-24 Thread Christoph Hellwig
On Mon, Sep 24, 2007 at 03:06:06PM +0200, Miklos Szeredi wrote:
 If a network filesystem protocol can't handle operations (be it data
 or metadata) on an unlinked file, we must do sillirenaming, so that
 the file is not actually unlinked.

Or not support such a broken protocol at all.

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch 1/2] VFS: new fgetattr() file operation

2007-09-24 Thread Matthew Wilcox
On Mon, Sep 24, 2007 at 03:06:06PM +0200, Miklos Szeredi wrote:
 A file isn't deleted while there are still links or open files
 refering to it.  So getting the attributes for a file with nlink==0 is
 perfectly valid while the file is still open.

Is it?  Why not just pretend that the attributes are wiped when the file
is deleted.  Effectively, they are, since they can't affect anything.

 If a network filesystem protocol can't handle operations (be it data
 or metadata) on an unlinked file, we must do sillirenaming, so that
 the file is not actually unlinked.

Or you could call getattr right before you unlink and cache the result
in the client.

-- 
Intel are signing my paycheques ... these opinions are still mine
Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step.
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch 1/2] VFS: new fgetattr() file operation

2007-09-24 Thread Miklos Szeredi
  If a network filesystem protocol can't handle operations (be it data
  or metadata) on an unlinked file, we must do sillirenaming, so that
  the file is not actually unlinked.
 
 Or not support such a broken protocol at all.

Wonder what people would say if we removed support for NFSv[23].

Just because a protocol does not support perfect UNIX semantics, it
doesn't mean it's broken.  By that standard almost all network
filesystem protocols are severely broken.

Miklos
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch 1/2] VFS: new fgetattr() file operation

2007-09-24 Thread Miklos Szeredi
 On Mon, Sep 24, 2007 at 03:06:06PM +0200, Miklos Szeredi wrote:
  A file isn't deleted while there are still links or open files
  refering to it.  So getting the attributes for a file with nlink==0 is
  perfectly valid while the file is still open.
 
 Is it?  Why not just pretend that the attributes are wiped when the file
 is deleted.

You mean when finally unlinked?  Delete happens when the file is
closed.

 Effectively, they are, since they can't affect anything.

Sure it can.  It may be open on the server as well.

  If a network filesystem protocol can't handle operations (be it data
  or metadata) on an unlinked file, we must do sillirenaming, so that
  the file is not actually unlinked.
 
 Or you could call getattr right before you unlink and cache the result
 in the client.

The file can still be modified after being unlinked.

And even if we did this caching thing and modify the attributes when
the file is modified, it would not deal with access on the remote end,
and would be much more complex than the other alternatives.

Miklos
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch 1/2] VFS: new fgetattr() file operation

2007-09-24 Thread Christoph Hellwig
On Mon, Sep 24, 2007 at 03:18:10PM +0200, Miklos Szeredi wrote:
  Or not support such a broken protocol at all.
 
 Wonder what people would say if we removed support for NFSv[23].
 
 Just because a protocol does not support perfect UNIX semantics, it
 doesn't mean it's broken.  By that standard almost all network
 filesystem protocols are severely broken.

Well, they are broken by these and other standards.  At least nfs and
cifs maintainers do the workarounds for this brokeness where they belong.
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch 1/2] VFS: new fgetattr() file operation

2007-09-24 Thread Alan Cox
 But it's has various dawbacks, like rmdir doesn't work if there are
 open files within an otherwise empty directory.
 
 I'd happily accept suggestions on how to deal with this differenty.

NFS has that problem because it really has to sillyrename into the same
directory. I don't see that ssh/sftp needs to do that. Instead it can
sillyrename anywhere in the filesystem.

Alan
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch 1/2] VFS: new fgetattr() file operation

2007-09-24 Thread Miklos Szeredi
 On Mon, Sep 24, 2007 at 03:18:10PM +0200, Miklos Szeredi wrote:
   Or not support such a broken protocol at all.
  
  Wonder what people would say if we removed support for NFSv[23].
  
  Just because a protocol does not support perfect UNIX semantics, it
  doesn't mean it's broken.  By that standard almost all network
  filesystem protocols are severely broken.
 
 Well, they are broken by these and other standards.  At least nfs and
 cifs maintainers do the workarounds for this brokeness where they belong.

And my patch is not working around a problem, rather solving a problem
in a correct way.

Let me summarise it:

There are valid reasons we have fstat() in addition to stat/lstat.
For example we want to protect agains races involving unlink/rename on
an open file.

Say I want to implement a network filesystem with a pure unprivileged
userspace sever (this is basically what sshfs does).

I want my filesystem client implementation to keep all these
advantages of fstat().  So how can I do that?  There's a simple way:
implement this operation with fstat() on the server.  I get all the
advantages on the remote system automatically.

But for that the filesystem needs to have the open file that the
fstat() on the client was performed on.

It's that simple.  There's really no ugly hacks going on behind the
scenes.  It's just that we do want to delegate some properties of this
operation onto the server, and the simplest and best implementation is
to just let the filesystem have the information it needs.

Why is that such a big problem?

Miklos
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch 1/2] VFS: new fgetattr() file operation

2007-09-24 Thread Miklos Szeredi
  But it's has various dawbacks, like rmdir doesn't work if there are
  open files within an otherwise empty directory.
  
  I'd happily accept suggestions on how to deal with this differenty.
 
 NFS has that problem because it really has to sillyrename into the same
 directory. I don't see that ssh/sftp needs to do that. Instead it can
 sillyrename anywhere in the filesystem.

I don't think it can.  How can we find in a reliable way another
directory, which is writable by the user?

Miklos
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/