Here's one last patch for the merge window. It allows the cluster stack to
negotiate a common protocol version with mounting nodes, as opposed to
requiring an exact match as we do today. The patch itself requires a
protocol change, so it'd be nice to see this upstream with all the other
network changes we did for 2.6.25. I held this patch back from my initial
pull request so that we could a bit more testing on it.
--Mark
Please pull from 'upstream-linus' branch of
git://git.kernel.org/pub/scm/linux/kernel/git/mfasheh/ocfs2.git upstream-linus
to receive the following updates:
fs/ocfs2/cluster/tcp_internal.h | 11 ++-
fs/ocfs2/dlm/dlmapi.h | 7 +-
fs/ocfs2/dlm/dlmcommon.h | 24 +++++-
fs/ocfs2/dlm/dlmdomain.c | 195 +++++++++++++++++++++++++++++++++-----
fs/ocfs2/dlm/dlmfs.c | 15 +++-
fs/ocfs2/dlm/userdlm.c | 5 +-
fs/ocfs2/dlm/userdlm.h | 3 +-
fs/ocfs2/dlmglue.c | 29 ++++++-
fs/ocfs2/dlmglue.h | 1 +
fs/ocfs2/ocfs2.h | 1 +
fs/ocfs2/ocfs2_lockingver.h | 30 ++++++
fs/ocfs2/super.c | 1 +
12 files changed, 288 insertions(+), 34 deletions(-)
create mode 100644 fs/ocfs2/ocfs2_lockingver.h
Joel Becker (1):
ocfs2: Negotiate locking protocol versions.
diff --git a/fs/ocfs2/cluster/tcp_internal.h b/fs/ocfs2/cluster/tcp_internal.h
index b2e832a..d25b9af 100644
--- a/fs/ocfs2/cluster/tcp_internal.h
+++ b/fs/ocfs2/cluster/tcp_internal.h
@@ -38,6 +38,15 @@
* locking semantics of the file system using the protocol. It should
* be somewhere else, I'm sure, but right now it isn't.
*
+ * With version 11, we separate out the filesystem locking portion. The
+ * filesystem now has a major.minor version it negotiates. Version 11
+ * introduces this negotiation to the o2dlm protocol, and as such the
+ * version here in tcp_internal.h should not need to be bumped for
+ * filesystem locking changes.
+ *
+ * New in version ...On Thu, 7 Feb 2008 12:09:44 -0800 It's somewhat obnoxious that what appears to be a straightforward compare-two-things-and-return-result function will actually modify one of the things which it is allegedly comparing. Please integrate checkpatch into your processes - this one had a few little glitches. --
Yeah, a better name would probably help with readability. Joel, how about
FWIW - I've run all patches through checkpatch.pl since your last review.
This one went through a couple cycles of checkpatch actually :) There's
three warnings that I get:
ERROR: "foo * bar" should be "foo *bar"
#70: FILE: fs/ocfs2/dlm/dlmapi.h:200:
+struct dlm_ctxt * dlm_register_domain(const char *domain, u32 key,
WARNING: line over 80 characters
#269: FILE: fs/ocfs2/dlm/dlmdomain.c:813:
+
#&dlm->fs_locking_proto,
WARNING: line over 80 characters
#270: FILE: fs/ocfs2/dlm/dlmdomain.c:814:
+
#&query->fs_proto)) {
total: 1 errors, 2 warnings, 569 lines checked
The "foo * bar" one is from existing code which got moved, and I felt that
leaving them unmodified was cleaner from a patch-reading perspective.
The over 80 characters warnings were ignored as the code seemed more
readable as-is.
I guess a lot of this can be subjective though, so I can be super strict if
you really feel it's necessary.
Thanks,
--Mark
--
Mark Fasheh
Principal Software Developer, Oracle
mark.fasheh@oracle.com
--
On Thu, 7 Feb 2008 13:37:15 -0800 I tend to clean those things up as we go, because it's a free patch. otoh I see that dlm style is presently space-after-asterisk so there's not yes, I tend to ignore those warnings unless the mess is really gratuitous or No, you shouldn't view checkpatch as a things-i-must-do. It is a things-i-might-have-missed tool. If you _meant_ things to be that way then fine, ignore it. --
Even better is to move the update outside the function. What do
we think of the following?
From d667a08d73cd6659219d21cc57f67882f46e42d1 Mon Sep 17 00:00:00 2001
From: Joel Becker <joel.becker@oracle.com>
Date: Thu, 7 Feb 2008 14:25:11 -0800
Subject: [PATCH] ocfs2: Clean up locking protocol negotiation.
The comparison functions for protocol negotiation (introduced in commit
d24fbcda0c4988322949df3d759f1cfb32b32953) were confusing.
Separate out the comparison and value update parts.
Signed-off-by: Joel Becker <joel.becker@oracle.com>
---
fs/ocfs2/dlm/dlmdomain.c | 102 ++++++++++++++++++++++------------------------
1 files changed, 49 insertions(+), 53 deletions(-)
diff --git a/fs/ocfs2/dlm/dlmdomain.c b/fs/ocfs2/dlm/dlmdomain.c
index 638d2eb..de802a7 100644
--- a/fs/ocfs2/dlm/dlmdomain.c
+++ b/fs/ocfs2/dlm/dlmdomain.c
@@ -144,8 +144,6 @@ static int dlm_cancel_join_handler(struct o2net_msg *msg, u32 len, void *data,
void **ret_data);
static int dlm_exit_domain_handler(struct o2net_msg *msg, u32 len, void *data,
void **ret_data);
-static int dlm_protocol_compare(struct dlm_protocol_version *existing,
- struct dlm_protocol_version *request);
static void dlm_unregister_domain_handlers(struct dlm_ctxt *dlm);
@@ -681,36 +679,48 @@ void dlm_unregister_domain(struct dlm_ctxt *dlm)
}
EXPORT_SYMBOL_GPL(dlm_unregister_domain);
+/*
+ * Compare a requested locking protocol version against the current one.
+ *
+ * If the major numbers are different, they are incompatible.
+ * If the current minor is greater than the request, they are incompatible.
+ * If the current minor is less than or equal to the request, they are
+ * compatible, and the requester should run at the current minor version.
+ */
+static int dlm_protocol_compatible(struct dlm_protocol_version *existing,
+ struct dlm_protocol_version *request)
+{
+ if (existing->pv_major != request->pv_major)
+ return 0;
+
+ if (existing->pv_minor > request->pv_minor)
+ return ...