Re: [Cluster-devel] Re: [PATCH 1/2] NLM failover unlock commands

2008-01-28 Thread Felix Blyakher
On Jan 28, 2008, at 9:56 AM, Wendy Cheng wrote: Felix Blyakher wrote: (I think Wendy's pretty close to that api already after adding the second method to start grace?) For reclaiming grace period issues, maybe we should move to https://www.redhat.com/archives/cluster-devel/2008-January/

Re: [Cluster-devel] Re: [PATCH 1/2] NLM failover unlock commands

2008-01-28 Thread Wendy Cheng
Felix Blyakher wrote: (I think Wendy's pretty close to that api already after adding the second method to start grace?) For reclaiming grace period issues, maybe we should move to https://www.redhat.com/archives/cluster-devel/2008-January/msg00340.html thread ? I view this (unlock) patch

Re: [Cluster-devel] Re: [PATCH 1/2] NLM failover unlock commands

2008-01-27 Thread Felix Blyakher
Hi Bruce, On Jan 24, 2008, at 10:00 AM, J. Bruce Fields wrote: On Thu, Jan 17, 2008 at 03:23:42PM -0500, J. Bruce Fields wrote: To summarize a phone conversation from today: On Thu, Jan 17, 2008 at 01:07:02PM -0500, Wendy Cheng wrote: J. Bruce Fields wrote: Would there be any advantage to

[Cluster-devel] Re: [PATCH 1/2] NLM failover unlock commands

2008-01-24 Thread J. Bruce Fields
On Thu, Jan 17, 2008 at 03:23:42PM -0500, J. Bruce Fields wrote: To summarize a phone conversation from today: On Thu, Jan 17, 2008 at 01:07:02PM -0500, Wendy Cheng wrote: J. Bruce Fields wrote: Would there be any advantage to enforcing that requirement in the server? (For example,

[Cluster-devel] Re: [PATCH 1/2] NLM failover unlock commands

2008-01-24 Thread Wendy Cheng
J. Bruce Fields wrote: On Thu, Jan 17, 2008 at 03:23:42PM -0500, J. Bruce Fields wrote: To summarize a phone conversation from today: On Thu, Jan 17, 2008 at 01:07:02PM -0500, Wendy Cheng wrote: J. Bruce Fields wrote: Would there be any advantage to enforcing that requirement

[Cluster-devel] Re: [PATCH 1/2] NLM failover unlock commands

2008-01-24 Thread Wendy Cheng
J. Bruce Fields wrote: On Thu, Jan 24, 2008 at 04:06:49PM -0500, Wendy Cheng wrote: J. Bruce Fields wrote: On Thu, Jan 24, 2008 at 02:45:37PM -0500, Wendy Cheng wrote: J. Bruce Fields wrote: In practice, it seems that both the unlock_ip and unlock_pathname

[Cluster-devel] Re: [PATCH 1/2] NLM failover unlock commands

2008-01-23 Thread Neil Brown
On Tuesday January 22, [EMAIL PROTECTED] wrote: ? ! (i.e. Acked-By: NeilBrown [EMAIL PROTECTED]) tnx.NB --b. commit 6685389d610950126f700d25f3d010c7049441c3 Author: J. Bruce Fields [EMAIL PROTECTED] Date: Tue Jan 22 17:40:42 2008 -0500 nfsd: more careful input validation in

[Cluster-devel] Re: [PATCH 1/2] NLM failover unlock commands

2008-01-18 Thread Wendy Cheng
Frank van Maarseveen wrote: shell echo 10.1.1.2 /proc/fs/nfsd/unlock_ip shell echo /mnt/sfs1 /proc/fs/nfsd/unlock_filesystem The expected sequence of events can be: 1. Tear down the IP address You might consider using iptables at this point for dropping outgoing packets with that

[Cluster-devel] Re: [PATCH 1/2] NLM failover unlock commands

2008-01-17 Thread J. Bruce Fields
On Thu, Jan 17, 2008 at 11:17:08AM -0500, Wendy Cheng wrote: J. Bruce Fields wrote: Yeah, sounds good. Maybe under Documentation/filesystems? And it might also be helpful to leave a reference to it in the code, e.g., in nfsctl.c: /* * The following are used for failover; see

[Cluster-devel] Re: [PATCH 1/2] NLM failover unlock commands

2008-01-17 Thread Wendy Cheng
J. Bruce Fields wrote: Yeah, sounds good. Maybe under Documentation/filesystems? And it might also be helpful to leave a reference to it in the code, e.g., in nfsctl.c: /* * The following are used for failover; see * Documentation/filesystems/nfsd-failover.txt for

[Cluster-devel] Re: [PATCH 1/2] NLM failover unlock commands

2008-01-17 Thread Wendy Cheng
Add a more detailed description into the top of the patch itself. I'm working on the resume patch now - it will include an overall write-up in the Documentation directory. -- Wendy

[Cluster-devel] Re: [PATCH 1/2] NLM failover unlock commands

2008-01-17 Thread J. Bruce Fields
On Tue, Jan 15, 2008 at 05:48:00PM -0500, Wendy Cheng wrote: Revised version of the patch: * based on comment from Neil Brown, use sscanf() to parse IP address (a cool idea imo). * the ret inside nlm_traverse_files() is now the file count that can't be unlocked. * other minor changes

[Cluster-devel] Re: [PATCH 1/2] NLM failover unlock commands

2008-01-17 Thread J. Bruce Fields
On Thu, Jan 17, 2008 at 10:48:56AM -0500, Wendy Cheng wrote: J. Bruce Fields wrote: Remind me: why do we need both per-ip and per-filesystem methods? In practice, I assume that we'll always do *both*? Failover normally is done via virtual IP address - so per-ip base method should be

[Cluster-devel] Re: [PATCH 1/2] NLM failover unlock commands

2008-01-17 Thread Wendy Cheng
J. Bruce Fields wrote: On Thu, Jan 17, 2008 at 10:48:56AM -0500, Wendy Cheng wrote: J. Bruce Fields wrote: Remind me: why do we need both per-ip and per-filesystem methods? In practice, I assume that we'll always do *both*? Failover normally is done via virtual IP address -

[Cluster-devel] Re: [PATCH 1/2] NLM failover unlock commands

2008-01-17 Thread Wendy Cheng
J. Bruce Fields wrote: On Thu, Jan 17, 2008 at 11:31:22AM -0500, Wendy Cheng wrote: J. Bruce Fields wrote: On Thu, Jan 17, 2008 at 10:48:56AM -0500, Wendy Cheng wrote: J. Bruce Fields wrote: Remind me: why do we need both per-ip and per-filesystem methods? In

[Cluster-devel] Re: [PATCH 1/2] NLM failover unlock commands

2008-01-17 Thread Wendy Cheng
Frank Filz wrote: I assume the intent here with this implementation is that the node taking over will start lock recovery for the ip address? With that perspective, I guess it would be important that each file system only be accessed with a single ip address otherwise lock recovery will not

[Cluster-devel] Re: [PATCH 1/2] NLM failover unlock commands

2008-01-17 Thread J. Bruce Fields
To summarize a phone conversation from today: On Thu, Jan 17, 2008 at 01:07:02PM -0500, Wendy Cheng wrote: J. Bruce Fields wrote: Would there be any advantage to enforcing that requirement in the server? (For example, teaching nlm to reject any locking request for a certain filesystem that

[Cluster-devel] Re: [PATCH 1/2] NLM failover unlock commands

2008-01-15 Thread J. Bruce Fields
On Tue, Jan 15, 2008 at 11:14:54AM -0500, Wendy Cheng wrote: J. Bruce Fields wrote: +static ssize_t failover_unlock_ip(struct file *file, char *buf, size_t size) +{ + __be32 server_ip; + char *fo_path; + char *mesg; + + /* sanity check */ + if (size = 0) + return

[Cluster-devel] Re: [PATCH 1/2] NLM failover unlock commands

2008-01-15 Thread Wendy Cheng
Neil Brown wrote: On Saturday January 12, [EMAIL PROTECTED] wrote: This is a combined patch that has: * changes made by Christoph Hellwig * code segment that handles f_locks so we would not walk inode-i_flock list twice. . if (unlikely(failover) !failover(data, file))

[Cluster-devel] Re: [PATCH 1/2] NLM failover unlock commands

2008-01-15 Thread Wendy Cheng
Neil Brown wrote: On Tuesday January 15, [EMAIL PROTECTED] wrote: I don't feel comfortable to change the existing code structure, especially a BUG() statement. It would be better to separate lock failover function away from lockd code clean-up. This is to make it easier for problem

[Cluster-devel] Re: [PATCH 1/2] NLM failover unlock commands

2008-01-15 Thread Wendy Cheng
Revised version of the patch: * based on comment from Neil Brown, use sscanf() to parse IP address (a cool idea imo). * the ret inside nlm_traverse_files() is now the file count that can't be unlocked. * other minor changes from latest round of review. Thanks to all for the review ! --

[Cluster-devel] Re: [PATCH 1/2] NLM failover unlock commands

2008-01-15 Thread Neil Brown
On Tuesday January 15, [EMAIL PROTECTED] wrote: Revised version of the patch: * based on comment from Neil Brown, use sscanf() to parse IP address (a cool idea imo). * the ret inside nlm_traverse_files() is now the file count that can't be unlocked. * other minor changes from latest

[Cluster-devel] Re: [PATCH 1/2] NLM failover unlock commands

2008-01-14 Thread Neil Brown
On Monday January 14, [EMAIL PROTECTED] wrote: +static ssize_t failover_unlock_ip(struct file *file, char *buf, size_t size) +{ + __be32 server_ip; + char *fo_path; + char *mesg; + + /* sanity check */ + if (size = 0) + return -EINVAL; Not only is

[Cluster-devel] Re: [PATCH 1/2] NLM failover unlock commands

2008-01-14 Thread Neil Brown
On Saturday January 12, [EMAIL PROTECTED] wrote: This is a combined patch that has: * changes made by Christoph Hellwig * code segment that handles f_locks so we would not walk inode-i_flock list twice. If agreed, please re-add your ack-by and signed-off lines respectively. Thanks ...

[Cluster-devel] Re: [PATCH 1/2] NLM failover unlock commands

2008-01-10 Thread Christoph Hellwig
On Wed, Jan 09, 2008 at 06:02:15PM +, Christoph Hellwig wrote: On Tue, Jan 08, 2008 at 03:57:45PM -0500, Wendy Cheng wrote: Christoph Hellwig wrote: Ok, I played around with this and cleaned up the ip/path codepathes to be entirely setup which helped the code quite a bit. Also a few

[Cluster-devel] Re: [PATCH 1/2] NLM failover unlock commands

2008-01-09 Thread J. Bruce Fields
On Tue, Jan 08, 2008 at 10:49:17PM -0500, Wendy Cheng wrote: Christoph Hellwig wrote: +/* cluster failover support */ + +typedef struct { + int cmd; + int stat; + int gp; + void*datap; +} nlm_fo_cmd; please don't introduce typedefs for struct types.

[Cluster-devel] Re: [PATCH 1/2] NLM failover unlock commands

2008-01-09 Thread Christoph Hellwig
On Tue, Jan 08, 2008 at 03:57:45PM -0500, Wendy Cheng wrote: Christoph Hellwig wrote: Ok, I played around with this and cleaned up the ip/path codepathes to be entirely setup which helped the code quite a bit. Also a few other Thanks for doing this :) . In the middle of running it with

[Cluster-devel] Re: [PATCH 1/2] NLM failover unlock commands

2008-01-08 Thread Christoph Hellwig
On Mon, Jan 07, 2008 at 12:39:25AM -0500, Wendy Cheng wrote: +#define DEBUG 0 +#define fo_printk(x...) ((void)(DEBUG printk(x))) Please don't introduce more debugging helpers but use the existing ones. +extern __u32 in_aton(const char *str); This is properly declared in linux/inet.h +

[Cluster-devel] Re: [PATCH 1/2] NLM failover unlock commands

2008-01-08 Thread Christoph Hellwig
Ok, I played around with this and cleaned up the ip/path codepathes to be entirely setup which helped the code quite a bit. Also a few other cleanups and two bugfixes (postive error code returned and missing path_release) fell out of it. I still don't like what's going on in fs/lockd/svcsubs.c,

[Cluster-devel] Re: [PATCH 1/2] NLM failover unlock commands

2008-01-08 Thread Wendy Cheng
Christoph Hellwig wrote: Ok, I played around with this and cleaned up the ip/path codepathes to be entirely setup which helped the code quite a bit. Also a few other Thanks for doing this :) . In the middle of running it with our cluster test - if passed, will repost it. Get your signed-off

[Cluster-devel] Re: [PATCH 1/2] NLM failover unlock commands

2008-01-08 Thread Wendy Cheng
Neil Brown wrote: On Monday January 7, [EMAIL PROTECTED] wrote: We've implemented two new NFSD procfs files: o /proc/fs/nfsd/unlock_ip o /proc/fs/nfsd/unlock_filesystem They are intended to allow admin or user mode script to release NLM locks based on either a path name or a server

[Cluster-devel] Re: [PATCH 1/2] NLM failover unlock commands

2008-01-08 Thread Wendy Cheng
Christoph Hellwig wrote: +/* cluster failover support */ + +typedef struct { + int cmd; + int stat; + int gp; + void*datap; +} nlm_fo_cmd; please don't introduce typedefs for struct types. I don't do much community version of linux code so its

[Cluster-devel] Re: [PATCH 1/2] NLM failover unlock commands

2008-01-07 Thread Neil Brown
On Monday January 7, [EMAIL PROTECTED] wrote: We've implemented two new NFSD procfs files: o /proc/fs/nfsd/unlock_ip o /proc/fs/nfsd/unlock_filesystem They are intended to allow admin or user mode script to release NLM locks based on either a path name or a server in-bound ip address