On Wed, 09 Jul 2014 14:21:42 -0400
Steve Dickson <SteveD@xxxxxxxxxx> wrote:
> Hey Jeff,
>
> On 07/09/2014 06:32 AM, Jeff Layton wrote:
> > On Tue, 8 Jul 2014 10:33:39 -0400
> > Steve Dickson <steved@xxxxxxxxxx> wrote:
> >
> >> When there is no kernel modules loaded the rpc_pipefs
> >> directory is empty, which cause rpc.gssd to silently
> >> exit.
> >>
> >> This patch adds a check to see if the topdirs_list
> >> is empty. If so error out without dropping a core.
> >>
> >> Signed-off-by: Steve Dickson <steved@xxxxxxxxxx>
> >> ---
> >> utils/gssd/gssd_main_loop.c | 11 ++++++++---
> >> 1 file changed, 8 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/utils/gssd/gssd_main_loop.c b/utils/gssd/gssd_main_loop.c
> >> index 9970028..6946ab6 100644
> >> --- a/utils/gssd/gssd_main_loop.c
> >> +++ b/utils/gssd/gssd_main_loop.c
> >> @@ -173,6 +173,10 @@ topdirs_init_list(void)
> >> if (ret)
> >> goto out_err;
> >> }
> >> + if (TAILQ_EMPTY(&topdirs_list)) {
> >> + printerr(0, "ERROR: rpc_pipefs directory '%s' is empty!\n", pipefs_dir);
> >> + return -1;
> >> + }
> >> closedir(pipedir);
> >> return 0;
> >> out_err:
> >> @@ -233,9 +237,10 @@ gssd_run()
> >> sigaddset(&set, DNOTIFY_SIGNAL);
> >> sigprocmask(SIG_UNBLOCK, &set, NULL);
> >>
> >> - if (topdirs_init_list() != 0)
> >> - return;
> >> -
> >> + if (topdirs_init_list() != 0) {
> >> + /* Error msg is already printed */
> >> + exit(1);
> >> + }
> >> init_client_list();
> >>
> >> printerr(1, "beginning poll\n");
> >
> > Does it drop a core now? It looks sort of like it would just exit(1)
> > silently.
> No. But whenever gssd_run() returns, in main, abort() is called,
> which is probably a bit brain dead... but it is what it is...
>
> >
> > In any case, this patch is certainly better than crashing, but gssd
> > looks sort of like it's doing the wrong thing here. Should it not just
> > wait idly for directories to show up instead of exiting if none are
> > present?
> The purpose/cope of this patch is to stop gssd from silently exiting
> which the patch does...
>
> The question of whether gssd should or should not be exiting is
> a different problem... a problem this patch is not trying to solve.
>
> I tend to agree with you, that gssd probably should just hang out
> but in what scenario is gssd start and none of the kernel modules
> are loaded... Its not a normal one...
>
> >
> > Also, because topdir_init_list is run only once, it looks like gssd
> > doesn't properly handle the case where there may be some directories in
> > rpc_pipefs when gssd starts, but then others show up later. Any that
> > show up after gssd is started are just ignored currently, right? That
> > seems like a subtle source of bugs if you just happen to start gssd a
> > little early.
> >
> Again all this patch does is document the exit... Not processing late
> showing up directories would be a problem.... but as always....
> patches are welcomed!! ;-)
>
Fair enough. I don't forsee myself having much time for this anytime
soon, so the best we can do is probably make note of it until someone
else has the time and desire to fix it.
It's always hard to predict the order that things will be started,
particularly now that we have a trend toward more parallel startup with
systemd. Having daemons that are robust enough to deal with it when
things are not well-ordered is certainly ideal.
--
Jeff Layton <jlayton@xxxxxxxxxxxxxxx>
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
