systemd properties: support for *Sec values

kolyshkin · kolyshkin · commit 1cd71dfd7101 · 2020-02-17T16:07:19.000-08:00
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 &lt;kolyshkin@gmail.com&gt;
diff --git 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
@@ -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
@@ -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"},
