The options hash is never used except for BOUND events, so don't
bother caching it in the DHCP client object. Just pass it along
with the BOUND state change, like the IP configuration object.
DHCP failure should just clean up the client in all cases. This
also has the benefit of removing the signal handler for the DHCP
client's state-change signal before telling the client to terminate,
which will simplify some DHCP code later.
No reason to have two signals for the same thing. Previously, the
TIMEOUT signal was used for the internal overall DHCP transaction
bound, while DHCP_STATE_TIMEOUT/DHC_TIMEOUT was a signal from
the DHCP client itself that something had timed out. But in both
cases the results should be the same, so just collapse the
stand-alone TIMEOUT signal into the DHCP_STATE_TIMEOUT state.
The existing DHC_* states are pretty specific to dhclient, and aren't
useful for more generalized DHCP. NetworkManager wasn't using many
of the states anyway, and doesn't need to differentiate between
states like REBOOT/REBIND/RENEW anyway. So simplify the DHCP states
into the ones we really care about.
No need to keep references of the singleton and take an additional ref
when accessing nm_firewall_manager_get().
Especially, since the firewall manager instance was nowhere passed in from
externally, it doesn't even sense for some vague testing purporse. Not to
mention, that there are no tests that actually inject a firewall manager stub.
Signed-off-by: Thomas Haller <thaller@redhat.com>
An assertion in nm_dhcp4_config_get_dbus_path() has been actually fixed
by 3d6936b2cc (hopefully for all cases).
But still I think we should check _config here instead of _client.
backtrace:
#0 0x0000003370c504e9 in g_logv (log_domain=0x4c14cc "NetworkManager", log_level=G_LOG_LEVEL_CRITICAL, format=<optimized out>, args=args@entry=0x7fff420d05c0) at gmessages.c:989
#1 0x0000003370c5063f in g_log (log_domain=log_domain@entry=0x4c14cc "NetworkManager", log_level=log_level@entry=G_LOG_LEVEL_CRITICAL, format=format@entry=0x3370cbc89a "%s: assertion '%s' failed") at gmessages.c:1025
#2 0x0000003370c50679 in g_return_if_fail_warning (log_domain=log_domain@entry=0x4c14cc "NetworkManager", pretty_function=pretty_function@entry=0x4b4710 <__PRETTY_FUNCTION__.35285> "nm_device_get_iface",
expression=expression@entry=0x4b160a "self != NULL") at gmessages.c:1034
#3 0x000000000042e39b in nm_device_get_iface (self=0x0) at devices/nm-device.c:501
#4 0x00000000004393cc in nm_device_activate_stage3_ip_config_start (user_data=<optimized out>) at devices/nm-device.c:4062
#5 0x0000003370c492a6 in g_main_dispatch (context=0x153b190) at gmain.c:3066
#6 g_main_context_dispatch (context=context@entry=0x153b190) at gmain.c:3642
#7 0x0000003370c49628 in g_main_context_iterate (context=0x153b190, block=block@entry=1, dispatch=dispatch@entry=1, self=<optimized out>) at gmain.c:3713
#8 0x0000003370c49a3a in g_main_loop_run (loop=0x153ae30) at gmain.c:3907
#9 0x0000000000429f15 in main (argc=1, argv=0x7fff420d0b78) at main.c:678
Signed-off-by: Thomas Haller <thaller@redhat.com>
When the slave device gets removed, the master is not in a state-change when
calling nm_device_release_one_slave(). This triggers a warning [1].
[1] backtrace:
#0 0x0000003370c504e9 in g_logv (log_domain=0x4c144c "NetworkManager", log_level=G_LOG_LEVEL_WARNING, format=<optimized out>, args=args@entry=0x7fff8b1d35b0) at gmessages.c:989
#1 0x0000003370c5063f in g_log (log_domain=log_domain@entry=0x4c144c "NetworkManager", log_level=log_level@entry=G_LOG_LEVEL_WARNING, format=format@entry=0x3370cc1fdc "%s") at gmessages.c:1025
#2 0x0000003370c50956 in g_warn_message (domain=domain@entry=0x4c144c "NetworkManager", file=file@entry=0x4b14d5 "devices/nm-device.c", line=line@entry=841, func=func@entry=0x4b48f0 <__FUNCTION__.35456> "nm_device_release_one_slave",
warnexpr=warnexpr@entry=0x0) at gmessages.c:1058
#3 0x0000000000436c30 in nm_device_release_one_slave (dev=dev@entry=0xd92540, slave=slave@entry=0xdb0e50, configure=configure@entry=1) at devices/nm-device.c:841
#4 0x000000000043a779 in slave_state_changed (slave=0xdb0e50, slave_new_state=NM_DEVICE_STATE_UNMANAGED, slave_old_state=NM_DEVICE_STATE_ACTIVATED, reason=<optimized out>, self=0xd92540) at devices/nm-device.c:1214
#5 0x0000003371805d8c in ffi_call_unix64 () at ../src/x86/unix64.S:76
#6 0x00000033718056bc in ffi_call (cif=cif@entry=0x7fff8b1d3a00, fn=0x43a677 <slave_state_changed>, rvalue=0x7fff8b1d3970, avalue=avalue@entry=0x7fff8b1d38f0) at ../src/x86/ffi64.c:522
#7 0x0000003371c10ad8 in g_cclosure_marshal_generic (closure=0xd76120, return_gvalue=0x0, n_param_values=<optimized out>, param_values=<optimized out>, invocation_hint=<optimized out>, marshal_data=0x0) at gclosure.c:1454
#8 0x0000003371c10298 in g_closure_invoke (closure=0xd76120, return_value=return_value@entry=0x0, n_param_values=4, param_values=param_values@entry=0x7fff8b1d3c00, invocation_hint=invocation_hint@entry=0x7fff8b1d3ba0) at gclosure.c:777
#9 0x0000003371c2235d in signal_emit_unlocked_R (node=node@entry=0xd9d850, detail=detail@entry=0, instance=instance@entry=0xdb0e50, emission_return=emission_return@entry=0x0,
instance_and_params=instance_and_params@entry=0x7fff8b1d3c00) at gsignal.c:3586
#10 0x0000003371c2a0f2 in g_signal_emit_valist (instance=instance@entry=0xdb0e50, signal_id=signal_id@entry=68, detail=detail@entry=0, var_args=var_args@entry=0x7fff8b1d3e38) at gsignal.c:3330
#11 0x0000003371c2a8f8 in g_signal_emit_by_name (instance=instance@entry=0xdb0e50, detailed_signal=detailed_signal@entry=0x4c3ce2 "state-changed") at gsignal.c:3426
#12 0x000000000043754f in _set_state_full (device=device@entry=0xdb0e50, state=state@entry=NM_DEVICE_STATE_UNMANAGED, reason=reason@entry=NM_DEVICE_STATE_REASON_REMOVED, quitting=quitting@entry=0) at devices/nm-device.c:6689
#13 0x000000000043797c in nm_device_state_changed (device=device@entry=0xdb0e50, state=state@entry=NM_DEVICE_STATE_UNMANAGED, reason=reason@entry=NM_DEVICE_STATE_REASON_REMOVED) at devices/nm-device.c:6823
#14 0x0000000000439fe7 in nm_device_set_unmanaged (device=device@entry=0xdb0e50, flag=flag@entry=NM_UNMANAGED_INTERNAL, unmanaged=unmanaged@entry=1, reason=reason@entry=NM_DEVICE_STATE_REASON_REMOVED) at devices/nm-device.c:5983
#15 0x00000000004799c2 in remove_device (manager=manager@entry=0xd86150, device=0xdb0e50, quitting=quitting@entry=0) at nm-manager.c:769
#16 0x000000000047e3bf in platform_link_cb (platform=<optimized out>, ifindex=35, plink=<optimized out>, change_type=<optimized out>, reason=<optimized out>, user_data=<optimized out>) at nm-manager.c:2123
#17 0x0000003371805d8c in ffi_call_unix64 () at ../src/x86/unix64.S:76
#18 0x00000033718056bc in ffi_call (cif=cif@entry=0x7fff8b1d4320, fn=0x47e34b <platform_link_cb>, rvalue=0x7fff8b1d4290, avalue=avalue@entry=0x7fff8b1d4210) at ../src/x86/ffi64.c:522
#19 0x0000003371c10ad8 in g_cclosure_marshal_generic (closure=0xd32e40, return_gvalue=0x0, n_param_values=<optimized out>, param_values=<optimized out>, invocation_hint=<optimized out>, marshal_data=0x0) at gclosure.c:1454
#20 0x0000003371c10298 in g_closure_invoke (closure=0xd32e40, return_value=return_value@entry=0x0, n_param_values=5, param_values=param_values@entry=0x7fff8b1d4520, invocation_hint=invocation_hint@entry=0x7fff8b1d44c0) at gclosure.c:777
#21 0x0000003371c2235d in signal_emit_unlocked_R (node=node@entry=0xcf5780, detail=detail@entry=0, instance=instance@entry=0xcf78a0, emission_return=emission_return@entry=0x0,
instance_and_params=instance_and_params@entry=0x7fff8b1d4520) at gsignal.c:3586
#22 0x0000003371c2a0f2 in g_signal_emit_valist (instance=instance@entry=0xcf78a0, signal_id=signal_id@entry=2, detail=detail@entry=0, var_args=var_args@entry=0x7fff8b1d4778) at gsignal.c:3330
#23 0x0000003371c2a8f8 in g_signal_emit_by_name (instance=instance@entry=0xcf78a0, detailed_signal=detailed_signal@entry=0x4b183d "link-changed") at gsignal.c:3426
#24 0x000000000044c6f3 in announce_object (platform=platform@entry=0xcf78a0, object=0xde1720, change_type=change_type@entry=NM_PLATFORM_SIGNAL_REMOVED, reason=reason@entry=NM_PLATFORM_REASON_EXTERNAL)
at platform/nm-linux-platform.c:1572
#25 0x000000000044ec07 in event_notification (msg=<optimized out>, user_data=<optimized out>) at platform/nm-linux-platform.c:1886
#26 0x0000003844c1117f in nl_cb_call (msg=<optimized out>, type=<optimized out>, cb=<optimized out>) at ../include/netlink-private/netlink.h:141
#27 recvmsgs (cb=0xcf7240, sk=0xcf7330) at nl.c:952
#28 nl_recvmsgs_report (sk=0xcf7330, cb=0xcf7240) at nl.c:1003
#29 0x0000003844c11549 in nl_recvmsgs (sk=<optimized out>, cb=<optimized out>) at nl.c:1027
#30 0x0000003844c11569 in nl_recvmsgs_default (sk=<optimized out>) at nl.c:1041
#31 0x000000000044e955 in event_handler (channel=<optimized out>, io_condition=<optimized out>, user_data=0xcf78a0) at platform/nm-linux-platform.c:3804
#32 0x0000003370c492a6 in g_main_dispatch (context=0xcf5190) at gmain.c:3066
#33 g_main_context_dispatch (context=context@entry=0xcf5190) at gmain.c:3642
#34 0x0000003370c49628 in g_main_context_iterate (context=0xcf5190, block=block@entry=1, dispatch=dispatch@entry=1, self=<optimized out>) at gmain.c:3713
#35 0x0000003370c49a3a in g_main_loop_run (loop=0xcf4e30) at gmain.c:3907
#36 0x0000000000429f15 in main (argc=1, argv=0x7fff8b1d4f38) at main.c:678
Signed-off-by: Thomas Haller <thaller@redhat.com>
dhcp4_cleanup() should first clear @dhcp4_client variables before
clearing @dhcp4_config. Otherwise the following assert fails [1] and
the DBUS property NM_DEVICE_DHCP4_CONFIG is set to %NULL.
Analog to dhcp6_cleanup(), dhcp6_client, and NM_DEVICE_DHCP6_CONFIG.
[1] backtrace:
#0 0x0000003370c504e9 in g_logv (log_domain=0x4c148c "unrecognized-specs-changed", log_level=G_LOG_LEVEL_CRITICAL, format=<optimized out>, args=args@entry=0x7fff4710ed60) at gmessages.c:989
#1 0x0000003370c5063f in g_log (log_domain=log_domain@entry=0x4c144c "NetworkManager", log_level=log_level@entry=G_LOG_LEVEL_CRITICAL, format=format@entry=0x3370cbc89a "%s: assertion '%s' failed") at gmessages.c:1025
#2 0x0000003370c50679 in g_return_if_fail_warning (log_domain=log_domain@entry=0x4c144c "NetworkManager", pretty_function=pretty_function@entry=0x4c6140 <__PRETTY_FUNCTION__.15969> "nm_dhcp4_config_get_dbus_path",
expression=expression@entry=0x4c60d9 "NM_IS_DHCP4_CONFIG (self)") at gmessages.c:1034
#3 0x000000000046b1d4 in nm_dhcp4_config_get_dbus_path (self=0x0) at nm-dhcp4-config.c:115
#4 0x0000000000434791 in get_property (object=0x9d2320, prop_id=13, value=0x9618a0, pspec=0x9bbc20) at devices/nm-device.c:7539
#5 0x0000003371c18e73 in object_get_property (value=0x9618a0, pspec=<optimized out>, object=0x9d2320) at gobject.c:1303
#6 g_object_get_property (object=0x9d2320, property_name=<optimized out>, value=0x9618a0) at gobject.c:2402
#7 0x000000000048482c in idle_id_reset (data=<optimized out>) at nm-properties-changed-signal.c:123
#8 0x0000003371c13055 in g_cclosure_marshal_VOID__PARAM (closure=0x9618a0, return_value=0xffffffff, n_param_values=0, param_values=0x7fff4710f130, invocation_hint=0x0, marshal_data=0x4b5201) at gmarshal.c:1037
#9 0x0000003371c10298 in g_closure_invoke (closure=0x2, closure@entry=0x919d00, return_value=return_value@entry=0x0, n_param_values=1192292656, param_values=0x9d2320, param_values@entry=0x7fff4710f130,
invocation_hint=invocation_hint@entry=0x7fff4710f0d0) at gclosure.c:777
#10 0x0000003371c21b87 in signal_emit_unlocked_R (node=node@entry=0x919d90, detail=detail@entry=667, instance=instance@entry=0x9d2320, emission_return=emission_return@entry=0x0,
instance_and_params=instance_and_params@entry=0x7fff4710f130) at gsignal.c:3516
#11 0x0000003371c2a0f2 in g_signal_emit_valist (instance=<optimized out>, signal_id=<optimized out>, detail=<optimized out>, var_args=var_args@entry=0x7fff4710f2c0) at gsignal.c:3330
#12 0x0000003371c2a3af in g_signal_emit (instance=<optimized out>, signal_id=<optimized out>, detail=<optimized out>) at gsignal.c:3386
#13 0x0000003371c14945 in g_object_dispatch_properties_changed (object=0x9d2320, n_pspecs=4294967295, pspecs=0x0) at gobject.c:1047
#14 0x0000003371c17019 in g_object_notify_by_spec_internal (pspec=<optimized out>, object=0x9d2320) at gobject.c:1141
#15 g_object_notify (object=0x9d2320, property_name=property_name@entry=0x4c400f "dhcp4-config") at gobject.c:1183
#16 0x0000000000434332 in dhcp4_cleanup (self=self@entry=0x9d2320, stop=stop@entry=1, release=release@entry=0) at devices/nm-device.c:2581
#17 0x0000000000434cab in _cleanup_generic_pre (self=self@entry=0x9d2320, deconfigure=deconfigure@entry=1) at devices/nm-device.c:6448
#18 0x0000000000436db1 in nm_device_cleanup (self=self@entry=0x9d2320, reason=reason@entry=NM_DEVICE_STATE_REASON_REMOVED) at devices/nm-device.c:6524
#19 0x0000000000437358 in _set_state_full (device=device@entry=0x9d2320, state=state@entry=NM_DEVICE_STATE_UNMANAGED, reason=reason@entry=NM_DEVICE_STATE_REASON_REMOVED, quitting=quitting@entry=0) at devices/nm-device.c:6641
#20 0x000000000043797c in nm_device_state_changed (device=device@entry=0x9d2320, state=state@entry=NM_DEVICE_STATE_UNMANAGED, reason=reason@entry=NM_DEVICE_STATE_REASON_REMOVED) at devices/nm-device.c:6823
#21 0x0000000000439fe7 in nm_device_set_unmanaged (device=device@entry=0x9d2320, flag=flag@entry=NM_UNMANAGED_INTERNAL, unmanaged=unmanaged@entry=1, reason=reason@entry=NM_DEVICE_STATE_REASON_REMOVED) at devices/nm-device.c:5983
#22 0x000000000043a193 in nm_device_set_unmanaged_quitting (device=0x9d2320) at devices/nm-device.c:5998
#23 0x00000000004799f9 in remove_device (manager=0x9b2150, device=0x9d2320, quitting=1) at nm-manager.c:775
#24 0x000000000047bf47 in dispose (object=0x9b2150) at nm-manager.c:4935
#25 0x0000003371c14ee8 in g_object_unref (_object=0x9b2150) at gobject.c:3160
#26 0x0000000000429f43 in main (argc=1, argv=0x7fff4710f9a8) at main.c:681
Signed-off-by: Thomas Haller <thaller@redhat.com>
Remove all remaining GParamSpec name and blurb strings (and fix
indentation while we're there), and add G_PARAM_STATIC_STRINGS to all
paramspecs that were lacking it.
When delaying the deactivation of a device during dispatcher-pre-down,
we must preseve the reason to pass it on.
This is especially important, because nm_device_slave_notify_release()
checks for the reason, and does not deactivate the slave if no reason is
given. This error caused slaves the be left up when deactivating the master.
Also update the call to nm_device_slave_notify_release() to ensure we
have a valid state reason when configuring the slave. This would have
pointed out the issue and would even work around it.
Regression introduced by commit d00e2147de.
Signed-off-by: Thomas Haller <thaller@redhat.com>
On shutdown we can't defer the response to a callback, so we need to
use synchronous D-Bus calls. Second, sometimes we want to block on
the dispatcher response, like for pre-down.
The NMDevice dispose() function contained some badly-duplicated logic
about when to deactivate a device on its last ref. This logic should
only run when the device is removed by the manager, since the manager
controls the device's life-cycle, and the manager knows best when to
clean up the device. But since it was tied to the device's refcount,
it could have run later than the manager wanted, or not at all.
It gets better. Dispose duplicated logic that was already done in
nm_device_cleanup(), and then *called* nm_device_cleanup() if the
device was still activated and managed. But the manager already
unmanages the device when removing it, which triggers a call to
nm_device_cleanup(), takes the device down, and resets the IPv6
sysctl properties, which dispose() duplicated too. So by the time
dispose() runs, the device should already be unmanaged if the
manager wants to deconfigure it, and most of the dispose() code
should be a no-op.
Clean all that up and remove duplicated functions. Now, the flow
should be like this:
1) manager decides to remove the device and calls remove_device()
2) if the device should be deconfigured, the manager unmanages
the device
3) the NMDevice state change handler tears down the active connection
via nm_device_cleanup() and resets IPv6 sysctl properties
4) when the device's last reference is finally released, only internal
data members are freed in dispose() because the device should
already have been cleaned up by the manager and be unmanaged
5) if the device should be left running because it has an assumable
connection, then the device is not unmanaged, and no cleanup
happens in the state change handler or in dispose()
Ensure autoip4 is cleaned up when disposing, like we clean up DHCP.
Move things that only free stuff to finalize(), and move things that
unref stuff to dispose (eg, the Firewall Manager stuff).
dhcp6_cleanup() frees priv->dhcp6_ip6_config so dispose() doesn't need
to do that. Also use g_clear_object() when appropriate.
Lastly, notify that the DHCP4/6 config objects have changed *after*
clearing them, so that the PropertiesChanged signal is emitted with a
blank path to indicate the object is gone. Previously the PC signal
would have been emitted with the valid path of the DHCP4/6 config object,
but the object would already be dead.
For any function in nm-device.h which is not used outside of
nm-device.c, remove the public prototypes. Functions that
are actually used get moved above their caller, and functions
that have no callers are removed.
Move the GObject-related stuff to the bottom of the file and get
rid of no-longer-necessary forward prototypes since the stuff that
required those prototypes is now below the implementation.
Instead of creating most routes with metric 0 and then fixing them
just before applying them, create the routes with the correct metric
in the first place (so that NMIP4Config and NMIP6Config don't have to
try to guess whether "metric 0" means "unset" or "actually metric 0").
If a link's "master" property changes unexpectedly (ie, from outside
NM), update the master and slave NMDevices to reflect it, without
making any changes to them.
The process of activating a slave requires that its master have an
NMActiveConnection. So don't allow generating a connection on a slave
until we have generated the connection on the master.
nm_device_generate_connection() was allowing connections for master
devices to have no IP config, but this didn't really make much sense,
since they would just fail at stage3 in that case anyway.
Now that we get multiple tries at generating a connection on a device,
we can just ignore the device until it has a proper connection.
If the initial attempt to assume a connection on a device fails, and
the device remains un-activated, but then something changes its
configuration externally, try to generate a new connection and assume
that.
If the IP config changes on a device that has assumed a generated
connection, then update the connection's NMSettingIP4Config /
NMSettingIP6Config, under the assumption that the configuration of
that device was in progress but incomplete when NM first observed it.
NMIP4Config and NMIP6Config had methods to update an existing
NMSetting. However, the functions would really only work correctly if
the passed-in setting was empty.
Change them from "update_setting" to "create_setting", and have them
create the NMSetting themselves, and update NMDevice for that.
(If we need update_setting later, we can add it, after figuring out
exactly how it's actually supposed to work.)
Firewalld call addInterface() fails with ZONE_CONFLICT if the interface
is already part of another zone. This complicates the code in NM,
because we would have to keep better track of the zone in which the
interface currently is. Which might be quite difficult because
the zone might be changed from an external program (so we would have
to monitor the firewall configuration and work around potential races).
A better and simpler fix is to simply always use the changeZone() call.
This will do the right thing, regardless if the interface is already part
of a zone or not.
https://bugzilla.redhat.com/show_bug.cgi?id=1103782
Signed-off-by: Thomas Haller <thaller@redhat.com>
The firewalld removeInterface call fails with ZONE_CONFLICT when
removing an interface from a wrong zone. This can happen, when the
connection gets modified, while being active (which is related to
bgo#724041).
By not specifying any zone, we remove the interface from the zone
where it currently is added. This behavior was introduced in upstream
firewalld with commit cc3101ab70a3997228be7bc9f45a069c7fccfa36, March 2012,
r0_2_3-1.
This is the behavior we actually want and we don't have to keep proper track
of the current zone.
https://bugzilla.redhat.com/show_bug.cgi?id=1103782
Signed-off-by: Thomas Haller <thaller@redhat.com>
