From eddb23679087f5a9cc10752fe5f1dc535a0b3bf9 Mon Sep 17 00:00:00 2001 From: neilbrown Date: Thu, 3 Nov 2005 05:33:07 +0000 Subject: [PATCH] idmapd update from Steve Dickson --- ChangeLog | 46 ++++++++++++++++++++++++++++++++++++++++++ utils/idmapd/idmapd.c | 47 ++++++++++++++++++++++--------------------- 2 files changed, 70 insertions(+), 23 deletions(-) diff --git a/ChangeLog b/ChangeLog index 11d3a22..3d8aca2 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,49 @@ +2005-11-03 Steve Dickson NeilBrown + *utils/idmapd/idmaps.c: + + 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. + 2005-10-14 NeilBrown *utils/mountd/cache.c(nfsd_fh): Understand type 2 and type 3 filesystem identifiers, which are used with device numbers diff --git a/utils/idmapd/idmapd.c b/utils/idmapd/idmapd.c index 3be8148..4c13273 100644 --- a/utils/idmapd/idmapd.c +++ b/utils/idmapd/idmapd.c @@ -92,18 +92,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); @@ -122,7 +132,7 @@ static void idtonameres(struct idmap_msg *); static void nametoidres(struct idmap_msg *); static int nfsdopen(char *); -static int nfsdopenone(struct idmap_client *, short, char *); +static int nfsdopenone(struct idmap_client *); static void nfsdreopen(void); size_t strlcat(char *, const char *, size_t); @@ -136,7 +146,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; @@ -162,10 +171,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; } @@ -596,9 +605,13 @@ nfsdreopen_one(struct idmap_client *ic) if (verbose > 0) 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); @@ -608,9 +621,6 @@ nfsdreopen_one(struct idmap_client *ic) } } -/* - * Note: nfsdreopen assumes nfsdopen has already been called - */ static void nfsdreopen() { @@ -622,18 +632,13 @@ nfsdreopen() static int nfsdopen(char *path) { - return ((nfsdopenone(&nfsd_ic[IC_NAMEID], IC_NAMEID, path) == 0 && - nfsdopenone(&nfsd_ic[IC_IDNAME], IC_IDNAME, path) == 0) ? 0 : -1); + return ((nfsdopenone(&nfsd_ic[IC_NAMEID]) == 0 && + nfsdopenone(&nfsd_ic[IC_IDNAME]) == 0) ? 0 : -1); } static int -nfsdopenone(struct idmap_client *ic, short which, char *path) +nfsdopenone(struct idmap_client *ic) { - 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) warnx("Opening %s failed: errno %d (%s)", @@ -644,10 +649,6 @@ nfsdopenone(struct idmap_client *ic, short which, char *path) 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) warnx("Opened %s", ic->ic_path); -- 2.39.2