From 1dd50924c8881aca1b5e42e29d32256886f84d03 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 23 Jun 2014 11:18:45 +0200 Subject: [PATCH] device: fix warning releasing of slave when slave device gets removed 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=, 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=, 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 , 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=, param_values=, invocation_hint=, 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=, ifindex=35, plink=, change_type=, reason=, user_data=) 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 , 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=, param_values=, invocation_hint=, 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=, user_data=) at platform/nm-linux-platform.c:1886 #26 0x0000003844c1117f in nl_cb_call (msg=, type=, cb=) 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=, cb=) at nl.c:1027 #30 0x0000003844c11569 in nl_recvmsgs_default (sk=) at nl.c:1041 #31 0x000000000044e955 in event_handler (channel=, io_condition=, 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=) 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 --- src/devices/nm-device.c | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index c27e34c786..174e776a44 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -800,6 +800,7 @@ nm_device_enslave_slave (NMDevice *dev, NMDevice *slave, NMConnection *connectio * @dev: the master device * @slave: the slave device to release * @configure: whether @dev needs to actually release @slave + * @reason: the state change reason for the @slave * * If @dev is capable of enslaving other devices (ie it's a bridge, bond, team, * etc) then this function releases the previously enslaved @slave and/or @@ -809,12 +810,11 @@ nm_device_enslave_slave (NMDevice *dev, NMDevice *slave, NMConnection *connectio * other devices, or if @slave was never enslaved. */ static gboolean -nm_device_release_one_slave (NMDevice *dev, NMDevice *slave, gboolean configure) +nm_device_release_one_slave (NMDevice *dev, NMDevice *slave, gboolean configure, NMDeviceStateReason reason) { NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (dev); SlaveInfo *info; gboolean success = FALSE; - NMDeviceStateReason reason; g_return_val_if_fail (slave != NULL, FALSE); g_return_val_if_fail (NM_DEVICE_GET_CLASS (dev)->release_slave != NULL, FALSE); @@ -831,13 +831,10 @@ nm_device_release_one_slave (NMDevice *dev, NMDevice *slave, gboolean configure) */ } - if (!configure) + if (!configure) { + g_warn_if_fail (reason == NM_DEVICE_STATE_REASON_NONE); reason = NM_DEVICE_STATE_REASON_NONE; - else if (priv->state == NM_DEVICE_STATE_FAILED) - reason = NM_DEVICE_STATE_REASON_DEPENDENCY_FAILED; - else if (priv->state_reason != NM_DEVICE_STATE_REASON_NONE) - reason = priv->state_reason; - else { + } else if (reason == NM_DEVICE_STATE_REASON_NONE) { g_warn_if_reached (); reason = NM_DEVICE_STATE_REASON_UNKNOWN; } @@ -1081,7 +1078,7 @@ device_link_changed (NMDevice *device, NMPlatformLink *info) nm_platform_link_get_name (info->master)); } } else if (priv->enslaved && !info->master) - nm_device_release_one_slave (priv->master, device, FALSE); + nm_device_release_one_slave (priv->master, device, FALSE, NM_DEVICE_STATE_REASON_NONE); if (klass->link_changed) klass->link_changed (device, info); @@ -1211,7 +1208,7 @@ slave_state_changed (NMDevice *slave, } if (release) { - nm_device_release_one_slave (self, slave, TRUE); + nm_device_release_one_slave (self, slave, TRUE, reason); /* Bridge/bond/team interfaces are left up until manually deactivated */ if (priv->slaves == NULL && priv->state == NM_DEVICE_STATE_ACTIVATED) { nm_log_dbg (LOGD_DEVICE, "(%s): last slave removed; remaining activated", @@ -1346,15 +1343,20 @@ static void nm_device_master_release_slaves (NMDevice *self) { NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self); + NMDeviceStateReason reason; /* Don't release the slaves if this connection doesn't belong to NM. */ if (nm_device_uses_generated_connection (self)) return; + reason = priv->state_reason; + if (priv->state == NM_DEVICE_STATE_FAILED) + reason = NM_DEVICE_STATE_REASON_DEPENDENCY_FAILED; + while (priv->slaves) { SlaveInfo *info = priv->slaves->data; - nm_device_release_one_slave (self, info->slave, TRUE); + nm_device_release_one_slave (self, info->slave, TRUE, reason); } }