Re: [PATCH nfs-utils] systemd: remove the nfs-config service

[NeilBrown <nfbrown@novell.com>]

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

On Wed, Mar 16 2016, Benjamin Coddington wrote:

>> On Wed, Mar 16 2016, Benjamin Coddington wrote:
>>
>> > On Wed, 16 Mar 2016, NeilBrown wrote:
>> >
>> >> On Sat, Mar 12 2016, Benjamin Coddington wrote:
>> >>
>> >> > The nfs-config service exists to translate distro-specific startup
>> >> > configuration into the command line arguments used by systemd to start nfs
>> >> > utilities.  Unfortunately, it is not clear to most users that this service
>> >> > must be recycled every time startup configurations have been modified, as
>> >> > this requirement is a change for at least one distro.
>> >> >
>> >> > We can get rid of nfs-config by generating the startup arguments in an
>> >> > ExecStartPre option for each service that needs them.  We'll also have to
>> >> > break out the EnvironmentFile into a separate file for each service to
>> >> > avoid races overwriting this file.
>> >> >
>> >> > A distro taking this change should also modify their
>> >> > /usr/lib/systemd/scripts/nfs-utils_env.sh script to include the new
>> >> > EnvironmentFile location for each service.
>> >> >
>> >> > Signed-off-by: Benjamin Coddington <bcodding@redhat.com>
>> >>
>> >> I can't say I really like this, but it does make sense and solves a real
>> >> problem.
>> >> Maybe: I would never have written it myself, but I'm kind-a glad you did
>> >> :-)
>> >
>> > Which bits are unlikeable?  I am guessing that all these little intermediate
>> > files are the issue; that is what I hate.  I admit that it is a least-effort
>> > approach to fixing the problem.  What does the ideal fix look like to you?
>>
>> I don't like having multiple intermediate config files that are all
>> identical, and I don't like having to run the config script every time
>> any daemon is (re)started.
>>
>> I'm not sure what "ideal" would be.
>>
>> One improvement would be if systemd could be given an executable in
>> place of an environment file with the implication that it would run the
>> executable and read environment variables from the stdout.
>> That would remove the multiple config files.
>> It would still mean the script is run every time, but I could be
>> convinced that isn't so bad.
>>
>> An alternate idea is that maybe systemd could have a directive so that
>> in any transaction were "this" unit is started, "that" unit will also be
>> started.  Then we could replace "Wants=nfs-config.server" with
>> "Prepare=nfs-config.service" so if systemd ever needed to start any
>> nfs-related service, it would make preparations by first running
>> nfs-config.service - only once per transaction.
>>
>> But unless you want to hack on systemd, I think what you have is good
>> enough.
>>
>>
>> .... hmmm.  I wonder what would happen if we just removed
>> "RemainAfterExit=y" from nfs-config.service.  Would it cause things to
>> fail, or would it cause nfs-config.service to be run every time.
>>
>> Hey, that looks like it works!  Can you double check for me?
>> i.e. revert your patch, remove "RemainAfterExit=y" from
>> nfs-config.service, and then see if nfs-config.service gets run any time
>> any nfs server is started.
>
> Yep.. that appears to work just fine.  Seems like a simpler solution than
> this change.  It appears to me ( from systemd.service(5) ) that systemd's
> state transitions for Type=Oneshot will avoid races that might overwrite the
> environment file.  Nice find!
>

Great.  Thanks for testing and thanks for prodding me to explain
myself.  Very happy with the outcome!

I've posted a patch to Steve.

NeilBrown

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]