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.
This commit is contained in:
Dan Winship 2014-02-25 13:23:07 -05:00
parent 0332850627
commit 5fe94852ef
5 changed files with 33 additions and 24 deletions

View file

@ -73,7 +73,7 @@ sysctl_set (NMPlatform *platform, const char *path, const char *value)
} }
static char * 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); 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); 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 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); 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 static gboolean

View file

@ -1435,8 +1435,13 @@ sysctl_set (NMPlatform *platform, const char *path, const char *value)
fd = open (path, O_WRONLY | O_TRUNC); fd = open (path, O_WRONLY | O_TRUNC);
if (fd == -1) { if (fd == -1) {
error ("sysctl: failed to open '%s': (%d) %s", if (errno == ENOENT) {
path, errno, strerror (errno)); 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; 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 */ /* Try to write the entire value three times if a partial write occurs */
len = strlen (actual); len = strlen (actual);
for (tries = 0, nwrote = 0; tries < 3 && nwrote != len; tries++) { for (tries = 0, nwrote = 0; tries < 3 && nwrote != len; tries++) {
errno = 0;
nwrote = write (fd, actual, len); nwrote = write (fd, actual, len);
if (nwrote == -1) { if (nwrote == -1) {
if (errno == EINTR) { if (errno == EINTR) {
error ("sysctl: interrupted, will try again"); debug ("sysctl: interrupted, will try again");
continue; continue;
} }
break; break;
} }
} }
if (nwrote != len && errno != EEXIST) { if (nwrote == -1 && errno != EEXIST) {
error ("sysctl: failed to set '%s' to '%s': (%d) %s", 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); g_free (actual);
@ -1473,13 +1480,17 @@ sysctl_set (NMPlatform *platform, const char *path, const char *value)
} }
static char * static char *
sysctl_get (NMPlatform *platform, const char *path, gboolean silent_on_error) sysctl_get (NMPlatform *platform, const char *path)
{ {
GError *error = NULL; GError *error = NULL;
char *contents; char *contents;
if (!g_file_get_contents (path, &contents, NULL, &error)) { 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); error ("error reading %s: %s", path, error->message);
g_clear_error (&error); g_clear_error (&error);
return NULL; return NULL;
@ -1902,7 +1913,7 @@ link_get_physical_port_id (NMPlatform *platform, int ifindex)
return NULL; return NULL;
path = g_strdup_printf ("/sys/class/net/%s/phys_port_id", ifname); 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); g_free (path);
return id; 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); 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 * static const char *
@ -2153,7 +2164,7 @@ tun_get_properties (NMPlatform *platform, int ifindex, NMPlatformTunProperties *
return FALSE; return FALSE;
path = g_strdup_printf ("/sys/class/net/%s/owner", ifname); 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); g_free (path);
if (val) { if (val) {
props->owner = nm_utils_ascii_str_to_int64 (val, 10, -1, G_MAXINT64, -1); 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; success = FALSE;
path = g_strdup_printf ("/sys/class/net/%s/group", ifname); 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); g_free (path);
if (val) { if (val) {
props->group = nm_utils_ascii_str_to_int64 (val, 10, -1, G_MAXINT64, -1); 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; success = FALSE;
path = g_strdup_printf ("/sys/class/net/%s/tun_flags", ifname); 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); g_free (path);
if (val) { if (val) {
gint64 flags; gint64 flags;

View file

@ -267,20 +267,18 @@ nm_platform_sysctl_set (const char *path, const char *value)
/** /**
* nm_platform_sysctl_get: * nm_platform_sysctl_get:
* @path: Absolute path to sysctl * @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. * Returns: (transfer full): Contents of the virtual sysctl file.
*/ */
char * char *
nm_platform_sysctl_get (const char *path, gboolean silent_on_error) nm_platform_sysctl_get (const char *path)
{ {
reset_error (); reset_error ();
g_return_val_if_fail (path, NULL); g_return_val_if_fail (path, NULL);
g_return_val_if_fail (klass->sysctl_get, 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); g_return_val_if_fail (path, fallback);
if (path) if (path)
value = nm_platform_sysctl_get (path, FALSE); value = nm_platform_sysctl_get (path);
if (!value) { if (!value) {
errno = EINVAL; errno = EINVAL;

View file

@ -261,7 +261,7 @@ typedef struct {
gboolean (*setup) (NMPlatform *); gboolean (*setup) (NMPlatform *);
gboolean (*sysctl_set) (NMPlatform *, const char *path, const char *value); 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 *); GArray *(*link_get_all) (NMPlatform *);
gboolean (*link_add) (NMPlatform *, const char *name, NMLinkType type); 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); void nm_platform_query_devices (void);
gboolean nm_platform_sysctl_set (const char *path, const char *value); 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); gint32 nm_platform_sysctl_get_int32 (const char *path, gint32 fallback);
GArray *nm_platform_link_get_all (void); GArray *nm_platform_link_get_all (void);

View file

@ -47,7 +47,7 @@ do_sysctl_set (char **argv)
static gboolean static gboolean
do_sysctl_get (char **argv) 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); printf ("%s\n", value);