From 1a4fe308e85f0980bc9acf75047659a0e4571d2c Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 10 Sep 2018 12:22:20 +0200 Subject: [PATCH] dhcp: return error reason from DHCP client start --- src/devices/nm-device.c | 40 +++++--- src/dhcp/nm-dhcp-client.c | 14 ++- src/dhcp/nm-dhcp-client.h | 14 +-- src/dhcp/nm-dhcp-dhclient.c | 175 +++++++++++++++++++++-------------- src/dhcp/nm-dhcp-dhcpcanon.c | 90 ++++++++++-------- src/dhcp/nm-dhcp-dhcpcd.c | 62 +++++++------ src/dhcp/nm-dhcp-manager.c | 42 ++++++--- src/dhcp/nm-dhcp-manager.h | 6 +- src/dhcp/nm-dhcp-systemd.c | 79 ++++++++-------- src/nm-iface-helper.c | 9 +- 10 files changed, 318 insertions(+), 213 deletions(-) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index c5f30ea28..8ca899a28 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -7561,6 +7561,7 @@ dhcp4_start (NMDevice *self) gs_unref_bytes GBytes *hwaddr = NULL; gs_unref_bytes GBytes *client_id = NULL; NMConnection *connection; + GError *error = NULL; connection = nm_device_get_applied_connection (self); g_return_val_if_fail (connection, FALSE); @@ -7591,10 +7592,14 @@ dhcp4_start (NMDevice *self) client_id, get_dhcp_timeout (self, AF_INET), priv->dhcp_anycast_address, - NULL); + NULL, + &error); - if (!priv->dhcp4.client) + if (!priv->dhcp4.client) { + _LOGW (LOGD_DHCP4, "failure to start DHCP: %s", error->message); + g_clear_error (&error); return NM_ACT_STAGE_RETURN_FAILURE; + } priv->dhcp4.state_sigid = g_signal_connect (priv->dhcp4.client, NM_DHCP_CLIENT_SIGNAL_STATE_CHANGED, @@ -8396,6 +8401,7 @@ dhcp6_start_with_link_ready (NMDevice *self, NMConnection *connection) gs_unref_bytes GBytes *hwaddr = NULL; gs_unref_bytes GBytes *duid = NULL; gboolean enforce_duid = FALSE; + GError *error = NULL; const NMPlatformIP6Address *ll_addr = NULL; @@ -8435,23 +8441,29 @@ dhcp6_start_with_link_ready (NMDevice *self, NMConnection *connection) priv->dhcp_anycast_address, (priv->dhcp6.mode == NM_NDISC_DHCP_LEVEL_OTHERCONF) ? TRUE : FALSE, nm_setting_ip6_config_get_ip6_privacy (NM_SETTING_IP6_CONFIG (s_ip6)), - priv->dhcp6.needed_prefixes); - - if (priv->dhcp6.client) { - priv->dhcp6.state_sigid = g_signal_connect (priv->dhcp6.client, - NM_DHCP_CLIENT_SIGNAL_STATE_CHANGED, - G_CALLBACK (dhcp6_state_changed), - self); - priv->dhcp6.prefix_sigid = g_signal_connect (priv->dhcp6.client, - NM_DHCP_CLIENT_SIGNAL_PREFIX_DELEGATED, - G_CALLBACK (dhcp6_prefix_delegated), - self); + priv->dhcp6.needed_prefixes, + &error); + if (!priv->dhcp6.client) { + _LOGW (LOGD_DHCP6, "failure to start DHCPv6: %s", error->message); + g_clear_error (&error); + if (nm_device_sys_iface_state_is_external_or_assume (self)) + priv->dhcp6.was_active = TRUE; + return FALSE; } + priv->dhcp6.state_sigid = g_signal_connect (priv->dhcp6.client, + NM_DHCP_CLIENT_SIGNAL_STATE_CHANGED, + G_CALLBACK (dhcp6_state_changed), + self); + priv->dhcp6.prefix_sigid = g_signal_connect (priv->dhcp6.client, + NM_DHCP_CLIENT_SIGNAL_PREFIX_DELEGATED, + G_CALLBACK (dhcp6_prefix_delegated), + self); + if (nm_device_sys_iface_state_is_external_or_assume (self)) priv->dhcp6.was_active = TRUE; - return !!priv->dhcp6.client; + return TRUE; } static gboolean diff --git a/src/dhcp/nm-dhcp-client.c b/src/dhcp/nm-dhcp-client.c index 9fc7d2c1c..16db83068 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 b50ea515b..86d60e387 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 d384da8a4..46d2339bf 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,11 +379,7 @@ 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_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; @@ -383,8 +387,12 @@ dhclient_start (NMDhcpClient *client, _LOGW ("failed to copy leasefile %s to %s: %s", (s_path = g_file_get_path (src)), (d_path = g_file_get_path (dst)), - error->message); - g_clear_error (&error); + 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); } } @@ -393,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; } } @@ -455,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; } @@ -473,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 @@ -511,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; } @@ -534,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 @@ -560,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 92aa7f8ca..de4030206 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 10094e5c2..de04bbda8 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); @@ -153,24 +158,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); + _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, &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); + 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 +190,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 672543237..2fa86f7c9 100644 --- a/src/dhcp/nm-dhcp-manager.c +++ b/src/dhcp/nm-dhcp-manager.c @@ -172,22 +172,22 @@ 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; - 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); 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 +216,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 +259,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 +297,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 +319,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 +335,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 +425,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 7eb32c37d..1d9e5c21d 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 2d0202bb7..b51c6e7b2 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; } diff --git a/src/nm-iface-helper.c b/src/nm-iface-helper.c index 308c9e1f5..805c6e4fd 100644 --- a/src/nm-iface-helper.c +++ b/src/nm-iface-helper.c @@ -380,7 +380,7 @@ int main (int argc, char *argv[]) { char *bad_domains = NULL; - GError *error = NULL; + gs_free_error GError *error = NULL; gboolean wrote_pidfile = FALSE; gs_free char *pidfile = NULL; gs_unref_object NMDhcpClient *dhcp4_client = NULL; @@ -516,8 +516,11 @@ main (int argc, char *argv[]) client_id, NM_DHCP_TIMEOUT_DEFAULT, NULL, - global_opt.dhcp4_address); - g_assert (dhcp4_client); + global_opt.dhcp4_address, + &error); + if (!dhcp4_client) + g_error ("failure to start DHCP: %s", error->message); + g_signal_connect (dhcp4_client, NM_DHCP_CLIENT_SIGNAL_STATE_CHANGED, G_CALLBACK (dhcp4_state_changed),