Cockpit would like to support teams, but we can't unfortunately force the NetworkManager-team package to be installed. The next best thing is to detect at run-time whether it is installed, via the NetworkManager D-Bus API. I am happy to do the work, with a little initial help.
We're incorporating a hack to work around this, but it's very ugly and we would like to remove the hack once the real solution is in place: https://github.com/cockpit-project/cockpit/pull/5045
Better then thinking only about "plugins", these should be more general "capabilities", "abilities", or "behaviors". Like [ ] This NM instance is capable to managed Team I think we could make use of this extensively and thus it should be easy to extend and support a large number of capabilities. Capabilities could be either a bitmask, a set of string values, or a set of guint32. I would tend to a set of guint32, as then the actual numbers are more flexible but it's still more lightweight then a list of strings. Can capabilities change during a NetworkManager run? Team-support can currently not change because device-plugins are only loaded at the beginning. If they can change (maybe in the future), it might need a way to notify about the change. - capabilities are a set of guint32 values. Upstream NM only uses the range 0x001 to 0x6FFF. For downstream, we reserve 0x7000 to 0x7FFF. - either (1) add a method GetCapabilities() -> "au" which returns a list of all supported capabilities. (2) add a property Capabilities "au" which also returns all capabilities. (3) add a method "CheckCapability("u")->"b" to query one particular capability. (4) add a method "CheckCapabilities("au")->"ab" to query multiple capabilities. I think (1) is best - If we we later need to notify about mutable capabilities, I would add a signal CapabilityChanged to notify which capabilities changed. So, that wouldn't be a problem.
(In reply to Thomas Haller from comment #2) > Better then thinking only about "plugins", these should be more general > "capabilities", "abilities", or "behaviors". Yes, that makes sense. What about the pretty neutral term "Flags"? > I think we could make use of this extensively and thus it should be easy to > extend and support a large number of capabilities. And a plugin would then set a specific capability flag during its initialization after it has been loaded? Would you want to change all plugins, or only those that people are actually expressing interest in, like the "team" plugin here? > - capabilities are a set of guint32 values. > Upstream NM only uses the range 0x001 to 0x6FFF. > For downstream, we reserve 0x7000 to 0x7FFF. Right. > - either > (1) add a method GetCapabilities() -> "au" which returns a list of all > supported capabilities. > (2) add a property Capabilities "au" which also returns all capabilities. > (3) add a method "CheckCapability("u")->"b" to query one particular > capability. > (4) add a method "CheckCapabilities("au")->"ab" to query multiple > capabilities. > I think (1) is best I vote for (2), because it _is_ a property, no? You would just be duplicating the Property API for no good reason. > - If we we later need to notify about mutable capabilities, I would add > a signal CapabilityChanged to notify which capabilities changed. So, that > wouldn't be a problem. It would raise the question why this isn't a property. Would you like me to work a bit on this? A pull request at https://github.com/NetworkManager/NetworkManager would be easiest for me, I guess.
(In reply to Marius Vollmer from comment #3) > (In reply to Thomas Haller from comment #2) > > Better then thinking only about "plugins", these should be more general > > "capabilities", "abilities", or "behaviors". > > Yes, that makes sense. What about the pretty neutral term "Flags"? hm. "Flags"seems too generic as a name, but what do others think? > > I think we could make use of this extensively and thus it should be easy to > > extend and support a large number of capabilities. > > And a plugin would then set a specific capability flag during its > initialization after it has been loaded? Yes. > Would you want to change all plugins, or only those that people are actually > expressing interest in, like the "team" plugin here? The latter. I think those "flags" are anyway well-known to NetworkManager-core and the D-Bus API users who care about them. It doesn't have to be generic. Also, device-plugins are entirely in-tree. There is no stable API/ABI for such plugins, as such every capability is known at compile-time. Maybe now, capabilities should be immutable. Thus, there is a phase where device-plugins are loaded (and the team-capability-flag is set), but once the capabilities are accessed the first time, there value should be frozen. Preferably, there is an assertion if somebody tries to change a capability after it's frozen.. Maybe like: static struct { bool initalized; ... } _capabilities; void nm_manager_capability_init (NMCapability capability, gboolean has) { if (_capabilities.initialized) g_return_if_reached (); if (!NM_IN_SET (capability, NM_CAPABILITY_TEAM)) g_return_if_reached (); ... } > > > - capabilities are a set of guint32 values. > > Upstream NM only uses the range 0x001 to 0x6FFF. > > For downstream, we reserve 0x7000 to 0x7FFF. > > Right. > > > - either > > (1) add a method GetCapabilities() -> "au" which returns a list of all > > supported capabilities. > > (2) add a property Capabilities "au" which also returns all capabilities. > > (3) add a method "CheckCapability("u")->"b" to query one particular > > capability. > > (4) add a method "CheckCapabilities("au")->"ab" to query multiple > > capabilities. > > I think (1) is best > > I vote for (2), because it _is_ a property, no? You would just be > duplicating the Property API for no good reason. I dislike properties, because ObjectManager and libnm always read them all. I think most users don't actually care, so this adds overhead for every user. Arguably, it's just one single property... ok, fine with me. > > - If we we later need to notify about mutable capabilities, I would add > > a signal CapabilityChanged to notify which capabilities changed. So, that > > wouldn't be a problem. > > It would raise the question why this isn't a property. > > > Would you like me to work a bit on this? A pull request at > https://github.com/NetworkManager/NetworkManager would be easiest for me, I > guess. that would be great.
(In reply to Thomas Haller from comment #4) > Maybe like: > [...] Thanks, I'll work from that. > I dislike properties, because ObjectManager and libnm always read them all. > I think most users don't actually care, so this adds overhead for every user. Yeah, that's the only reason against properties that I can think of. Really expensive properties need some special handling, but Capabilities should be fine. > Arguably, it's just one single property... ok, fine with me. Cool. > > Would you like me to work a bit on this? A pull request at > > https://github.com/NetworkManager/NetworkManager would be easiest for me, I > > guess. > > that would be great. I got it to compile so the rest should be easy. :-)
probably, the _capabilities shouldn't be a static variable, but a member in the NMManagerPrivate. The static variable was just a quick idea, solve it as you think it's best.
https://github.com/NetworkManager/NetworkManager/pull/9
merged upstream and will be in NM-1.6.0: master: https://cgit.freedesktop.org/NetworkManager/NetworkManager/commit/?id=0b48ad4e48c22ca4d392d5d64701df268ad5ed8e. I am closing this as RESOLVED|RAWHIDE, as we usually don't backport new API to old-stable branches. And usually we don't upgrade the NetworkManager version in already released Fedora versions. If this should be actually backported to F24/F25, please reopen... but better don't ;-)