From 1cd71dfd7101c1d4b0f224fdcba207793dbd4a91 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Thu, 13 Feb 2020 13:02:15 -0800 Subject: [PATCH] systemd properties: support for *Sec values Some systemd properties are documented as having "Sec" suffix (e.g. "TimeoutStopSec") but are expected to have "USec" suffix when passed over dbus, so let's provide appropriate conversion to improve compatibility. This means, one can specify TimeoutStopSec with a numeric argument, in seconds, and it will be properly converted to TimeoutStopUsec with the argument in microseconds. As a side bonus, even float values are converted, so e.g. TimeoutStopSec=1.5 is possible. This turned out a bit more tricky to implement when I was originally expected, since there are a handful of numeric types in dbus and each one requires explicit conversion. Signed-off-by: Kir Kolyshkin --- docs/systemd-properties.md | 9 +--- libcontainer/specconv/spec_linux.go | 40 +++++++++++++++++ libcontainer/specconv/spec_linux_test.go | 55 ++++++++++++++++++++++++ 3 files changed, 96 insertions(+), 8 deletions(-) diff --git a/docs/systemd-properties.md b/docs/systemd-properties.md index 8e58087a..737e2415 100644 --- a/docs/systemd-properties.md +++ b/docs/systemd-properties.md @@ -24,11 +24,4 @@ The values must be in the gvariant format (for details, see [gvariant documentation](https://developer.gnome.org/glib/stable/gvariant-text.html)). To find out which type systemd expects for a particular parameter, please -consult systemd sources. In particular, parameters with `USec` suffix are -in microseconds, and those require an `uint64` typed argument. Since -gvariant assumes int32 for a numeric values, the explicit type is required. - -**Note** that time-typed systemd parameter names must have the `USec` -suffix, while they are documented with `Sec` suffix. -For example, the stop timeout used in the example above must be -set as `TimeoutStopUSec` but is shown and documented as `TimeoutStopSec`. +consult systemd sources. diff --git a/libcontainer/specconv/spec_linux.go b/libcontainer/specconv/spec_linux.go index f0cfd47d..90731f50 100644 --- a/libcontainer/specconv/spec_linux.go +++ b/libcontainer/specconv/spec_linux.go @@ -5,6 +5,7 @@ package specconv import ( + "errors" "fmt" "os" "path/filepath" @@ -304,6 +305,38 @@ func createLibcontainerMount(cwd string, m specs.Mount) *configs.Mount { // systemd property name check: latin letters only, at least 3 of them var isValidName = regexp.MustCompile(`^[a-zA-Z]{3,}$`).MatchString +var isSecSuffix = regexp.MustCompile(`[a-z]Sec$`).MatchString + +// Some systemd properties are documented as having "Sec" suffix +// (e.g. TimeoutStopSec) but are expected to have "USec" suffix +// here, so let's provide conversion to improve compatibility. +func convertSecToUSec(value dbus.Variant) (dbus.Variant, error) { + var sec uint64 + const M = 1000000 + vi := value.Value() + switch value.Signature().String() { + case "y": + sec = uint64(vi.(byte)) * M + case "n": + sec = uint64(vi.(int16)) * M + case "q": + sec = uint64(vi.(uint16)) * M + case "i": + sec = uint64(vi.(int32)) * M + case "u": + sec = uint64(vi.(uint32)) * M + case "x": + sec = uint64(vi.(int64)) * M + case "t": + sec = vi.(uint64) * M + case "d": + sec = uint64(vi.(float64) * M) + default: + return value, errors.New("not a number") + } + return dbus.MakeVariant(sec), nil +} + func initSystemdProps(spec *specs.Spec) ([]systemdDbus.Property, error) { const keyPrefix = "org.systemd.property." var sp []systemdDbus.Property @@ -320,6 +353,13 @@ func initSystemdProps(spec *specs.Spec) ([]systemdDbus.Property, error) { if err != nil { return nil, fmt.Errorf("Annotation %s=%s value parse error: %v", k, v, err) } + if isSecSuffix(name) { + name = strings.TrimSuffix(name, "Sec") + "USec" + value, err = convertSecToUSec(value) + if err != nil { + return nil, fmt.Errorf("Annotation %s=%s value parse error: %v", k, v, err) + } + } sp = append(sp, systemdDbus.Property{Name: name, Value: value}) } diff --git a/libcontainer/specconv/spec_linux_test.go b/libcontainer/specconv/spec_linux_test.go index 228c8044..aca5ca5e 100644 --- a/libcontainer/specconv/spec_linux_test.go +++ b/libcontainer/specconv/spec_linux_test.go @@ -471,6 +471,61 @@ func TestInitSystemdProps(t *testing.T) { in: inT{"org.systemd.property.TimeoutStopUSec", "uint64 123456789"}, exp: expT{false, "TimeoutStopUSec", uint64(123456789)}, }, + { + desc: "convert USec to Sec (default numeric type)", + in: inT{"org.systemd.property.TimeoutStopSec", "456"}, + exp: expT{false, "TimeoutStopUSec", uint64(456000000)}, + }, + { + desc: "convert USec to Sec (byte)", + in: inT{"org.systemd.property.TimeoutStopSec", "byte 234"}, + exp: expT{false, "TimeoutStopUSec", uint64(234000000)}, + }, + { + desc: "convert USec to Sec (int16)", + in: inT{"org.systemd.property.TimeoutStopSec", "int16 234"}, + exp: expT{false, "TimeoutStopUSec", uint64(234000000)}, + }, + { + desc: "convert USec to Sec (uint16)", + in: inT{"org.systemd.property.TimeoutStopSec", "uint16 234"}, + exp: expT{false, "TimeoutStopUSec", uint64(234000000)}, + }, + { + desc: "convert USec to Sec (int32)", + in: inT{"org.systemd.property.TimeoutStopSec", "int32 234"}, + exp: expT{false, "TimeoutStopUSec", uint64(234000000)}, + }, + { + desc: "convert USec to Sec (uint32)", + in: inT{"org.systemd.property.TimeoutStopSec", "uint32 234"}, + exp: expT{false, "TimeoutStopUSec", uint64(234000000)}, + }, + { + desc: "convert USec to Sec (int64)", + in: inT{"org.systemd.property.TimeoutStopSec", "int64 234"}, + exp: expT{false, "TimeoutStopUSec", uint64(234000000)}, + }, + { + desc: "convert USec to Sec (uint64)", + in: inT{"org.systemd.property.TimeoutStopSec", "uint64 234"}, + exp: expT{false, "TimeoutStopUSec", uint64(234000000)}, + }, + { + desc: "convert USec to Sec (float)", + in: inT{"org.systemd.property.TimeoutStopSec", "234.789"}, + exp: expT{false, "TimeoutStopUSec", uint64(234789000)}, + }, + { + desc: "convert USec to Sec (bool -- invalid value)", + in: inT{"org.systemd.property.TimeoutStopSec", "false"}, + exp: expT{true, "", ""}, + }, + { + desc: "convert USec to Sec (string -- invalid value)", + in: inT{"org.systemd.property.TimeoutStopSec", "'covfefe'"}, + exp: expT{true, "", ""}, + }, { in: inT{"org.systemd.property.CollectMode", "'inactive-or-failed'"}, exp: expT{false, "CollectMode", "inactive-or-failed"},