On Sat, Nov 12 2016, Chuck Lever wrote:
> Hiya Neil-
>
>> On Nov 10, 2016, at 5:32 PM, NeilBrown <neilb@xxxxxxxx> wrote:
>>
>> On Fri, Nov 11 2016, Chuck Lever wrote:
>>
>>>>
>>>> No objection here, especially if we make it so that we have existing
>>>> behavior when there is no config file. nfs-utils even has some config
>>>> file parsing routines now in support/nfs that should be sufficient.
>>>
>>> Just a general thought:
>>>
>>> It's fine to store the configuration data under /etc, but for new
>>> administrative interfaces, please create a command line UI which
>>> is used to edit said configuration data. This makes it much easier
>>> for users to script administrative changes, to build GUIs around,
>>> and for our Q/A teams to construct automated tests.
>>
>> If that is an important concern (and I don't disagree) then I would
>> suggest dropping the idea of a config file with sections and just have a
>> file with simple (case sensitive) "name=value" assignments.
>
> I'm not necessarily advocating this, but we already have a single
> central key-value file that stores NFS configuration information
> (at least on RH-based distros), and that's /etc/sysconfig/nfs.
SUSE also has /etc/sysconfig/nfs, but with different names having
different meanings.
Debian has /etc/default/nfs_common, again with different names.
>
>
>> Then sed
>> can be used to modify the file.
>>
>> Or.... just use git to modify the config file.
>>
>> sudo git config --file /etc/idmapd.conf mapping.nobody-user neilb
>>
>> did what I expected it to do :-)
>
> Clever! But to be a little more clear, this is not about the
> technology used to modify a config file.
>
> After an administrator finds and modifies the correct file, there's
> a set of dodgy distribution-dependent instructions to figure out what
> daemons need to be poked with a sharp stick to pick up the new config.
>
> For idmapd.conf changes, for example, it's going to be a different
> set of pokes depending on whether its server or client.
>
> A kernel cache has to be invalidated in some important cases. This
> is true for modifying things related to the FH or export cache, and
> that is already wrapped in an administrative CLI: exportfs.
>
> For gssd, Bruce recently showed me a series of (as far as I know,
> undocumented) systemctl commands that are needed to pick up changes
> to gssd command line options on RHEL 7.
Ahh, I understand your concern better now I think.
I would hope that "systemctl restart nfs-utils" would do everything
needed to make sure all config changes are effective. That was
certainly the intention.
This may be a little heavy-handed (killing and restarting, rather than
just reloading config) but all the state lost should be refillable
caches so I don't think it should be a problem.
The questions is: does this command really do all that is needed?
If not, it should be fixed.
>
> If I had a lot of time to explore this, my strategy would be:
>
> a) Provide one or a few well-known tools to edit NFS config files
>
> b) Mechanize the acts of putting said edits into effect, and integrate
> those mechanisms into the tools
>
> c) Tie the tools directly to documentation (--help or a man page)
I'm not at all convinced about the need for an editing tool but I do
agree that documentation would be a big help. I imagine a "nfs.systemd"
man page (or maybe "systemd.nfs", but that would cause arguments about
ownership of the namespace) which set out which services should be
enabled or masked or restarted to provide which outcomes.
>
> If there was a library that managed this for us, open-coded config
> file parsing for nfsmount.conf and for idmapd.conf in nfs-utils
> could be replaced.
It already was. support/nfs/conffile.c is used by both mount and
idmapd.
e.g.
conf_init();
verbose = conf_get_num("General", "Verbosity", 0);
cache_entry_expiration = conf_get_num("General",
"Cache-Expiration", DEFAULT_IDMAP_CACHE_EXPIRY);
CONF_SAVE(xpipefsdir, conf_get_str("General", "Pipefs-Directory"));
if (xpipefsdir != NULL)
strlcpy(pipefsdir, xpipefsdir, sizeof(pipefsdir));
CONF_SAVE(nobodyuser, conf_get_str("Mapping", "Nobody-User"));
CONF_SAVE(nobodygroup, conf_get_str("Mapping", "Nobody-Group"));
Thanks,
NeilBrown
Attachment:
signature.asc
Description: PGP signature
