From: Steve Dickson <SteveD@RedHat.com>
To: Doug Nazar <nazard@nazar.ca>, linux-nfs@vger.kernel.org
Subject: Re: [PATCH 2/4] idmapd: Add graceful exit and resource cleanup
Date: Thu, 23 Jul 2020 13:56:19 -0400 [thread overview]
Message-ID: <c136e451-f10a-c3a2-7f50-12735463275f@RedHat.com> (raw)
In-Reply-To: <20200722055354.28132-3-nazard@nazar.ca>
Hello,
On 7/22/20 1:53 AM, Doug Nazar wrote:
> Signed-off-by: Doug Nazar <nazard@nazar.ca>
> ---
> utils/idmapd/idmapd.c | 75 +++++++++++++++++++++++++++++++++++++------
> 1 file changed, 65 insertions(+), 10 deletions(-)
>
> diff --git a/utils/idmapd/idmapd.c b/utils/idmapd/idmapd.c
> index 491ef54c..dae5e567 100644
> --- a/utils/idmapd/idmapd.c
> +++ b/utils/idmapd/idmapd.c
> @@ -155,6 +155,7 @@ static void idtonameres(struct idmap_msg *);
> static void nametoidres(struct idmap_msg *);
>
> static int nfsdopen(void);
> +static void nfsdclose(void);
> static int nfsdopenone(struct idmap_client *);
> static void nfsdreopen_one(struct idmap_client *);
> static void nfsdreopen(void);
> @@ -167,6 +168,20 @@ static char *nobodyuser, *nobodygroup;
> static uid_t nobodyuid;
> static gid_t nobodygid;
> static struct event_base *evbase = NULL;
> +static bool signal_received = false;
> +
> +static void
> +sig_die(int signal)
> +{
> + if (signal_received) {
> + xlog_warn("forced exiting on signal %d\n", signal);
> + exit(0);
> + }
> +
> + signal_received = true;
> + xlog_warn("exiting on signal %d\n", signal);
> + event_base_loopexit(evbase, NULL);
> +}
>
> static int
> flush_nfsd_cache(char *path, time_t now)
> @@ -210,14 +225,15 @@ main(int argc, char **argv)
> {
> int wd = -1, opt, fg = 0, nfsdret = -1;
> struct idmap_clientq icq;
> - struct event *rootdirev, *clntdirev, *svrdirev, *inotifyev;
> - struct event *initialize;
> + struct event *rootdirev = NULL, *clntdirev = NULL,
> + *svrdirev = NULL, *inotifyev = NULL;
> + struct event *initialize = NULL;
> struct passwd *pw;
> struct group *gr;
> struct stat sb;
> char *xpipefsdir = NULL;
> int serverstart = 1, clientstart = 1;
> - int inotify_fd;
> + int inotify_fd = -1;
> int ret;
> char *progname;
> char *conf_path = NULL;
> @@ -290,6 +306,9 @@ main(int argc, char **argv)
> serverstart = 0;
> }
>
> + /* Not needed anymore */
> + conf_cleanup();
> +
I'm a bit confused by this comment... If it is not needed why as the call added?
steved.
> while ((opt = getopt(argc, argv, GETOPTSTR)) != -1)
> switch (opt) {
> case 'v':
> @@ -398,6 +417,9 @@ main(int argc, char **argv)
>
> TAILQ_INIT(&icq);
>
> + signal(SIGINT, sig_die);
> + signal(SIGTERM, sig_die);
> +
> /* These events are persistent */
> rootdirev = evsignal_new(evbase, SIGUSR1, dirscancb, &icq);
> if (rootdirev == NULL)
> @@ -435,7 +457,25 @@ main(int argc, char **argv)
> if (event_base_dispatch(evbase) < 0)
> xlog_err("main: event_dispatch returns errno %d (%s)",
> errno, strerror(errno));
> - /* NOTREACHED */
> +
> + nfs4_term_name_mapping();
> + nfsdclose();
> +
> + if (inotifyev)
> + event_free(inotifyev);
> + if (inotify_fd != -1)
> + close(inotify_fd);
> +
> + if (initialize)
> + event_free(initialize);
> + if (rootdirev)
> + event_free(rootdirev);
> + if (clntdirev)
> + event_free(clntdirev);
> + if (svrdirev)
> + event_free(svrdirev);
> + event_base_free(evbase);
> +
> return 1;
> }
>
> @@ -760,6 +800,19 @@ out:
> event_add(ic->ic_event, NULL);
> }
>
> +static void
> +nfsdclose_one(struct idmap_client *ic)
> +{
> + if (ic->ic_event) {
> + event_free(ic->ic_event);
> + ic->ic_event = NULL;
> + }
> + if (ic->ic_fd != -1) {
> + close(ic->ic_fd);
> + ic->ic_fd = -1;
> + }
> +}
> +
> static void
> nfsdreopen_one(struct idmap_client *ic)
> {
> @@ -769,12 +822,7 @@ nfsdreopen_one(struct idmap_client *ic)
> xlog_warn("ReOpening %s", ic->ic_path);
>
> if ((fd = open(ic->ic_path, O_RDWR, 0)) != -1) {
> - if (ic->ic_event && event_initialized(ic->ic_event)) {
> - event_del(ic->ic_event);
> - event_free(ic->ic_event);
> - }
> - if (ic->ic_fd != -1)
> - close(ic->ic_fd);
> + nfsdclose_one(ic);
>
> ic->ic_fd = fd;
> ic->ic_event = event_new(evbase, ic->ic_fd, EV_READ, nfsdcb, ic);
> @@ -807,6 +855,13 @@ nfsdopen(void)
> nfsdopenone(&nfsd_ic[IC_IDNAME]) == 0) ? 0 : -1);
> }
>
> +static void
> +nfsdclose(void)
> +{
> + nfsdclose_one(&nfsd_ic[IC_NAMEID]);
> + nfsdclose_one(&nfsd_ic[IC_IDNAME]);
> +}
> +
> static int
> nfsdopenone(struct idmap_client *ic)
> {
>
next prev parent reply other threads:[~2020-07-23 17:56 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-07-22 5:53 [PATCH 0/4] nfs-utils patches Doug Nazar
2020-07-22 5:53 ` [PATCH 1/4] exportfs: Fix a few valgrind warnings Doug Nazar
2020-07-22 5:53 ` [PATCH 2/4] idmapd: Add graceful exit and resource cleanup Doug Nazar
2020-07-23 17:56 ` Steve Dickson [this message]
2020-07-23 18:25 ` Doug Nazar
2020-07-23 19:16 ` Steve Dickson
2020-07-22 5:53 ` [PATCH 3/4] idmapd: Fix client mode support Doug Nazar
2020-07-22 5:53 ` [PATCH 4/4] nfsidmap: Allow overriding location of method libraries Doug Nazar
2020-07-27 14:42 ` [PATCH 0/4] nfs-utils patches Steve Dickson
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=c136e451-f10a-c3a2-7f50-12735463275f@RedHat.com \
--to=steved@redhat.com \
--cc=linux-nfs@vger.kernel.org \
--cc=nazard@nazar.ca \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).