config.go add omitempty to device.Major and device.Minor

[zhouhao3]

Signed-off-by: zhouhao [email protected]

[crosbymichael]

Can you please explain why you think this should change?

It is hard to provide a reason to reject or approve this change without knowing first why you think it should be different.

[zhouhao3]

Modify according to the following spec：
1major, minor (int64, REQUIRED unless type is p)
2.Both MAJOR and MINOR must be specified when TYPE is b, c, or u, and they must be omitted when TYPE is p.

[crosbymichael]

I don't think this should be changed. A device type of p is not valid for the devices cgroup. I think adding omitempty is enough

[wking]

I don't think this should be changed. A device type of p is not valid for the devices cgroup.

cb01bcd is updating the LinuxDevice struct, which is used for linux.devices. The cgroup uses linux.resources.devices, which is backed by LinuxDeviceCgroup. The current specs for both (unchanged since v1.0.0) are here (for linux.devices):

• major, minor (int64, REQUIRED unless type is p) - [major, minor numbers][devices] for the device.

and here (for linux.resources.devices):

major, minor (int64, OPTIONAL) - [major, minor numbers][devices] for the device.

That means that both cases should have omitempty for Major and Minor and LinuxDeviceCgroup already does.

Once we add an omitempty, we need to decide on pointer vs. non-pointer. I don't think we have a clear policy on this at the moment (#772), but LinuxDeviceCgroup uses pointers for the same occasionally-unset situation, so cb01bcd keeps us consistent with that. Keeping non-pointers would also work for Major, since major zero is reserved, and that would make migration easier for Go consumers. But using a non-pointer for Minor is a no-go, because 0 is a valid minor value (e.g. here). And the slightly easier migration for:

Major int64 `json:"major,omitempty"`
Minor *int64 `json:"minor,omitempty"`

does not seem worth breaking consistency with LinuxDeviceCgroup.

[vbatts]

NACK
I don't think this is needed. This sets up implementations to now get a nil where that was not possible before, so this would need a version bump for not a particular gain or fix.