2007-06-08 22:14:59

by J.Bruce Fields

[permalink] [raw]
Subject: [PATCH 4/5] gfs2: stop giving out non-cluster-coherent leases

From: Marc Eshel <[email protected]>

Since gfs2 can't prevent conflicting opens or leases on other nodes, we
probably shouldn't allow it to give out leases at all.

Put the newly defined lease operation into use in gfs2 by turning off
lease, unless we're using the "nolock' locking module (in which case all
locking is local anyway).

Signed-off-by: Marc Eshel <[email protected]>
---
fs/gfs2/ops_file.c | 26 ++++++++++++++++++++++++++
1 files changed, 26 insertions(+), 0 deletions(-)

diff --git a/fs/gfs2/ops_file.c b/fs/gfs2/ops_file.c
index 064df88..654000b 100644
--- a/fs/gfs2/ops_file.c
+++ b/fs/gfs2/ops_file.c
@@ -489,6 +489,31 @@ static int gfs2_fsync(struct file *file, struct dentry *dentry, int datasync)
}

/**
+ * gfs2_setlease - acquire/release a file lease
+ * @file: the file pointer
+ * @arg: lease type
+ * @fl: file lock
+ *
+ * Returns: errno
+ */
+
+static int gfs2_setlease(struct file *file, long arg, struct file_lock **fl)
+{
+ struct gfs2_sbd *sdp = GFS2_SB(file->f_mapping->host);
+ int ret = EAGAIN;
+
+ if (sdp->sd_args.ar_localflocks) {
+ return setlease(file, arg, fl);
+ }
+
+ /* For now fail the delegation request. Cluster file system can not
+ allow any node in the cluster to get a local lease until it can
+ be managed centrally by the cluster file system.
+ */
+ return ret;
+}
+
+/**
* gfs2_lock - acquire/release a posix lock on a file
* @file: the file pointer
* @cmd: either modify or retrieve lock state, possibly wait
@@ -639,6 +664,7 @@ const struct file_operations gfs2_file_fops = {
.flock = gfs2_flock,
.splice_read = generic_file_splice_read,
.splice_write = generic_file_splice_write,
+ .setlease = gfs2_setlease,
};

const struct file_operations gfs2_dir_fops = {
--
1.5.2


-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs


2007-06-09 14:18:07

by Steven Whitehouse

[permalink] [raw]
Subject: Re: [PATCH 4/5] gfs2: stop giving out non-cluster-coherent leases

Hi,

On Fri, 2007-06-08 at 18:14 -0400, J. Bruce Fields wrote:
> From: Marc Eshel <[email protected]>
>
> Since gfs2 can't prevent conflicting opens or leases on other nodes, we
> probably shouldn't allow it to give out leases at all.
>
> Put the newly defined lease operation into use in gfs2 by turning off
> lease, unless we're using the "nolock' locking module (in which case all
> locking is local anyway).
>
> Signed-off-by: Marc Eshel <[email protected]>

In general it looks fine, but EAGAIN seems like an odd error to return
when we really mean that this function isn't supported in cluster mode.
Is that really the correct value for that?

Steve.



2007-06-09 16:35:52

by Marc Eshel

[permalink] [raw]
Subject: Re: [PATCH 4/5] gfs2: stop giving out non-cluster-coherent leases

This is the return code that setlease() currently returns when the lease
can not be obtained. Although ENOTSUPP would be more accurately describing
the error it will be a new return code from setlease() that is currently
not expected by callers to setlease(), but either return code should work.
Marc.

Steven Whitehouse <[email protected]> wrote on 06/09/2007 07:18:07 AM:

> Hi,
>
> On Fri, 2007-06-08 at 18:14 -0400, J. Bruce Fields wrote:
> > From: Marc Eshel <[email protected]>
> >
> > Since gfs2 can't prevent conflicting opens or leases on other nodes,
we
> > probably shouldn't allow it to give out leases at all.
> >
> > Put the newly defined lease operation into use in gfs2 by turning off
> > lease, unless we're using the "nolock' locking module (in which case
all
> > locking is local anyway).
> >
> > Signed-off-by: Marc Eshel <[email protected]>
>
> In general it looks fine, but EAGAIN seems like an odd error to return
> when we really mean that this function isn't supported in cluster mode.
> Is that really the correct value for that?
>
> Steve.
>
>


2007-06-11 09:38:18

by Steven Whitehouse

[permalink] [raw]
Subject: Re: [PATCH 4/5] gfs2: stop giving out non-cluster-coherent leases

Hi,

On Sat, 2007-06-09 at 09:35 -0700, Marc Eshel wrote:
> This is the return code that setlease() currently returns when the lease
> can not be obtained. Although ENOTSUPP would be more accurately describing
> the error it will be a new return code from setlease() that is currently
> not expected by callers to setlease(), but either return code should work.
> Marc.
>
I prefer EOPNOTSUPP as it more accurately describes whats going on here.
EAGAIN seems to me to imply that its something thats blocking
temporarily and that trying again at a later time might result in
success,

Steve.



2007-06-11 17:07:19

by J. Bruce Fields

[permalink] [raw]
Subject: Re: [PATCH 4/5] gfs2: stop giving out non-cluster-coherent leases

On Mon, Jun 11, 2007 at 10:38:18AM +0100, Steven Whitehouse wrote:
> Hi,
>
> On Sat, 2007-06-09 at 09:35 -0700, Marc Eshel wrote:
> > This is the return code that setlease() currently returns when the lease
> > can not be obtained. Although ENOTSUPP would be more accurately describing
> > the error it will be a new return code from setlease() that is currently
> > not expected by callers to setlease(), but either return code should work.
> > Marc.
> >
> I prefer EOPNOTSUPP as it more accurately describes whats going on here.
> EAGAIN seems to me to imply that its something thats blocking
> temporarily and that trying again at a later time might result in
> success,

OK. Well, if we hold to the theory that nobody sane was getting leases
in these situations before anyway, then I guess we can assume we're free
to define the error returns that make sense to us unconstrained by
application behavior.

So for now I've made the returns -EOPNOTSUPP in both the nfs and gfs2
cases; updated version available from:

git://linux-nfs.org/~bfields/linux.git server-cluster-lease-api

--b.

diff --git a/fs/gfs2/ops_file.c b/fs/gfs2/ops_file.c
index 654000b..e34d9bd 100644
--- a/fs/gfs2/ops_file.c
+++ b/fs/gfs2/ops_file.c
@@ -500,7 +500,7 @@ static int gfs2_fsync(struct file *file, struct dentry *dentry, int datasync)
static int gfs2_setlease(struct file *file, long arg, struct file_lock **fl)
{
struct gfs2_sbd *sdp = GFS2_SB(file->f_mapping->host);
- int ret = EAGAIN;
+ int ret = -EOPNOTSUPP;

if (sdp->sd_args.ar_localflocks) {
return setlease(file, arg, fl);
diff --git a/fs/nfs/file.c b/fs/nfs/file.c
index 467671b..97c1a3d 100644
--- a/fs/nfs/file.c
+++ b/fs/nfs/file.c
@@ -560,5 +560,5 @@ static int nfs_flock(struct file *filp, int cmd, struct file_lock *fl)

static int nfs_setlease(struct file *file, long arg, struct file_lock **fl)
{
- return -EINVAL;
+ return -EOPNOTSUPP;
}