The goal is to get rid of gprop_to_dbus_fcn() uses.
Note that there is a change in behavior. The "dns" GPtrArray in
NMSettingIPConfig is never NULL (the default of the boxed property),
thus the previous code always serialized the property, even the
empty list.
Now, empty dns properties are omitted from D-Bus.
Also, there is another change in behavior: nm_utils_ip4_dns_to_variant()
will now skip over strings that are not valid IPv4 addresses.
Previously, it would have added 0.0.0.0 (or some undefined address).
So far, we only have NMSettingClass.compare_property() hook.
The ugliness is that this hook is per-setting, when basically
all implementations only compare one property.
It feels cleaner to have a per-property hook and call that consistently.
In step one, we give all properties (the same) compare_fcn() implementation,
which delegates to the existing NMSettingClass.compare_property().
In a second step, this will be untangled.
There is one problem with this approach: NMSettInfoPropertType grows by
one pointer size, and we have potentially many such types. That should
be addressed by unifying types in the future.
Various NMSetting API would accept a property_idx parameter. Together
with the NMSettInfoSetting instance, this was useful to find the actual
NMSettInfoProperty instance.
The idea was, to provide the most of the functionality. That is, if you
might need the property_idx too, you had it -- after all, the
property_info you could lookup yourself.
However,
- literally zero users care about the property_idx. The care about
the property_info.
- if the user really, really required the property_idx, then it
is a given that it can be easily computed by
(property_info - sett_info->property_infos)
For our property meta data handling we require that all the meta data is
associated with one GType.
NMSettingIPConfig is a parent class of NMSettingIP[46]Config. Note that
we already have _nm_sett_info_property_override_create_array_ip_config()
because the meta data must be initialized together at one place.
We will require that we can find the offset for properties based on one
offset per type. That is cumbersome, if NMSettingIPConfigPrivate is
private itself.
Simplify that, by internally sharing NMSettingIPConfigPrivate and let
the subclasses embed the private data in their own private data.
Optimally we would simply embed the private struct as field into
NMSettingIPConfig. But that would be an ABI change as that struct
was public before 1.32. Don't change ABI for now, so we have to
awkwardly place it into the subclasses private data.
NMSetting instances either have no private data, they use
g_type_add_class_private(), or they embed the private data in the
NMSetting struct.
In all cases, we can find the private data at a fixed offset. Track that
offset in the NMSettInfoSetting meta data.
This will be useful, because properties really are stored in simple
fields, like a boolean property can be stored in a "bool" field. We will
extend the property meta data to track the offset of this property
field, but we also need to know where the offset starts.
For each property we have meta data in form of NMSettInfoProperty.
Each meta data also has a NMSettInfoProperty.property_type
(NMSettInfoPropertType).
The property type is supposed to define common behaviors for properties,
while the property meta data has individual properties. The idea is that
several properties can share the same property-type, and that
per-property meta data is part of NMSettInfoProperty.
The distinction is not very strong, but note that all remaining uses
of NMSettInfoPropertType.gprop_to_dbus_fcn were part of a property
type that was only used for one single property. That lack of
reusability hints to a wrong use.
Move gprop_to_dbus_fcn to the property meta data as a new field
NMSettInfoProperty.to_dbus_data.
Note that NMSettInfoPropertType.gprop_from_dbus_fcn still suffers from
the same problem. But the from-dbus side is not yet addressed.
If a property can be converted to D-Bus, then always set the
to_dbus_fcn() handler. The only caller of to_dbus_fcn() is
property_to_dbus(), so this means that property_to_dbus()
has no more default implementation and always delegates to
to_dbus_fcn().
The code is easier to understand if all properties implement
to_dbus_fcn() the same way.
Also, there is supposed to be a split between NMSettInfoProperty (info about
the property) and NMSettInfoPropertType (the type). The idea is that
each property (obviously) requires its distinct NMSettInfoProperty, but
they can share a common type implementation.
With NMSettInfoPropertType.gprop_to_dbus_fcn that is often violated because
many properties that implement NMSettInfoPropertType.gprop_to_dbus_fcn
require a special type implementation. As such, gprop_to_dbus_fcn should
be part of the property info and not the property type. The first step towards
that is unifying all properties to use to_dbus_fcn().
When subclassing a GObject type, the class and object structs
must be available and defined in the header.
For libnm, and in particular for NMSetting classes, we don't want
users to subclass NMSetting. It also doesn't work, because libnm
has internal code that is necessary to hook up the NMSetting class.
You cannot define your own type and make it work together with
libnm.
Having the structs in public headers limits what we can do with them.
For example, we could embed the private data directly in the structures
and avoid the additional indirection.
This is an API break, but for something that most likely nobody cares
about. Or better, nobody should care about. API is not what is
accidentally defined in a header, API was the library provides to
meaningfully use. Subclassing these types is not meaningful and was
only accidentally possible so far.
Only hide the structs for now. More cleanup is possible later. We shall
however aim to keep the padding and struct layout to not also break ABI.
(cherry picked from commit e46d484fae)
nm-settings-connection.c has code similar to this in two places:
/* FIXME: improve NMConnection API so we can avoid the overhead of cloning the connection,
* in particular if there are no secrets to begin with. */
connection_cloned = nm_simple_connection_new_clone(new);
/* Clear out unwanted secrets */
_nm_connection_clear_secrets_by_secret_flags(connection_cloned,
NM_SETTING_SECRET_FLAG_NOT_SAVED
| NM_SETTING_SECRET_FLAG_AGENT_OWNED);
secrets = nm_g_variant_ref_sink(
nm_connection_to_dbus(connection_cloned, NM_CONNECTION_SERIALIZE_ONLY_SECRETS));
It seems the secrets filtering can be done by nm_connection_to_dbus() if
the NM_CONNECTION_SERIALIZE_* flags are extended. The current set of
flags contains flags that start with NO, ONLY and WITH prefixes, which
makes it useless for combining the flags because most combinations of
more than one flag don't have a clear interpretation. So they're mostly
useful when used alone, i.e. you'd need to add a new enum value for
each new subset of settings to be serialized.
To get the most flexibility from a small set of flags they should
either all be of the WITH_* type or NO_* type. In the former case they
could be combined to extend the subset of properties serialized, in the
latter case each flag would reduce the subset. After trying both
options I found it's easier to adapt the current set of flags to the
WITH_* schema while keeping binary and source compatibility. This
commit changes the set of flags in the following way:
NM_CONNECTION_SERIALIZE_ALL is kept for compatibility but is equivalent
to a combination of other flags.
NM_CONNECTION_SERIALIZE_WITH_NON_SECRET is added with the same value as
NM_CONNECTION_SERIALIZE_NO_SECRETS, it implies that non-secret
properties are included but doesn't prevent including other properties.
Since it couldn't be meaningfully combined with any other flag this
change shouldn't break compatibility.
Similarly NM_CONNECTION_SERIALIZE_WITH_SECRETS is added with the same
value as existing NM_CONNECTION_SERIALIZE_ONLY_SECRETS with the same
consideration about compatibility.
NM_CONNECTION_SERIALIZE_WITH_SECRETS_AGENT_OWNED and the new
NM_CONNECTION_SERIALIZE_WITH_SECRETS_SYSTEM_OWNED and
NM_CONNECTION_SERIALIZE_WITH_SECRETS_NOT_SAVED add only subsets of
secrets and can be combined. For backwards compatibility
NM_CONNECTION_SERIALIZE_ONLY_SECRETS is basically ignored when either of
these three is present, so that the value:
..ONLY_SECRETS | ..AGENT_OWNED works as previously.
"libnm-core/" is rather complicated. It provides a static library that
is linked into libnm.so and NetworkManager. It also contains public
headers (like "nm-setting.h") which are part of public libnm API.
Then we have helper libraries ("libnm-core/nm-libnm-core-*/") which
only rely on public API of libnm-core, but are themself static
libraries that can be used by anybody who uses libnm-core. And
"libnm-core/nm-libnm-core-intern" is used by libnm-core itself.
Move "libnm-core/" to "src/". But also split it in different
directories so that they have a clearer purpose.
The goal is to have a flat directory hierarchy. The "src/libnm-core*/"
directories correspond to the different modules (static libraries and set
of headers that we have). We have different kinds of such modules because
of how we combine various code together. The directory layout now reflects
this.
2021-02-18 19:46:51 +01:00
Renamed from libnm-core/nm-setting-ip4-config.c (Browse further)
