The new hash table should destroy elements stolen from the hash table
returned by nm_utils_parse_variant_attributes().
Fixes: d094914120
(cherry picked from commit 31bda1b837)
After an update of the connection.mdns property, a reactivation is
needed to apply the new value.
Also, the ifcfg-rh variable name was wrong.
Fixes: 2e2ff6f27a
Autocompletion doesn't work in some cases because we present a prompt
ending with ":", but compare it with the string without ":" in the
autocomplete function. Fix this.
While at it, also add missing colon after prompt where needed.
Before, we would not autocomplete connection types that have an alias:
Connection type: <TAB><TAB>
6lowpan cdma macvlan vlan
802-11-olpc-mesh dummy olpc-mesh vpn
802-11-wireless ethernet ovs-bridge vxlan
802-3-ethernet generic ovs-interface wifi
adsl gsm ovs-port wimax
bluetooth infiniband pppoe wpan
bond ip-tunnel team
bridge macsec tun
Connection type: 8<TAB> [-> no completion]
Don't treat the default connection type (for example,
"802-3-ethernet") in a special way and allow it to be autocompleted,
because we already display it when the user did not enter any text.
The array returned by the completion function follows a special
convention. If the first element is set, it is used as the
completion. Otherwise, the remaining entries are the possible
completions.
_meta_abstract_complete() just returned an array of matching words and
so the first element was always used as completion. Instead, we must
use rl_completion_matches() to generate the array passing a generator
function.
https://bugzilla.redhat.com/show_bug.cgi?id=1588952
Add a new 'match' setting containing properties to match a connection
to devices. At the moment only the interface-name property is present
and, contrary to connection.interface-name, it allows the use of
wildcards.
As of upstream kernel v4.18-rc8.
Note that we name the features like they are called in ethtool's
ioctl API ETH_SS_FEATURES.
Except, for features like "tx-gro", which ethtool utility aliases
as "gro". So, for those features where ethtool has a built-in,
alternative name, we prefer the alias.
And again, note that a few aliases of ethtool utility ("sg", "tso", "tx")
actually affect more than one underlying kernel feature.
Note that 3 kernel features which are announced via ETH_SS_FEATURES are
explicitly exluded because kernel marks them as "never_changed":
#define NETIF_F_NEVER_CHANGE (NETIF_F_VLAN_CHALLENGED | \
NETIF_F_LLTX | NETIF_F_NETNS_LOCAL)
We will add a large number of offload features. That means, the output
of `nmcli connection show "$PROFILE"` would be very verbose, in case
the profile has a [ethtool] option.
Since this is newly added API, don't do that. Don't show ethtool properties
that are left unset.
A minor problem here is, that it becomes no longer obvious which
properties exist. We should however counter that by documentation.
Also, one could do:
$ nmcli connection modify "$PROFILE" ethtool.xxx x
Error: invalid property 'xxx': 'xxx' not among [feature-gro, feature-gso, feature-lro, feature-ntuple, feature-rx, feature-rxhash, feature-rxvlan, feature-sg, feature-tso, feature-tx, feature-txvlan, feature-tx-tcp6-segmentation, feature-tx-tcp-segmentation].
Likewise, bash completion still works as one would expect.
$ nmcli --complete-args connection modify "$PROFILE" ethtool.
ethtool.feature-gro
ethtool.feature-gso
ethtool.feature-lro
[...]
Note the output of
$ nmcli -f ethtool.feature-gro connection show "$PROFILE"
gives now nothing (if there is an ethtool section, but not this
particular feature). Maybe this shouldn't be like that. On the other
hand, specifying a connection setting that doesn't exist also gives
no output:
$ nmcli -f bond connection show "$PROFILE"
So, maybe this behavior is fine.
Historically, nmcli printed all fields during operations like
`nmcli connection show "$PROFILE"`. As we supported more and
more options, this resulted in a verbose output, of most properties
just being the default values.
To counter that, we added the '-overview' option. When given,
it would hide options that are set at their default. This option
was not the default, to preserve established behavior.
However, for new options, we can afford to hide them. Add a mechanism,
that property getters can mark their value to be hidden. At the moment,
there is no way to show these properties. However, we could add a
'-verbose' option, with the opposite meaning of '-overview'. Anyway,
that does not seem necessary at the moment.
Hiding properties from output is only acceptable for new properties
(otherwise we badly change behavior), and if the properties are set
at their default values (otherwise, we hide important information).
Also, add two more features "tx-tcp-segmentation" and
"tx-tcp6-segmentation". There are two reasons for that:
- systemd-networkd supports setting these two features,
so lets support them too (apparently they are important
enough for networkd).
- these two features are already implicitly covered by "tso".
Like for the "ethtool" program, "tso" is an alias for several
actual features. By adding two features that are already
also covered by an alias (which sets multiple kernel names
at once), we showcase how aliases for the same feature can
coexist. In particular, note how setting
"tso on tx-tcp6-segmentation off" will behave as one would
expect: all 4 tso features covered by the alias are enabled,
except that particular one.
Note that in NetworkManager API (D-Bus, libnm, and nmcli),
the features are called "feature-xyz". The "feature-" prefix
is used, because NMSettingEthtool possibly will gain support
for options that are not only -K|--offload|--features, for
example -C|--coalesce.
The "xzy" suffix is either how ethtool utility calls the feature
("tso", "rx"). Or, if ethtool utility specifies no alias for that
feature, it's the name from kernel's ETH_SS_FEATURES ("tx-tcp6-segmentation").
If possible, we prefer ethtool utility's naming.
Also note, how the features "feature-sg", "feature-tso", and
"feature-tx" actually refer to multiple underlying kernel features
at once. This too follows what ethtool utility does.
The functionality is not yet implemented server-side.
Previously, each (non abstract) NMSetting class had to register
its name and priority via _nm_register_setting().
Note, that libnm-core.la already links against "nm-meta-setting.c",
which also redundantly keeps track of the settings name and gtype
as well.
Re-use NMMetaSettingInfo also in libnm-core.la, to track this meta
data.
The goal is to get rid of private data structures that track
meta data about NMSetting classes. In this case, "registered_settings"
hash. Instead, we should have one place where all this meta data
is tracked. This was, it is also accessible as internal API,
which can be useful (for keyfile).
Note that NMSettingClass has some overlap with NMMetaSettingInfo.
One difference is, that NMMetaSettingInfo is const, while NMSettingClass
is only initialized during the class_init() method. Appart from that,
it's mostly a matter of taste, whether we attach meta data to
NMSettingClass, to NMMetaSettingInfo, or to a static-array indexed
by NMMetaSettingType.
Note, that previously, _nm_register_setting() was private API. That
means, no user could subclass a functioning NMSetting instance. The same
is still true: NMMetaSettingInfo is internal API and users cannot access
it to create their own NMSetting subclasses. But that is almost desired.
libnm is not designed, to be extensible via subclassing, nor is it
clear why that would be a useful thing to do. One day, we should remove
the NMSetting and NMSettingClass definitions from public headers. Their
only use is subclassing the types, which however does not work.
While libnm-core was linking already against nm-meta-setting.c,
nm_meta_setting_infos was unreferenced. So, this change increases
the binary size of libnm and NetworkManager (1032 bytes). Note however
that roughly the same information was previously allocated at runtime.
Not all properties that we want to handle in nmcli are actual GObject
properties. For the moment that was the case, but that's about to change.
This is a change in behavior with respect of the order in which
properties are reported. For example, print_setting_description()
now prints the property descriptions in a different order. However,
one might argue that this order makes more sense because:
- it's the same order as properties are listed in
"nm-meta-setting-desc.c". At that place, we have explict
control over the order and set it intentionally to suite
our needs best. The order of the GObject properties is
much less well defined.
- the order from "nm-meta-setting-desc.c" is used at several other
places. So, it makes sense to use the same order everywhere.
Add a new option that allows to activate a profile multiple times
(at the same time). Previoulsy, all profiles were implicitly
NM_SETTING_CONNECTION_MULTI_CONNECT_SINGLE, meaning, that activating
a profile that is already active will deactivate it first.
This will make more sense, as we also add more match-options how
profiles can be restricted to particular devices. We already have
connection.type, connection.interface-name, and (ethernet|wifi).mac-address
to restrict a profile to particular devices. For example, it is however
not possible to specify a wildcard like "eth*" to match a profile to
a set of devices by interface-name. That is another missing feature,
and once we extend the matching capabilities, it makes more sense to
activate a profile multiple times.
See also https://bugzilla.redhat.com/show_bug.cgi?id=997998, which
previously changed that a connection is restricted to a single activation
at a time. This work relaxes that again.
This only adds the new property, it is not used nor implemented yet.
https://bugzilla.redhat.com/show_bug.cgi?id=1555012
NM_SETTING_NAME is a special property that only has relevance
to libnm. It is inherited by all NMSetting instances. It is
read-only, and it has no corresponding value on D-Bus or nmcli.
Skip it during generate-setting-docs.py.
This also drops it from `man nm-settings`, where it doesn't belong.
The virtual function NMMetaType.get_nested() has only one caller:
nm_meta_abstract_info_get_nested(). That caller makes sure to
always pass in an @out_to_free argument, and that it is initialized
to NULL.
The local variable was just a copy of the (unchanging)
configuration nmc_config->multiline_output.
It is complicated enough to understand how nmc_config->print_output
and nmc_config->multiline_output affects nmc_print(). Don't use an
alias when referencing to nmc_config->multiline_output because it
hides where the value is used and where it causes a difference.
PrintDataCol contains a reference to the parent structure, for which
it was created. Previously, that was expressed via the "parent_idx"
field, which is an index into the list of all PrintDataCol entries.
That is inconvenient. Resolve the index to the actual pointer.
Note that during _output_selection_append() we still need to use
the index instead of resolving the pointer right away. That is because
_output_selection_append() grows the GArray into which the parent_idx
pointers to. So, obtaining the real pointer at that point would result
in using a dangling pointer later on.
Introduce a new step _output_selection_complete() which converts the
index into the actual pointer.
The print-output knows three modes: NORMAL, PRETTY, TERSE.
Never compare whether a mode is != a particular mode.
Instead, always explicitly check for the modes for which we enable a
particular behavior.
The effect is, that we always do something when the mode is set to a
particular value. We don't ever do something, if the mode is not set to
a particular value. Consistently opt-in is easier to understand.
The print-output knows three modes: NORMAL, PRETTY, TERSE.
Instead of using local variables "pretty" and "terse",
check for the output mode directly.
Note how we have tree modes, so mapping them to two boolean
variables is confusing. Especially at one place where we did:
pretty = (nmc_config->print_output != NMC_PRINT_TERSE);
while at other places:
pretty = (nmc_config->print_output == NMC_PRINT_PRETTY);
The header-cell is the header for a column with possibly many
rows.
We must not set:
header_cell->skip = nmc_config->overview && is_default;
for a particular cell, because it does not take into accound
the neighbouring rows. It is only correct, if there is only
one row.
Invert the logical meaning and rename "strip" to "to_print".
Each cell then can opt-in, whether it needs to be printed.
Only if no cell opts-in, it will be skipped.
Fixes: 9a19bbcb2f
Instead of re-fetching the UUID each time with a D-Bus call,
cache/memoize it.
On my system, this improves
$ make check-local-clients-tests-test-client
from 20.9 to 20.4 seconds (- 2.6%). That is not stellar, but noticible
enough to warrant this simple change.
These nmcli calls are inside a Util.iter_nmcli_output_modes()
loop, hence, --terse will passed to them already.
Specifying --terse more than once, causes nmcli to fail.
We didn't actually want to test how nmcli rejects such
duplicate arguments. Adjust the test.
Add more tests for various output modes of nmcli.
This most interestingly includes a terse output for
$ nmcli device status
Which was not tested previously (but a later commit will change
behavior here).
This blows up the number of tests even further.
Previously, the test invoked nmcli 850 times (taking ~15 seconds
ony my machine), afterwards, it is 1334 times (taking ~20 seconds).
No doubt, this seems excessive and questionable.
However, I maintain that the computer doesn't mind running a lot of
redundant tests. The important thing is, that we cover as many cases
as possible. Trying at the same time to minimize the number of tests
by avoiding duplicates and redundant tests, is just a lot of manual
labor. Manual labor that must be repeated whenever new tests are added, to
ensure that this test case is not yet covered. So, I am fine
with the large number of tests. Let the computer do the work.
Instead of letting each nmcli run write an individual .expected file,
combine the output of multiple runs in one file (per test).
Advantages:
- since there is a very large number of tests, having a file for each
tests is cumbersome. For example, since they are all added to
$(EXTRA_DIST) (and since we use non-recursive make), autoconf can
easily hit a length limit when processing all the file names.
- previously, whenever we added tests, all .expected files shifted
and the diff was large. Now, there is a chance that the diff is
smaller and more accurate.
1) the command line gets shorter. I frequently run `make V=1` to see
the command line arguments for the compiler, and there is a lot
of noise.
2) define each of these variables at one place. This makes it easy
to verify that for all compilation units, a particular
define has the same value. Previously that was not obvious or
even not the case (see commit e5d1a71396
and commit d63cf1ef2f).
The point is to avoid redundancy.
3) not all compilation units need all defines. In fact, most modules
would only need a few of these defines. We aimed to pass the necessary
minium of defines to each compilation unit, but that was non-obvious
to get right and often we set a define that wasn't used. See for example
"src_settings_plugins_ibft_cppflags" which needlessly had "-DSYSCONFDIR".
This question is now entirely avoided by just defining all variables in
a header. We don't care to find the minimum, because every component
gets anyway all defines from the header.
4) this also avoids the situation, where a module that previously did
not use a particular define gets modified to require it. Previously,
that would have required to identify the missing define, and add
it to the CFLAGS of the complation unit. Since every compilation
now includes "config-extra.h", all defines are available everywhere.
5) the fact that each define is now available in all compilation units
could be perceived as a downside. But it isn't, because these defines
should have a unique name and one specific value. Defining the same
name with different values, or refer to the same value by different
names is a bug, not a desirable feature. Since these defines should
be unique accross the entire tree, there is no problem in providing
them to every compilation unit.
6) the reason why we generate "config-extra.h" this way, instead of using
AC_DEFINE() in configure.ac, is due to the particular handling of
autoconf for directory variables. See [1].
With meson, it would be trivial to put them into "config.h.meson".
While that is not easy with autoconf, the "config-extra.h" workaround
seems still preferable to me.
[1] https://www.gnu.org/software/autoconf/manual/autoconf-2.63/html_node/Installation-Directory-Variables.html
We commonly don't use the glib typedefs for char/short/int/long,
but their C types directly.
$ git grep '\<g\(char\|short\|int\|long\|float\|double\)\>' | wc -l
587
$ git grep '\<\(char\|short\|int\|long\|float\|double\)\>' | wc -l
21114
One could argue that using the glib typedefs is preferable in
public API (of our glib based libnm library) or where it clearly
is related to glib, like during
g_object_set (obj, PROPERTY, (gint) value, NULL);
However, that argument does not seem strong, because in practice we don't
follow that argument today, and seldomly use the glib typedefs.
Also, the style guide for this would be hard to formalize, because
"using them where clearly related to a glib" is a very loose suggestion.
Also note that glib typedefs will always just be typedefs of the
underlying C types. There is no danger of glib changing the meaning
of these typedefs (because that would be a major API break of glib).
A simple style guide is instead: don't use these typedefs.
No manual actions, I only ran the bash script:
FILES=($(git ls-files '*.[hc]'))
sed -i \
-e 's/\<g\(char\|short\|int\|long\|float\|double\)\>\( [^ ]\)/\1\2/g' \
-e 's/\<g\(char\|short\|int\|long\|float\|double\)\> /\1 /g' \
-e 's/\<g\(char\|short\|int\|long\|float\|double\)\>/\1/g' \
"${FILES[@]}"
The function nmc_print() receives a list of "targets". These are essentially
the rows that should be printed (while the "fields" list represents the columns).
When filling the cells with values, it calles repeatedly get_fcn() on the
column descriptors (fields), by passing each row (target).
The caller must be well aware that the fields and targets are
compatible. For example, in some cases the targets are NMDevice
instances and the target type must correspond to what get_fcn()
expects.
Add another user-data pointer that is passed on along with the
targets. That is useful, if we have a list of targets/rows, but
pass in additional data that applies to all rows alike.
It is still unused.
