summary refs log tree commit diff
path: root/src/dhcp
diff options
context:
space:
mode:
Diffstat (limited to 'src/dhcp')
-rw-r--r--src/dhcp/nm-dhcp-client.c14
-rw-r--r--src/dhcp/nm-dhcp-client.h14
-rw-r--r--src/dhcp/nm-dhcp-dhclient.c181
-rw-r--r--src/dhcp/nm-dhcp-dhcpcanon.c90
-rw-r--r--src/dhcp/nm-dhcp-dhcpcd.c66
-rw-r--r--src/dhcp/nm-dhcp-manager.c59
-rw-r--r--src/dhcp/nm-dhcp-manager.h6
-rw-r--r--src/dhcp/nm-dhcp-systemd.c79
8 files changed, 309 insertions, 200 deletions
diff --git a/src/dhcp/nm-dhcp-client.c b/src/dhcp/nm-dhcp-client.c
index 9fc7d2c1..16db8306 100644
--- a/src/dhcp/nm-dhcp-client.c
+++ b/src/dhcp/nm-dhcp-client.c
@@ -510,7 +510,8 @@ nm_dhcp_client_start_ip4 (NMDhcpClient *self,
                           GBytes *client_id,
                           const char *dhcp_anycast_addr,
                           const char *hostname,
-                          const char *last_ip4_address)
+                          const char *last_ip4_address,
+                          GError **error)
 {
 	NMDhcpClientPrivate *priv;
 
@@ -531,7 +532,10 @@ nm_dhcp_client_start_ip4 (NMDhcpClient *self,
 	g_clear_pointer (&priv->hostname, g_free);
 	priv->hostname = g_strdup (hostname);
 
-	return NM_DHCP_CLIENT_GET_CLASS (self)->ip4_start (self, dhcp_anycast_addr, last_ip4_address);
+	return NM_DHCP_CLIENT_GET_CLASS (self)->ip4_start (self,
+	                                                   dhcp_anycast_addr,
+	                                                   last_ip4_address,
+	                                                   error);
 }
 
 static GBytes *
@@ -548,7 +552,8 @@ nm_dhcp_client_start_ip6 (NMDhcpClient *self,
                           const struct in6_addr *ll_addr,
                           const char *hostname,
                           NMSettingIP6ConfigPrivacy privacy,
-                          guint needed_prefixes)
+                          guint needed_prefixes,
+                          GError **error)
 {
 	NMDhcpClientPrivate *priv;
 	gs_free char *str = NULL;
@@ -584,7 +589,8 @@ nm_dhcp_client_start_ip6 (NMDhcpClient *self,
 	                                                   ll_addr,
 	                                                   privacy,
 	                                                   priv->duid,
-	                                                   needed_prefixes);
+	                                                   needed_prefixes,
+	                                                   error);
 }
 
 void
diff --git a/src/dhcp/nm-dhcp-client.h b/src/dhcp/nm-dhcp-client.h
index b50ea515..86d60e38 100644
--- a/src/dhcp/nm-dhcp-client.h
+++ b/src/dhcp/nm-dhcp-client.h
@@ -76,18 +76,18 @@ typedef enum {
 typedef struct {
 	GObjectClass parent;
 
-	/* Methods */
-
 	gboolean (*ip4_start)     (NMDhcpClient *self,
 	                           const char *anycast_addr,
-	                           const char *last_ip4_address);
+	                           const char *last_ip4_address,
+	                           GError **error);
 
 	gboolean (*ip6_start)     (NMDhcpClient *self,
 	                           const char *anycast_addr,
 	                           const struct in6_addr *ll_addr,
 	                           NMSettingIP6ConfigPrivacy privacy,
 	                           GBytes *duid,
-	                           guint needed_prefixes);
+	                           guint needed_prefixes,
+	                           GError **error);
 
 	void (*stop)              (NMDhcpClient *self,
 	                           gboolean release,
@@ -151,7 +151,8 @@ gboolean nm_dhcp_client_start_ip4 (NMDhcpClient *self,
                                    GBytes *client_id,
                                    const char *dhcp_anycast_addr,
                                    const char *hostname,
-                                   const char *last_ip4_address);
+                                   const char *last_ip4_address,
+                                   GError **error);
 
 gboolean nm_dhcp_client_start_ip6 (NMDhcpClient *self,
                                    GBytes *client_id,
@@ -160,7 +161,8 @@ gboolean nm_dhcp_client_start_ip6 (NMDhcpClient *self,
                                    const struct in6_addr *ll_addr,
                                    const char *hostname,
                                    NMSettingIP6ConfigPrivacy privacy,
-                                   guint needed_prefixes);
+                                   guint needed_prefixes,
+                                   GError **error);
 
 void nm_dhcp_client_stop (NMDhcpClient *self, gboolean release);
 
