From 5fe94852ef6869d88dbedfa54c2d79613f6cb4df Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Tue, 25 Feb 2014 13:23:07 -0500 Subject: [PATCH] platform: change sysctl_get/set error logging Remove the "silent_on_error" flag from nm_platform_sysctl_get(), and make both get() and set() log at debug level on ENOENT and error level on all other errors, always. Also ensure that we don't sometimes write "failed to set 'x' to 'y': Success" when a partial write occurs. --- src/platform/nm-fake-platform.c | 6 +++--- src/platform/nm-linux-platform.c | 37 +++++++++++++++++++++----------- src/platform/nm-platform.c | 8 +++---- src/platform/nm-platform.h | 4 ++-- src/platform/tests/platform.c | 2 +- 5 files changed, 33 insertions(+), 24 deletions(-) diff --git a/src/platform/nm-fake-platform.c b/src/platform/nm-fake-platform.c index d0127c4733..cabaa8365d 100644 --- a/src/platform/nm-fake-platform.c +++ b/src/platform/nm-fake-platform.c @@ -73,7 +73,7 @@ sysctl_set (NMPlatform *platform, const char *path, const char *value) } static char * -sysctl_get (NMPlatform *platform, const char *path, gboolean silent_on_error) +sysctl_get (NMPlatform *platform, const char *path) { NMFakePlatformPrivate *priv = NM_FAKE_PLATFORM_GET_PRIVATE (platform); @@ -567,7 +567,7 @@ master_get_option (NMPlatform *platform, int master, const char *option) { auto_g_free char *path = g_strdup_printf ("master:%d:%s", master, option); - return sysctl_get (platform, path, FALSE); + return sysctl_get (platform, path); } static gboolean @@ -583,7 +583,7 @@ slave_get_option (NMPlatform *platform, int slave, const char *option) { auto_g_free char *path = g_strdup_printf ("slave:%d:%s", slave, option); - return sysctl_get (platform, path, FALSE); + return sysctl_get (platform, path); } static gboolean diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c index 94cc81acf7..db734c96e2 100644 --- a/src/platform/nm-linux-platform.c +++ b/src/platform/nm-linux-platform.c @@ -1435,8 +1435,13 @@ sysctl_set (NMPlatform *platform, const char *path, const char *value) fd = open (path, O_WRONLY | O_TRUNC); if (fd == -1) { - error ("sysctl: failed to open '%s': (%d) %s", - path, errno, strerror (errno)); + if (errno == ENOENT) { + debug ("sysctl: failed to open '%s': (%d) %s", + path, errno, strerror (errno)); + } else { + error ("sysctl: failed to open '%s': (%d) %s", + path, errno, strerror (errno)); + } return FALSE; } @@ -1452,19 +1457,21 @@ sysctl_set (NMPlatform *platform, const char *path, const char *value) /* Try to write the entire value three times if a partial write occurs */ len = strlen (actual); for (tries = 0, nwrote = 0; tries < 3 && nwrote != len; tries++) { - errno = 0; nwrote = write (fd, actual, len); if (nwrote == -1) { if (errno == EINTR) { - error ("sysctl: interrupted, will try again"); + debug ("sysctl: interrupted, will try again"); continue; } break; } } - if (nwrote != len && errno != EEXIST) { + if (nwrote == -1 && errno != EEXIST) { error ("sysctl: failed to set '%s' to '%s': (%d) %s", - path, value, errno, strerror (errno)); + path, value, errno, strerror (errno)); + } else if (nwrote < len) { + error ("sysctl: failed to set '%s' to '%s' after three attempts", + path, value); } g_free (actual); @@ -1473,13 +1480,17 @@ sysctl_set (NMPlatform *platform, const char *path, const char *value) } static char * -sysctl_get (NMPlatform *platform, const char *path, gboolean silent_on_error) +sysctl_get (NMPlatform *platform, const char *path) { GError *error = NULL; char *contents; if (!g_file_get_contents (path, &contents, NULL, &error)) { - if (!silent_on_error) + /* We assume FAILED means EOPNOTSUP */ + if ( g_error_matches (error, G_FILE_ERROR, G_FILE_ERROR_NOENT) + || g_error_matches (error, G_FILE_ERROR, G_FILE_ERROR_FAILED)) + debug ("error reading %s: %s", path, error->message); + else error ("error reading %s: %s", path, error->message); g_clear_error (&error); return NULL; @@ -1902,7 +1913,7 @@ link_get_physical_port_id (NMPlatform *platform, int ifindex) return NULL; path = g_strdup_printf ("/sys/class/net/%s/phys_port_id", ifname); - id = sysctl_get (platform, path, TRUE); + id = sysctl_get (platform, path); g_free (path); return id; @@ -2021,7 +2032,7 @@ link_get_option (int master, const char *category, const char *option) { auto_g_free char *path = link_option_path (master, category, option); - return path ? nm_platform_sysctl_get (path, FALSE) : NULL; + return path ? nm_platform_sysctl_get (path) : NULL; } static const char * @@ -2153,7 +2164,7 @@ tun_get_properties (NMPlatform *platform, int ifindex, NMPlatformTunProperties * return FALSE; path = g_strdup_printf ("/sys/class/net/%s/owner", ifname); - val = nm_platform_sysctl_get (path, TRUE); + val = nm_platform_sysctl_get (path); g_free (path); if (val) { props->owner = nm_utils_ascii_str_to_int64 (val, 10, -1, G_MAXINT64, -1); @@ -2164,7 +2175,7 @@ tun_get_properties (NMPlatform *platform, int ifindex, NMPlatformTunProperties * success = FALSE; path = g_strdup_printf ("/sys/class/net/%s/group", ifname); - val = nm_platform_sysctl_get (path, TRUE); + val = nm_platform_sysctl_get (path); g_free (path); if (val) { props->group = nm_utils_ascii_str_to_int64 (val, 10, -1, G_MAXINT64, -1); @@ -2175,7 +2186,7 @@ tun_get_properties (NMPlatform *platform, int ifindex, NMPlatformTunProperties * success = FALSE; path = g_strdup_printf ("/sys/class/net/%s/tun_flags", ifname); - val = nm_platform_sysctl_get (path, TRUE); + val = nm_platform_sysctl_get (path); g_free (path); if (val) { gint64 flags; diff --git a/src/platform/nm-platform.c b/src/platform/nm-platform.c index f9e31d1d6e..9ceab53ce6 100644 --- a/src/platform/nm-platform.c +++ b/src/platform/nm-platform.c @@ -267,20 +267,18 @@ nm_platform_sysctl_set (const char *path, const char *value) /** * nm_platform_sysctl_get: * @path: Absolute path to sysctl - * @silent_on_error: don't log an error message when the value - * could not be read. * * Returns: (transfer full): Contents of the virtual sysctl file. */ char * -nm_platform_sysctl_get (const char *path, gboolean silent_on_error) +nm_platform_sysctl_get (const char *path) { reset_error (); g_return_val_if_fail (path, NULL); g_return_val_if_fail (klass->sysctl_get, NULL); - return klass->sysctl_get (platform, path, silent_on_error); + return klass->sysctl_get (platform, path); } /** @@ -302,7 +300,7 @@ nm_platform_sysctl_get_int32 (const char *path, gint32 fallback) g_return_val_if_fail (path, fallback); if (path) - value = nm_platform_sysctl_get (path, FALSE); + value = nm_platform_sysctl_get (path); if (!value) { errno = EINVAL; diff --git a/src/platform/nm-platform.h b/src/platform/nm-platform.h index bdf82b39fd..88bb5f1bc7 100644 --- a/src/platform/nm-platform.h +++ b/src/platform/nm-platform.h @@ -261,7 +261,7 @@ typedef struct { gboolean (*setup) (NMPlatform *); gboolean (*sysctl_set) (NMPlatform *, const char *path, const char *value); - char * (*sysctl_get) (NMPlatform *, const char *path, gboolean silent_on_error); + char * (*sysctl_get) (NMPlatform *, const char *path); GArray *(*link_get_all) (NMPlatform *); gboolean (*link_add) (NMPlatform *, const char *name, NMLinkType type); @@ -382,7 +382,7 @@ const char *nm_platform_get_error_msg (void); void nm_platform_query_devices (void); gboolean nm_platform_sysctl_set (const char *path, const char *value); -char *nm_platform_sysctl_get (const char *path, gboolean silent_on_error); +char *nm_platform_sysctl_get (const char *path); gint32 nm_platform_sysctl_get_int32 (const char *path, gint32 fallback); GArray *nm_platform_link_get_all (void); diff --git a/src/platform/tests/platform.c b/src/platform/tests/platform.c index 3c636f5393..7ca2cb5258 100644 --- a/src/platform/tests/platform.c +++ b/src/platform/tests/platform.c @@ -47,7 +47,7 @@ do_sysctl_set (char **argv) static gboolean do_sysctl_get (char **argv) { - auto_g_free char *value = nm_platform_sysctl_get (argv[0], FALSE); + auto_g_free char *value = nm_platform_sysctl_get (argv[0]); printf ("%s\n", value);