From 8bed2c9edc644287fa708d6837de6f601f4b5849 Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Tue, 13 Dec 2022 12:25:04 +0100
Subject: [PATCH] core: add "VersionInfo" property on D-Bus and NMClient

This exposes NM_VERSION as number (contrary to the "Version", which is a
string). That is in particular useful, because the number can be
compared with <> due to the encoding of the version.

While at it, don't make it a single number. Expose an array of numbers,
where the following numbers are a bitfield of capabilities.

Note that before commit 3c67a1ec5e8e ('cli: remove version check against
NM'), we used to parse the "Version" string to detect the version. As
such, the information that "VersionInfo" exposes now, was already
(somewhat) available, you just had to parse the string. The main benefit of
"VersionInfo" is that it can expose capabilities (patched behavior) in
in a lightweight bitfield. To include the numerical version there is
just useful on top.

Currently no additional capabilities are exposed. The idea is of course
to have a place in the future, where we can expose additional
capabilities. Adding a capability flag is most useful for behavior that we
backport to older branches. Otherwise, we could just check the daemon version
alone. But since we only add "VersionInfo" property only now, we cannot backport
any capability further than this, because the "VersionInfo" property itself
won't be backported. As such, this will only be useful in the future by having
a place where we can add (and backport) capabilities.

Note that there is some overlap with the existing "Capability" property
and NMCapability enum. The difference is that adding a capability via "VersionInfo"
is only one bit, and thus cheaper. Most importantly, having it cheaper means
the downsides of adding a capability flag is significantly removed. In
practice, we could live without capabilities for a long time, so they
must be very cheap for them to be worth to add. Another difference might be,
that we will want that the VersionInfo is about compile time defaults (e.g.
a certain patch/behavior that is in or not), while NM_CAPABILITY_TEAM depends on
whether the team plugin is loaded at runtime.
---
 .../org.freedesktop.NetworkManager.xml        |  16 +++
 src/core/nm-manager.c                         |  40 +++++++
 src/core/nm-manager.h                         |   1 +
 src/libnm-client-impl/libnm.ver               |   2 +
 src/libnm-client-impl/nm-client.c             | 105 +++++++++++++++---
 src/libnm-client-public/nm-client.h           |  11 +-
 src/libnm-core-public/nm-dbus-interface.h     |  14 +++
 7 files changed, 172 insertions(+), 17 deletions(-)

diff --git a/introspection/org.freedesktop.NetworkManager.xml b/introspection/org.freedesktop.NetworkManager.xml
index 161857062..c92c80174 100644
--- a/introspection/org.freedesktop.NetworkManager.xml
+++ b/introspection/org.freedesktop.NetworkManager.xml
@@ -456,6 +456,22 @@
     -->
     <property name="Version" type="s" access="read"/>
 
+    <!--
+        VersionInfo:
+
+        NetworkManager version and capabilities.
+
+        The first element in the array is the NM_VERSION of the daemon. It is a binary representation
+        of the "Version" and can be compared numerically. The version is encoded as
+        "(major &lt;&lt; 16 | minor &lt;&lt; 8 | micro)".
+
+        The following elements are a bitfield of static capabilities of the daemon. See
+        #NMVersionInfoCapability for the available capability numbers.
+
+        Since: 1.42
+    -->
+    <property name="VersionInfo" type="au" access="read"/>
+
     <!--
         Capabilities:
 
diff --git a/src/core/nm-manager.c b/src/core/nm-manager.c
index 54711bb3b..ef5158aaa 100644
--- a/src/core/nm-manager.c
+++ b/src/core/nm-manager.c
@@ -115,6 +115,7 @@ static guint signals[LAST_SIGNAL] = {0};
 
 NM_GOBJECT_PROPERTIES_DEFINE(NMManager,
                              PROP_VERSION,
+                             PROP_VERSION_INFO,
                              PROP_CAPABILITIES,
                              PROP_STATE,
                              PROP_STARTUP,
@@ -420,6 +421,31 @@ static NM_CACHED_QUARK_FCN("autoconnect-root", autoconnect_root_quark);
 
 /*****************************************************************************/
 
+static GVariant *
+_version_info_get(void)
+{
+    const guint32 arr[] = {
+        NM_VERSION,
+    };
+
+    /* The array contains as first element NM_VERSION, which can be
+     * used to numerically compare the version (see also NM_ENCODE_VERSION,
+     * nm_utils_version(), nm_encode_version() and nm_decode_version().
+     *
+     * The following elements of the array are a bitfield of capabilities.
+     * These capabilities should only depend on compile-time abilities
+     * (unlike NM_MANAGER_CAPABILITIES, NMCapability). The supported values
+     * are from NMVersionInfoCapability enum. This way to expose capabilities
+     * is more cumbersome but more efficient compared to NM_MANAGER_CAPABILITIES.
+     * As such, it is cheap to add capabilities for something, where you would
+     * avoid it as NM_MANAGER_CAPABILITIES due to the overhead.
+     */
+
+    return nm_g_variant_new_au(arr, G_N_ELEMENTS(arr));
+}
+
+/*****************************************************************************/
+
 static gboolean
 _connection_is_vpn(NMConnection *connection)
 {
@@ -8133,6 +8159,9 @@ get_property(GObject *object, guint prop_id, GValue *value, GParamSpec *pspec)
     case PROP_VERSION:
         g_value_set_string(value, VERSION);
         break;
+    case PROP_VERSION_INFO:
+        g_value_set_variant(value, _version_info_get());
+        break;
     case PROP_CAPABILITIES:
         g_value_set_variant(value,
                             nm_g_variant_new_au(nm_g_array_first_p(priv->capabilities, guint32),
@@ -8646,6 +8675,9 @@ static const NMDBusInterfaceInfoExtended interface_info_manager = {
                                                            NM_MANAGER_ACTIVATING_CONNECTION),
             NM_DEFINE_DBUS_PROPERTY_INFO_EXTENDED_READABLE("Startup", "b", NM_MANAGER_STARTUP),
             NM_DEFINE_DBUS_PROPERTY_INFO_EXTENDED_READABLE("Version", "s", NM_MANAGER_VERSION),
+            NM_DEFINE_DBUS_PROPERTY_INFO_EXTENDED_READABLE("VersionInfo",
+                                                           "au",
+                                                           NM_MANAGER_VERSION_INFO),
             NM_DEFINE_DBUS_PROPERTY_INFO_EXTENDED_READABLE("Capabilities",
                                                            "au",
                                                            NM_MANAGER_CAPABILITIES),
@@ -8704,6 +8736,14 @@ nm_manager_class_init(NMManagerClass *manager_class)
                                                        NULL,
                                                        G_PARAM_READABLE | G_PARAM_STATIC_STRINGS);
 
+    obj_properties[PROP_VERSION_INFO] =
+        g_param_spec_variant(NM_MANAGER_VERSION_INFO,
+                             "",
+                             "",
+                             G_VARIANT_TYPE("au"),
+                             NULL,
+                             G_PARAM_READABLE | G_PARAM_STATIC_STRINGS);
+
     obj_properties[PROP_CAPABILITIES] =
         g_param_spec_variant(NM_MANAGER_CAPABILITIES,
                              "",
diff --git a/src/core/nm-manager.h b/src/core/nm-manager.h
index fcb002272..18c371e75 100644
--- a/src/core/nm-manager.h
+++ b/src/core/nm-manager.h
@@ -20,6 +20,7 @@
     (G_TYPE_INSTANCE_GET_CLASS((obj), NM_TYPE_MANAGER, NMManagerClass))
 
 #define NM_MANAGER_VERSION                      "version"
+#define NM_MANAGER_VERSION_INFO                 "version-info"
 #define NM_MANAGER_CAPABILITIES                 "capabilities"
 #define NM_MANAGER_STATE                        "state"
 #define NM_MANAGER_STARTUP                      "startup"
diff --git a/src/libnm-client-impl/libnm.ver b/src/libnm-client-impl/libnm.ver
index 7f531b6de..9083ac3fd 100644
--- a/src/libnm-client-impl/libnm.ver
+++ b/src/libnm-client-impl/libnm.ver
@@ -1881,6 +1881,7 @@ global:
 
 libnm_1_42_0 {
 global:
+	nm_client_get_version_info;
 	nm_client_wait_shutdown;
 	nm_client_wait_shutdown_finish;
 	nm_device_loopback_get_type;
@@ -1906,4 +1907,5 @@ global:
 	nm_setting_ovs_port_remove_trunk_by_value;
 	nm_setting_vlan_get_protocol;
 	nm_utils_ensure_gtypes;
+	nm_version_info_capability_get_type;
 } libnm_1_40_0;
diff --git a/src/libnm-client-impl/nm-client.c b/src/libnm-client-impl/nm-client.c
index 6eb5bc48c..44a969942 100644
--- a/src/libnm-client-impl/nm-client.c
+++ b/src/libnm-client-impl/nm-client.c
@@ -217,6 +217,7 @@ NM_GOBJECT_PROPERTIES_DEFINE(NMClient,
                              PROP_DNS_RC_MANAGER,
                              PROP_DNS_CONFIGURATION,
                              PROP_CHECKPOINTS,
+                             PROP_VERSION_INFO,
                              PROP_CAPABILITIES,
                              PROP_PERMISSIONS_STATE, );
 
@@ -305,7 +306,9 @@ typedef struct {
         char             *connectivity_check_uri;
         char             *version;
         guint32          *capabilities_arr;
+        guint32          *version_info_arr;
         gsize             capabilities_len;
+        gsize             version_info_len;
         guint32           connectivity;
         guint32           state;
         guint32           metered;
@@ -6300,6 +6303,57 @@ nm_client_get_capabilities(NMClient *client, gsize *length)
     return priv->nm.capabilities_arr;
 }
 
+/**
+ * nm_client_get_version_info:
+ * @client: the #NMClient instance
+ * @length: (out): the number of returned capabilities.
+ *
+ * If available, the first element in the array is NM_VERSION which
+ * encodes the daemon version as "(major << 16 | minor << 8 | micro)".
+ * The following elements are a bitfield of %NMVersionInfoCapabilities
+ * that indicate that the daemon supports a certain capability.
+ *
+ * Returns: (transfer none) (array length=length): the
+ *   list of capabilities reported by the server or %NULL
+ *   if the capabilities are unknown.
+ *
+ * Since: 1.42
+ */
+const guint32 *
+nm_client_get_version_info(NMClient *client, gsize *length)
+{
+    NMClientPrivate *priv;
+
+    g_return_val_if_fail(NM_IS_CLIENT(client), NULL);
+    g_return_val_if_fail(length, NULL);
+
+    priv = NM_CLIENT_GET_PRIVATE(client);
+
+    *length = priv->nm.version_info_len;
+    return priv->nm.version_info_arr;
+}
+
+static NMLDBusNotifyUpdatePropFlags
+_notify_update_prop_nm_au(guint32 **p_arr, gsize *p_len, GVariant *value)
+{
+    nm_clear_g_free(p_arr);
+    *p_len = 0;
+
+    if (value) {
+        const guint32 *arr;
+        gsize          len;
+
+        arr    = g_variant_get_fixed_array(value, &len, sizeof(guint32));
+        *p_len = len;
+        *p_arr = g_new(guint32, len + 1);
+        if (len > 0)
+            memcpy(*p_arr, arr, len * sizeof(guint32));
+        (*p_arr)[len] = 0;
+    }
+
+    return NML_DBUS_NOTIFY_UPDATE_PROP_FLAGS_NOTIFY;
+}
+
 static NMLDBusNotifyUpdatePropFlags
 _notify_update_prop_nm_capabilities(NMClient               *self,
                                     NMLDBusObject          *dbobj,
@@ -6311,22 +6365,21 @@ _notify_update_prop_nm_capabilities(NMClient               *self,
 
     nm_assert(G_OBJECT(self) == dbobj->nmobj);
 
-    nm_clear_g_free(&priv->nm.capabilities_arr);
-    priv->nm.capabilities_len = 0;
+    return _notify_update_prop_nm_au(&priv->nm.capabilities_arr, &priv->nm.capabilities_len, value);
+}
 
-    if (value) {
-        const guint32 *arr;
-        gsize          len;
+static NMLDBusNotifyUpdatePropFlags
+_notify_update_prop_nm_version_info(NMClient               *self,
+                                    NMLDBusObject          *dbobj,
+                                    const NMLDBusMetaIface *meta_iface,
+                                    guint                   dbus_property_idx,
+                                    GVariant               *value)
+{
+    NMClientPrivate *priv = NM_CLIENT_GET_PRIVATE(self);
 
-        arr                       = g_variant_get_fixed_array(value, &len, sizeof(guint32));
-        priv->nm.capabilities_len = len;
-        priv->nm.capabilities_arr = g_new(guint32, len + 1);
-        if (len > 0)
-            memcpy(priv->nm.capabilities_arr, arr, len * sizeof(guint32));
-        priv->nm.capabilities_arr[len] = 0;
-    }
+    nm_assert(G_OBJECT(self) == dbobj->nmobj);
 
-    return NML_DBUS_NOTIFY_UPDATE_PROP_FLAGS_NOTIFY;
+    return _notify_update_prop_nm_au(&priv->nm.version_info_arr, &priv->nm.version_info_len, value);
 }
 
 /*****************************************************************************/
@@ -7541,13 +7594,15 @@ get_property(GObject *object, guint prop_id, GValue *value, GParamSpec *pspec)
     case PROP_CHECKPOINTS:
         g_value_take_boxed(value, _nm_utils_copy_object_array(nm_client_get_checkpoints(self)));
         break;
+    case PROP_VERSION_INFO:
     case PROP_CAPABILITIES:
     {
         const guint32 *arr;
         GArray        *out;
         gsize          len;
 
-        arr = nm_client_get_capabilities(self, &len);
+        arr = (prop_id == PROP_VERSION_INFO) ? nm_client_get_version_info(self, &len)
+                                             : nm_client_get_capabilities(self, &len);
         if (arr) {
             out = g_array_new(TRUE, FALSE, sizeof(guint32));
             g_array_append_vals(out, arr, len);
@@ -8104,6 +8159,10 @@ const NMLDBusMetaIface _nml_dbus_meta_iface_nm = NML_DBUS_META_IFACE_INIT_PROP(
         NML_DBUS_META_PROPERTY_INIT_B("Startup", PROP_STARTUP, NMClient, _priv.nm.startup),
         NML_DBUS_META_PROPERTY_INIT_U("State", PROP_STATE, NMClient, _priv.nm.state),
         NML_DBUS_META_PROPERTY_INIT_S("Version", PROP_VERSION, NMClient, _priv.nm.version),
+        NML_DBUS_META_PROPERTY_INIT_FCN("VersionInfo",
+                                        PROP_VERSION_INFO,
+                                        "au",
+                                        _notify_update_prop_nm_version_info, ),
         NML_DBUS_META_PROPERTY_INIT_IGNORE("WimaxEnabled", "b"),
         NML_DBUS_META_PROPERTY_INIT_IGNORE("WimaxHardwareEnabled", "b"),
         NML_DBUS_META_PROPERTY_INIT_B("WirelessEnabled",
@@ -8242,6 +8301,24 @@ nm_client_class_init(NMClientClass *client_class)
                                                        NULL,
                                                        G_PARAM_READABLE | G_PARAM_STATIC_STRINGS);
 
+    /**
+     * NMClient:version-info: (type GArray(guint32))
+     *
+     * Expose version info and capabilities of NetworkManager. If non-empty,
+     * the first element is NM_VERSION, which encodes the version of the
+     * daemon as "(major << 16 | minor << 8 | micro)". The following elements
+     * is a bitfields of %NMVersionInfoCapabilities. If a bit is set, then
+     * the running NetworkManager has the respective capability.
+     *
+     * Since: 1.42
+     */
+    obj_properties[PROP_VERSION_INFO] =
+        g_param_spec_boxed(NM_CLIENT_VERSION_INFO,
+                           "",
+                           "",
+                           G_TYPE_ARRAY,
+                           G_PARAM_READABLE | G_PARAM_STATIC_STRINGS);
+
     /**
      * NMClient:state:
      *
diff --git a/src/libnm-client-public/nm-client.h b/src/libnm-client-public/nm-client.h
index daa5bcb1a..9d54827ec 100644
--- a/src/libnm-client-public/nm-client.h
+++ b/src/libnm-client-public/nm-client.h
@@ -47,6 +47,7 @@ typedef enum /*< flags >*/ {
 #define NM_CLIENT_GET_CLASS(obj)  (G_TYPE_INSTANCE_GET_CLASS((obj), NM_TYPE_CLIENT, NMClientClass))
 
 #define NM_CLIENT_VERSION         "version"
+#define NM_CLIENT_VERSION_INFO    "version-info"
 #define NM_CLIENT_STATE           "state"
 #define NM_CLIENT_STARTUP         "startup"
 #define NM_CLIENT_NM_RUNNING      "nm-running"
@@ -187,9 +188,13 @@ NM_AVAILABLE_IN_1_22
 const char *nm_client_get_dbus_name_owner(NMClient *client);
 
 const char *nm_client_get_version(NMClient *client);
-NMState     nm_client_get_state(NMClient *client);
-gboolean    nm_client_get_startup(NMClient *client);
-gboolean    nm_client_get_nm_running(NMClient *client);
+
+NM_AVAILABLE_IN_1_42
+const guint32 *nm_client_get_version_info(NMClient *client, gsize *length);
+
+NMState  nm_client_get_state(NMClient *client);
+gboolean nm_client_get_startup(NMClient *client);
+gboolean nm_client_get_nm_running(NMClient *client);
 
 NMObject *nm_client_get_object_by_path(NMClient *client, const char *dbus_path);
 
diff --git a/src/libnm-core-public/nm-dbus-interface.h b/src/libnm-core-public/nm-dbus-interface.h
index 0baf14bbc..bf8eba67d 100644
--- a/src/libnm-core-public/nm-dbus-interface.h
+++ b/src/libnm-core-public/nm-dbus-interface.h
@@ -90,6 +90,20 @@
 #define NM_DBUS_INTERFACE_DNS_MANAGER "org.freedesktop.NetworkManager.DnsManager"
 #define NM_DBUS_PATH_DNS_MANAGER      "/org/freedesktop/NetworkManager/DnsManager"
 
+/**
+ * NMVersionInfoCapability:
+ * %_NM_VERSION_INFO_CAPABILITY_UNUSED: a dummy capability. It has no meaning,
+ *   don't use it.
+ *
+ * Currently no enum values are defined. These capabilities are exposed
+ * on D-Bus in the "VersionInfo" bit field.
+ *
+ * Since: 1.42
+ */
+typedef enum {
+    _NM_VERSION_INFO_CAPABILITY_UNUSED = 0x7FFFFFFFu,
+} NMVersionInfoCapability;
+
 /**
  * NMCapability:
  * @NM_CAPABILITY_TEAM: Teams can be managed. This means the team device plugin