On Fri, 13 Nov 2020 at 21:32:37 +0900, Norbert Preining wrote:
[I wrote:]
> > I still think doing this as a user service
> > would be a better approach to use by default
> 
> But isn't that what we are doing by shipping
>       /usr/lib/systemd/user/onedrive.service
> ?

Yes it is, but you started this thread by asking for advice about how
to restart an instanced *system* service on upgrade, which gave me the
impression that you consider the system service to be what you recommend
to users as the most normal way to run this onedrive service.

If I'm understanding correctly, each user who wants to run this service
in monitor mode needs to either:
- enable it as a systemd user service (using the unit in
  /usr/lib/systemd/user)
or
- (ask their sysadmin to) enable it as an instanced system service with
  instance name = their username (using the unit in /lib/systemd/system),
or
- run it in some way that doesn't involve systemd
but they need to choose exactly one of those ways, and it is pointless
to have more than one for the same user. Is that right?

If I'm correct to think that, then I would say that you should
recommend the user-service as the most-preferred way, with the others
as alternatives for people with unusual needs.

> We don't enable **anything** by default since it requires interaction by
> the user (log into the onedrive account, provide response URL).

That's reasonable. We don't have a particularly good answer for what to do
with per-user services that are only conditionally useful: mpd is another
example of this (at the moment it autostarts for all users, but is only
practically useful for users who have configured it).

> That is also the reason why I don't see any use for a system service.

Now I'm confused. You started this thread asking for advice about how to
restart instanced system services, but now you say you don't see any use
for a system service.

Is what you mean here: a user-service is a viable route to use this
program; and an instanced system service that is run per user and
behaves as though it wants to be a user-service is also viable; but you
see no use for a system service that is genuinely running on behalf of
the entire multi-user system, similar to atd.service or cups.service
or rsyslog.service?

If that's what you mean, then yes, I agree; but any unit in
/lib/systemd/system is, by definition, a system service.

> > I assume this package is a FUSE filesystem (or some other daemon)
> 
> No, it is a user space program that syncs a directory with the content
> in the OneDrive cloud, more similar to the dropbox client or insync
> client. It can run in one-shot sync mode, as well as monitor mode using
> notify events.

OK, so it behaves like a daemon when run with --monitor, which is the case
we're interested in here.

> As I said, I think nothing should be automatically enabled from the
> system side (equivalent of systemctl --user --global disable
> onedrive.service), so the only information necessary would be
> 
>      Users can enable the service for themselves by running the command:
>  
>          $ systemctl --user enable onedrive.service

OK, that makes sense. You should be able to achieve this with:

    dh_installsystemduser --no-enable

(in a sufficiently new compat level) if you aren't doing that already.

> Bottomline for me is that it seems there is no reliable way to restart
> user services (services started via systemctl --user) besides waiting
> for reboot.

I think you are correct. If there is a way to do this in future, it is
most likely to involve dh_installsystemduser, so making sure that your
units are registered with dh_installsystemduser would be a good step
towards this.

A few packages use killall or pkill to tell user services to restart or
reload, but that isn't exactly robust: it relies on rummaging through
/proc and assuming that anything with a matching executable name is in
fact the service you had in mind.

Using a full path match, like gvfs-backends.postinst does, is probably
the least-bad way to do this on a per-package basis - but I hesitate to
say "best", only "least bad".

As far as I know, needrestart-session is the closest we have to an
actually good solution. You might well think it is too trigger-happy
about what to restart to want to use it on your own system, and that's
fair, but the design at least seems reasonable. My understanding is that
it works by running a daemon in the user session that receives advisory
notifications from a matching system-level component. The per-user daemon
makes its own decisions about what to (offer to) restart for that user,
which means the privileged system-level part is as small as possible,
and is not trying to "look into" distrusted session processes.

Honestly, the more experience I get in distro development, the more
convinced I am that "reboot to update" is the robust thing to do for
end-user systems. For developers who are using a rolling release or
installing their own patched/updated packages, it does often make sense
to optimize away the reboot into just restarting individual programs and
services, because developers are updating frequently and for relatively
trivial changes, and can cope with the bugs that sometimes happen when a
running program gets confused by the world changing under it and breaking
its assumptions; but I don't think we should imagine that just because
this is appropriate for *us*, it is also the most appropriate thing for
non-technical users.

Doing upgrades across a reboot guarantees that the system state after
the reboot genuinely reflects what was updated, with no old versions
of libraries lingering in RAM - particularly important if the bug being
fixed is a security flaw, which a majority of updates to a stable system
should be.

> Looking at the insync postinst code (which does that for logged in
> users), hmmm, I am a bit in doubt though:
...
>       su - $user -c "insync quit"

Defnitely please don't do this. Having system-level code reach into
user sessions like this is far from robust: system code should never
be trusting user sessions to behave reasonably. There is nothing to
stop the user from having a broken or malicious insync executable in
their PATH that sleeps forever, or prints an unbounded amount of text
to stdout/stderr, or any number of other ways to cause denial of service
for the postinst.

    smcv

Reply via email to