Re: [chrony-dev] [PATCH] main: imply -x if time can't be set

[ Thread Index | Date Index | More chrony.tuxfamily.org/chrony-dev Archives ]




On Thu, Mar 8, 2018 at 4:34 PM, Miroslav Lichvar <mlichvar@xxxxxxxxxx> wrote:
On Thu, Mar 08, 2018 at 04:20:48PM +0100, Christian Ehrhardt wrote:
> On Thu, Mar 8, 2018 at 3:52 PM, Miroslav Lichvar <mlichvar@xxxxxxxxxx>
> wrote:
> > I'd rather keep it simple and avoid implementing an automatic fallback
> > in chrony.
> >
>
> I understand the POV, but without even a default-off function for it I'm
> kind of forced to wrap a round it to be able to support the use-case in
> some way at least.

I still don't see a good use case for the option.

Is your intention to use in the default configuration? If not, what
difference will it make for the user to add -x or some other option
which enables -x?

No, by adding a new option like -x=fallback (or whatever) to chrony we could have the best of both worlds.

This is based on the following proposed changes:
- add an option to not set clock if not able to adjust it
- drop the condition check on CAP_SYS_TIME from the service file
- If unable to set time, but the new option is not set call out more clearly why chrony doesn't work in this environment

1. the option would not be default on, so "normal" users/installations would not be affected
2. cases that want the fallback behavior, but are unable to probe/detect at the time:
   - so they can not decide to use normal -x
   - also the environment might change on them withut reconfig
   Both of the above would be solved by them dropping the new -x=fallback option for their case.

Our container folks will outline the CAP_SYS_TIME issue I mentioned before, so really the best test for my suggested SYS_IsTimeAdjustable would be (on top to what I have to check the Cap) a adjtime no-op.
I tried via adjtimex cmdline and thought maybe "adjtimex -s 0" (in C from chrony eventually) would be a no-op check I'd think
If the above is not ok as a probe in your opinion, if you happen to know how a timex struct should look like to essentially change nothing (as it is only a probe) but trigger EPERM if forbidden that would be great to know about.

If yes, will be the users that don't want chronyd to be started in
containers (but have it installed for some reason they cannot change)
happy with that?

The default install would not enable it, so users would not be affected in general.
 
> In general such a wrapper is inherently always slightly behind as not all
> downstreams share it, and I'd much more prefer coding something up that we
> share..

A script like that could be included the contrib directory if you
think others will find it useful.

> If that is a pre-nack to a patch adding a "-Please-fall-back-if-failing"
> then I don't have to waste the time writing it.
> So is there a chance at all to add such a switch in your opinion?

Not until I better understand the use case :).

Ok, that stance is totally fair - thanks.
Going on to discuss and at some point sending a new patch then.


Mail converted by MHonArc 2.6.19+ http://listengine.tuxfamily.org/