We don't need DEFINE_SETTER_STR_LIST_MULTI() to define a static function
that is called by the set_fcn() implementation. Instead, let the macro
define the actual set_fcn() function.
It's fundamentally wrong to have separate "remove_fcn" and "set_fcn"
implementations. Set, reset, add, and remove are all similar, and should
be implemented in a similar manner.
Merge the implementations all in set-property, which now can:
- reset the value (value == NULL && modifier == '\0')
- set a value (value != NULL && modifier == '\0')
- add a value (value != NULL && modifier == '+')
- remove a value (value != NULL && modifier == '-')
The real problem is that remove_fcn() behaves fundamentally different
from set_fcn(). You can do "+setting.property value1,value2" but you
cannot remove them the same way. That is because most remove_fcn()
implementations don't expect a list of values. But it's also because of
the unnatural split between set_fcn() and remove_fcn().
The next commit will merge set_fcn(), remove_fcn() and reset property.
This commit just merges them all in nmc_setting_set_property().
ipv4_method_changed_cb() and ipv6_method_changed_cb() are horrible hacks, as they
use a static variable to cache the previous value.
Anyway, don't fix that now.
We are going to drop nmc_property_get_gvalue()/nmc_property_set_gvalue()
because that works only with GObject based properties -- and having this
API around wrongly suggests it works in general. Use the native types
directly.
"reset" is just a special case of "set". We can keep nmc_setting_reset_property() as a convenience
function, but it must be implemented based on nmc_setting_set_property().
Also, reset only used nmc_property_set_default_value(), which only works
with GObject based properties. It's wrong to assume that all properties
are GObject based. By implementing it based via nmc_setting_set_property()
we can fix this (later).
Not all implementations support having the value being an index.
For example, the implementations that are done via DEFINE_REMOVER_OPTION() macro.
The meaning of the "value" string must not be determined by
nmc_setting_remove_property_option(). It's up to the implementation
to decide whether to allow an index and how to interpret it.
$ nmcli connection add type vlan autoconnect no con-name v dev vlan.1 id 1
$ nmcli connection modify v +vlan.ingress-priority-map 1:2,2:3
$ nmcli connection modify v +vlan.ingress-priority-map 2:3,4:5
$ nmcli connection modify v -vlan.ingress-priority-map 1:2,4:5
Warning: only one mapping at a time is supported; taking the first one (1:2)
Part 1, which addresses the issue for simple properties that have
a plain remove-by-value function.
Rationale:
Removing a value/index that does not exist should not be a failure.
Woule you like:
$ nmcli connection modify "$PROFILE" autoconnect no
$ nmcli connection modify "$PROFILE" autoconnect no
Error: autoconnect is already disabled
So, why would it be a good idea to fail during
$ nmcli connection modify "$PROFILE" -vpn.data ca
$ nmcli connection modify "$PROFILE" -vpn.data ca
Error: failed to remove a value from vpn.data: invalid option 'ca'.
Generally, it should not be an error to remove an option, as long
as the option itself is valid. For example,
$ nmcli connection modify "$PROFILE" -vlan.ingress-priority-map bogus
should fail, but
$ nmcli connection modify "$PROFILE" -vlan.ingress-priority-map 1:5
should succeed even if there was nothing to remove.
The "from" part is like a key for the egress/ingress priority map.
Extend nm_setting_vlan_remove_priority_str_by_value() to accept only the
"from" part when finding and deleting value. This allows for:
$ nmcli connection modify "$PROFILE" -vlan.ingress-priority-map '4:'
$ nmcli connection modify "$PROFILE" -vlan.ingress-priority-map '4:*'
to fuzzy match the value to remove.
How hard can it be to use strtol()? Apparently very. You need to check
errno, the endpointer, and also not pass in an empty string.
Previously, nmc_string_to_uint_base() would silently accept "" as valid
number. That's a bug.
Btw, let's not use strtol() (or nmc_string_to_uint*()). We have
_nm_utils_ascii_str_to_int64(), which gets all of this right.
We have code in "shared/nm-utils" which are general purpose
helpers, independent of "libnm", "libnm-core", "clients" and "src".
We have shared code like "shared/nm-ethtool-utils.h" and
"shared/nm-meta-setting.h", which is statically linked, shared
code that contains libnm related helpers. But these helpers already
have a specific use (e.g. they are related to ethtool or NMSetting
metadata).
Add a general purpose helper that:
- depends (and extends) libnm-core
- contains unrelated helpers
- can be shared (meaning it will be statically linked).
- this code can be used by any library user of "libnm.so"
(nmcli, nm-applet) and by "libnm-core" itself. Thus, "src/"
and "libnm/" may also use this code indirectly, via "libnm-core/".
Support importing ".conf" files as `wg-quick up` supports it.
`wg-quick` parses several options under "[Interface]" and
passes the remainder to `wg setconf`.
The PreUp/PreDown/PostUp/PostDown options are of course not supported.
"Table" for the moment behaves different.
(cherry picked from commit a3a8583c31)
If we already specified "type" or "file", don't offer it for
completion again.
$ nmcli connection import type openvpn <TAB>
file type
(cherry picked from commit fea0f4a5ea)
@secrets is unreferenced at the end of request_secrets_from_ui() and
so try_spawn_vpn_auth_helper() must take a reference to it.
Fixes: 1a0fc8d437
(cherry picked from commit b57a3a4cc6)
The tests run nmcli with Polish locale and compare the output.
After modifying Polish translation we must regenerate the expected
output.
Fixes: 01b7b32afb
(cherry picked from commit 3089b7df52)
Rework the explicit implementation of NM_SETTING_802_1X_PASSWORD_RAW
handling to generically handle GBytes properties.
Note that the NM_SETTING_802_1X_PASSWORD_RAW setter accepts a legacy
format where hex-words are separated by space. I don't think we want
to support this format for new options.
So, there are two possibilities:
1) either leave _set_fcn_802_1x_password_raw() as-is, with the special
handling.
2) interpret a property-data gobject_bytes.legacy_format.
1) seems to make more sense, because there is only one such property,
and we won't use this for new properties. However let's do 2), because
it shows nicely the two styles side-by-side. In other words, let's
password-raw also be a _pt_gobject_bytes typed property, with some
special legacy handling. Instead, of having it an entirely separate
property type (with a different setter implementation). I think it's
better to have the parts where they differ pushed down (the "stack") as
much as possible.
- it's less lines of code (for the caller).
- it's a function that can be easier unit-tested on its own.
Possibly there are already other unit-tests that cover it.
- it's more efficient than the GString based implementation.
- it reuses our one and only bin-to-hexstr implementation.
For now only add the core settings, no peers' data.
To support peers and the allowed-ips of the peers is more complicated
and will be done later. It's more complicated because these are nested
lists (allowed-ips) inside a list (peers). That is quite unusual and to
conveniently support that in D-Bus API, in keyfile format, in libnm,
and nmcli, is a effort.
Also, it's further complicated by the fact that each peer has a secret (the
preshared-key). Thus we probably need secret flags for each peer, which
is a novelty as well (until now we require a fixed set of secrets per
profile that is well known).
Previously, Wi-Fi scans uses polkit action
"org.freedesktop.NetworkManager.network-control". This is introduced
in commit 5e3e19d0. But in a system with restrict polkit rules, for
example "org.freedesktop.NetworkManager.network-control" was set as
auth_admin. When you open the network panel of GNOME Control Center, a
polkit dialog will keep showing up asking for admin password, as GNOME
Control Center scans the Wi-Fi list every 15 seconds.
Fix that by adding a new polkit action
"org.freedesktop.NetworkManager.wifi.scan" so that distributions can
add specific rule to allow Wi-Fi scans.
[thaller@redhat.com: fix macro in "shared/nm-common-macros.h"]
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/merge_requests/68
When asking for the preshared-key for WireGuard peers, the secret request
will be very verbose with redundant information. Allow suppressing the entry
id from the prompt.
It's not really used, but we shouldn't just forget about it.
Currently, we fill requests only based on the connection-type, ignoring
the setting-name. I guess, the concept of requesting secrets for a setting
is utterly broken. But equally broken it is to just look at the connection
(type). At least, don't just throw parts of the request away but keep
it.
