nm_property_compare() makes a misguided attempt to compare dictionaries
regardless of their order.
However, if variants contain duplicate keys, then the implementation
is wrong and cannot handle it correctly.
Regardless of that. While in some sense the order of dictionary keys is
irrelevant, this is not the right place to perform such normalization.
If the order of things doesn't matter, then NMSetting must normalize the
property (e.g. by sorting the keys). At that point, the GVariant shall
be compared fully.
The supervision address is read-only. It is constructed by kernel and
only the last byte can be modified by setting the multicast-spec as
documented indeed.
As 1.46 was not released yet, we still can drop the whole API for this
setting property. We are keeping the NMDeviceHsr property as it is a
nice to have for reading it.
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1823
Fixes: 5426bdf4a1 ('HSR: add support to HSR/PRP interface')
Rename _nm_gobject_notify_together_impl##suffix() to
_nm_gobject_notify_together_full_v##suffix(). This name makes a bit more
sense. The "_v" suffix indicates that this takes an array of properties.
Also, commonly, when we have an array and a length parameter, the array comes
first. Reorder the arguments.
With `nmcli connection modify`, later options should overwrite earlier
ones. That did not work correctly with
nmcli --offline connection add type wifi \
wifi.ssid xxxx \
wifi.cloned-mac-address permanent \
wifi.mac-address-randomization 0
That's because "wifi.mac-address-randomization" is a mostly redundant
alias for certain "wifi.cloned-mac-address" options, and libnm does
various normalizations to make that somewhat seamless.
However, once "cloned-mac-address" property is set, setting any value of
"wifi.mac-address-randomization" has no effect, as it gets normalized
away by libnm. This is a sensible thing to do, in most cases to best
handle the deprecation/aliasing.
For nmcli, if the user sets "wifi.mac-address-randomization", it really
means to also reset the "cloned-mac-address". Thus nmcli needs to do
extra work to get this right.
The previous code is not entirely obvious, because as always,
verify() and normalize() must agree in what they are about to
do.
Make that clearer by adding _nm_setting_wireless_normalize_mac_address_randomization(),
which evaluates the desired settings. This is the used both by verify()
and normalize().
Some warnings in the generation of the translation files indicate real
errors, like strings that cannot be extracted for translations. Check
that no warnings are emitted.
Detected thanks to this message when generating the pot files:
id.po:14392: warning: internationalized messages should not contain
the '\v' escape sequence.
Due to the formatting of the string that contains it, it seems clear
that the \v character and the preceding work was put there by mistake.
Glib format specifiers are not gettext friendly. It even emits a
warning: src/core/main-utils.c:196: warning: Although being used in a
format string position, the msgid is not a valid C format string."
One possible solution is to use the equivalent format specifiers from
<inttypes.h> like PRId64, available since C99.
Even simpler is to cast the value to a type that is big enough to hold
it according to C specs (i.e. for int64: long long).
Fixes: 50f34217f9 ('main: use _nm_utils_ascii_str_to_int64 instead of strtol for reading pid')
A SSID of zero length, really looks "empty". Let
nm_utils_is_empty_ssid() indicate so too.
This affects some places, that try to guess what a hidden SSID looks
like. In general, it seems that treating a length of zero as empty, is
suitable also then.
nm_utils_escape_ssid() uses a static buffer, which makes it non
thread-safe. We shouldn't have such API in libnm. We could improve that
by using a thread-local storage, but that brings overhead, for a
function that really isn't useful.
It's not useful, because the escaping is very naive. You are better
served with:
- nm_utils_ssid_to_utf8(): gives UTF-8, but looses information.
- nm_utils_bin2hexstr(): does not loose information, but makes the
name unreadable.
Maybe the best way to escape the SSID (which can be binary, but usually
is UTF-8), are the utf8safe functions. That is because it makes the
blob UFT-8, while not loosing/hiding any bytes (the escaping can be
reversed). This API is currently not exposed to the users, if there were
a need, then this could be done as a 3rd way for printing SSIDs.
However, nm_utils_escape_ssid() is bad either way. Deprecate.
Setting
wifi.cloned-mac-address="stable-ssid"
should generate the same SSID as
connection.stable-id="${NETWORK_SSID}"
wifi.cloned-mac-address="stable"
For that to work correctly, we need to post-process the generated stable
id.
Fixes: d210923c0f ('wifi: add "wifi.cloned-mac-address=stable-ssid"')
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1813
Specially in the case of BEST_EFFORT it's not completely clear what each
flag means. For example: with BEST_EFFORT, in the case of partial
success should we return an error value or a success value?
Add some comments and documentation to clarify.
Check for invalid DNS, addresses and routes errors in the `_from_dbus`
functions. With NM_SETTING_PARSE_FLAGS_STRICT, stop parsing and return
error at first error. With NM_SETTING_PARSE_FLAGS_BEST_EFFORT don't
return any error and return the values of the list which are valid.
This is the same that was done in a previous commit for ipv4 properties.
Report an error if the data type of the address-labels received via DBus
is not the expected.
Also, fix the assignment of the labels to their corresponding addresses.
As they are matched by array position, if any invalid address was
received, the array of addresses that we generate is shorter than the
array of address-labels. We were not considering this so we were
assigning the address-labels to incorrect addresses. Fix it by moving the
assignment of the labels to _nm_utils_ip4_addresses_from_variant, where
we still have the information of what the original position in the array
the address had.
Invalid addresses received via DBUS were just ignored and filtered out,
only emitting a warning to the logs. If there were still some valid
addresses, those were configured and the client was unaware of the
errors. Only if there was not any valid address at all and method was
manual, an error was returned from `verify`, but not reflecting the
real cause:
ipv4.addresses: this property cannot be empty for 'method=manual'
Check for invalid addresses errors in the `_from_dbus` functions. With
NM_SETTING_PARSE_FLAG_STRICT, parsing is aborted on first error and
error is returned. With NM_SETTING_PARSE_BEST_EFFORT, we keep parsing
and set only the valid values.
Actually, the invalid addresses were dropped in a helper function that
converts from GVariant to NMIPAddress. As it is part of the public API,
we can't change now its signature to add the GError argument. Instead,
create a new internal function and call it from the public one. The
public function will ignore the error, as it was doing previously, but
it won't emit any warning to avoid spamming the logs (we don't even
know if ignoring the invalid values was intentional when calling the
function). The new internal function might be made public in
the future, deprecating the other, but probably it is not necessary
because clients are never going to receive invalid addresses from the
daemon.
Do the same as explained above for DNS entries and routes.
Also, fix the documentation of nm_utils_ip_routes_to/from_dbus, which
said that it accepts new style routes but described the old style ones.
With enabled assertions via LIBNM_CLIENT_DEBUG=WARN or
LIBNM_CLIENT_DEBUG=ERROR, still print the warning/error message to the
default destination, along the trace/debug messages.
For example, when you set LIBNM_CLIENT_DEBUG_FILE, then we want that
those messages end up in the file too, not only in g_log() output.
Also, g_warning() prints to stderr. If you set
LIBNM_CLIENT_DEBUG="WARN,trace,stdout", then we printed the warning to
stderr and the trace messages to stdout.
All debug messages should and up at the same place, and the g_warning()
and g_critical() messages are additional.
Also because glib's g_log() supports its own redirection and suppression
mechanism.
Previously, it was odd. The enum values like NML_DBUS_LOG_LEVEL_DEBUG were
actually the bit mask of all the levels "debug", "warn" and "error".
On the other hand, when parsing _nml_dbus_log_level, that variable only contained
the flags that were exactly requested. E.g. when setting LIBNM_CLIENT_DEBUG=trace,
then _nml_dbus_log_level only contained the trace flag 0x02. That was useful,
because with "LIBNM_CLIENT_DEBUG=warn,trace" the "warn" flag was not redundant,
it was used to enable printing via g_warning(). That was confusing.
Now, "LIBNM_CLIENT_DEBUG=warn,trace" is the same as "LIBNM_CLIENT_DEBUG=trace".
To enable printing via g_warning(), use "LIBNM_CLIENT_DEBUG=WARN,trace".
With this, we don't need this backward representation of the flags. Invert
it. The level enums are now just single bits.
Document LIBNM_CLIENT_DEBUG under nm_utils_print().
Also, add an alias "warn" for "warning" flag.
Also, no longer special treat "error" and "warning" flags to indicate
printing via g_criticial()/g_warning(). Previously, you could get
assertions via
$ G_DEBUG=fatal-warnings LIBNM_CLIENT_DEBUG=error,warning,trace nmcli
or you could enable all messages (including <error>/<warn> level)
without assertions via
$ G_DEBUG=fatal-warnings LIBNM_CLIENT_DEBUG=trace nmcli
However, it was not possible to enable only <error>/<warn> levels
without those assertions.
Now, "error"/"warn"/"warning" behave just like "debug"/"trace" to enable
message up to the specified level. It only implies printing to stderr
(or stdout or file, depending on "stdout" flag and
LIBNM_CLIENT_DEBUG_FILE).
Now, to enable redirect to g_warning()/g_error() use the new keywords
"ERROR"/"WARN"/"WARNING".
For testing, we probably want to enable such assertions. So to be
mostly backward compatible, we can run with
$ G_DEBUG=fatal-warnings LIBNM_CLIENT_DEBUG=error,warning,WARN nmcli
with that, the "error","warning" flags are redundant on newer libnm and
the WARN is ignored on older libnm.
This option was only introduced only to allow keeping the old behavior
in RHEL7, while the default order was changed from 'ifindex' to 'name'
in RHEL8. The usefulness of this option is questionable, as 'name'
together with predictable interface names should give predictable order.
When not using predictable interface names, the name is unpredictable
but so is the ifindex.
https://issues.redhat.com/browse/NMT-926https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1814
Now that we require glib 2.42, we can use G_PARAM_EXPLICIT_NOTIFY flag.
The benefit is that this flag saves a notification, when the property
value does not change.
The downside is, that implementations of set_property() must remember to
emit _notify() when required. This is somewhat alleviated by using
_nm_setting_property_set_property_direct(), which does this
automatically.
Se the flag for G_PARAM_EXPLICIT_NOTIFY for direct boolean properties.
For now, only do it for boolean properties, because of the danger of
getting this wrong. We must review all callers to make sure that they
don't implement set_properties() and don't forget to notify.
