Open cdoern opened 2 years ago
@guelfey PTAL
Generally makes sense, however I'd like to stick to the GVariant format from GLib since this seems to be the convention for other tools, and DBus structs are called "tuples" there: https://www.freedesktop.org/software/gstreamer-sdk/data/docs/2012.5/glib/gvariant-text.html#gvariant-text-tuples ar the best docs I can find. So normal parantheses and separating the field with a comma.
@guelfey ok thanks for the feedback. I wasn't sure what the format should be so I'll repush with those changes tonight.
@giuseppe, using this PR in c/common the unit file generated looks like
# This is a transient unit file, created programmatically via the systemd API. Do not edit.
[Unit]
Description=cgroup machine2-libpod_pod_105.slice
Wants=machine2.slice
DefaultDependencies=no
[Slice]
MemoryAccounting=yes
CPUAccounting=yes
IOAccounting=yes
CPUWeight=4
MemoryMax=900
MemorySwapMax=1000
CPUQuotaPeriodSec=100ms
CPUQuota=100%
AllowedCPUs=4-5
AllowedMemoryNodes=4-5
IOReadBandwidthMax=
IOReadBandwidthMax=8:16 2097152
which is incorrectly adding an extra blank line, nothing is being added to io.max. The final property being provided is {IOReadBandwidthMax [("8:16", 2097152)]}
does this seem right to you? not sure where to go from here.
how does the generated value you pass to systemd look like? Not sure if the empty io.max
depends on that extra empty line
@giuseppe running something like:
...
for k, v := range structMap {
vari := dbus.MakeVariant(v)
fmt.Println(vari.Signature(), vari.String(), vari.Value())
...
prints:
a(st) @a(st) [("8:16" 2097152)] [{8:16 2097152}]
as a matter of fact, the extra lines are added to the unit file even without these modifications...
@guelfey this should be good to go! works with my PR in containers/common
@guelfey sorry I took so long here. To answer your question about why this is needed, mainly for testing and verification that systemd is getting the right values before we merge something into containers/common. Otherwise, debug statements just print INVALID
currently, structs are not parsed properly since
format()
has no handling for the reflect type. add a switch case to handle structs and encode some sane defaults if given an empty oneresolves #328
Signed-off-by: Charlie Doern cdoern@redhat.com