Our NMObject implementations should behave in a similar manner.
For example, string properties should be coerced the a consistent
manner.
Add functions _nml_coerce_property_*() for that. Of course, they
are trivial. Their value is not that they encapsulate some complex
implementation, but that they convey a general concept of how we
want to handle certain properties in NMClient's object cache.
This looks up the GParamSpec from the obj_properties and is
thus more efficient. Also, the generated _notify() function
has the proper argument type and is thus generally preferable.
This is not merely cosmetic. I will need the obj_properties
array to lookup GParamSpec by their PROP_* enum value. The
alternative would be lookup by name, which is more expensive.
Drop uses of nmdbus_manager_call_get_permissions_sync().
Of course, we should ever call synchronous API while initizliaing the
NMClient. Needs to be fixed eventually.
We still need the bits in "nm-manager.c", to wait until the
NMActiveConnection instance is ready. This is now done by
nm_manager_complete_active_connection().
Eventually, I will refactor libnm to no longer use gdbus-codegen and
no GDBusProxy. In preparation of that, we must stop using that
API.
As first step, change nm_client_deactivate_connection(). Note how this
was done previously:
- nm_client_deactivate_connection() calls nm_manager_deactivate_connection()
- nmdbus_manager_call_deactivate_connection_sync() calls g_dbus_proxy_call_sync()
- g_dbus_proxy_call_sync() calls g_dbus_connection_call_sync()
Currently this is still a bit ugly, because NMClient doesn't directly
track the GDBusConnection nor the name owner. Instead, we need to peel
it out of the object manager. One day, that will all be nicer, but first
get rid of gdbus-codegen.
Synchrnous initialization is problmatic and needs cleanup.
get_permissions_sync() is an internal function, that has only one
caller. We need to keep track of functions that make synchronous D-Bus
calls. Move the synchronous call into the caller, so that it's clearer
who calls such API.
We no longer add these. If you use Emacs, configure it yourself.
Also, due to our "smart-tab" usage the editor anyway does a subpar
job handling our tabs. However, on the upside every user can choose
whatever tab-width he/she prefers. If "smart-tabs" are used properly
(like we do), every tab-width will work.
No manual changes, just ran commands:
F=($(git grep -l -e '-\*-'))
sed '1 { /\/\* *-\*- *[mM]ode.*\*\/$/d }' -i "${F[@]}"
sed '1,4 { /^\(#\|--\|dnl\) *-\*- [mM]ode/d }' -i "${F[@]}"
Check remaining lines with:
git grep -e '-\*-'
The ultimate purpose of this is to cleanup our files and eventually use
SPDX license identifiers. For that, first get rid of the boilerplate lines.
"libnm-core" implements common functionality for "NetworkManager" and
"libnm".
Note that clients like "nmcli" cannot access the internal API provided
by "libnm-core". So, if nmcli wants to do something that is also done by
"libnm-core", , "libnm", or "NetworkManager", the code would have to be
duplicated.
Instead, such code can be in "libnm-libnm-core-{intern|aux}.la".
Note that:
0) "libnm-libnm-core-intern.la" is used by libnm-core itsself.
On the other hand, "libnm-libnm-core-aux.la" is not used by
libnm-core, but provides utilities on top of it.
1) they both extend "libnm-core" with utlities that are not public
API of libnm itself. Maybe part of the code should one day become
public API of libnm. On the other hand, this is code for which
we may not want to commit to a stable interface or which we
don't want to provide as part of the API.
2) "libnm-libnm-core-intern.la" is statically linked by "libnm-core"
and thus directly available to "libnm" and "NetworkManager".
On the other hand, "libnm-libnm-core-aux.la" may be used by "libnm"
and "NetworkManager".
Both libraries may be statically linked by libnm clients (like
nmcli).
3) it must only use glib, libnm-glib-aux.la, and the public API
of libnm-core.
This is important: it must not use "libnm-core/nm-core-internal.h"
nor "libnm-core/nm-utils-private.h" so the static library is usable
by nmcli which couldn't access these.
Note that "shared/nm-meta-setting.c" is an entirely different case,
because it behaves differently depending on whether linking against
"libnm-core" or the client programs. As such, this file must be compiled
twice.
(cherry picked from commit af07ed01c0)
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
#0 0x00007fffea7481e5 in _g_log_abort (breakpoint=1) at gmessages.c:554
#1 0x00007fffea74951d in g_logv (log_domain=0x7fffea78e00e "GLib", log_level=G_LOG_LEVEL_CRITICAL, format=<optimized out>, args=args@entry=0x7fffffffcbb0)
at gmessages.c:1371
#2 0x00007fffea7496f3 in g_log
(log_domain=log_domain@entry=0x7fffea78e00e "GLib", log_level=log_level@entry=G_LOG_LEVEL_CRITICAL, format=format@entry=0x7fffea798320 "%s: assertion '%s' failed")
at gmessages.c:1413
#3 0x00007fffea749f2d in g_return_if_fail_warning
(log_domain=log_domain@entry=0x7fffea78e00e "GLib", pretty_function=pretty_function@entry=0x7fffea799d40 <__func__.4759> "g_atomic_ref_count_dec", expression=expression@entry=0x7fffea799ca1 "g_atomic_int_get (arc) > 0") at gmessages.c:2762
#4 0x00007fffea754c12 in g_atomic_ref_count_dec (arc=arc@entry=0x5555558c5280) at grefcount.c:260
#5 0x00007fffea7302c6 in g_hash_table_unref (hash_table=0x5555558c5240) at ghash.c:1101
#6 0x00007fffea4b6dbc in clear_op_res (simple=0x55555587ed90 [GSimpleAsyncResult]) at gsimpleasyncresult.c:248
#7 0x00007fffea4b6dbc in g_simple_async_result_finalize (object=0x55555587ed90 [GSimpleAsyncResult]) at gsimpleasyncresult.c:268
#8 0x00007fffea67b949 in g_object_unref (_object=<optimized out>) at gobject.c:3346
#9 0x00007fffea67b949 in g_object_unref (_object=0x55555587ed90) at gobject.c:3238
#10 0x00007fffe95dea2d in checkpoint_rollback_cb (object=<optimized out>, result=<optimized out>, user_data=0x55555587ed90) at libnm/nm-manager.c:1584
#11 0x00007fffea4ca834 in g_task_return_now (task=0x5555558b5c80 [GTask]) at gtask.c:1148
#12 0x00007fffea4cb196 in g_task_return (task=0x5555558b5c80 [GTask], type=<optimized out>) at gtask.c:1206
#13 0x00007fffea5096bb in reply_cb (connection=<optimized out>, res=<optimized out>, user_data=0x5555558b5c80) at gdbusproxy.c:2596
#14 0x00007fffea4ca834 in g_task_return_now (task=0x5555558b5d50 [GTask]) at gtask.c:1148
#15 0x00007fffea4cb196 in g_task_return (task=0x5555558b5d50 [GTask], type=<optimized out>) at gtask.c:1206
#16 0x00007fffea4fdd4a in g_dbus_connection_call_done (source=<optimized out>, result=0x5555558b5e20, user_data=0x5555558b5d50) at gdbusconnection.c:5715
#17 0x00007fffea4ca834 in g_task_return_now (task=0x5555558b5e20 [GTask]) at gtask.c:1148
#18 0x00007fffea4ca86d in complete_in_idle_cb (task=task@entry=0x5555558b5e20) at gtask.c:1162
#19 0x00007fffea73e97b in g_idle_dispatch (source=0x7fffdc04eb90, callback=0x7fffea4ca860 <complete_in_idle_cb>, user_data=0x5555558b5e20) at gmain.c:5620
#20 0x00007fffea74206d in g_main_dispatch (context=0x5555557c8410) at gmain.c:3182
#21 0x00007fffea74206d in g_main_context_dispatch (context=context@entry=0x5555557c8410) at gmain.c:3847
#22 0x00007fffea742438 in g_main_context_iterate (context=0x5555557c8410, block=block@entry=1, dispatch=dispatch@entry=1, self=<optimized out>) at gmain.c:3920
#23 0x00007fffea742762 in g_main_loop_run (loop=0x55555584ed00) at gmain.c:4116
Fixes: c3efedf54b
Add a "a{sv}" output argument to "AddAndActivate2" D-Bus API.
"AddAndActivate2" replaces "AddAndActivate" with more options.
It also has a dictionary argument to be forward compatible so that we
hopefully won't need an "AddAndActivate3". However, it lacked a similar
output dictionary. Add it for future extensibility. I think this is
really to workaround a shortcoming of D-Bus, which does provide strong
typing and type information about its API, but does not allow to extend
an existing API in a backward compatible manner. So we either resort to
Method(), Method2(), Method3() variants, or a catch-all variant with a
generic "a{sv}" input/output argument.
In libnm, rename "nm_client_add_and_activate_connection_options()" to
"nm_client_add_and_activate_connection2()". I think libnm API should have
an obvious correspondence with D-Bus API. Or stated differently, if
"AddAndActivateOptions" would be a better name, then the D-Bus API should
be renamed. We should prefer one name over the other, but regardless
of which is preferred, the naming for D-Bus and libnm API should
correspond.
In this case, I do think that AddAndActivate2() is a better name than
AddAndActivateOptions(). Hence I rename the libnm API.
Also, unless necessary, let libnm still call "AddAndActivate" instead of
"AddAndActivate2". Our backward compatibility works the way that libnm
requires a server version at least as new as itself. As such, libnm
theoretically could assume that server version is new enough to support
"AddAndActivate2" and could always use the more powerful variant.
However, we don't need to break compatibility intentionally and for
little gain. Here, it's easy to let libnm also handle old server API, by
continuing to use "AddAndActivate" for nm_client_add_and_activate_connection().
Note that during package update, we don't restart the currently running
NetworkManager instance. In such a scenario, it can easily happen that
nmcli/libnm is newer than the server version. Let's try a bit harder
to not break that.
Changes as discussed in [1].
[1] https://gitlab.freedesktop.org/NetworkManager/NetworkManager/merge_requests/37#note_79876
This adds the new methods nm_client_add_and_activate_connection_options_*
and ports the existing methods to use the new AddAndActivateConnection2
call rather than AddAndActivateConnection, allowing further parameters
to be passed in.
We must disconnect ActivateInfo before invoking callbacks.
Otherwise, it can happen that the callee cancels the cancellable,
which in turn enters activate_info_complete() again, and leads
to a crash.
https://bugzilla.redhat.com/show_bug.cgi?id=1642625
Coccinelle:
@@
expression a, b;
@@
-a ? a : b
+a ?: b
Applied with:
spatch --sp-file ternary.cocci --in-place --smpl-spacing --dir .
With some manual adjustments on spots that Cocci didn't catch for
reasons unknown.
Thanks to the marvelous effort of the GNU compiler developer we can now
spare a couple of bits that could be used for more important things,
like this commit message. Standards commitees yet have to catch up.
The libnm API fir checkpoints was only introduced with 1.11. It
is not yet stable, so there is still time to adjust it. Note that
this changes API/ABI of the development branch.
Changes:
- we only add async variants of the checkpoint functions. I believe
that synchronous D-Bus methods are fundamentally flawed, because
they mess up the ordering of events.
Rename the async functions by removing the "_async" suffix. This
matches glib style, for which the async form is also not specially
marked.
- for function that refere to a particular checkpoint (rollback and
destroy), accept the D-Bus path as string, instead of an NMCheckpoint
instance. This form is more flexible, because it allows to use
the function without having a NMCheckpoint instance at hand. On the
other hand, if one has a NMCheckpoint instance, he can trivially
obtain the path to make the call.
This allows to adjust the timeout of an existing checkpoint.
The main usecase of checkpoints, is to have a fail-safe when
configuring the network remotely. By allowing to reset the timeout,
the user can perform a series of actions, and keep bumping the
timeout. That way, the entire series is still guarded by the same
checkpoint, but the user can start with short timeout, and
re-adjust the timeout as he goes along.
The libnm API only implements the async form (at least for now).
Sync methods are fundamentally wrong with D-Bus, and it's probably
not needed. Also, follow glib convenction, where the async form
doesn't have the _async name suffix. Also, accept a D-Bus path
as argument, not a NMCheckpoint instance. The libnm API should
not be more restricted than the underlying D-Bus API. It would
be cumbersome to require the user to lookup the NMCheckpoint
instance first, especially since libnm doesn't provide an efficient
or convenient lookup-by-path method. On the other hand, retrieving
the path from a NMCheckpoint instance is always possible.
Now that the D-Bus signals in server are reordered, creating
a checkpoint in libnm crashes:
$ examples/python/gi/checkpoint.py create 4
#0 0x00007ffff6d011ee in __strcmp_sse2_unaligned () at /lib64/libc.so.6
#1 0x00007fffeb611c90 in find_checkpoint_info (manager=manager@entry=0x5555559e5110 [NMManager], path=0x7fffdc0092f0 "/org/freedesktop/NetworkManager/Checkpoint/6")
at libnm/nm-manager.c:153
#2 0x00007fffeb611d8f in checkpoint_added (manager=0x5555559e5110 [NMManager], checkpoint=checkpoint@entry=0x555555a122d0 [NMCheckpoint]) at libnm/nm-manager.c:1194
#3 0x00007fffef7db929 in g_cclosure_marshal_VOID__OBJECTv (closure=0x5555559e4b30, return_value=<optimized out>, instance=<optimized out>, args=<optimized out>, marshal_data=<optimized out>, n_params=<optimized out>, param_types=0x5555559e2fc0) at gmarshal.c:2102
#4 0x00007fffef7d8976 in _g_closure_invoke_va (closure=0x5555559e4b30, return_value=0x0, instance=0x5555559e5110, args=0x7fffffffc1c8, n_params=1, param_types=0x5555559e2fc0)
at gclosure.c:867
#5 0x00007fffef7f3ff4 in g_signal_emit_valist (instance=instance@entry=0x5555559e5110, signal_id=signal_id@entry=97, detail=0, var_args=var_args@entry=0x7fffffffc1c8) at gsignal.c:3300
#6 0x00007fffef7f4b48 in g_signal_emit_by_name (instance=instance@entry=0x5555559e5110, detailed_signal=detailed_signal@entry=0x7fffffffc310 "checkpoint-added") at gsignal.c:3487
#7 0x00007fffeb6156d1 in deferred_notify_cb (data=0x5555559e5110) at libnm/nm-object.c:219
#8 0x00007fffeb615ae7 in object_property_maybe_complete (self=0x5555559e5110 [NMManager]) at libnm/nm-object.c:555
#9 0x00007fffeb615e5d in object_created (obj=<optimized out>, path=<optimized out>, user_data=<optimized out>) at libnm/nm-object.c:576
#10 0x00007fffeb61648b in handle_object_array_property (pi=<optimized out>, value=0x7fffdc075070, property_name=0x7fffeb67f117 "checkpoints", self=0x5555559e5110 [NMManager])
at libnm/nm-object.c:671
#11 0x00007fffeb61648b in handle_property_changed (self=self@entry=0x5555559e5110 [NMManager], dbus_name=<optimized out>, value=<optimized out>) at libnm/nm-object.c:740
#12 0x00007fffeb6166e9 in properties_changed (proxy=<optimized out>, changed_properties=<optimized out>, invalidated_properties=<optimized out>, user_data=0x5555559e5110)
at libnm/nm-object.c:772
...
That is, because NetworkManager now first emits signals that the checkpoint
object was created, before answering the D-Bus request. That makes more
sense, but leads to this crash.
The ugliness of how libnm handles object visibility is considerable.
libnm hides objects until they are fully initialized. So, when
the async create-checkpoint operation returns, the object might not
yet be ready to be exposed. We need to delay the result. It would be
better if the API would simply return the created path.
If an operation is cancelled through the GCancellable, then the idiom is
that the operation is always cancelled, even if it has finished
successfully. To ensure this is the case, add calls to
g_simple_async_result_set_check_cancellable everywhere.
Without this, e.g. gnome-control-center will crash when switching away
from the power panel quickly, as the NMClient creation finishes
asynchronously and g-c-c assume that G_IO_ERROR_CANCELLED is returned to
ensure it doesn't access the now invalid user_data parameter.
https://bugzilla.gnome.org/show_bug.cgi?id=794088
We also do this for libnm and libnm-core, where it causes visible changes
in behavior. But if somebody would rely on the hashing implementation
for hash tables, it would be seriously flawed.
We also do this for libnm, where it causes visible changes
in behavior. But if somebody would rely on the hashing implementation
for hash tables, it would be seriously flawed.
GHashTable optimizes a NULL equality function to use direct pointer
comparison. That saves the overhead of calling g_direct_equal().
This is also documented behavior for g_hash_table_new().
While at it, also don't pass g_direct_hash() but use the default
of %NULL. The behavior is the same, but consistently don't use
g_direct_hash().
Currently, after a client performs a connectivity check it cannot
access the up-to-date value of the manager.connectivity property right
away, but it must wait that the queued PropertiesChanged signal is
processed, which is cumbersome.
Arguably, clients already receive the new connectivity value as the
result of the connectivity check call, so they don't have to read it
from the object; however it would be better if the right value of the
object property was available immediately as well.
https://bugzilla.gnome.org/show_bug.cgi?id=784629
