From 68ca77cdb5d6faff9b54312c0b34085b583df603 Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Wed, 18 Feb 2009 09:57:33 -0500 Subject: [PATCH] dhcp: always clean up DHCP client watch callback when stopping DHCP Also fix a bug where failure to start the DHCP client wouldn't be handled. --- src/dhcp-manager/nm-dhcp-dhclient.c | 18 ++++++--------- src/dhcp-manager/nm-dhcp-dhcpcd.c | 18 ++++++--------- src/dhcp-manager/nm-dhcp-manager.c | 34 +++++++++++++++++------------ src/dhcp-manager/nm-dhcp-manager.h | 4 ++-- 4 files changed, 36 insertions(+), 38 deletions(-) diff --git a/src/dhcp-manager/nm-dhcp-dhclient.c b/src/dhcp-manager/nm-dhcp-dhclient.c index 552014e0e..946be0c9a 100644 --- a/src/dhcp-manager/nm-dhcp-dhclient.c +++ b/src/dhcp-manager/nm-dhcp-dhclient.c @@ -215,14 +215,13 @@ dhclient_child_setup (gpointer user_data G_GNUC_UNUSED) } -gboolean +GPid nm_dhcp_client_start (NMDHCPDevice *device, NMSettingIP4Config *s_ip4) { - GPtrArray * dhclient_argv = NULL; - GPid pid; - GError * error = NULL; - gboolean success = FALSE; - char * pid_contents = NULL; + GPtrArray *dhclient_argv = NULL; + GPid pid = 0; + GError *error = NULL; + char *pid_contents = NULL; if (!g_file_test (DHCP_CLIENT_PATH, G_FILE_TEST_EXISTS)) { nm_warning (DHCP_CLIENT_PATH " does not exist."); @@ -249,7 +248,7 @@ nm_dhcp_client_start (NMDHCPDevice *device, NMSettingIP4Config *s_ip4) unsigned long int tmp = strtoul (pid_contents, NULL, 10); if (!((tmp == ULONG_MAX) && (errno == ERANGE))) - nm_dhcp_client_stop (device->iface, (pid_t) tmp); + nm_dhcp_client_stop (device, (pid_t) tmp); remove (device->pid_file); } @@ -282,13 +281,10 @@ nm_dhcp_client_start (NMDHCPDevice *device, NMSettingIP4Config *s_ip4) nm_info ("dhclient started with pid %d", pid); - device->pid = pid; - success = TRUE; - out: g_free (pid_contents); g_ptr_array_free (dhclient_argv, TRUE); - return success; + return pid; } static const char ** diff --git a/src/dhcp-manager/nm-dhcp-dhcpcd.c b/src/dhcp-manager/nm-dhcp-dhcpcd.c index 62f09688a..bc04c5f02 100644 --- a/src/dhcp-manager/nm-dhcp-dhcpcd.c +++ b/src/dhcp-manager/nm-dhcp-dhcpcd.c @@ -59,14 +59,13 @@ dhcpcd_child_setup (gpointer user_data G_GNUC_UNUSED) } -gboolean +GPid nm_dhcp_client_start (NMDHCPDevice *device, NMSettingIP4Config *s_ip4) { - GPtrArray * argv = NULL; - GPid pid; - GError * error = NULL; - gboolean success = FALSE; - char * pid_contents = NULL; + GPtrArray *argv = NULL; + GPid pid = 0; + GError *error = NULL; + char *pid_contents = NULL; if (!g_file_test (DHCP_CLIENT_PATH, G_FILE_TEST_EXISTS)) { nm_warning (DHCP_CLIENT_PATH " does not exist."); @@ -84,7 +83,7 @@ nm_dhcp_client_start (NMDHCPDevice *device, NMSettingIP4Config *s_ip4) unsigned long int tmp = strtoul (pid_contents, NULL, 10); if (!((tmp == ULONG_MAX) && (errno == ERANGE))) - nm_dhcp_client_stop (device->iface, (pid_t) tmp); + nm_dhcp_client_stop (device, (pid_t) tmp); remove (device->pid_file); } @@ -112,13 +111,10 @@ nm_dhcp_client_start (NMDHCPDevice *device, NMSettingIP4Config *s_ip4) nm_info ("dhcpcd started with pid %d", pid); - device->pid = pid; - success = TRUE; - out: g_free (pid_contents); g_ptr_array_free (argv, TRUE); - return success; + return pid; } gboolean diff --git a/src/dhcp-manager/nm-dhcp-manager.c b/src/dhcp-manager/nm-dhcp-manager.c index 44e49a9c6..b2e352d1b 100644 --- a/src/dhcp-manager/nm-dhcp-manager.c +++ b/src/dhcp-manager/nm-dhcp-manager.c @@ -172,10 +172,9 @@ nm_dhcp_device_destroy (NMDHCPDevice *device) int ret; nm_dhcp_device_timeout_cleanup (device); - nm_dhcp_device_watch_cleanup (device); if (device->pid) - nm_dhcp_client_stop (device->iface, device->pid); + nm_dhcp_client_stop (device, device->pid); if (device->options) g_hash_table_destroy (device->options); @@ -577,9 +576,9 @@ static void dhcp_watch_cb (GPid pid, gint status, gpointer user_data) gboolean nm_dhcp_manager_begin_transaction (NMDHCPManager *manager, - const char *iface, - NMSettingIP4Config *s_ip4, - guint32 timeout) + const char *iface, + NMSettingIP4Config *s_ip4, + guint32 timeout) { NMDHCPManagerPrivate *priv; NMDHCPDevice *device; @@ -593,13 +592,17 @@ nm_dhcp_manager_begin_transaction (NMDHCPManager *manager, if (!device) device = nm_dhcp_device_new (manager, iface); - if (state_is_bound (device->state) || (device->state == DHC_START)) { + if (device->pid && (device->state < DHC_ABEND)) { /* Cancel any DHCP transaction already in progress */ nm_dhcp_manager_cancel_transaction_real (device); } nm_info ("Activation (%s) Beginning DHCP transaction.", iface); + device->pid = nm_dhcp_client_start (device, s_ip4); + if (device->pid == 0) + return FALSE; + if (timeout == 0) timeout = NM_DHCP_TIMEOUT; @@ -607,8 +610,6 @@ nm_dhcp_manager_begin_transaction (NMDHCPManager *manager, device->timeout_id = g_timeout_add_seconds (timeout, nm_dhcp_manager_handle_timeout, device); - - nm_dhcp_client_start (device, s_ip4); device->watch_id = g_child_watch_add (device->pid, (GChildWatchFunc) dhcp_watch_cb, device); @@ -616,10 +617,17 @@ nm_dhcp_manager_begin_transaction (NMDHCPManager *manager, } void -nm_dhcp_client_stop (const char * iface, pid_t pid) +nm_dhcp_client_stop (NMDHCPDevice *device, pid_t pid) { int i = 15; /* 3 seconds */ + g_return_if_fail (pid > 0); + + /* Clean up the watch handler since we're explicitly killing + * the daemon + */ + nm_dhcp_device_watch_cleanup (device); + /* Tell it to quit; maybe it wants to send out a RELEASE message */ kill (pid, SIGTERM); @@ -643,7 +651,7 @@ nm_dhcp_client_stop (const char * iface, pid_t pid) } if (i <= 0) { - nm_warning ("%s: dhcp client pid %d didn't exit, will kill it.", iface, pid); + nm_warning ("%s: dhcp client pid %d didn't exit, will kill it.", device->iface, pid); kill (pid, SIGKILL); nm_debug ("waiting for dhcp client pid %d to exit", pid); @@ -655,10 +663,9 @@ nm_dhcp_client_stop (const char * iface, pid_t pid) static void nm_dhcp_manager_cancel_transaction_real (NMDHCPDevice *device) { - if (!device->pid) - return; + g_return_if_fail (device->pid > 0); - nm_dhcp_client_stop (device->iface, device->pid); + nm_dhcp_client_stop (device, device->pid); nm_info ("%s: canceled DHCP transaction, dhcp client pid %d", device->iface, @@ -688,7 +695,6 @@ nm_dhcp_manager_cancel_transaction_real (NMDHCPDevice *device) device->conf_file = NULL; } - nm_dhcp_device_watch_cleanup (device); nm_dhcp_device_timeout_cleanup (device); g_hash_table_remove_all (device->options); } diff --git a/src/dhcp-manager/nm-dhcp-manager.h b/src/dhcp-manager/nm-dhcp-manager.h index 74bdd087d..c3527387e 100644 --- a/src/dhcp-manager/nm-dhcp-manager.h +++ b/src/dhcp-manager/nm-dhcp-manager.h @@ -102,8 +102,8 @@ gboolean nm_dhcp_manager_foreach_dhcp4_option (NMDHCPManager *self, gpointer user_data); /* The following are implemented by the DHCP client backends */ -gboolean nm_dhcp_client_start (NMDHCPDevice *device, NMSettingIP4Config *s_ip4); -void nm_dhcp_client_stop (const char *iface, pid_t pid); +GPid nm_dhcp_client_start (NMDHCPDevice *device, NMSettingIP4Config *s_ip4); +void nm_dhcp_client_stop (NMDHCPDevice *device, pid_t pid); gboolean nm_dhcp_client_process_classless_routes (GHashTable *options, NMIP4Config *ip4_config,