Merge pull request #27411 from yuwata/udev-iocost-follow-ups

udev/iocost: several follow ups

1 files changed, 61 insertions, 71 deletions

diff --git a/src/udev/iocost/iocost.c b/src/udev/iocost/iocost.c
index 54b50b4a8d..fc8223e2b6 100644
--- a/src/udev/iocost/iocost.c
+++ b/src/udev/iocost/iocost.c
@@ -25,7 +25,9 @@ static int parse_config(void) {
         static const ConfigTableItem items[] = {
                 { "IOCost", "TargetSolution", config_parse_string, 0, &arg_target_solution },
         };
-        return config_parse(
+        int r;
+
+        r = config_parse(
                         NULL,
                         "/etc/udev/iocost.conf",
                         NULL,
@@ -35,6 +37,17 @@ static int parse_config(void) {
                         CONFIG_PARSE_WARN,
                         NULL,
                         NULL);
+        if (r < 0)
+                return r;
+
+        if (!arg_target_solution) {
+                arg_target_solution = strdup("naive");
+                if (!arg_target_solution)
+                        return log_oom();
+        }
+
+        log_debug("Target solution: %s", arg_target_solution);
+        return 0;
 }
 
 static int help(void) {
@@ -88,41 +101,37 @@ static int parse_argv(int argc, char *argv[]) {
         return 1;
 }
 
-static int get_known_solutions(sd_device *device, char ***ret_solutions) {
+static int get_known_solutions(sd_device *device, int log_level, char ***ret_solutions, const char **ret_selected) {
         _cleanup_free_ char **s = NULL;
-        const char *value;
+        const char *value, *found;
         int r;
 
         assert(ret_solutions);
+        assert(ret_selected);
 
         r = sd_device_get_property_value(device, "IOCOST_SOLUTIONS", &value);
+        if (r == -ENOENT)
+                return log_device_full_errno(device, log_level, r, "No iocost solution found for device.");
         if (r < 0)
-                return r;
+                return log_device_error_errno(device, r, "Failed to query solutions from device: %m");
 
         s = strv_split(value, WHITESPACE);
         if (!s)
-                return -ENOMEM;
-
-        *ret_solutions = TAKE_PTR(s);
-
-        return 0;
-}
-
-static int choose_solution(char **solutions, const char **ret_name) {
-        assert(ret_name);
-
-        if (strv_isempty(solutions))
-                return log_error_errno(
-                                SYNTHETIC_ERRNO(EINVAL), "IOCOST_SOLUTIONS exists in hwdb but is empty.");
-
-        if (arg_target_solution && strv_find(solutions, arg_target_solution)) {
-                *ret_name = arg_target_solution;
-                log_debug("Selected solution based on target solution: %s", *ret_name);
+                return log_oom();
+        if (strv_isempty(s))
+                return log_device_error_errno(device, SYNTHETIC_ERRNO(EINVAL),
+                                              "IOCOST_SOLUTIONS exists in hwdb but is empty.");
+
+        found = strv_find(s, arg_target_solution);
+        if (found) {
+                *ret_selected = found;
+                log_device_debug(device, "Selected solution based on target solution: %s", *ret_selected);
         } else {
-                *ret_name = solutions[0];
-                log_debug("Selected first available solution: %s", *ret_name);
+                *ret_selected = s[0];
+                log_device_debug(device, "Selected first available solution: %s", *ret_selected);
         }
 
+        *ret_solutions = TAKE_PTR(s);
         return 0;
 }
 
@@ -132,29 +141,14 @@ static int query_named_solution(
                 const char **ret_model,
                 const char **ret_qos) {
 
-        _cleanup_strv_free_ char **solutions = NULL;
         _cleanup_free_ char *upper_name = NULL, *qos_key = NULL, *model_key = NULL;
-        const char *qos = NULL, *model = NULL;
+        const char *qos, *model;
         int r;
 
+        assert(name);
         assert(ret_qos);
         assert(ret_model);
 
-        /* If NULL is passed we query the default solution, which is the first one listed
-         * in the IOCOST_SOLUTIONS key or the one specified by the TargetSolution setting.
-         */
-        if (!name) {
-                r = get_known_solutions(device, &solutions);
-                if (r == -ENOENT)
-                        return log_device_debug_errno(device, r, "No entry found for device, skipping iocost logic.");
-                if (r < 0)
-                        return log_device_error_errno(device, r, "Failed to query solutions from device: %m");
-
-                r = choose_solution(solutions, &name);
-                if (r < 0)
-                        return r;
-        }
-
         upper_name = strdup(name);
         if (!upper_name)
                 return log_oom();
@@ -174,7 +168,7 @@ static int query_named_solution(
         if (r == -ENOENT)
                 return log_device_debug_errno(device, r, "No value found for key %s, skipping iocost logic.", qos_key);
         if (r < 0)
-                return log_device_error_errno(device, r, "Failed to obtain model for iocost solution from device: %m");
+                return log_device_error_errno(device, r, "Failed to obtain QoS for iocost solution from device: %m");
 
         r = sd_device_get_property_value(device, model_key, &model);
         if (r == -ENOENT)
@@ -190,8 +184,9 @@ static int query_named_solution(
 
 static int apply_solution_for_path(const char *path, const char *name) {
         _cleanup_(sd_device_unrefp) sd_device *device = NULL;
+        _cleanup_strv_free_ char **solutions = NULL;
         _cleanup_free_ char *qos = NULL, *model = NULL;
-        const char *qos_params = NULL, *model_params = NULL;
+        const char *qos_params, *model_params;
         dev_t devnum;
         int r;
 
@@ -199,16 +194,24 @@ static int apply_solution_for_path(const char *path, const char *name) {
         if (r < 0)
                 return log_error_errno(r, "Error looking up device: %m");
 
+        r = sd_device_get_devnum(device, &devnum);
+        if (r < 0)
+                return log_device_error_errno(device, r, "Error getting devnum: %m");
+
+        if (!name) {
+                r = get_known_solutions(device, LOG_DEBUG, &solutions, &name);
+                if (r == -ENOENT)
+                        return 0;
+                if (r < 0)
+                        return r;
+        }
+
         r = query_named_solution(device, name, &model_params, &qos_params);
         if (r == -ENOENT)
                 return 0;
         if (r < 0)
                 return r;
 
-        r = sd_device_get_devnum(device, &devnum);
-        if (r < 0)
-                return log_device_error_errno(device, r, "Error getting devnum: %m");
-
         if (asprintf(&qos, DEVNUM_FORMAT_STR " enable=1 ctrl=user %s", DEVNUM_FORMAT_VAL(devnum), qos_params) < 0)
                 return log_oom();
 
@@ -216,8 +219,9 @@ static int apply_solution_for_path(const char *path, const char *name) {
                 return log_oom();
 
         log_debug("Applying iocost parameters to %s using solution '%s'\n"
-                        "\tio.cost.qos: %s\n"
-                        "\tio.cost.model: %s\n", path, name ?: "default", qos, model);
+                  "\tio.cost.qos: %s\n"
+                  "\tio.cost.model: %s\n",
+                  path, name, qos, model);
 
         r = cg_set_attribute("io", NULL, "io.cost.qos", qos);
         if (r < 0) {
@@ -237,8 +241,7 @@ static int apply_solution_for_path(const char *path, const char *name) {
 static int query_solutions_for_path(const char *path) {
         _cleanup_(sd_device_unrefp) sd_device *device = NULL;
         _cleanup_strv_free_ char **solutions = NULL;
-        const char *default_solution = NULL;
-        const char *model_name = NULL;
+        const char *selected_solution, *model_name;
         int r;
 
         r = sd_device_new_from_path(&device, path);
@@ -257,15 +260,9 @@ static int query_solutions_for_path(const char *path) {
         if (r < 0)
                 return log_device_error_errno(device, r, "Model name for device %s is unknown", path);
 
-        r = get_known_solutions(device, &solutions);
-        if (r == -ENOENT) {
-                log_device_info(device, "Attribute IOCOST_SOLUTIONS missing, model not found in hwdb.");
+        r = get_known_solutions(device, LOG_INFO, &solutions, &selected_solution);
+        if (r == -ENOENT)
                 return 0;
-        }
-        if (r < 0)
-                return log_device_error_errno(device, r, "Couldn't access IOCOST_SOLUTIONS for device %s, model name %s on hwdb: %m\n", path, model_name);
-
-        r = choose_solution(solutions, &default_solution);
         if (r < 0)
                 return r;
 
@@ -273,13 +270,12 @@ static int query_solutions_for_path(const char *path) {
                  "Preferred solution: %s\n"
                  "Solution that would be applied: %s",
                  path, model_name,
-                 arg_target_solution, default_solution);
+                 arg_target_solution, selected_solution);
 
         STRV_FOREACH(s, solutions) {
-                const char *model = NULL, *qos = NULL;
+                const char *model, *qos;
 
-                r = query_named_solution(device, *s, &model, &qos);
-                if (r < 0 || !model || !qos)
+                if (query_named_solution(device, *s, &model, &qos) < 0)
                         continue;
 
                 log_info("%s: io.cost.qos: %s\n"
@@ -318,15 +314,9 @@ static int run(int argc, char *argv[]) {
         if (r <= 0)
                 return r;
 
-        (void) parse_config();
-
-        if (!arg_target_solution) {
-                arg_target_solution = strdup("naive");
-                if (!arg_target_solution)
-                        return log_oom();
-        }
-
-        log_debug("Target solution: %s.", arg_target_solution);
+        r = parse_config();
+        if (r < 0)
+                return r;
 
         return iocost_main(argc, argv);
 }