diff --git a/src/dhcp/nm-dhcp-dhclient.c b/src/dhcp/nm-dhcp-dhclient.c
index 8408e3f9..46d2339b 100644
--- a/src/dhcp/nm-dhcp-dhclient.c
+++ b/src/dhcp/nm-dhcp-dhclient.c
@@ -174,13 +174,14 @@ merge_dhclient_config (NMDhcpDhclient *self,
                        GBytes **out_new_client_id,
                        GError **error)
 {
-	char *orig = NULL, *new;
-	gboolean success = FALSE;
+	gs_free char *orig = NULL;
+	gs_free char *new = NULL;
 
-	g_return_val_if_fail (iface != NULL, FALSE);
-	g_return_val_if_fail (conf_file != NULL, FALSE);
+	g_return_val_if_fail (iface, FALSE);
+	g_return_val_if_fail (conf_file, FALSE);
 
-	if (orig_path && g_file_test (orig_path, G_FILE_TEST_EXISTS)) {
+	if (   orig_path
+	    && g_file_test (orig_path, G_FILE_TEST_EXISTS)) {
 		GError *read_error = NULL;
 
 		if (!g_file_get_contents (orig_path, &orig, NULL, &read_error)) {
@@ -190,14 +191,22 @@ merge_dhclient_config (NMDhcpDhclient *self,
 		}
 	}
 
-	new = nm_dhcp_dhclient_create_config (iface, addr_family, client_id, anycast_addr, hostname, timeout,
-	                                      use_fqdn, orig_path, orig, out_new_client_id);
+	new = nm_dhcp_dhclient_create_config (iface,
+	                                      addr_family,
+	                                      client_id,
+	                                      anycast_addr,
+	                                      hostname,
+	                                      timeout,
+	                                      use_fqdn,
+	                                      orig_path,
+	                                      orig,
+	                                      out_new_client_id);
 	g_assert (new);
-	success = g_file_set_contents (conf_file, new, -1, error);
-	g_free (new);
-	g_free (orig);
 
-	return success;
+	return g_file_set_contents (conf_file,
+	                            new,
+	                            -1,
+	                            error);
 }
 
 static char *
@@ -282,13 +291,14 @@ create_dhclient_config (NMDhcpDhclient *self,
                         gboolean use_fqdn,
                         GBytes **out_new_client_id)
 {
-	char *orig = NULL, *new = NULL;
+	gs_free char *orig = NULL;
+	char *new = NULL;
 	GError *error = NULL;
-	gboolean success = FALSE;
 
 	g_return_val_if_fail (iface != NULL, NULL);
 
 	new = g_strdup_printf (NMSTATEDIR "/dhclient%s-%s.conf", _addr_family_to_path_part (addr_family), iface);
+
 	_LOGD ("creating composite dhclient config %s", new);
 
 	orig = find_existing_config (self, addr_family, iface, uuid);
@@ -297,15 +307,12 @@ create_dhclient_config (NMDhcpDhclient *self,
 	else
 		_LOGD ("no existing dhclient configuration to merge");
 
-	error = NULL;
-	success = merge_dhclient_config (self, addr_family, iface, new, client_id, dhcp_anycast_addr,
-	                                 hostname, timeout, use_fqdn, orig, out_new_client_id, &error);
-	if (!success) {
+	if (!merge_dhclient_config (self, addr_family, iface, new, client_id, dhcp_anycast_addr,
+	                            hostname, timeout, use_fqdn, orig, out_new_client_id, &error)) {
 		_LOGW ("error creating dhclient configuration: %s", error->message);
-		g_error_free (error);
+		g_clear_error (&error);
 	}
 
-	g_free (orig);
 	return new;
 }
 
@@ -315,13 +322,14 @@ dhclient_start (NMDhcpClient *client,
                 GBytes *duid,
                 gboolean release,
                 pid_t *out_pid,
-                int prefixes)
+                int prefixes,
+                GError **error)
 {
 	NMDhcpDhclient *self = NM_DHCP_DHCLIENT (client);
 	NMDhcpDhclientPrivate *priv = NM_DHCP_DHCLIENT_GET_PRIVATE (self);
 	gs_unref_ptrarray GPtrArray *argv = NULL;
 	pid_t pid;
-	GError *error = NULL;
+	gs_free_error GError *local = NULL;
 	const char *iface;
 	const char *uuid;
 	const char *system_bus_address;
@@ -339,7 +347,7 @@ dhclient_start (NMDhcpClient *client,
 
 	dhclient_path = nm_dhcp_dhclient_get_path ();
 	if (!dhclient_path) {
-		_LOGW ("dhclient could not be found");
+		nm_utils_error_set_literal (error, NM_UTILS_ERROR_UNKNOWN, "dhclient binary not found");
 		return FALSE;
 	}
 
@@ -371,16 +379,20 @@ dhclient_start (NMDhcpClient *client,
 		gs_unref_object GFile *dst = g_file_new_for_path (preferred_leasefile_path);
 
 		/* Try to copy the existing leasefile to the preferred location */
-		if (g_file_copy (src, dst, G_FILE_COPY_OVERWRITE, NULL, NULL, NULL, &error)) {
-			/* Success; use the preferred leasefile path */
-			g_free (priv->lease_file);
-			priv->lease_file = g_strdup (g_file_get_path (dst));
-		} else {
+		if (!g_file_copy (src, dst, G_FILE_COPY_OVERWRITE, NULL, NULL, NULL, &local)) {
+			gs_free char *s_path = NULL;
+			gs_free char *d_path = NULL;
+
 			/* Failure; just use the existing leasefile */
 			_LOGW ("failed to copy leasefile %s to %s: %s",
-			       g_file_get_path (src), g_file_get_path (dst),
-			       error->message);
-			g_clear_error (&error);
+			       (s_path = g_file_get_path (src)),
+			       (d_path = g_file_get_path (dst)),
+			       local->message);
+			g_clear_error (&local);
+		} else {
+			/* Success; use the preferred leasefile path */
+			g_free (priv->lease_file);
+			priv->lease_file = g_file_get_path (dst);
 		}
 	}
 
@@ -389,9 +401,12 @@ dhclient_start (NMDhcpClient *client,
 		gs_free char *escaped = NULL;
 
 		escaped = nm_dhcp_dhclient_escape_duid (duid);
-		if (!nm_dhcp_dhclient_save_duid (priv->lease_file, escaped, &error)) {
-			_LOGW ("failed to save DUID to %s: %s", priv->lease_file, error->message);
-			g_clear_error (&error);
+		if (!nm_dhcp_dhclient_save_duid (priv->lease_file, escaped, &local)) {
+			nm_utils_error_set (error,
+			                    NM_UTILS_ERROR_UNKNOWN,
+			                    "failed to save DUID to '%s': %s",
+			                    priv->lease_file,
+			                    local->message);
 			return FALSE;
 		}
 	}
@@ -451,9 +466,11 @@ dhclient_start (NMDhcpClient *client,
 
 	if (!g_spawn_async (NULL, (char **) argv->pdata, NULL,
 	                    G_SPAWN_DO_NOT_REAP_CHILD | G_SPAWN_STDOUT_TO_DEV_NULL | G_SPAWN_STDERR_TO_DEV_NULL,
-	                    nm_utils_setpgid, NULL, &pid, &error)) {
-		_LOGW ("dhclient failed to start: '%s'", error->message);
-		g_error_free (error);
+	                    nm_utils_setpgid, NULL, &pid, &local)) {
+		nm_utils_error_set (error,
+		                    NM_UTILS_ERROR_UNKNOWN,
+		                    "dhclient failed to start: %s",
+		                    local->message);
 		return FALSE;
 	}
 
@@ -469,36 +486,46 @@ dhclient_start (NMDhcpClient *client,
 }
 
 static gboolean
-ip4_start (NMDhcpClient *client, const char *dhcp_anycast_addr, const char *last_ip4_address)
+ip4_start (NMDhcpClient *client,
+           const char *dhcp_anycast_addr,
+           const char *last_ip4_address,
+           GError **error)
 {
 	NMDhcpDhclient *self = NM_DHCP_DHCLIENT (client);
 	NMDhcpDhclientPrivate *priv = NM_DHCP_DHCLIENT_GET_PRIVATE (self);
 	GBytes *client_id;
 	gs_unref_bytes GBytes *new_client_id = NULL;
-	const char *iface, *uuid, *hostname;
-	guint32 timeout;
-	gboolean success = FALSE;
-	gboolean use_fqdn;
 
-	iface = nm_dhcp_client_get_iface (client);
-	uuid = nm_dhcp_client_get_uuid (client);
 	client_id = nm_dhcp_client_get_client_id (client);
-	hostname = nm_dhcp_client_get_hostname (client);
-	timeout = nm_dhcp_client_get_timeout (client);
-	use_fqdn = nm_dhcp_client_get_use_fqdn (client);
 
-	priv->conf_file = create_dhclient_config (self, AF_INET, iface, uuid, client_id, dhcp_anycast_addr,
-	                                          hostname, timeout, use_fqdn, &new_client_id);
-	if (priv->conf_file) {
-		if (new_client_id) {
-			nm_assert (!client_id);
-			nm_dhcp_client_set_client_id (client, new_client_id);
-		}
-		success = dhclient_start (client, NULL, NULL, FALSE, NULL, 0);
-	} else
-		_LOGW ("error creating dhclient configuration file");
+	priv->conf_file = create_dhclient_config (self,
+	                                          AF_INET,
+	                                          nm_dhcp_client_get_iface (client),
+	                                          nm_dhcp_client_get_uuid (client),
+	                                          client_id,
+	                                          dhcp_anycast_addr,
+	                                          nm_dhcp_client_get_hostname (client),
+	                                          nm_dhcp_client_get_timeout (client),
+	                                          nm_dhcp_client_get_use_fqdn (client),
+	                                          &new_client_id);
+	if (!priv->conf_file) {
+		nm_utils_error_set_literal (error,
+		                            NM_UTILS_ERROR_UNKNOWN,
+		                            "error creating dhclient configuration file");
+		return FALSE;
+	}
 
-	return success;
+	if (new_client_id) {
+		nm_assert (!client_id);
+		nm_dhcp_client_set_client_id (client, new_client_id);
+	}
+	return dhclient_start (client,
+	                       NULL,
+	                       NULL,
+	                       FALSE,
+	                       NULL,
+	                       0,
+	                       error);
 }
 
 static gboolean
@@ -507,22 +534,26 @@ ip6_start (NMDhcpClient *client,
            const struct in6_addr *ll_addr,
            NMSettingIP6ConfigPrivacy privacy,
            GBytes *duid,
-           guint needed_prefixes)
+           guint needed_prefixes,
+           GError **error)
 {
 	NMDhcpDhclient *self = NM_DHCP_DHCLIENT (client);
 	NMDhcpDhclientPrivate *priv = NM_DHCP_DHCLIENT_GET_PRIVATE (self);
-	const char *iface, *uuid, *hostname;
-	guint32 timeout;
-
-	iface = nm_dhcp_client_get_iface (client);
-	uuid = nm_dhcp_client_get_uuid (client);
-	hostname = nm_dhcp_client_get_hostname (client);
-	timeout = nm_dhcp_client_get_timeout (client);
 
-	priv->conf_file = create_dhclient_config (self, AF_INET6, iface, uuid, NULL, dhcp_anycast_addr,
-	                                          hostname, timeout, TRUE, NULL);
+	priv->conf_file = create_dhclient_config (self,
+	                                          AF_INET6,
+	                                          nm_dhcp_client_get_iface (client),
+	                                          nm_dhcp_client_get_uuid (client),
+	                                          NULL,
+	                                          dhcp_anycast_addr,
+	                                          nm_dhcp_client_get_hostname (client),
+	                                          nm_dhcp_client_get_timeout (client),
+	                                          TRUE,
+	                                          NULL);
 	if (!priv->conf_file) {
-		_LOGW ("error creating dhclient configuration file");
+		nm_utils_error_set_literal (error,
+		                            NM_UTILS_ERROR_UNKNOWN,
+		                            "error creating dhclient configuration file");
 		return FALSE;
 	}
 
@@ -530,7 +561,11 @@ ip6_start (NMDhcpClient *client,
 	                       nm_dhcp_client_get_info_only (NM_DHCP_CLIENT (self))
 	                         ? "-S"
 	                         : "-N",
-	                       duid, FALSE, NULL, needed_prefixes);
+	                       duid,
+	                       FALSE,
+	                       NULL,
+	                       needed_prefixes,
+	                       error);
 }
 
 static void
@@ -556,7 +591,13 @@ stop (NMDhcpClient *client, gboolean release, GBytes *duid)
 	if (release) {
 		pid_t rpid = -1;
 
-		if (dhclient_start (client, NULL, duid, TRUE, &rpid, 0)) {
+		if (dhclient_start (client,
+		                    NULL,
+		                    duid,
+		                    TRUE,
+		                    &rpid,
+		                    0,
+		                    NULL)) {
 			/* Wait a few seconds for the release to happen */
 			nm_dhcp_client_stop_pid (rpid, nm_dhcp_client_get_iface (client));
 		}
diff --git a/src/dhcp/nm-dhcp-dhcpcanon.c b/src/dhcp/nm-dhcp-dhcpcanon.c
index 92aa7f8c..de403020 100644
--- a/src/dhcp/nm-dhcp-dhcpcanon.c
+++ b/src/dhcp/nm-dhcp-dhcpcanon.c
@@ -82,28 +82,36 @@ dhcpcanon_start (NMDhcpClient *client,
                 GBytes *duid,
                 gboolean release,
                 pid_t *out_pid,
-                int prefixes)
+                guint needed_prefixes,
+                GError **error)
 {
 	NMDhcpDhcpcanon *self = NM_DHCP_DHCPCANON (client);
 	NMDhcpDhcpcanonPrivate *priv = NM_DHCP_DHCPCANON_GET_PRIVATE (self);
-	GPtrArray *argv = NULL;
+	gs_unref_ptrarray GPtrArray *argv = NULL;
 	pid_t pid;
-	GError *error = NULL;
-	const char *iface, *system_bus_address, *dhcpcanon_path = NULL;
-	char *binary_name, *cmd_str, *pid_file = NULL, *system_bus_address_env = NULL;
+	gs_free_error GError *local = NULL;
+	const char *iface;
+	const char *system_bus_address;
+	const char *dhcpcanon_path;
+	gs_free char *binary_name = NULL;
+	gs_free char *pid_file = NULL;
+	gs_free char *system_bus_address_env = NULL;
 	int addr_family;
 
-	g_return_val_if_fail (priv->pid_file == NULL, FALSE);
+	g_return_val_if_fail (!priv->pid_file, FALSE);
 
 	iface = nm_dhcp_client_get_iface (client);
+
 	addr_family = nm_dhcp_client_get_addr_family (client);
+
 	dhcpcanon_path = nm_dhcp_dhcpcanon_get_path ();
-	_LOGD ("dhcpcanon_path: %s", dhcpcanon_path);
 	if (!dhcpcanon_path) {
-		_LOGW ("dhcpcanon could not be found");
+		nm_utils_error_set_literal (error, NM_UTILS_ERROR_UNKNOWN, "dhcpcanon binary not found");
 		return FALSE;
 	}
 
+	_LOGD ("dhcpcanon_path: %s", dhcpcanon_path);
+
 	pid_file = g_strdup_printf (RUNSTATEDIR "/dhcpcanon%c-%s.pid",
 	                            nm_utils_addr_family_to_char (addr_family),
 	                            iface);
@@ -112,7 +120,6 @@ dhcpcanon_start (NMDhcpClient *client,
 	/* Kill any existing dhcpcanon from the pidfile */
 	binary_name = g_path_get_basename (dhcpcanon_path);
 	nm_dhcp_client_stop_existing (pid_file, binary_name);
-	g_free (binary_name);
 
 	argv = g_ptr_array_new ();
 	g_ptr_array_add (argv, (gpointer) dhcpcanon_path);
@@ -120,10 +127,8 @@ dhcpcanon_start (NMDhcpClient *client,
 	g_ptr_array_add (argv, (gpointer) "-sf"); /* Set script file */
 	g_ptr_array_add (argv, (gpointer) nm_dhcp_helper_path);
 
-	if (pid_file) {
-		g_ptr_array_add (argv, (gpointer) "-pf"); /* Set pid file */
-		g_ptr_array_add (argv, (gpointer) pid_file);
-	}
+	g_ptr_array_add (argv, (gpointer) "-pf"); /* Set pid file */
+	g_ptr_array_add (argv, (gpointer) pid_file);
 
 	if (priv->conf_file) {
 		g_ptr_array_add (argv, (gpointer) "-cf"); /* Set interface config file */
@@ -144,33 +149,43 @@ dhcpcanon_start (NMDhcpClient *client,
 	g_ptr_array_add (argv, (gpointer) iface);
 	g_ptr_array_add (argv, NULL);
 
-	cmd_str = g_strjoinv (" ", (char **) argv->pdata);
-	g_free (cmd_str);
-
-	if (g_spawn_async (NULL, (char **) argv->pdata, NULL,
-	                   G_SPAWN_DO_NOT_REAP_CHILD | G_SPAWN_STDOUT_TO_DEV_NULL | G_SPAWN_STDERR_TO_DEV_NULL,
-	                   nm_utils_setpgid, NULL, &pid, &error)) {
-		 g_assert (pid > 0);
-		_LOGI ("dhcpcanon started with pid %d", pid);
-		nm_dhcp_client_watch_child (client, pid);
-		priv->pid_file = pid_file;
-	} else {
-		_LOGW ("dhcpcanon failed to start: '%s'", error->message);
-		g_error_free (error);
-		g_free (pid_file);
+	if (!g_spawn_async (NULL,
+	                   (char **) argv->pdata,
+	                   NULL,
+	                     G_SPAWN_DO_NOT_REAP_CHILD
+	                   | G_SPAWN_STDOUT_TO_DEV_NULL
+	                   | G_SPAWN_STDERR_TO_DEV_NULL,
+	                   nm_utils_setpgid,
+	                   NULL,
+	                   &pid,
+	                   &local)) {
+		nm_utils_error_set (error,
+		                    NM_UTILS_ERROR_UNKNOWN,
+		                    "dhcpcanon failed to start: %s",
+		                    local->message);
+		return FALSE;
 	}
 
-	g_ptr_array_free (argv, TRUE);
-	g_free (system_bus_address_env);
-	return pid > 0 ? TRUE : FALSE;
+	nm_assert (pid > 0);
+	_LOGI ("dhcpcanon started with pid %d", pid);
+	nm_dhcp_client_watch_child (client, pid);
+	priv->pid_file = g_steal_pointer (&pid_file);
+	return TRUE;
 }
 
 static gboolean
-ip4_start (NMDhcpClient *client, const char *dhcp_anycast_addr, const char *last_ip4_address)
+ip4_start (NMDhcpClient *client,
+           const char *dhcp_anycast_addr,
+           const char *last_ip4_address,
+           GError **error)
 {
-	gboolean success = FALSE;
-	success = dhcpcanon_start (client, NULL, NULL, FALSE, NULL, 0);
-	return success;
+	return dhcpcanon_start (client,
+	                        NULL,
+	                        NULL,
+	                        FALSE,
+	                        NULL,
+	                        0,
+	                        error);
 }
 
 static gboolean
@@ -179,11 +194,10 @@ ip6_start (NMDhcpClient *client,
            const struct in6_addr *ll_addr,
            NMSettingIP6ConfigPrivacy privacy,
            GBytes *duid,
-           guint needed_prefixes)
+           guint needed_prefixes,
+           GError **error)
 {
-	NMDhcpDhcpcanon *self = NM_DHCP_DHCPCANON (client);
-
-	_LOGW ("the dhcpcd backend does not support IPv6");
+	nm_utils_error_set_literal (error, NM_UTILS_ERROR_UNKNOWN, "dhcpcanon plugin does not support IPv6");
 	return FALSE;
 }
 static void
diff --git a/src/dhcp/nm-dhcp-dhcpcd.c b/src/dhcp/nm-dhcp-dhcpcd.c
index 10094e5c..98ab5342 100644
--- a/src/dhcp/nm-dhcp-dhcpcd.c
+++ b/src/dhcp/nm-dhcp-dhcpcd.c
@@ -81,15 +81,21 @@ nm_dhcp_dhcpcd_get_path (void)
 }
 
 static gboolean
-ip4_start (NMDhcpClient *client, const char *dhcp_anycast_addr, const char *last_ip4_address)
+ip4_start (NMDhcpClient *client,
+           const char *dhcp_anycast_addr,
+           const char *last_ip4_address,
+           GError **error)
 {
 	NMDhcpDhcpcd *self = NM_DHCP_DHCPCD (client);
 	NMDhcpDhcpcdPrivate *priv = NM_DHCP_DHCPCD_GET_PRIVATE (self);
-	GPtrArray *argv = NULL;
+	gs_unref_ptrarray GPtrArray *argv = NULL;
 	pid_t pid = -1;
-	GError *error = NULL;
-	char *pid_contents = NULL, *binary_name, *cmd_str;
-	const char *iface, *dhcpcd_path, *hostname;
+	GError *local = NULL;
+	gs_free char *cmd_str = NULL;
+	gs_free char *binary_name = NULL;
+	const char *iface;
+	const char *dhcpcd_path;
+	const char *hostname;
 
 	g_return_val_if_fail (priv->pid_file == NULL, FALSE);
 
@@ -102,14 +108,13 @@ ip4_start (NMDhcpClient *client, const char *dhcp_anycast_addr, const char *last
 
 	dhcpcd_path = nm_dhcp_dhcpcd_get_path ();
 	if (!dhcpcd_path) {
-		_LOGW ("dhcpcd could not be found");
+		nm_utils_error_set_literal (error, NM_UTILS_ERROR_UNKNOWN, "dhcpcd binary not found");
 		return FALSE;
 	}
 
 	/* Kill any existing dhcpcd from the pidfile */
 	binary_name = g_path_get_basename (dhcpcd_path);
 	nm_dhcp_client_stop_existing (priv->pid_file, binary_name);
-	g_free (binary_name);
 
 	argv = g_ptr_array_new ();
 	g_ptr_array_add (argv, (gpointer) dhcpcd_path);
@@ -128,13 +133,11 @@ ip4_start (NMDhcpClient *client, const char *dhcp_anycast_addr, const char *last
 	g_ptr_array_add (argv, (gpointer) "-c");    /* Set script file */
 	g_ptr_array_add (argv, (gpointer) nm_dhcp_helper_path);
 
-#ifdef DHCPCD_SUPPORTS_IPV6
 	/* IPv4-only for now.  NetworkManager knows better than dhcpcd when to
 	 * run IPv6, and dhcpcd's automatic Router Solicitations cause problems
 	 * with devices that don't expect them.
 	 */
 	g_ptr_array_add (argv, (gpointer) "-4");
-#endif
 
 	hostname = nm_dhcp_client_get_hostname (client);
 
@@ -153,24 +156,30 @@ ip4_start (NMDhcpClient *client, const char *dhcp_anycast_addr, const char *last
 	g_ptr_array_add (argv, (gpointer) iface);
 	g_ptr_array_add (argv, NULL);
 
-	cmd_str = g_strjoinv (" ", (char **) argv->pdata);
-	_LOGD ("running: %s", cmd_str);
-	g_free (cmd_str);
-
-	if (g_spawn_async (NULL, (char **) argv->pdata, NULL,
-	                   G_SPAWN_DO_NOT_REAP_CHILD | G_SPAWN_STDOUT_TO_DEV_NULL | G_SPAWN_STDERR_TO_DEV_NULL,
-	                   nm_utils_setpgid, NULL, &pid, &error)) {
-		g_assert (pid > 0);
-		_LOGI ("dhcpcd started with pid %d", pid);
-		nm_dhcp_client_watch_child (client, pid);
-	} else {
-		_LOGW ("dhcpcd failed to start, error: '%s'", error->message);
-		g_error_free (error);
+	_LOGD ("running: %s",
+	       (cmd_str = g_strjoinv (" ", (char **) argv->pdata)));
+
+	if (!g_spawn_async (NULL,
+	                    (char **) argv->pdata, NULL,
+	                      G_SPAWN_DO_NOT_REAP_CHILD
+	                    | G_SPAWN_STDOUT_TO_DEV_NULL
+	                    | G_SPAWN_STDERR_TO_DEV_NULL,
+	                    nm_utils_setpgid,
+	                    NULL,
+	                    &pid,
+	                    &local)) {
+		nm_utils_error_set (error,
+		                    NM_UTILS_ERROR_UNKNOWN,
+		                    "dhcpcd failed to start: %s",
+		                    local->message);
+		g_error_free (local);
+		return FALSE;
 	}
 
-	g_free (pid_contents);
-	g_ptr_array_free (argv, TRUE);
-	return pid > 0 ? TRUE : FALSE;
+	nm_assert (pid > 0);
+	_LOGI ("dhcpcd started with pid %d", pid);
+	nm_dhcp_client_watch_child (client, pid);
+	return TRUE;
 }
 
 static gboolean
@@ -179,11 +188,10 @@ ip6_start (NMDhcpClient *client,
            const struct in6_addr *ll_addr,
            NMSettingIP6ConfigPrivacy privacy,
            GBytes *duid,
-           guint needed_prefixes)
+           guint needed_prefixes,
+           GError **error)
 {
-	NMDhcpDhcpcd *self = NM_DHCP_DHCPCD (client);
-
-	_LOGW ("the dhcpcd backend does not support IPv6");
+	nm_utils_error_set_literal (error, NM_UTILS_ERROR_UNKNOWN, "dhcpcd plugin does not support IPv6");
 	return FALSE;
 }
 
diff --git a/src/dhcp/nm-dhcp-manager.c b/src/dhcp/nm-dhcp-manager.c
index 67254323..6c71af9d 100644
--- a/src/dhcp/nm-dhcp-manager.c
+++ b/src/dhcp/nm-dhcp-manager.c
@@ -172,22 +172,39 @@ client_start (NMDhcpManager *self,
               gboolean info_only,
               NMSettingIP6ConfigPrivacy privacy,
               const char *last_ip4_address,
-              guint needed_prefixes)
+              guint needed_prefixes,
+              GError **error)
 {
 	NMDhcpManagerPrivate *priv;
 	NMDhcpClient *client;
 	gboolean success = FALSE;
+	gsize hwaddr_len;
 
-	g_return_val_if_fail (self, NULL);
 	g_return_val_if_fail (NM_IS_DHCP_MANAGER (self), NULL);
 	g_return_val_if_fail (ifindex > 0, NULL);
 	g_return_val_if_fail (uuid != NULL, NULL);
 	g_return_val_if_fail (!dhcp_client_id || g_bytes_get_size (dhcp_client_id) >= 2, NULL);
+	g_return_val_if_fail (!error || !*error, NULL);
+
+	if (!hwaddr) {
+		nm_utils_error_set (error,
+		                    NM_UTILS_ERROR_UNKNOWN,
+		                    "missing MAC address");
+		return NULL;
+	}
+
+	hwaddr_len = g_bytes_get_size (hwaddr);
+	if (   hwaddr_len == 0
+	    || hwaddr_len > NM_UTILS_HWADDR_LEN_MAX) {
+		nm_utils_error_set (error,
+		                    NM_UTILS_ERROR_UNKNOWN,
+		                    "invalid MAC address");
+		g_return_val_if_reached (NULL) ;
+	}
 
 	priv = NM_DHCP_MANAGER_GET_PRIVATE (self);
 
-	if (!priv->client_factory)
-		return NULL;
+	nm_assert (priv->client_factory);
 
 	/* Kill any old client instance */
 	client = get_client_for_ifindex (self, addr_family, ifindex);
@@ -216,10 +233,24 @@ client_start (NMDhcpManager *self,
 	c_list_link_tail (&priv->dhcp_client_lst_head, &client->dhcp_client_lst);
 	g_signal_connect (client, NM_DHCP_CLIENT_SIGNAL_STATE_CHANGED, G_CALLBACK (client_state_changed), self);
 
-	if (addr_family == AF_INET)
-		success = nm_dhcp_client_start_ip4 (client, dhcp_client_id, dhcp_anycast_addr, hostname, last_ip4_address);
-	else
-		success = nm_dhcp_client_start_ip6 (client, dhcp_client_id, enforce_duid, dhcp_anycast_addr, ipv6_ll_addr, hostname, privacy, needed_prefixes);
+	if (addr_family == AF_INET) {
+		success = nm_dhcp_client_start_ip4 (client,
+		                                    dhcp_client_id,
+		                                    dhcp_anycast_addr,
+		                                    hostname,
+		                                    last_ip4_address,
+		                                    error);
+	} else {
+		success = nm_dhcp_client_start_ip6 (client,
+		                                    dhcp_client_id,
+		                                    enforce_duid,
+		                                    dhcp_anycast_addr,
+		                                    ipv6_ll_addr,
+		                                    hostname,
+		                                    privacy,
+		                                    needed_prefixes,
+		                                    error);
+	}
 
 	if (!success) {
 		remove_client_unref (self, client);
@@ -245,7 +276,8 @@ nm_dhcp_manager_start_ip4 (NMDhcpManager *self,
                            GBytes *dhcp_client_id,
                            guint32 timeout,
                            const char *dhcp_anycast_addr,
-                           const char *last_ip_address)
+                           const char *last_ip_address,
+                           GError **error)
 {
 	NMDhcpManagerPrivate *priv;
 	const char *hostname = NULL;
@@ -282,7 +314,7 @@ nm_dhcp_manager_start_ip4 (NMDhcpManager *self,
 	return client_start (self, AF_INET, multi_idx, iface, ifindex, hwaddr, uuid,
 	                     route_table, route_metric, NULL,
 	                     dhcp_client_id, 0, timeout, dhcp_anycast_addr, hostname,
-	                     use_fqdn, FALSE, 0, last_ip_address, 0);
+	                     use_fqdn, FALSE, 0, last_ip_address, 0, error);
 }
 
 /* Caller owns a reference to the NMDhcpClient on return */
@@ -304,7 +336,8 @@ nm_dhcp_manager_start_ip6 (NMDhcpManager *self,
                            const char *dhcp_anycast_addr,
                            gboolean info_only,
                            NMSettingIP6ConfigPrivacy privacy,
-                           guint needed_prefixes)
+                           guint needed_prefixes,
+                           GError **error)
 {
 	NMDhcpManagerPrivate *priv;
 	const char *hostname = NULL;
@@ -319,7 +352,7 @@ nm_dhcp_manager_start_ip6 (NMDhcpManager *self,
 	return client_start (self, AF_INET6, multi_idx, iface, ifindex, hwaddr, uuid,
 	                     route_table, route_metric, ll_addr, duid, enforce_duid,
 	                     timeout, dhcp_anycast_addr, hostname, TRUE, info_only,
-	                     privacy, NULL, needed_prefixes);
+	                     privacy, NULL, needed_prefixes, error);
 }
 
 void
@@ -409,7 +442,7 @@ nm_dhcp_manager_init (NMDhcpManager *self)
 		}
 	}
 
-	nm_assert (client_factory);
+	g_return_if_fail (client_factory);
 
 	nm_log_info (LOGD_DHCP, "dhcp-init: Using DHCP client '%s'", client_factory->name);
 
diff --git a/src/dhcp/nm-dhcp-manager.h b/src/dhcp/nm-dhcp-manager.h
index 7eb32c37..1d9e5c21 100644
--- a/src/dhcp/nm-dhcp-manager.h
+++ b/src/dhcp/nm-dhcp-manager.h
@@ -59,7 +59,8 @@ NMDhcpClient * nm_dhcp_manager_start_ip4     (NMDhcpManager *manager,
                                               GBytes *dhcp_client_id,
                                               guint32 timeout,
                                               const char *dhcp_anycast_addr,
-                                              const char *last_ip_address);
+                                              const char *last_ip_address,
+                                              GError **error);
 
 NMDhcpClient * nm_dhcp_manager_start_ip6     (NMDhcpManager *manager,
                                               struct _NMDedupMultiIndex *multi_idx,
@@ -78,7 +79,8 @@ NMDhcpClient * nm_dhcp_manager_start_ip6     (NMDhcpManager *manager,
                                               const char *dhcp_anycast_addr,
                                               gboolean info_only,
                                               NMSettingIP6ConfigPrivacy privacy,
-                                              guint needed_prefixes);
+                                              guint needed_prefixes,
+                                              GError **error);
 
 /* For testing only */
 extern const char* nm_dhcp_helper_path;
diff --git a/src/dhcp/nm-dhcp-systemd.c b/src/dhcp/nm-dhcp-systemd.c
index 2d0202bb..b51c6e7b 100644
--- a/src/dhcp/nm-dhcp-systemd.c
+++ b/src/dhcp/nm-dhcp-systemd.c
@@ -567,7 +567,10 @@ get_arp_type (GBytes *hwaddr)
 }
 
 static gboolean
-ip4_start (NMDhcpClient *client, const char *dhcp_anycast_addr, const char *last_ip4_address)
+ip4_start (NMDhcpClient *client,
+           const char *dhcp_anycast_addr,
+           const char *last_ip4_address,
+           GError **error)
 {
 	NMDhcpSystemd *self = NM_DHCP_SYSTEMD (client);
 	NMDhcpSystemdPrivate *priv = NM_DHCP_SYSTEMD_GET_PRIVATE (self);
@@ -590,7 +593,7 @@ ip4_start (NMDhcpClient *client, const char *dhcp_anycast_addr, const char *last
 
 	r = sd_dhcp_client_new (&priv->client4, FALSE);
 	if (r < 0) {
-		_LOGW ("failed to create client (%d)", r);
+		nm_utils_error_set_errno (error, r, "failed to create dhcp-client: %s");
 		return FALSE;
 	}
 
@@ -598,8 +601,8 @@ ip4_start (NMDhcpClient *client, const char *dhcp_anycast_addr, const char *last
 
 	r = sd_dhcp_client_attach_event (priv->client4, NULL, 0);
 	if (r < 0) {
-		_LOGW ("failed to attach event (%d)", r);
-		goto error;
+		nm_utils_error_set_errno (error, r, "failed to attach event: %s");
+		goto errout;
 	}
 
 	hwaddr = nm_dhcp_client_get_hw_addr (client);
@@ -613,21 +616,21 @@ ip4_start (NMDhcpClient *client, const char *dhcp_anycast_addr, const char *last
 		                            len,
 		                            get_arp_type (hwaddr));
 		if (r < 0) {
-			_LOGW ("failed to set MAC address (%d)", r);
-			goto error;
+			nm_utils_error_set_errno (error, r, "failed to set MAC address: %s");
+			goto errout;
 		}
 	}
 
 	r = sd_dhcp_client_set_ifindex (priv->client4, nm_dhcp_client_get_ifindex (client));
 	if (r < 0) {
-		_LOGW ("failed to set ififindex (%d)", r);
-		goto error;
+		nm_utils_error_set_errno (error, r, "failed to set ifindex: %s");
+		goto errout;
 	}
 
 	r = sd_dhcp_client_set_callback (priv->client4, dhcp_event_cb, client);
 	if (r < 0) {
-		_LOGW ("failed to set callback (%d)", r);
-		goto error;
+		nm_utils_error_set_errno (error, r, "failed to set callback: %s");
+		goto errout;
 	}
 
 	dhcp_lease_load (&lease, priv->lease_file);
@@ -640,8 +643,8 @@ ip4_start (NMDhcpClient *client, const char *dhcp_anycast_addr, const char *last
 	if (last_addr.s_addr) {
 		r = sd_dhcp_client_set_request_address (priv->client4, &last_addr);
 		if (r < 0) {
-			_LOGW ("failed to set last IPv4 address (%d)", r);
-			goto error;
+			nm_utils_error_set_errno (error, r, "failed to set last IPv4 address: %s");
+			goto errout;
 		}
 	}
 
@@ -681,25 +684,25 @@ ip4_start (NMDhcpClient *client, const char *dhcp_anycast_addr, const char *last
 		 */
 		r = sd_dhcp_client_set_hostname (priv->client4, hostname);
 		if (r < 0) {
-			_LOGW ("failed to set DHCP hostname to '%s' (%d)", hostname, r);
-			goto error;
+			nm_utils_error_set_errno (error, r, "failed to set DHCP hostname: %s");
+			goto errout;
 		}
 	}
 
 	r = sd_dhcp_client_start (priv->client4);
 	if (r < 0) {
-		_LOGW ("failed to start client (%d)", r);
-		goto error;
+		nm_utils_error_set_errno (error, r, "failed to start DHCP client: %s");
+		goto errout;
 	}
 
 	nm_dhcp_client_start_timeout (client);
 
 	success = TRUE;
 
-error:
+errout:
 	sd_dhcp_lease_unref (lease);
 	if (!success)
-		priv->client4 = sd_dhcp_client_unref (priv->client4);
+		sd_dhcp_client_unref (g_steal_pointer (&priv->client4));
 	return success;
 }
 
@@ -864,7 +867,8 @@ ip6_start (NMDhcpClient *client,
            const struct in6_addr *ll_addr,
            NMSettingIP6ConfigPrivacy privacy,
            GBytes *duid,
-           guint needed_prefixes)
+           guint needed_prefixes,
+           GError **error)
 {
 	NMDhcpSystemd *self = NM_DHCP_SYSTEMD (client);
 	NMDhcpSystemdPrivate *priv = NM_DHCP_SYSTEMD_GET_PRIVATE (self);
@@ -888,7 +892,7 @@ ip6_start (NMDhcpClient *client,
 
 	r = sd_dhcp6_client_new (&priv->client6);
 	if (r < 0) {
-		_LOGW ("failed to create client (%d)", r);
+		nm_utils_error_set_errno (error, r, "failed to create dhcp-client: %s");
 		return FALSE;
 	}
 
@@ -907,14 +911,14 @@ ip6_start (NMDhcpClient *client,
 	                              &duid_arr[2],
 	                              duid_len - 2);
 	if (r < 0) {
-		_LOGW ("failed to set DUID (%d)", r);
+		nm_utils_error_set_errno (error, r, "failed to set DUID: %s");
 		return FALSE;
 	}
 
 	r = sd_dhcp6_client_attach_event (priv->client6, NULL, 0);
 	if (r < 0) {
-		_LOGW ("failed to attach event (%d)", r);
-		goto error;
+		nm_utils_error_set_errno (error, r, "failed to attach event: %s");
+		goto errout;
 	}
 
 	hwaddr = nm_dhcp_client_get_hw_addr (client);
@@ -928,21 +932,21 @@ ip6_start (NMDhcpClient *client,
 		                             len,
 		                             get_arp_type (hwaddr));
 		if (r < 0) {
-			_LOGW ("failed to set MAC address (%d)", r);
-			goto error;
+			nm_utils_error_set_errno (error, r, "failed to set MAC address: %s");
+			goto errout;
 		}
 	}
 
 	r = sd_dhcp6_client_set_ifindex (priv->client6, nm_dhcp_client_get_ifindex (client));
 	if (r < 0) {
-		_LOGW ("failed to set ifindex (%d)", r);
-		goto error;
+		nm_utils_error_set_errno (error, r, "failed to set ifindex: %s");
+		goto errout;
 	}
 
 	r = sd_dhcp6_client_set_callback (priv->client6, dhcp6_event_cb, client);
 	if (r < 0) {
-		_LOGW ("failed to set callback (%d)", r);
-		goto error;
+		nm_utils_error_set_errno (error, r, "failed to set callback: %s");
+		goto errout;
 	}
 
 	/* Add requested options */
@@ -953,30 +957,29 @@ ip6_start (NMDhcpClient *client,
 
 	r = sd_dhcp6_client_set_local_address (priv->client6, ll_addr);
 	if (r < 0) {
-		_LOGW ("failed to set local address (%d)", r);
-		goto error;
+		nm_utils_error_set_errno (error, r, "failed to set local address: %s");
+		goto errout;
 	}
 
 	hostname = nm_dhcp_client_get_hostname (client);
 	r = sd_dhcp6_client_set_fqdn (priv->client6, hostname);
 	if (r < 0) {
-		_LOGW ("failed to set DHCP hostname to '%s' (%d)", hostname, r);
-		goto error;
+		nm_utils_error_set_errno (error, r, "failed to set DHCP hostname: %s");
+		goto errout;
 	}
 
 	r = sd_dhcp6_client_start (priv->client6);
 	if (r < 0) {
-		_LOGW ("failed to start client (%d)", r);
-		goto error;
+		nm_utils_error_set_errno (error, r, "failed to start client: %s");
+		goto errout;
 	}
 
 	nm_dhcp_client_start_timeout (client);
 
 	return TRUE;
 
-error:
-	sd_dhcp6_client_unref (priv->client6);
-	priv->client6 = NULL;
+errout:
+	sd_dhcp6_client_unref (g_steal_pointer (&priv->client6));
 	return FALSE;
 }