Merge pull request #7301 from poettering/loginctl-ellipsize
Fix loginctl seat sysfs tree ellipsation logic.
Simple reproducer:
loginctl --full seat-status seat0|cat
→ after this PR, all lines are shown in full. Before, lines were ellipsized to terminal width.
Yu Watanabe [Sat, 11 Nov 2017 12:35:49 +0000 (21:35 +0900)]
core: add support to specify errno in SystemCallFilter=
This makes each system call in SystemCallFilter= blacklist optionally
takes errno name or number after a colon. The errno takes precedence
over the one given by SystemCallErrorNumber=.
loginctl: rework sysfs tree dump, to honour --full and friends
Let's hook up the sysfs tree output with the output flags logic,
already used when dumping log lines or process trees. This way we get
very similar output handling for line breaking/ellipsation in all three
outputs of structured data.
string-util: when ellipsizing to a length if (size_t) -1, become a NOP
Let's say that (size_t) -1 (i.e. SIZE_T_MAX) is equivalent to
"unbounded" ellipsation, i.e. ellipsation as NOP. In which case the
relevant functions become little more than strdup()/strndup().
This is useful to simplify caller code in case we want to turn off
ellipsation in certain code paths with minimal caller-side handling for
this.
merge two lines in our get_output_flags() functions
loginctl, machinectl, systemctl all have very similar implementations of
a get_output_flags() functions. Simplify it by merging two lines that
set the same flag.
core: make "tmpfs" dependencies on swapfs a "default" dep, not an "implicit"
There should be a way to turn this logic of, and DefaultDependencies=
appears to be the right option for that, hence let's downgrade this
dependency type from "implicit" to "default, and thus honour
DefaultDependencies=.
This also drops mount_get_fstype() as we only have a single user needing
this now.
core: remove SYSTEMD_WANTS udev property configured dependencies at the right moment
Previously dependencies configured with SYSTEMD_WANTS would be collected
on a device unit as long as it was loaded. let's fix that, and remove
dependencies again when SYTEMD_WANTS changes.
This replaces the dependencies Set* objects by Hashmap* objects, where
the key is the depending Unit, and the value is a bitmask encoding why
the specific dependency was created.
The bitmask contains a number of different, defined bits, that indicate
why dependencies exist, for example whether they are created due to
explicitly configured deps in files, by udev rules or implicitly.
Note that memory usage is not increased by this change, even though we
store more information, as we manage to encode the bit mask inside the
value pointer each Hashmap entry contains.
Why this all? When we know how a dependency came to be, we can update
dependencies correctly when a configuration source changes but others
are left unaltered. Specifically:
1. We can fix UDEV_WANTS dependency generation: so far we kept adding
dependencies configured that way, but if a device lost such a
dependency we couldn't them again as there was no scheme for removing
of dependencies in place.
2. We can implement "pin-pointed" reload of unit files. If we know what
dependencies were created as result of configuration in a unit file,
then we know what to flush out when we want to reload it.
3. It's useful for debugging: "systemd-analyze dump" now shows
this information, helping substantially with understanding how
systemd's dependency tree came to be the way it came to be.
mkosi: fix build script to use right sysvinit path
On Fedora /etc/init.d is a symlink to /etc/rc.d/init.d. Our build
scripts default to /etc/init.d since that is the LSB default. Let's make
sure the build script thus follows the symlink correctly and configures
to path explicitly, since otherwise our build artifacts in $DESTDIR are
incompatible with the setup we actually need for Fedora.
Alan Jenkins [Fri, 10 Nov 2017 14:57:52 +0000 (14:57 +0000)]
core: add missing error_message cases (#6911)
We neglected to set error_message for errors which occur _after_ the
`finish` label. These fatal errors only happen in paths where `finish`
was reached successfully, i.e. error_message has not already been set
(and this analysis is simple enough that this need not cause too much
headaches. Also our new assignments to error_message come immediately
after execve() calls, which would have lost the error_message if it had
been set).
Also print a status message when we fail to exec init, otherwise the only
sign the user will see is `# ` :).
This addresses the lack of error messages pointed out in issue #6827.
basic/hashmap: add cleanup of memory pools (#7164)
It was dropped in 89439d4fc0d29f04ac68432fd06ab84bc4e36e20. As a result, every
process that uses a hashmap allocates and then leaks the hashmap mempools.
The mempools are only allocated in the main thread, but we don't know where
the memory is used.
So let's check if we are the last thread and free the mempools then. This is
fairly heavy, because /proc/self/status has to be opened and parsed, but we do
it only when compiled for valgrind, i.e. not by default, and compared to running
under valgrind or asan, the extra cost is acceptable. The big advantage is that
we don't have to think or filter out this false positive.
As a micro-opt, cleanup is attempted only in the main thread. We could allow
any thread to check if it is the last one and perform cleanup, but that'd mean
that we'd have to _do_ the check in every thread. We don't use threads like
that, our non-main threads are always short-lived, so let's just accept the
possibility that we'll leak memory if a thread survives. The check is also
non-atomic, but it's called in a destructor of the main thread _and_ we do
cleanup only when there are no other threads, so the risk of some library
suddenly spawning another thread is very low. All in all, this is not perfect,
but should work in 999‰ of cases.
Fixes the following valgrind warning:
==22564== HEAP SUMMARY:
==22564== in use at exit: 8,192 bytes in 2 blocks
==22564== total heap usage: 243 allocs, 241 frees, 151,905 bytes allocated
==22564==
==22564== 4,096 bytes in 1 blocks are still reachable in loss record 1 of 2
==22564== at 0x4C2FB6B: malloc (vg_replace_malloc.c:299)
==22564== by 0x4F08A8C: mempool_alloc_tile (mempool.c:62)
==22564== by 0x4F08B16: mempool_alloc0_tile (mempool.c:81)
==22564== by 0x4EF8DE0: hashmap_base_new (hashmap.c:748)
==22564== by 0x4EF8ED9: internal_hashmap_new (hashmap.c:782)
==22564== by 0x11045D: test_hashmap_copy (test-hashmap-plain.c:87)
==22564== by 0x115722: test_hashmap_funcs (test-hashmap-plain.c:914)
==22564== by 0x10FC9D: main (test-hashmap.c:60)
==22564==
==22564== 4,096 bytes in 1 blocks are still reachable in loss record 2 of 2
==22564== at 0x4C2FB6B: malloc (vg_replace_malloc.c:299)
==22564== by 0x4F08A8C: mempool_alloc_tile (mempool.c:62)
==22564== by 0x4F08B16: mempool_alloc0_tile (mempool.c:81)
==22564== by 0x4EF8DE0: hashmap_base_new (hashmap.c:748)
==22564== by 0x4EF8EF8: internal_ordered_hashmap_new (hashmap.c:786)
==22564== by 0x10A2A0: test_ordered_hashmap_copy (test-hashmap-ordered.c:89)
==22564== by 0x10F70F: test_ordered_hashmap_funcs (test-hashmap-ordered.c:916)
==22564== by 0x10FCA2: main (test-hashmap.c:61)
==22564==
==22564== LEAK SUMMARY:
==22564== definitely lost: 0 bytes in 0 blocks
==22564== indirectly lost: 0 bytes in 0 blocks
==22564== possibly lost: 0 bytes in 0 blocks
==22564== still reachable: 8,192 bytes in 2 blocks
==22564== suppressed: 0 bytes in 0 blocks
Yu Watanabe [Fri, 10 Nov 2017 09:15:55 +0000 (18:15 +0900)]
core/load-fragment: add RemoveIPC= (#7288)
PR #3865 introduced RemoveIPC= but the option is not listed in
load-fragment-gperf.gperf. So, the option could be used only via d-bus.
This adds RemoveIPC= in load-fragment-gperf.gperf. Then, now we can
set the option in unit files.
Tom Stellard [Tue, 31 Oct 2017 15:46:24 +0000 (08:46 -0700)]
udev-rules: Permission changes for /dev/dri/renderD*
- Remove the uaccess tag from /dev/dri/renderD*.
- Change the owning group from video to render.
- Change default mode to 0666.
- Add an option to allow users to set the access mode for these devices at
compile time.
NeilBrown [Wed, 8 Nov 2017 08:29:32 +0000 (19:29 +1100)]
umount: always use MNT_FORCE in umount_all() (#7213)
The linux umount2() systemcall accepts a MNT_FORCE flags
which some filesystems honor, particularly FUSE and various
network filesystems such as NFS.
These filesystems can sometimes wait for an indefinite period
for a response from an external service, and the wait if
sometimes "uninterruptible" meaning that the process cannot be
killed.
Using MNT_FORCE causes any such request that are outstanding to
be aborted. This normally allows the waiting process to
be killed. It will then realease and reference it has to the
filesytem, this allowing the filesystem to be unmounted.
If there remain active references to the filesystem, MNT_FORCE
is *not* forcefull enough to unmount the filesystem anyway.
By the time that umount_all() is run by systemd-shutdown, all
filesystems *should* be unmounted, and sync() will have been
called. Anything that remains cannot be unmounted in a
completely clean manner and just nees to be dealt with as firmly
as possible. So use MNT_FORCE and try to explain why in the
comment.
Also enhance an earlier comment to explain why umount2() is
safe even though mount(MNT_REMOUNT) isn't.
Yu Watanabe [Wed, 8 Nov 2017 06:50:58 +0000 (15:50 +0900)]
core/execute: do not create RuntimeDirectory= under private/ sub-directory
RuntimeDirectory= often used for sharing files or sockets with other
services. So, if creating them under private/ sub-directory, we cannot
set DynamicUser= to service units which want to share something through
RuntimeDirectory=.
This makes the directories given by RuntimeDirectory= are created under
/run/ even if DynamicUser= is set.
Yu Watanabe [Fri, 13 Oct 2017 12:22:25 +0000 (21:22 +0900)]
core: ReadWritePaths= and friends assume '+' prefix when BindPaths= or freinds are set
When at least one of BindPaths=, BindReadOnlyPaths=, RootImage=,
RuntimeDirectory= or their friends are set, systemd prepares
a namespace under /run/systemd/unit-root. Thus, ReadWritePaths=
or their friends without '+' prefix is completely meaningless.
So, let's assume '+' prefix when one of them are set.
Alan Jenkins [Tue, 7 Nov 2017 16:14:15 +0000 (16:14 +0000)]
core: simplify - don't add jobs to dbus queue if we immediately remove them (#7251)
job_finish_and_invalidate() calls job_free() to destroy jobs (and remove
them from the dbus queue). So we don't need to add them to the dbus queue
first.
We only want to add jobs to the dbus queue if they're a restart job, which
we're transmogrifying into a start job and putting back into the system.
Susant Sahani [Thu, 2 Nov 2017 12:36:03 +0000 (18:06 +0530)]
networkd: remove route - drop route type from netlink message. (#7240)
During startup of networkd we try to drop the configs. While droping
routes we filling ip route type and because of which message like
```
host: Could not drop route: Invalid argument
host: Could not drop route: Invalid argument
```
are shown.
util-lib: mark variable with _unused_ to silence clang warning
_unused_ means "the variable is meant to be possible unused and gcc
will not generate a warning about it", which is exactly what we need here,
since we're only declaring it for the side effect of _cleanup_.
journal: disable -Waddress-of-packed-member under clang
clang warns about a few sites like this:
../src/journal/journal-file.c:1780:48: warning: taking address of packed member 'entry_offset' of class or structure 'DataObject' may result in an unaligned pointer value [-Waddress-of-packed-member]
&o->data.entry_offset,
^~~~~~~~~~~~~~~~~~~~
but DataObject.entry_offset will always be 8-byte aligned as long as
the DataObject structure is aligned. Similarly in other cases, the
field is always aligned. Let's just silence the warning to avoid noise.
gcc does not know -Waddress-of-packed-member, and would warn about an unknown
warning, so we need to conditionalize on __clang__.
../src/network/networkd-link.c:3577:84: warning: format specifies type 'unsigned char' but the argument has type 'uint32_t' (aka 'unsigned int') [-Wformat]
route->dst_prefixlen, route->tos, route->priority, route->table, route->lifetime);
^~~~~~~~~~~~
../src/network/networkd-manager.c:1146:132: warning: format specifies type 'unsigned char' but the argument has type 'uint32_t' (aka 'unsigned int') [-Wformat]
rule->from_prefixlen, space ? " " : "", to_str, rule->to_prefixlen, rule->tos, rule->fwmark, rule->fwmask, rule->table);
^~~~~~~~~~~
Also add some line breaks to make it easier to see which argument is for which
part of the format string.
clang warns:
../src/import/importd.c:254:70: warning: 'break' is bound to current loop, GCC binds it to the enclosing loop [-Wgcc-compat]
while ((e < t->log_message + t->log_message_size) && IN_SET(*e, 0, '\n'))
^
Let's just play it safe and not use IN_SET here.
Alan Jenkins [Mon, 14 Aug 2017 10:15:38 +0000 (11:15 +0100)]
core: failure to spawn ExecStartPost should not run ExecStop
Failure to spawn ExecStartPost was being handled differently to e.g.
EXIT_FAILURE returned by ExecStartPost. It looks like this was an
oversight. Fix to match documented behaviour.
`man systemd.service`:
> Note that if any of the commands specified in ExecStartPre=, ExecStart=,
> or ExecStartPost= fail (and are not prefixed with "-", see above) or time
> out before the service is fully up, execution continues with commands
> specified in ExecStopPost=, the commands in ExecStop= are skipped.
Alan Jenkins [Mon, 2 Oct 2017 13:40:15 +0000 (14:40 +0100)]
core: distinguish "Killing"/"Terminating"/"Stopping" for mount unit timeout
Update the timeout warnings for remount and unmount. For consistency with
mount, for accuracy, and for consistency with their equivalents in
service.c.
Michal Sekletar [Wed, 1 Nov 2017 09:25:48 +0000 (02:25 -0700)]
manager: fix connecting to bus when dbus is actually around (#7205)
manager_connect_bus() is called *before* manager_coldplug(). As a last
thing in service_coldplug() we set service state to
s->deserialized_state, and thus before we do that all services are
inactive and try_connect always evaluates to false. To fix that we must
look at deserialized state instead of current unit state.