systemd: Do not check systemd version #154
Conversation
justxuewei
force-pushed
the
systemd-version
branch
2 times, most recently
from
f692c9d to
eca1a30
Compare
src/systemd/dbus/client.rs
Outdated
|
|
||
| /// Get the systemd version. | ||
| pub fn systemd_version(&self) -> Result<usize> { | ||
| pub fn systemd_version(&self) -> Result<String> { |
There was a problem hiding this comment.
I can see no callers of systemd_version() left - do we want to keep the function at all?
There was a problem hiding this comment.
Yes, I think it can be removed at this time.
The systemd version is purely informational and should not be parsed, as documented in the [1]. In practice, systemd version has different formats on OpenShift and Ubuntu. This commit skips the version check and allows all operations. The errors will be thrown from dbus when performing unsupported operations on obsolete versions of systemd. 1: https://www.freedesktop.org/software/systemd/man/latest/org.freedesktop.systemd1.html Signed-off-by: Xuewei Niu <[email protected]>
justxuewei
force-pushed
the
systemd-version
branch
from
eca1a30 to
c49bee2
Compare
| InvalidArgument, | ||
|
|
||
| #[error("obsolete systemd, please upgrade your systemd")] | ||
| ObsoleteSystemd, |
There was a problem hiding this comment.
Just a question - I like the resolution and level of detail of these error messages. The message I initially got was from the CorruptedSystemdVersion error which was specific enough to direct me right at the core problem although I had no previous experience with cgroups-rs at all.
Is there a way to preserve at least some of this? I tried to check and I think (not sure though) that users will now get one of zbus's Error variants. It doesn't seem too bad but if there was a way to turn it into something more specific in the context of cgroups-rs that would be awesome.
There was a problem hiding this comment.
Well, I prefer not. Most of errors from dbus are MethodError. The first item is OwnedErrorName which is a variant of string IMHO. For example, calling a non-existent method will result in an error with the value org.freedesktop.systemd1.NoSuchUnit. For the common errors, we can parse them, but it is hard to cover them all.
BTW, I think the errors are clear enough even if we don't parse it. I have not tried to trigger an error, but it should be expected as follows.
Caused by:
0: setup device after start vm
1: setup cgroups after start vm
2: update sandbox cgroups after start vm
3: set
4: systemd dbus error: dbus: No such unit
5: dbus: No such unit
There was a problem hiding this comment.
Well dbus: no such unit is pretty much exactly the opaque message that I'm afraid says nothing to anybody who doesn't happen to be very familiar with dbus. But fair enough, it's true that what I'd like to see would require non-trivial effort. Maybe later.
There was a problem hiding this comment.
I think the high-prio thing is to use cgroups-rs to manage cgroups for kata-agent. I am fixing the issue of that, but it is expected to take a while. I'll keep track of issues related to cgroups. If I find it slows down the debugging process, I'll take steps to make it clearer.
There was a problem hiding this comment.
Added one more question/suggestion but approving right away since that doesn't have to be decided now. Thanks @justxuewei !
3 participants
The systemd version is purely informational and should not be parsed, as
documented in the [1]. In practice, systemd version has different formats
on OpenShift and Ubuntu.
This commit skips the version check and allows all operations. The errors
will be thrown from dbus when performing unsupported operations on obsolete
versions of systemd.
1: https://www.freedesktop.org/software/systemd/man/latest/org.freedesktop.systemd1.html