[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [NFS] [Cluster-devel] [PATCH 0/4 Revised] NLM - lock failover



On Sat, Apr 28, 2007 at 02:51:17PM +1000, Neil Brown wrote:
> On Friday April 27, wcheng redhat com wrote:
[...]
> Certainly a lot closer.
> If we are creating "nlm_drop_locks" and "nlm_set_grace" interfaces, we
> should spend a few moments considering exactly what semantics they
> should have.
> 
> In both cases we write a filename.  Presumably it must start with a
> '/' and be null terminated, so you use "echo -n" rather than "echo".
> After all, a filename can contain a newline.

I don't care much about the trailing newline. Try mounting and
exporting it and mounting it on the client ;-). Truncating the
string at the first newline may be a practical thing to do.

> 
> Is there any extra info we might want to pass in or out at the same
> time?
> 
> For nlm_drop_locks, we might also want to be able to query locked -
> "Do you hold any locks on this filesystem".  Even "how many?".

The "no locks dropped" case might be useful. #locks dropped is
only informational (without client info) and covers the first case
too so that would be my choice but I don't have any strong opinion
about this.

> 
> Does it make sense to have a single file with composite semantics?

Only if that would avoid an otherwise unavoidable race. There are
just too many components involved with NFS so to avoid any race I'd
probably unplug it temporarily with iptables or "ip addr del..."
But I would like to be able to drop locks without entering grace
mode: a zero second grace mode when combined.

> 
> We write
>     XX/path/name
> where XX can be:

Try mounting and exporting pathnames with spaces.. that's not going
to work anytime soon, or even anytime at all (other unixes). So no
need to use / as separator.

>     a number, to set second remaining in grace period
>     a '?' (or empty string) to query state

You mean: write "?/path/name" to tell the kernel what subsequent
reads should query?

>     a '-' to remove all locks (and cancels any grace period)

That's a strange combination. But cancelling a grace period
is equivalent with setting it to zero seconds so no need for a
special case.

I'd go for simplicity: one file per function (unless there's an
unavoidable race). What about:

/proc/fs/nfsd/nlm_grace:
	Write a number to set the grace period in seconds
	(0==cancel). May be followed by a space + pathname to
	indicate the superblock/list of svc_something the grace
	period applies to (otherwise it's global). Truncate the
	string at a newline.

/proc/fs/nfsd/nlm_unlock:
	Write either a pathname or "" to drop locks. This has the
	same syntax as the second field of nlm_grace.


Optional: In addition to a pathname support "fsid=" syntax in
both cases.

If you wanna go wild then support a file= syntax to recover from
stale locks on individual files due to buggy clients.

-- 
Frank


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]