All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] nfs-utils - rpc.idmapd - nfsdreopen still broken
@ 2005-10-26 20:01 Steve Dickson
  2005-11-03  5:33 ` Neil Brown
  0 siblings, 1 reply; 3+ messages in thread
From: Steve Dickson @ 2005-10-26 20:01 UTC (permalink / raw)
  To: nfs, Neil Brown

[-- Attachment #1: Type: text/plain, Size: 1886 bytes --]

I've recently updated the nfs-utils in rawhide with the
latest patches from the SourceForge CVS tree and the
latest CITI patches (1.0.7-4).

In testing these patches, I notice that when the server was started
and a SIGHUP was sent to rpc.idmapd to open the nfs4.nametoid/channel
and nfs4.idtoname/channel files, the second open (the nfs4.idtoname one)
failed because the path (i.e. ic->ic_path) was NULL.

Now the reason the ic_path was NULL was because it was never set
during the call to nfsdopen(). nfsdopen() looks like:
nfsdopen(char *path)
{
     return ((nfsdopenone(&nfsd_ic[IC_NAMEID], IC_NAMEID, path) == 0 &&
             nfsdopenone(&nfsd_ic[IC_IDNAME], IC_IDNAME, path) == 0) ? 0 
: -1);
}

Note: the call to nfsdopenone() is how the path is set in each nfsd_ic[]
entry and nfsdopen() is only called once.

So when rpc.idmap comes up and the first call to nfsdopenone() fails
(because the server is not running) the path in nfsd_ic[IC_IDNAME] is
never filled in because the second nfsdopenone() never happen...

Now there was a CITI patche (idmapd_revert_fix_reopen_on_sighup.dif)
that tried to address this problem but did seem to fix it.. The
attached patch fix the problem by initializing both nfsd_ic[IC_IDNAME]
and nfsd_ic[IC_NAMEID] structures with the needed info...
I figured since there is no way of changing these paths or filenames
by command line args, why not just set them during compile time...
so that's what this patch does.

This patch also changes how nfsdreopen_one() handles the
case where the event has already been set. Unlike the CITI
patch (idmapd_revert_fix_reopen_on_sighup.dif) which just
just does not register the second event, my patch deletes
the old event and the registers the new one. It just seems like
the right thing to do since a SIGHUP means a new server just
started so we probably should create a new event as well...

steved.





[-- Attachment #2: nfs-utils-1.0.7-idmapd-mapinit.patch --]
[-- Type: text/x-patch, Size: 3477 bytes --]

This patch initializes the paths in idmap_client structure used
by the server side which avoid the problem of this structure
not being initialized when a SIGHUP comes in.

This also changes nfsdreopen_one() to delete old events and register new ones
when its called with an event already registered. 

Signed-off by: Steve Dickson <steved@redhat.com>

------------------------
--- nfs-utils-1.0.7/utils/idmapd/idmapd.c.orig	2005-10-24 10:29:08.000000000 -0400
+++ nfs-utils-1.0.7/utils/idmapd/idmapd.c	2005-10-24 10:32:13.000000000 -0400
@@ -94,18 +94,28 @@
 } while (0)
 
 #define IC_IDNAME 0
+#define IC_IDNAME_CHAN  NFSD_DIR "/nfs4.idtoname/channel"
+#define IC_IDNAME_FLUSH NFSD_DIR "/nfs4.idtoname/flush"
+
 #define IC_NAMEID 1
+#define IC_NAMEID_CHAN  NFSD_DIR "/nfs4.nametoid/channel"
+#define IC_NAMEID_FLUSH NFSD_DIR "/nfs4.nametoid/flush"
+
 struct idmap_client {
-	int                        ic_fd;
-	int                        ic_dirfd;
+	short                      ic_which;
 	char                       ic_clid[30];
+	char                      *ic_id;
 	char                       ic_path[PATH_MAX];
+	int                        ic_fd;
+	int                        ic_dirfd;
 	int                        ic_scanned;
 	struct event               ic_event;
-	char                      *ic_id;
-	short                      ic_which;
 	TAILQ_ENTRY(idmap_client)  ic_next;
 };
+static struct idmap_client nfsd_ic[2] = {
+{IC_IDNAME, "Server", "", IC_IDNAME_CHAN, -1, -1, 0},
+{IC_NAMEID, "Server", "", IC_NAMEID_CHAN, -1, -1, 0},
+};
 
 TAILQ_HEAD(idmap_clientq, idmap_client);
 
@@ -138,7 +148,6 @@ static char pipefsdir[PATH_MAX];
 static char *nobodyuser, *nobodygroup;
 static uid_t nobodyuid;
 static gid_t nobodygid;
-static struct idmap_client nfsd_ic[2];
 
 /* Used by cfg.c */
 char *conf_path;
@@ -164,10 +173,10 @@ flush_nfsd_idmap_cache(void)
 	time_t now = time(NULL);
 	int ret;
 
-	ret = flush_nfsd_cache("/proc/net/rpc/nfs4.idtoname/flush", now);
+	ret = flush_nfsd_cache(IC_IDNAME_FLUSH, now);
 	if (ret)
 		return ret;
-	ret = flush_nfsd_cache("/proc/net/rpc/nfs4.nametoid/flush", now);
+	ret = flush_nfsd_cache(IC_NAMEID_FLUSH, now);
 	return ret;
 }
 
@@ -675,9 +684,13 @@ nfsdreopen_one(struct idmap_client *ic)
 
 	if (verbose > 0)
 		idmapd_warnx("ReOpening %s", ic->ic_path);
+
 	if ((fd = open(ic->ic_path, O_RDWR, 0)) != -1) {
 		if (ic->ic_fd != -1)
 			close(ic->ic_fd);
+		if ((ic->ic_event.ev_flags & EVLIST_INIT))
+			event_del(&ic->ic_event);
+
 		ic->ic_event.ev_fd = ic->ic_fd = fd;
 		event_set(&ic->ic_event, ic->ic_fd, EV_READ, nfsdcb, ic);
 		event_add(&ic->ic_event, NULL);
@@ -687,9 +700,6 @@ nfsdreopen_one(struct idmap_client *ic)
 	}
 }
 
-/*
- * Note: nfsdreopen assumes nfsdopen has already been called
- */
 static void
 nfsdreopen()
 {
@@ -710,9 +720,6 @@ nfsdopenone(struct idmap_client *ic, sho
 {
 	char *whichstr;
 
-	whichstr = which == IC_IDNAME ? "idtoname" : "nametoid";
-	snprintf(ic->ic_path, sizeof(ic->ic_path),
-		"%s/nfs4.%s/channel", path, whichstr);
 	if ((ic->ic_fd = open(ic->ic_path, O_RDWR, 0)) == -1) {
 		if (verbose > 0)
 			idmapd_warnx("Opening %s failed: errno %d (%s)",
@@ -723,10 +730,6 @@ nfsdopenone(struct idmap_client *ic, sho
 	event_set(&ic->ic_event, ic->ic_fd, EV_READ, nfsdcb, ic);
 	event_add(&ic->ic_event, NULL);
 
-	ic->ic_which = which;
-	ic->ic_id = "Server";
-	strlcpy(ic->ic_clid, "Server", strlen("Server"));
-
 	if (verbose > 0)
 		idmapd_warnx("Opened %s", ic->ic_path);
 

^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2005-11-03 15:06 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2005-10-26 20:01 [PATCH] nfs-utils - rpc.idmapd - nfsdreopen still broken Steve Dickson
2005-11-03  5:33 ` Neil Brown
2005-11-03 15:06   ` Kevin Coffman

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.