TL;DR:

As many I've gone deeper, but none of the times in `stat` nor the
checksums of /usr/lib/systemd/system/hello.service did change.

Turns out this wasn't even about their file states.
And additionally my understanding was wrong, and potentially yours as well.

The state if this is outdated is not only per service via `struct 
UnitStatusInfo`,
but also globally across all services via `struct Manager`.

Snapd's way to enable its mount unit sets that global state and
therefore either needs to change how it enables units or run
a daemon-reload afterwards just like most .deb package installs do.


Details:

First we need to be careful, there are two ambiguous paths here that
can trigger the same message:

a) on service start
  start_unit_one
    -> if (need_daemon_reload(bus, name) > 0)
      -> warn_unit_file_changed(name);
  This is a function:
    int need_daemon_reload(sd_bus *bus, const char *unit)
  It will call out via dbus asking for the attribute NeedDaemonReload

b) on service status
  print_status_info
   -> if (i->need_daemon_reload)
      -> warn_unit_file_changed(i->id);

Also for storage, there are two:

c) `struct Manager` containing `unit_file_state_outdated` which is a global
  state for all units in that manager

d) Each `struct UnitStatusInfo` has a field `need_daemon_reload` (yes just
  named like the function above) that can flag this per unit.


And (a) isn't even per service.

The value of that can be fetched per service via dbus like:
$ dbus-send --system --print-reply --dest="org.freedesktop.systemd1" 
"/org/freedesktop/systemd1/unit/hello_2eservice" 
"org.freedesktop.DBus.Properties.Get" "string:org.freedesktop.systemd1.Unit" 
"string:NeedDaemonReload"
method return time=1713787033.411485 sender=:1.3 -> destination=:1.19 
serial=3833 reply_serial=2
   variant       boolean false

And the same can be fetched via `systemctl show` as well:

root@test:~# systemctl show hello | grep '^NeedDaemonReload'
NeedDaemonReload=yes

With the above in mind we can see that installing snapd renders ALL of them
as outdated. It was spotted with pro, reproduced with a simple example
and if you check the system it is all of them.

root@test:~# for u in $(systemctl list-units --output json | jq '.[].unit' | tr 
-d '"'); do systemctl show $u | grep '^NeedDaemonReload'; done 2>/dev/null | 
uniq -c
    143 NeedDaemonReload=no
root@test:~# snap install snapd
2024-04-22T12:44:50Z INFO Waiting for automatic snapd restart...
snapd 2.62 from Canonical✓ installed
root@test:~# for u in $(systemctl list-units --output json | jq '.[].unit' | tr 
-d '"'); do systemctl show $u | grep '^NeedDaemonReload'; done 2>/dev/null | 
uniq -c
    144 NeedDaemonReload=yes

Still the question is, which of the two data points is it switching?
It could be the global setting, but as well iterating and setting it per 
service.


I found that the global state could get changed in src/core/dbus-manager.c
in very similarly named methods:
- method_add_dependency_unit_files
- method_preset_all_unit_files
- method_revert_unit_files
- method_disable_unit_files_generic
- method_preset_unit_files_with_mode
- method_enable_unit_files_generic

All of them do eventually the same:
  m->unit_file_state_outdated = m->unit_file_state_outdated || n_changes > 0;

So it keeps the state or has n_changes applied and then sets it to true.

With GDB I found it happening in `method_enable_unit_files_generic`
of "snap-snapd-21465.mount":

2523            r = call(m->runtime_scope, flags, NULL, l, &changes, 
&n_changes);
2524            m->unit_file_state_outdated = m->unit_file_state_outdated || 
n_changes > 0; /* See comments for this variable in manager.h */
(gdb) p m->unit_file_state_outdated
$1 = false
(gdb) n
2525            if (r < 0)
(gdb) p m->unit_file_state_outdated
$2 = true
(gdb) p n_changes
$3 = 2
(gdb) p *l
$4 = 0x562ccfe389a0 "snap-snapd-21465.mount"

That "call" reference is in this case:

  $1 = {int (RuntimeScope, UnitFileFlags, const char *, char **,
        InstallChange **, size_t *)}
        0x7e2b6d34dd00 <unit_file_enable>

So the way snapd enabled its unit sets the global "out of date" state
for all things. I'm not challenging that, but like package postinst
it means snapd should run the implied daemon-reload to get back to
a stable state- WDYT?

