dhcp: handle DHCP clients that aren't subprocesses

Make the base class mostly ignorant of process IDs, except for a
few utility functions.

src/dhcp-manager/nm-dhcp-client.c

@@ -45,8 +45,7 @@ typedef struct {
 	GByteArray * duid;
 
 	guchar       state;
-	GPid         pid;
+	pid_t        pid;
-	gboolean     dead;
 	guint        timeout_id;
 	guint        watch_id;
 	guint32      remove_id;
@@ -82,7 +81,7 @@ enum {
 
 /********************************************/
 
-GPid
+pid_t
 nm_dhcp_client_get_pid (NMDHCPClient *self)
 {
 	g_return_val_if_fail (NM_IS_DHCP_CLIENT (self), -1);
@@ -147,7 +146,7 @@ watch_cleanup (NMDHCPClient *self)
 }
 
 void
-nm_dhcp_client_stop_pid (GPid pid, const char *iface)
+nm_dhcp_client_stop_pid (pid_t pid, const char *iface)
 {
 	char *name = iface ? g_strdup_printf ("dhcp-client-%s", iface) : NULL;
 
@@ -164,12 +163,13 @@ stop (NMDHCPClient *self, gboolean release, const GByteArray *duid)
 	g_return_if_fail (NM_IS_DHCP_CLIENT (self));
 
 	priv = NM_DHCP_CLIENT_GET_PRIVATE (self);
-	g_return_if_fail (priv->pid > 0);
 
-	/* Clean up the watch handler since we're explicitly killing the daemon */
+	if (priv->pid > 0) {
-	watch_cleanup (self);
+		/* Clean up the watch handler since we're explicitly killing the daemon */
-
+		watch_cleanup (self);
-	nm_dhcp_client_stop_pid (priv->pid, priv->iface);
+		nm_dhcp_client_stop_pid (priv->pid, priv->iface);
+		priv->pid = -1;
+	}
 
 	priv->info_only = FALSE;
 }
@@ -248,25 +248,26 @@ daemon_watch_cb (GPid pid, gint status, gpointer user_data)
 
 	watch_cleanup (self);
 	timeout_cleanup (self);
-	priv->dead = TRUE;
+	priv->pid = -1;
 
 	dhcp_client_set_state (self, new_state, TRUE, FALSE);
 }
 
-static void
+void
-start_monitor (NMDHCPClient *self)
+nm_dhcp_client_watch_child (NMDHCPClient *self, pid_t pid)
 {
 	NMDHCPClientPrivate *priv = NM_DHCP_CLIENT_GET_PRIVATE (self);
 
-	g_return_if_fail (priv->pid > 0);
+	g_return_if_fail (priv->pid == -1);
+	priv->pid = pid;
 
 	/* Set up a timeout on the transaction to kill it after the timeout */
+	g_assert (priv->timeout_id == 0);
 	priv->timeout_id = g_timeout_add_seconds (priv->timeout,
 	                                          daemon_timeout,
 	                                          self);
-	priv->watch_id = g_child_watch_add (priv->pid,
+	g_assert (priv->watch_id == 0);
-	                                    (GChildWatchFunc) daemon_watch_cb,
+	priv->watch_id = g_child_watch_add (pid, daemon_watch_cb, self);
-	                                    self);
 }
 
 gboolean
@@ -287,11 +288,7 @@ nm_dhcp_client_start_ip4 (NMDHCPClient *self,
 	nm_log_info (LOGD_DHCP, "Activation (%s) Beginning DHCPv4 transaction (timeout in %d seconds)",
 	             priv->iface, priv->timeout);
 
-	priv->pid = NM_DHCP_CLIENT_GET_CLASS (self)->ip4_start (self, dhcp_client_id, dhcp_anycast_addr, hostname);
+	return NM_DHCP_CLIENT_GET_CLASS (self)->ip4_start (self, dhcp_client_id, dhcp_anycast_addr, hostname);
-	if (priv->pid)
-		start_monitor (self);
-
-	return priv->pid ? TRUE : FALSE;
 }
 
 /* uuid_parse does not work for machine-id, so we use our own converter */
@@ -456,15 +453,11 @@ nm_dhcp_client_start_ip6 (NMDHCPClient *self,
 	nm_log_info (LOGD_DHCP, "Activation (%s) Beginning DHCPv6 transaction (timeout in %d seconds)",
 	             priv->iface, priv->timeout);
 
-	priv->pid = NM_DHCP_CLIENT_GET_CLASS (self)->ip6_start (self,
+	return NM_DHCP_CLIENT_GET_CLASS (self)->ip6_start (self,
-	                                                        dhcp_anycast_addr,
+	                                                   dhcp_anycast_addr,
-	                                                        hostname,
+	                                                   hostname,
-	                                                        info_only,
+	                                                   info_only,
-	                                                        priv->duid);
+	                                                   priv->duid);
-	if (priv->pid > 0)
-		start_monitor (self);
-
-	return priv->pid ? TRUE : FALSE;
 }
 
 void
@@ -492,7 +485,7 @@ nm_dhcp_client_stop_existing (const char *pid_file, const char *binary_name)
 				exe = proc_contents;
 
 			if (!strcmp (exe, binary_name))
-				nm_dhcp_client_stop_pid ((GPid) tmp, NULL);
+				nm_dhcp_client_stop_pid ((pid_t) tmp, NULL);
 		}
 	}
 
