godbus/dbus

`MakeVariant` cannot properly evaluate a(st)

cdoern opened this issue · 4 comments

in order to utilize the systemd BlockIOReadBandwidth cgroup entity, I need to be able to parse and pass an array of structs that each contain a string and a uint64 a(st). godbus is able to process the type via MakeVariant but the format() function in variant.go returns ["INVALID"] when given an a(st) type.

Am i doing something wrong here passing an array of structs or is there a gap in the parsing?

for example: giving a map of strings (a singular "BlockIOReadBandwidth") to structs map[string][]BlkioBpsThrottle that have the following format:

type BlkioBpsThrottle struct {
	Device   string
	Throttle uint64
}

and using the following loop to parse a map of length 1 with the device 8:16 and the throttle 2097152:

for k, v := range structMap {
			val := dbus.MakeVariant(v)
			fmt.Println(reflect.ValueOf(v), val.Signature(), val.Value(), val.String())
			p := systemdDbus.Property{
				Name:  k,
				Value: val,
			}

			properties = append(properties, p)

		}

results in: [{8:16 2097152}] a(st) [{8:16 2097152}] ["INVALID"]

the .String() function calls format() which has no handling for either a or (..). I have also noticed that MakeVariant seems to recursively call getSignature on a(st) even further confusing the output.

a(st) is a valid type according to: https://man7.org/linux/man-pages/man5/org.freedesktop.systemd1.5.html and looking at: https://dbus.freedesktop.org/doc/dbus-specification.html#type-system can be interpreted as an array of structs that contains a string and an int.

Not sure if I hit a new use case or if my interpretation here is incorrect.

@kolyshkin FYI, any help here either with what I am doing wrong or if I can make a PR to fix it would be appreciated.

Just for my understanding: the issue here is just that the String function returns a bad value, or are you doing something else with it? Note that conceptually, marshalling DBus values as strings and trying to parse them back is not a good idea since that's not standardized, in contrast to the DBus wire format itself. Just the title and the first lines of the issue confuse me: I can see that the formatting is wrong, but MakeVariant and ParseVariant don't have a problem with this type as far as I can tell.

Just for my understanding: the issue here is just that the String function returns a bad value, or are you doing something else with it? Note that conceptually, marshalling DBus values as strings and trying to parse them back is not a good idea since that's not standardized, in contrast to the DBus wire format itself. Just the title and the first lines of the issue confuse me: I can see that the formatting is wrong, but MakeVariant and ParseVariant don't have a problem with this type as far as I can tell.

I am using MakeVariant to pass form an array of attributes for systemd to place in a unit file. I noticed that my property array contains an attribute that says ["INVALID"]. the format functions seems to be used within the getSignature function so that was the point of improper evaluation. This is my first time looking at the dbus code so I could be wrong but the attached PR tested fixes all of my issues and systemd happily accepts the values

But from looking at containers/common#1082, I can't see why this would cause any issues with what you're trying to achieve. From what I can see, in the end you're just calling StartTransientUnitContext on systemd using the normal D-Bus interface. That should already work regardless of #329, since the communication through D-Bus uses a custom binary format which is completely independent of the string formatting functions that #329 touches.

I'm just trying to figure out where the code lives (if any) that depends on the exact format of the string, because as mentioned, that would still be a Bad Idea (tm).