I had a hard time tracking that down, to enter gdb in a somewhat helpful
state you might want to start with these saved breakpoints

root@test:~# cat sd.brk 
break ../src/core/dbus-manager.c:2524
  commands
    printf "Path %s nchanges %ld\n", *l, n_changes
    cont
  end
break ../src/core/dbus-manager.c:2597
  commands
    printf "Path %s nchanges %ld\n", *l, n_changes
    cont
  end
break ../src/core/dbus-manager.c:2651
  commands
    printf "Path %s nchanges %ld\n", *l, n_changes
    cont
  end
break ../src/core/dbus-manager.c:2694
  commands
    printf "Path %s nchanges %ld\n", *l, n_changes
    cont
  end
break ../src/core/dbus-manager.c:2767
  commands
    printf "Path %s nchanges %ld\n", *l, n_changes
    cont
  end
break ../src/core/dbus-manager.c:2807
  commands
    printf "Path %s nchanges %ld\n", *l, n_changes
    cont
  end
break method_enable_unit_files_generic

In a fresh noble container with just GDB installed

root@test:~# gdb -p 1
Enable debuginfod for this session? (y or [n]) y
(gdb) source sd.brk 
(gdb) c

And then in another window `snap install snapd`
That will hit method_enable_unit_files_generic and you'll see
it change the global state as shown above.

The call stack down from there for the question "where does n_changes come 
from" is:
From method_enable_unit_files_generic down to install_changes_add.
There it does:
15              c[(*n_changes)++] = (InstallChange) {

Stack here is like:

#0  install_changes_add (changes=changes@entry=0x7ffe97bcecc0, 
n_changes=n_changes@entry=0x7ffe97bcecb8, 
type=type@entry=INSTALL_CHANGE_SYMLINK, 
    path=path@entry=0x613bd300aa70 
"/etc/systemd/system/snapd.mounts.target.wants/snap-snapd-21465.mount", 
source=source@entry=0x613bd30dd7d0 "/etc/systemd/system/snap-snapd-21465.mount")
    at ../src/shared/install.c:321
#1  0x00007356043982eb in create_symlink (lp=lp@entry=0x7ffe97bcebf0, 
old_path=0x613bd30dd7d0 "/etc/systemd/system/snap-snapd-21465.mount", 
    new_path=new_path@entry=0x613bd300aa70 
"/etc/systemd/system/snapd.mounts.target.wants/snap-snapd-21465.mount", 
force=force@entry=true, changes=changes@entry=0x7ffe97bcecc0, 
    n_changes=n_changes@entry=0x7ffe97bcecb8) at ../src/shared/install.c:564
#2  0x000073560439b900 in install_info_symlink_wants 
(scope=scope@entry=RUNTIME_SCOPE_SYSTEM, file_flags=file_flags@entry=0, 
info=info@entry=0x613bd2fad040, lp=lp@entry=0x7ffe97bcebf0, 
    config_path=config_path@entry=0x613bd300a930 "/etc/systemd/system", 
list=<optimized out>, suffix=0x73560455a2c5 ".wants/", changes=0x7ffe97bcecc0, 
n_changes=0x7ffe97bcecb8) at ../src/shared/install.c:2027
#3  0x000073560439c13f in install_info_apply (n_changes=<optimized out>, 
changes=<optimized out>, config_path=<optimized out>, lp=<optimized out>, 
info=0x613bd2fad040, file_flags=<optimized out>, 
    scope=RUNTIME_SCOPE_SYSTEM) at ../src/shared/install.c:2099
#4  install_context_apply (ctx=<optimized out>, lp=0x7ffe97bcebf0, 
file_flags=0, config_path=0x613bd300a930 "/etc/systemd/system", 
flags=SEARCH_LOAD, changes=0x7ffe97bcecc0, n_changes=0x7ffe97bcecb8)
    at ../src/shared/install.c:2173
#5  0x000073560439ccc9 in do_unit_file_enable (lp=lp@entry=0x7ffe97bcebf0, 
scope=scope@entry=RUNTIME_SCOPE_SYSTEM, flags=flags@entry=0, 
config_path=0x613bd300a930 "/etc/systemd/system", 
    names_or_paths=names_or_paths@entry=0x613bd2f52fc0, 