@@ -508,23 +501,24 @@ void
 nm_dhcp_client_stop (NMDHCPClient *self, gboolean release)
 {
 	NMDHCPClientPrivate *priv;
+	pid_t old_pid = 0;
 
 	g_return_if_fail (NM_IS_DHCP_CLIENT (self));
 
 	priv = NM_DHCP_CLIENT_GET_PRIVATE (self);
 
 	/* Kill the DHCP client */
-	if (!priv->dead) {
+	old_pid = priv->pid;
-		NM_DHCP_CLIENT_GET_CLASS (self)->stop (self, release, priv->duid);
+	NM_DHCP_CLIENT_GET_CLASS (self)->stop (self, release, priv->duid);
-		priv->dead = TRUE;
+	if (old_pid > 0) {
-
 		nm_log_info (LOGD_DHCP, "(%s): canceled DHCP transaction, DHCP client pid %d",
-		             priv->iface, priv->pid);
+		             priv->iface, old_pid);
-	}
+	} else
+		nm_log_info (LOGD_DHCP, "(%s): canceled DHCP transaction", priv->iface);
+	g_assert (priv->pid == -1);
 
 	/* And clean stuff up */
 
-	priv->pid = -1;
 	dhcp_client_set_state (self, DHC_END, FALSE, TRUE);
 
 	g_hash_table_remove_all (priv->options);

src/dhcp-manager/nm-dhcp-client.h

@@ -83,20 +83,20 @@ typedef struct {
 
 	/* Methods */
 
-	GPid (*ip4_start)     (NMDHCPClient *self,
+	gboolean (*ip4_start)     (NMDHCPClient *self,
-	                       const char *dhcp_client_id,
+	                           const char *dhcp_client_id,
-	                       GByteArray *anycast_addr,
+	                           GByteArray *anycast_addr,
-	                       const char *hostname);
+	                           const char *hostname);
 
-	GPid (*ip6_start)     (NMDHCPClient *self,
+	gboolean (*ip6_start)     (NMDHCPClient *self,
-	                       GByteArray *anycast_addr,
+	                           GByteArray *anycast_addr,
-	                       const char *hostname,
+	                           const char *hostname,
-	                       gboolean info_only,
+	                           gboolean info_only,
-	                       const GByteArray *duid);
+	                           const GByteArray *duid);
 
-	void (*stop)          (NMDHCPClient *self,
+	void (*stop)              (NMDHCPClient *self,
-	                       gboolean release,
+	                           gboolean release,
-	                       const GByteArray *duid);
+	                           const GByteArray *duid);
 
 	/**
 	 * get_duid:
@@ -117,7 +117,7 @@ typedef struct {
 
 GType nm_dhcp_client_get_type (void);
 
-GPid nm_dhcp_client_get_pid (NMDHCPClient *self);
+pid_t nm_dhcp_client_get_pid (NMDHCPClient *self);
 
 const char *nm_dhcp_client_get_iface (NMDHCPClient *self);
 
@@ -154,7 +154,9 @@ NMIP6Config *nm_dhcp_client_get_ip6_config   (NMDHCPClient *self, gboolean test)
 /* Backend helpers */
 void nm_dhcp_client_stop_existing (const char *pid_file, const char *binary_name);
 
-void nm_dhcp_client_stop_pid (GPid pid, const char *iface);
+void nm_dhcp_client_stop_pid (pid_t pid, const char *iface);
+
+void nm_dhcp_client_watch_child (NMDHCPClient *self, pid_t pid);
 
 #endif /* NM_DHCP_CLIENT_H */
 

src/dhcp-manager/nm-dhcp-dhclient.c

@@ -324,7 +324,7 @@ dhclient_child_setup (gpointer user_data G_GNUC_UNUSED)
 	nm_unblock_posix_signals (NULL);
 }
 
