From: Adrian Vovk Date: Thu, 19 Sep 2024 15:44:42 +0000 (-0400) Subject: sysupdated: Rearrange error logging a little bit X-Git-Tag: v257-rc1~311^2~1 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=5faf65d9cf7ff94970a18589aefd550c01a651d1;p=thirdparty%2Fsystemd.git sysupdated: Rearrange error logging a little bit First, this fixes a case where an error is logged twice at the LOG_ERR level. Second, this goes through and raises the log level where appropriate so that sysupdated is easier to debug. Finally, it replaces EINVAL with EPROTO where appropriate, since EINVAL implies that the caller passed some incorrect arguments (which is incorrect; sysupdate passed some invalid JSON back to us) --- diff --git a/src/sysupdate/sysupdated.c b/src/sysupdate/sysupdated.c index 546f52a96ce..542e8ec7b1d 100644 --- a/src/sysupdate/sysupdated.c +++ b/src/sysupdate/sysupdated.c @@ -816,7 +816,7 @@ static int sysupdate_run_simple(sd_json_variant **ret, Target *t, ...) { r = sd_json_parse_file(f, "stdout", 0, &v, NULL, NULL); if (r < 0) - return log_error_errno(r, "Failed to parse JSON: %m"); + return log_debug_errno(r, "Failed to parse JSON: %m"); *ret = TAKE_PTR(v); return 0; @@ -824,8 +824,12 @@ static int sysupdate_run_simple(sd_json_variant **ret, Target *t, ...) { static BUS_DEFINE_PROPERTY_GET_ENUM(target_property_get_class, target_class, TargetClass); -#define log_sysupdate_bad_json(verb, msg) \ - log_debug("Invalid JSON response from 'systemd-sysupdate %s': %s", verb, msg) +#define log_sysupdate_bad_json_full(lvl, r, verb, msg) \ + log_full_errno((lvl), (r), "Invalid JSON response from 'systemd-sysupdate %s': %s", (verb), (msg)) +#define log_sysupdate_bad_json(r, verb, msg) \ + log_sysupdate_bad_json_full(LOG_ERR, (r), (verb), (msg)) +#define log_sysupdate_bad_json_debug(r, verb, msg) \ + log_sysupdate_bad_json_full(LOG_DEBUG, (r), (verb), (msg)) static int target_method_list_finish( sd_bus_message *msg, @@ -841,14 +845,12 @@ static int target_method_list_finish( assert(json); v = sd_json_variant_by_key(json, "all"); - if (!v) { - log_sysupdate_bad_json("list", "Missing key 'all'"); - return -EINVAL; - } + if (!v) + return log_sysupdate_bad_json(SYNTHETIC_ERRNO(EPROTO), "list", "Missing key 'all'"); r = sd_json_variant_strv(v, &versions); if (r < 0) - return r; + return log_sysupdate_bad_json(SYNTHETIC_ERRNO(EPROTO), "list", "Key 'all' should be strv"); r = sd_bus_message_new_method_return(msg, &reply); if (r < 0) @@ -991,10 +993,8 @@ static int target_method_check_new_finish( assert(json); sd_json_variant *v = sd_json_variant_by_key(json, "available"); - if (!v) { - log_sysupdate_bad_json("check-new", "Missing key 'available'"); - return -EINVAL; - } + if (!v) + return log_sysupdate_bad_json(SYNTHETIC_ERRNO(EPROTO), "check-new", "Missing key 'available'"); if (sd_json_variant_is_null(v)) reply = ""; @@ -1131,11 +1131,15 @@ static int target_method_vacuum_finish( sd_json_variant *json, sd_bus_error *error) { + sd_json_variant *v; uint64_t instances; assert(json); - instances = sd_json_variant_unsigned(sd_json_variant_by_key(json, "removed")); + v = sd_json_variant_by_key(json, "removed"); + if (!v) + return log_sysupdate_bad_json(SYNTHETIC_ERRNO(EPROTO), "vacuum", "Missing key 'removed'"); + instances = sd_json_variant_unsigned(v); return sd_bus_reply_method_return(msg, "u", instances); } @@ -1184,21 +1188,17 @@ static int target_method_get_version(sd_bus_message *msg, void *userdata, sd_bus r = sysupdate_run_simple(&v, t, "--offline", "list", NULL); if (r < 0) - return r; + return log_error_errno(r, "Failed to run 'systemd-sysupdate list': %m"); version_json = sd_json_variant_by_key(v, "current"); - if (!version_json) { - log_sysupdate_bad_json("list", "Missing key 'current'"); - return -EINVAL; - } + if (!version_json) + return log_sysupdate_bad_json(SYNTHETIC_ERRNO(EPROTO), "list", "Missing key 'current'"); if (sd_json_variant_is_null(version_json)) return sd_bus_reply_method_return(msg, "s", ""); - if (!sd_json_variant_is_string(version_json)) { - log_sysupdate_bad_json("list", "Expected string value for key 'current'"); - return -EINVAL; - } + if (!sd_json_variant_is_string(version_json)) + return log_sysupdate_bad_json(SYNTHETIC_ERRNO(EPROTO), "list", "Key 'current' should be a string"); return sd_bus_reply_method_return(msg, "s", sd_json_variant_string(version_json)); } @@ -1210,19 +1210,15 @@ static int target_get_appstream(Target *t, char ***ret) { r = sysupdate_run_simple(&v, t, "--offline", "list", NULL); if (r < 0) - return r; + return log_error_errno(r, "Failed to run 'systemd-sysupdate list': %m"); appstream_url_json = sd_json_variant_by_key(v, "appstream_urls"); - if (!appstream_url_json) { - log_sysupdate_bad_json("list", "Missing key 'appstream_urls'"); - return -EINVAL; - } + if (!appstream_url_json) + return log_sysupdate_bad_json(SYNTHETIC_ERRNO(EPROTO), "list", "Missing key 'appstream_urls'"); r = sd_json_variant_strv(appstream_url_json, ret); - if (r < 0) { - log_sysupdate_bad_json("list", "Expected array of strings for key 'appstream_urls'"); - return r; - } + if (r < 0) + return log_sysupdate_bad_json(SYNTHETIC_ERRNO(EPROTO), "list", "Key 'appstream_urls' should be strv"); return 0; } @@ -1257,19 +1253,19 @@ static int target_list_components(Target *t, char ***ret_components, bool *ret_h r = sysupdate_run_simple(&json, t, "components", NULL); if (r < 0) - return r; + return log_debug_errno(r, "Failed to run 'systemd-sysupdate components': %m"); v = sd_json_variant_by_key(json, "default"); if (!v) - return -EINVAL; + return log_sysupdate_bad_json_debug(SYNTHETIC_ERRNO(EPROTO), "components", "Missing key 'default'"); have_default = sd_json_variant_boolean(v); v = sd_json_variant_by_key(json, "components"); if (!v) - return -EINVAL; + return log_sysupdate_bad_json_debug(SYNTHETIC_ERRNO(EPROTO), "components", "Missing key 'components'"); r = sd_json_variant_strv(v, &components); if (r < 0) - return r; + return log_sysupdate_bad_json_debug(SYNTHETIC_ERRNO(EPROTO), "components", "Key 'components' should be a strv"); if (ret_components) *ret_components = TAKE_PTR(components); @@ -1670,7 +1666,11 @@ static int manager_enumerate_targets(Manager *m) { target_class_to_string(*class)); } - return manager_enumerate_components(m); + r = manager_enumerate_components(m); + if (r < 0) + log_warning_errno(r, "Failed to enumerate components, ignoring: %m"); + + return 0; } static int manager_ensure_targets(Manager *m) {