changes=changes@entry=0x7ffe97bcecc0, n_changes=0x7ffe97bcecb8) at 
../src/shared/install.c:2774
#6  0x000073560439cdc5 in unit_file_enable (scope=RUNTIME_SCOPE_SYSTEM, 
flags=0, root_dir=<optimized out>, names_or_paths=0x613bd2f52fc0, 
changes=0x7ffe97bcecc0, n_changes=0x7ffe97bcecb8)
    at ../src/shared/install.c:2800
#7  0x00007356046c01d7 in method_enable_unit_files_generic 
(message=0x613bd300bdc0, m=0x613bd2f38500, call=0x73560439cd00 
<unit_file_enable>, carries_install_info=<optimized out>, error=0x7ffe97bceda0)
    at ../src/core/dbus-manager.c:2523
#8  0x00007356044cd072 in method_callbacks_run (found_object=0x7ffe97bcee77, 
require_fallback=false, c=<optimized out>, m=0x613bd300bdc0, 
bus=0x613bd3028d30) at ../src/libsystemd/sd-bus/bus-objects.c:406
#9  object_find_and_run (bus=bus@entry=0x613bd3028d30, 
m=m@entry=0x613bd300bdc0, p=<optimized out>, 
require_fallback=require_fallback@entry=false, 
found_object=found_object@entry=0x7ffe97bcee77)
    at ../src/libsystemd/sd-bus/bus-objects.c:1319
#10 0x00007356044ce61b in bus_process_object (bus=0x613bd3028d30, 
m=0x613bd300bdc0) at ../src/libsystemd/sd-bus/bus-objects.c:1439
#11 0x00007356044e1fb8 in process_message (m=0x613bd300bdc0, 
bus=0x613bd3028d30) at ../src/libsystemd/sd-bus/sd-bus.c:3007
#12 process_running (ret=0x0, bus=0x613bd3028d30) at 
../src/libsystemd/sd-bus/sd-bus.c:3049
#13 bus_process_internal (bus=bus@entry=0x613bd3028d30, ret=ret@entry=0x0) at 
../src/libsystemd/sd-bus/sd-bus.c:3277
#14 0x00007356044e23f9 in sd_bus_process (bus=bus@entry=0x613bd3028d30, 
ret=ret@entry=0x0) at ../src/libsystemd/sd-bus/sd-bus.c:3304
#15 0x00007356044e2a5d in io_callback (s=<optimized out>, fd=<optimized out>, 
revents=<optimized out>, userdata=0x613bd3028d30) at 
../src/libsystemd/sd-bus/sd-bus.c:3646
#16 0x000073560453d724 in source_dispatch (s=s@entry=0x613bd3116730) at 
../src/libsystemd/sd-event/sd-event.c:4187
#17 0x000073560453da6d in sd_event_dispatch (e=<optimized out>, 
e@entry=0x613bd2f38f30) at ../src/libsystemd/sd-event/sd-event.c:4808
#18 0x000073560453f3d8 in sd_event_run (e=<optimized out>, 
timeout=18446744073709551615) at ../src/libsystemd/sd-event/sd-event.c:4869

And if changes comes back up to method_enable_unit_files_generic as non-
zero then the global state will be set.

I hope that helps to further track this down on the snapd side.

Related References
[1]: 
https://github.com/systemd/systemd/blob/main/src/systemctl/systemctl-util.c#L436
[2]: 
https://github.com/systemd/systemd/blob/main/src/systemctl/systemctl-start-unit.c#L146
[3]: 
https://github.com/systemd/systemd/blob/main/src/systemctl/systemctl-util.c#L403
[4]: https://github.com/systemd/systemd/blob/main/src/core/manager.h#L301
[5]: 
https://github.com/systemd/systemd/blob/main/src/systemctl/systemctl-show.c#L861
[6]: 
https://github.com/systemd/systemd/blob/main/src/systemctl/systemctl-show.c#L201

-- 
You received this bug notification because you are a member of Ubuntu
Bugs, which is subscribed to Ubuntu.
https://bugs.launchpad.net/bugs/2055239

Title:
  Warning: The unit file, source configuration file or drop-ins of {apt-
  news,esm-cache}.service changed on disk. Run 'systemctl daemon-reload'
  to reload units.

To manage notifications about this bug go to:
https://bugs.launchpad.net/snapd/+bug/2055239/+subscriptions


-- 
ubuntu-bugs mailing list
ubuntu-bugs@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs

Reply via email to