-static GPid
+static gboolean
 dhclient_start (NMDHCPClient *client,
                 const char *mode_opt,
                 const GByteArray *duid,
@@ -332,7 +332,7 @@ dhclient_start (NMDHCPClient *client,
 {
 	NMDHCPDhclientPrivate *priv = NM_DHCP_DHCLIENT_GET_PRIVATE (client);
 	GPtrArray *argv = NULL;
-	GPid pid = -1;
+	pid_t pid;
 	GError *error = NULL;
 	const char *iface, *uuid, *system_bus_address;
 	char *binary_name, *cmd_str, *pid_file = NULL, *system_bus_address_env = NULL;
@@ -340,7 +340,7 @@ dhclient_start (NMDHCPClient *client,
 	guint log_domain;
 	char *escaped, *preferred_leasefile_path = NULL;
 
-	g_return_val_if_fail (priv->pid_file == NULL, -1);
+	g_return_val_if_fail (priv->pid_file == NULL, FALSE);
 
 	iface = nm_dhcp_client_get_iface (client);
 	uuid = nm_dhcp_client_get_uuid (client);
@@ -350,7 +350,7 @@ dhclient_start (NMDHCPClient *client,
 
 	if (!g_file_test (priv->path, G_FILE_TEST_EXISTS)) {
 		nm_log_warn (log_domain, "%s does not exist.", priv->path);
-		return -1;
+		return FALSE;
 	}
 
 	pid_file = g_strdup_printf (LOCALSTATEDIR "/run/dhclient%s-%s.pid",
@@ -404,7 +404,8 @@ dhclient_start (NMDHCPClient *client,
 			             iface, priv->lease_file,
 			             error ? error->code : -1,
 			             error && error->message ? error->message : "(unknown)");
-			return -1;
+			g_free (pid_file);
+			return FALSE;
 		}
 	}
 
@@ -456,22 +457,24 @@ dhclient_start (NMDHCPClient *client,
 	nm_log_dbg (log_domain, "running: %s", cmd_str);
 	g_free (cmd_str);
 
-	if (!g_spawn_async (NULL, (char **) argv->pdata, NULL, G_SPAWN_DO_NOT_REAP_CHILD,
+	if (g_spawn_async (NULL, (char **) argv->pdata, NULL, G_SPAWN_DO_NOT_REAP_CHILD,
 	                    &dhclient_child_setup, NULL, &pid, &error)) {
+		g_assert (pid > 0);
+		nm_log_info (log_domain, "dhclient started with pid %d", pid);
+		nm_dhcp_client_watch_child (client, pid);
+		priv->pid_file = pid_file;
+	} else {
 		nm_log_warn (log_domain, "dhclient failed to start: '%s'", error->message);
 		g_error_free (error);
-		pid = -1;
+		g_free (pid_file);
-	} else {
-		nm_log_info (log_domain, "dhclient started with pid %d", pid);
-		priv->pid_file = pid_file;
 	}
 
 	g_ptr_array_free (argv, TRUE);
 	g_free (system_bus_address_env);
-	return pid;
+	return pid > 0 ? TRUE : FALSE;
 }
 
-static GPid
+static gboolean
 ip4_start (NMDHCPClient *client,
            const char *dhcp_client_id,
            GByteArray *dhcp_anycast_addr,
@@ -486,13 +489,13 @@ ip4_start (NMDHCPClient *client,
 	priv->conf_file = create_dhclient_config (iface, FALSE, uuid, dhcp_client_id, dhcp_anycast_addr, hostname);
 	if (!priv->conf_file) {
 		nm_log_warn (LOGD_DHCP4, "(%s): error creating dhclient configuration file.", iface);
-		return -1;
+		return FALSE;
 	}
 
 	return dhclient_start (client, NULL, NULL, FALSE);
 }
 
-static GPid
+static gboolean
 ip6_start (NMDHCPClient *client,
            GByteArray *dhcp_anycast_addr,
            const char *hostname,
@@ -508,7 +511,7 @@ ip6_start (NMDHCPClient *client,
 	priv->conf_file = create_dhclient_config (iface, TRUE, uuid, NULL, dhcp_anycast_addr, hostname);
 	if (!priv->conf_file) {
 		nm_log_warn (LOGD_DHCP6, "(%s): error creating dhclient6 configuration file.", iface);
-		return -1;
+		return FALSE;
 	}
 
 	return dhclient_start (client, info_only ? "-S" : "-N", duid, FALSE);
@@ -533,7 +536,7 @@ stop (NMDHCPClient *client, gboolean release, const GByteArray *duid)
 	}
 
 	if (release) {
-		GPid rpid;
+		pid_t rpid;
 
 		rpid = dhclient_start (client, NULL, duid, TRUE);
 		if (rpid > 0) {

src/dhcp-manager/nm-dhcp-dhcpcd.c

@@ -86,7 +86,7 @@ dhcpcd_child_setup (gpointer user_data G_GNUC_UNUSED)
 	nm_unblock_posix_signals (NULL);
 }
 
-static GPid
+static gboolean
 ip4_start (NMDHCPClient *client,
            const char *dhcp_client_id,
            GByteArray *dhcp_anycast_addr,
@@ -94,12 +94,12 @@ ip4_start (NMDHCPClient *client,
 {
 	NMDHCPDhcpcdPrivate *priv = NM_DHCP_DHCPCD_GET_PRIVATE (client);
 	GPtrArray *argv = NULL;
-	GPid pid = -1;
+	pid_t pid = -1;
 	GError *error = NULL;
 	char *pid_contents = NULL, *binary_name, *cmd_str;
 	const char *iface;
 
-	g_return_val_if_fail (priv->pid_file == NULL, -1);
+	g_return_val_if_fail (priv->pid_file == NULL, FALSE);
 
 	iface = nm_dhcp_client_get_iface (client);
 
@@ -110,7 +110,7 @@ ip4_start (NMDHCPClient *client,
 
 	if (!g_file_test (priv->path, G_FILE_TEST_EXISTS)) {
 		nm_log_warn (LOGD_DHCP4, "%s does not exist.", priv->path);
-		return -1;
+		return FALSE;
 	}
 
 	/* Kill any existing dhcpcd from the pidfile */
@@ -152,20 +152,22 @@ ip4_start (NMDHCPClient *client,
 	nm_log_dbg (LOGD_DHCP4, "running: %s", cmd_str);
 	g_free (cmd_str);
 
-	if (!g_spawn_async (NULL, (char **) argv->pdata, NULL, G_SPAWN_DO_NOT_REAP_CHILD,
+	if (g_spawn_async (NULL, (char **) argv->pdata, NULL, G_SPAWN_DO_NOT_REAP_CHILD,
-	                    &dhcpcd_child_setup, NULL, &pid, &error)) {
+	                   &dhcpcd_child_setup, NULL, &pid, &error)) {
+		g_assert (pid > 0);
+		nm_log_info (LOGD_DHCP4, "dhcpcd started with pid %d", pid);
+		nm_dhcp_client_watch_child (client, pid);
+	} else {
 		nm_log_warn (LOGD_DHCP4, "dhcpcd failed to start.  error: '%s'", error->message);
 		g_error_free (error);
-		pid = -1;
+	}
-	} else
-		nm_log_info (LOGD_DHCP4, "dhcpcd started with pid %d", pid);
 
 	g_free (pid_contents);
 	g_ptr_array_free (argv, TRUE);
-	return pid;
+	return pid > 0 ? TRUE : FALSE;
 }
 
-static GPid
+static gboolean
 ip6_start (NMDHCPClient *client,
            GByteArray *dhcp_anycast_addr,
            const char *hostname,
@@ -173,7 +175,7 @@ ip6_start (NMDHCPClient *client,
            const GByteArray *duid)
 {
 	nm_log_warn (LOGD_DHCP6, "the dhcpcd backend does not support IPv6.");
-	return -1;
+	return FALSE;
 }
 
 static void