diff options
-rw-r--r-- | src/core/dbus-manager.c | 119 | ||||
-rw-r--r-- | src/shared/bus-util.c | 21 | ||||
-rw-r--r-- | src/shared/install.c | 120 | ||||
-rw-r--r-- | src/shared/install.h | 7 | ||||
-rw-r--r-- | src/systemctl/systemctl.c | 83 |
5 files changed, 205 insertions, 145 deletions
diff --git a/src/core/dbus-manager.c b/src/core/dbus-manager.c index ddfde23028..d48b0ca69d 100644 --- a/src/core/dbus-manager.c +++ b/src/core/dbus-manager.c @@ -1585,15 +1585,19 @@ static int reply_unit_file_changes_and_free( if (r < 0) goto fail; - for (i = 0; i < n_changes; i++) { - r = sd_bus_message_append( - reply, "(sss)", - unit_file_change_type_to_string(changes[i].type), - changes[i].path, - changes[i].source); - if (r < 0) - goto fail; - } + for (i = 0; i < n_changes; i++) + if (changes[i].type >= 0) { + const char *change = unit_file_change_type_to_string(changes[i].type); + assert(change != NULL); + + r = sd_bus_message_append( + reply, "(sss)", + change, + changes[i].path, + changes[i].source); + if (r < 0) + goto fail; + } r = sd_bus_message_close_container(reply); if (r < 0) @@ -1607,17 +1611,56 @@ fail: return r; } -static int install_error(sd_bus_error *error, int c) { +/* Create an error reply, using the error information from changes[] + * if possible, and fall back to generating an error from error code c. + * The error message only describes the first error. + * + * Coordinate with unit_file_dump_changes() in install.c. + */ +static int install_error( + sd_bus_error *error, + int c, + UnitFileChange *changes, + unsigned n_changes) { + int r; + unsigned i; assert(c < 0); - if (c == -ERFKILL) - return sd_bus_error_setf(error, BUS_ERROR_UNIT_MASKED, "Unit file is masked."); - if (c == -EADDRNOTAVAIL) - return sd_bus_error_setf(error, BUS_ERROR_UNIT_GENERATED, "Unit file is transient or generated."); - if (c == -ELOOP) - return sd_bus_error_setf(error, BUS_ERROR_UNIT_LINKED, "Refusing to operate on linked unit file."); + for (i = 0; i < n_changes; i++) + switch(changes[i].type) { + case 0 ... INT_MAX: + continue; + case -EEXIST: + if (changes[i].source) + r = sd_bus_error_setf(error, BUS_ERROR_UNIT_EXISTS, + "File %s already exists and is a symlink to %s.", + changes[i].path, changes[i].source); + else + r = sd_bus_error_setf(error, BUS_ERROR_UNIT_EXISTS, + "File %s already exists.", + changes[i].path); + goto found; + case -ERFKILL: + r = sd_bus_error_setf(error, BUS_ERROR_UNIT_MASKED, + "Unit file %s is masked.", changes[i].path); + goto found; + case -EADDRNOTAVAIL: + r = sd_bus_error_setf(error, BUS_ERROR_UNIT_GENERATED, + "Unit %s is transient or generated.", changes[i].path); + goto found; + case -ELOOP: + r = sd_bus_error_setf(error, BUS_ERROR_UNIT_LINKED, + "Refusing to operate on linked unit file %s", changes[i].path); + goto found; + default: + r = sd_bus_error_set_errnof(error, changes[i].type, "File %s: %m", changes[i].path); + goto found; + } - return c; + r = c; + found: + unit_file_changes_free(changes, n_changes); + return r; } static int method_enable_unit_files_generic( @@ -1651,10 +1694,8 @@ static int method_enable_unit_files_generic( return 1; /* No authorization for now, but the async polkit stuff will call us again when it has it */ r = call(m->unit_file_scope, runtime, NULL, l, force, &changes, &n_changes); - if (r < 0) { - unit_file_changes_free(changes, n_changes); - return install_error(error, r); - } + if (r < 0) + return install_error(error, r, changes, n_changes); return reply_unit_file_changes_and_free(m, message, carries_install_info ? r : -1, changes, n_changes); } @@ -1719,10 +1760,8 @@ static int method_preset_unit_files_with_mode(sd_bus_message *message, void *use return 1; /* No authorization for now, but the async polkit stuff will call us again when it has it */ r = unit_file_preset(m->unit_file_scope, runtime, NULL, l, mm, force, &changes, &n_changes); - if (r < 0) { - unit_file_changes_free(changes, n_changes); - return install_error(error, r); - } + if (r < 0) + return install_error(error, r, changes, n_changes); return reply_unit_file_changes_and_free(m, message, r, changes, n_changes); } @@ -1757,10 +1796,8 @@ static int method_disable_unit_files_generic( return 1; /* No authorization for now, but the async polkit stuff will call us again when it has it */ r = call(m->unit_file_scope, runtime, NULL, l, &changes, &n_changes); - if (r < 0) { - unit_file_changes_free(changes, n_changes); - return install_error(error, r); - } + if (r < 0) + return install_error(error, r, changes, n_changes); return reply_unit_file_changes_and_free(m, message, -1, changes, n_changes); } @@ -1794,10 +1831,8 @@ static int method_revert_unit_files(sd_bus_message *message, void *userdata, sd_ return 1; /* No authorization for now, but the async polkit stuff will call us again when it has it */ r = unit_file_revert(m->unit_file_scope, NULL, l, &changes, &n_changes); - if (r < 0) { - unit_file_changes_free(changes, n_changes); - return install_error(error, r); - } + if (r < 0) + return install_error(error, r, changes, n_changes); return reply_unit_file_changes_and_free(m, message, -1, changes, n_changes); } @@ -1827,10 +1862,8 @@ static int method_set_default_target(sd_bus_message *message, void *userdata, sd return 1; /* No authorization for now, but the async polkit stuff will call us again when it has it */ r = unit_file_set_default(m->unit_file_scope, NULL, name, force, &changes, &n_changes); - if (r < 0) { - unit_file_changes_free(changes, n_changes); - return install_error(error, r); - } + if (r < 0) + return install_error(error, r, changes, n_changes); return reply_unit_file_changes_and_free(m, message, -1, changes, n_changes); } @@ -1869,10 +1902,8 @@ static int method_preset_all_unit_files(sd_bus_message *message, void *userdata, return 1; /* No authorization for now, but the async polkit stuff will call us again when it has it */ r = unit_file_preset_all(m->unit_file_scope, runtime, NULL, mm, force, &changes, &n_changes); - if (r < 0) { - unit_file_changes_free(changes, n_changes); - return install_error(error, r); - } + if (r < 0) + return install_error(error, r, changes, n_changes); return reply_unit_file_changes_and_free(m, message, -1, changes, n_changes); } @@ -1908,10 +1939,8 @@ static int method_add_dependency_unit_files(sd_bus_message *message, void *userd return -EINVAL; r = unit_file_add_dependency(m->unit_file_scope, runtime, NULL, l, target, dep, force, &changes, &n_changes); - if (r < 0) { - unit_file_changes_free(changes, n_changes); - return install_error(error, r); - } + if (r < 0) + return install_error(error, r, changes, n_changes); return reply_unit_file_changes_and_free(m, message, -1, changes, n_changes); } diff --git a/src/shared/bus-util.c b/src/shared/bus-util.c index 677970b7f0..6a1877d8aa 100644 --- a/src/shared/bus-util.c +++ b/src/shared/bus-util.c @@ -2200,20 +2200,16 @@ int bus_deserialize_and_dump_unit_file_changes(sd_bus_message *m, bool quiet, Un return bus_log_parse_error(r); while ((r = sd_bus_message_read(m, "(sss)", &type, &path, &source)) > 0) { - if (!quiet) { - if (streq(type, "symlink")) - log_info("Created symlink from %s to %s.", path, source); - else if (streq(type, "unlink")) - log_info("Removed symlink %s.", path); - else if (streq(type, "masked")) - log_info("Unit %s is masked, ignoring.", path); - else - log_notice("Manager reported unknown change type \"%s\" for %s.", type, path); + /* We expect only "success" changes to be sent over the bus. + Hence, reject anything negative. */ + UnitFileChangeType ch = unit_file_change_type_from_string(type); + + if (ch < 0) { + log_notice("Manager reported unknown change type \"%s\" for path \"%s\", ignoring.", type, path); + continue; } - r = unit_file_changes_add(changes, n_changes, - unit_file_change_type_from_string(type), - path, source); + r = unit_file_changes_add(changes, n_changes, ch, path, source); if (r < 0) return r; } @@ -2224,6 +2220,7 @@ int bus_deserialize_and_dump_unit_file_changes(sd_bus_message *m, bool quiet, Un if (r < 0) return bus_log_parse_error(r); + unit_file_dump_changes(0, NULL, *changes, *n_changes, false); return 0; } diff --git a/src/shared/install.c b/src/shared/install.c index 1522435f7f..febe33ed7b 100644 --- a/src/shared/install.c +++ b/src/shared/install.c @@ -276,6 +276,70 @@ void unit_file_changes_free(UnitFileChange *changes, unsigned n_changes) { free(changes); } +void unit_file_dump_changes(int r, const char *verb, const UnitFileChange *changes, unsigned n_changes, bool quiet) { + unsigned i; + bool logged = false; + + assert(changes || n_changes == 0); + /* If verb is not specified, errors are not allowed! */ + assert(verb || r >= 0); + + for (i = 0; i < n_changes; i++) { + assert(verb || changes[i].type >= 0); + + switch(changes[i].type) { + case UNIT_FILE_SYMLINK: + if (!quiet) + log_info("Created symlink %s, pointing to %s.", changes[i].path, changes[i].source); + break; + case UNIT_FILE_UNLINK: + if (!quiet) + log_info("Removed %s.", changes[i].path); + break; + case UNIT_FILE_IS_MASKED: + if (!quiet) + log_info("Unit %s is masked, ignoring.", changes[i].path); + break; + case -EEXIST: + if (changes[i].source) + log_error_errno(changes[i].type, + "Failed to %s unit, file %s already exists and is a symlink to %s.", + verb, changes[i].path, changes[i].source); + else + log_error_errno(changes[i].type, + "Failed to %s unit, file %s already exists.", + verb, changes[i].path); + logged = true; + break; + case -ERFKILL: + log_error_errno(changes[i].type, "Failed to %s unit, unit %s is masked.", + verb, changes[i].path); + logged = true; + break; + case -EADDRNOTAVAIL: + log_error_errno(changes[i].type, "Failed to %s unit, unit %s is transient or generated.", + verb, changes[i].path); + logged = true; + break; + case -ELOOP: + log_error_errno(changes[i].type, "Failed to %s unit, refusing to operate on linked unit file %s", + verb, changes[i].path); + logged = true; + break; + default: + assert(changes[i].type < 0); + log_error_errno(changes[i].type, "Failed to %s unit, file %s: %m.", + verb, changes[i].path); + logged = true; + } + } + + if (r < 0 && !logged) + log_error_errno(r, "Failed to %s: %m.", verb); +} + + + static int create_symlink( const char *old_path, const char *new_path, @@ -300,8 +364,10 @@ static int create_symlink( return 1; } - if (errno != EEXIST) + if (errno != EEXIST) { + unit_file_changes_add(changes, n_changes, -errno, new_path, NULL); return -errno; + } r = readlink_malloc(new_path, &dest); if (r < 0) @@ -310,8 +376,10 @@ static int create_symlink( if (path_equal(dest, old_path)) return 0; - if (!force) + if (!force) { + unit_file_changes_add(changes, n_changes, -EEXIST, new_path, dest); return -EEXIST; + } r = symlink_atomic(old_path, new_path); if (r < 0) @@ -421,6 +489,7 @@ static int remove_marked_symlinks_fd( p = path_make_absolute(de->d_name, path); if (!p) return -ENOMEM; + path_kill_slashes(p); q = readlink_malloc(p, &dest); if (q == -ENOENT) @@ -444,10 +513,10 @@ static int remove_marked_symlinks_fd( if (unlinkat(fd, de->d_name, 0) < 0 && errno != ENOENT) { if (r == 0) r = -errno; + unit_file_changes_add(changes, n_changes, -errno, p, NULL); continue; } - path_kill_slashes(p); (void) rmdir_parents(p, config_path); unit_file_changes_add(changes, n_changes, UNIT_FILE_UNLINK, p, NULL); @@ -745,19 +814,26 @@ static UnitFileInstallInfo *install_info_find(InstallContext *c, const char *nam return ordered_hashmap_get(c->will_process, name); } -static int install_info_may_process(UnitFileInstallInfo *i, const LookupPaths *paths) { +static int install_info_may_process( + UnitFileInstallInfo *i, + const LookupPaths *paths, + UnitFileChange **changes, + unsigned *n_changes) { assert(i); assert(paths); /* Checks whether the loaded unit file is one we should process, or is masked, transient or generated and thus * not subject to enable/disable operations. */ - if (i->type == UNIT_FILE_TYPE_MASKED) + if (i->type == UNIT_FILE_TYPE_MASKED) { + unit_file_changes_add(changes, n_changes, -ERFKILL, i->path, NULL); return -ERFKILL; - if (path_is_generator(paths, i->path)) - return -EADDRNOTAVAIL; - if (path_is_transient(paths, i->path)) + } + if (path_is_generator(paths, i->path) || + path_is_transient(paths, i->path)) { + unit_file_changes_add(changes, n_changes, -EADDRNOTAVAIL, i->path, NULL); return -EADDRNOTAVAIL; + } return 0; } @@ -1637,8 +1713,11 @@ int unit_file_unmask( return -ENOMEM; if (unlink(path) < 0) { - if (errno != ENOENT && r >= 0) - r = -errno; + if (errno != ENOENT) { + if (r >= 0) + r = -errno; + unit_file_changes_add(changes, n_changes, -errno, path, NULL); + } continue; } @@ -1953,7 +2032,7 @@ int unit_file_add_dependency( r = install_info_discover(scope, &c, &paths, target, SEARCH_FOLLOW_CONFIG_SYMLINKS, &target_info); if (r < 0) return r; - r = install_info_may_process(target_info, &paths); + r = install_info_may_process(target_info, &paths, changes, n_changes); if (r < 0) return r; @@ -1965,7 +2044,7 @@ int unit_file_add_dependency( r = install_info_discover(scope, &c, &paths, *f, SEARCH_FOLLOW_CONFIG_SYMLINKS, &i); if (r < 0) return r; - r = install_info_may_process(i, &paths); + r = install_info_may_process(i, &paths, changes, n_changes); if (r < 0) return r; @@ -2018,7 +2097,7 @@ int unit_file_enable( r = install_info_discover(scope, &c, &paths, *f, SEARCH_LOAD, &i); if (r < 0) return r; - r = install_info_may_process(i, &paths); + r = install_info_may_process(i, &paths, changes, n_changes); if (r < 0) return r; @@ -2131,7 +2210,7 @@ int unit_file_set_default( r = install_info_discover(scope, &c, &paths, name, 0, &i); if (r < 0) return r; - r = install_info_may_process(i, &paths); + r = install_info_may_process(i, &paths, changes, n_changes); if (r < 0) return r; @@ -2163,7 +2242,7 @@ int unit_file_get_default( r = install_info_discover(scope, &c, &paths, SPECIAL_DEFAULT_TARGET, SEARCH_FOLLOW_CONFIG_SYMLINKS, &i); if (r < 0) return r; - r = install_info_may_process(i, &paths); + r = install_info_may_process(i, &paths, NULL, 0); if (r < 0) return r; @@ -2425,7 +2504,9 @@ static int preset_prepare_one( InstallContext *minus, LookupPaths *paths, UnitFilePresetMode mode, - const char *name) { + const char *name, + UnitFileChange **changes, + unsigned *n_changes) { UnitFileInstallInfo *i; int r; @@ -2443,7 +2524,7 @@ static int preset_prepare_one( if (r < 0) return r; - r = install_info_may_process(i, paths); + r = install_info_may_process(i, paths, changes, n_changes); if (r < 0) return r; } else @@ -2482,7 +2563,7 @@ int unit_file_preset( if (!unit_name_is_valid(*i, UNIT_NAME_ANY)) return -EINVAL; - r = preset_prepare_one(scope, &plus, &minus, &paths, mode, *i); + r = preset_prepare_one(scope, &plus, &minus, &paths, mode, *i, changes, n_changes); if (r < 0) return r; } @@ -2537,7 +2618,8 @@ int unit_file_preset_all( if (!IN_SET(de->d_type, DT_LNK, DT_REG)) continue; - r = preset_prepare_one(scope, &plus, &minus, &paths, mode, de->d_name); + /* we don't pass changes[] in, because we want to handle errors on our own */ + r = preset_prepare_one(scope, &plus, &minus, &paths, mode, de->d_name, NULL, 0); if (r == -ERFKILL) r = unit_file_changes_add(changes, n_changes, UNIT_FILE_IS_MASKED, de->d_name, NULL); diff --git a/src/shared/install.h b/src/shared/install.h index 219b48f428..82c62095d5 100644 --- a/src/shared/install.h +++ b/src/shared/install.h @@ -77,8 +77,12 @@ enum UnitFileChangeType { _UNIT_FILE_CHANGE_TYPE_INVALID = -1 }; +/* type can either one of the UnitFileChangeTypes listed above, or a negative error. + * If source is specified, it should be the contents of the path symlink. + * In case of an error, source should be the existing symlink contents or NULL + */ struct UnitFileChange { - UnitFileChangeType type; + int type; /* UnitFileChangeType or bust */ char *path; char *source; }; @@ -233,6 +237,7 @@ Hashmap* unit_file_list_free(Hashmap *h); int unit_file_changes_add(UnitFileChange **changes, unsigned *n_changes, UnitFileChangeType type, const char *path, const char *source); void unit_file_changes_free(UnitFileChange *changes, unsigned n_changes); +void unit_file_dump_changes(int r, const char *verb, const UnitFileChange *changes, unsigned n_changes, bool quiet); int unit_file_query_preset(UnitFileScope scope, const char *root_dir, const char *name); diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index 5f87e4a97f..cfdc851a6d 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -1984,27 +1984,6 @@ static int get_default(int argc, char *argv[], void *userdata) { return 0; } -static void dump_unit_file_changes(const UnitFileChange *changes, unsigned n_changes) { - unsigned i; - - assert(changes || n_changes == 0); - - for (i = 0; i < n_changes; i++) - switch(changes[i].type) { - case UNIT_FILE_SYMLINK: - log_info("Created symlink %s, pointing to %s.", changes[i].path, changes[i].source); - break; - case UNIT_FILE_UNLINK: - log_info("Removed %s.", changes[i].path); - break; - case UNIT_FILE_IS_MASKED: - log_info("Unit %s is masked, ignoring.", changes[i].path); - break; - default: - assert_not_reached("bad change type"); - } -} - static int set_default(int argc, char *argv[], void *userdata) { _cleanup_free_ char *unit = NULL; int r; @@ -2021,14 +2000,9 @@ static int set_default(int argc, char *argv[], void *userdata) { unsigned n_changes = 0; r = unit_file_set_default(arg_scope, arg_root, unit, true, &changes, &n_changes); - if (r < 0) - return log_error_errno(r, "Failed to set default target: %m"); - - if (!arg_quiet) - dump_unit_file_changes(changes, n_changes); - + unit_file_dump_changes(r, "set default", changes, n_changes, arg_quiet); unit_file_changes_free(changes, n_changes); - r = 0; + return r; } else { _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; @@ -3117,7 +3091,7 @@ static int set_exit_code(uint8_t code) { NULL, "y", code); if (r < 0) - return log_error_errno(r, "Failed to execute operation: %s", bus_error_message(&error, r)); + return log_error_errno(r, "Failed to set exit code: %s", bus_error_message(&error, r)); return 0; } @@ -4967,7 +4941,7 @@ static int daemon_reload(int argc, char *argv[], void *userdata) { * reply */ r = 0; else if (r < 0) - return log_error_errno(r, "Failed to execute operation: %s", bus_error_message(&error, r)); + return log_error_errno(r, "Failed to reload daemon: %s", bus_error_message(&error, r)); return r < 0 ? r : 0; } @@ -5450,18 +5424,9 @@ static int enable_unit(int argc, char *argv[], void *userdata) { else assert_not_reached("Unknown verb"); - if (r == -ERFKILL) - return log_error_errno(r, "Unit file is masked."); - if (r == -EADDRNOTAVAIL) - return log_error_errno(r, "Unit file is transient or generated."); - if (r == -ELOOP) - return log_error_errno(r, "Refusing to operate on linked unit file."); + unit_file_dump_changes(r, verb, changes, n_changes, arg_quiet); if (r < 0) - return log_error_errno(r, "Operation failed: %m"); - - if (!arg_quiet) - dump_unit_file_changes(changes, n_changes); - + return r; r = 0; } else { _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL, *m = NULL; @@ -5542,7 +5507,7 @@ static int enable_unit(int argc, char *argv[], void *userdata) { r = sd_bus_call(bus, m, 0, &error, &reply); if (r < 0) - return log_error_errno(r, "Failed to execute operation: %s", bus_error_message(&error, r)); + return log_error_errno(r, "Failed to %s unit: %s", verb, bus_error_message(&error, r)); if (expect_carries_install_info) { r = sd_bus_message_read(reply, "b", &carries_install_info); @@ -5625,18 +5590,9 @@ static int add_dependency(int argc, char *argv[], void *userdata) { unsigned n_changes = 0; r = unit_file_add_dependency(arg_scope, arg_runtime, arg_root, names, target, dep, arg_force, &changes, &n_changes); - if (r == -ERFKILL) - return log_error_errno(r, "Unit file is masked."); - if (r == -EADDRNOTAVAIL) - return log_error_errno(r, "Unit file is transient or generated."); - if (r < 0) - return log_error_errno(r, "Can't add dependency: %m"); - - if (!arg_quiet) - dump_unit_file_changes(changes, n_changes); - + unit_file_dump_changes(r, "add dependency on", changes, n_changes, arg_quiet); unit_file_changes_free(changes, n_changes); - + return r; } else { _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL, *m = NULL; _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; @@ -5668,19 +5624,16 @@ static int add_dependency(int argc, char *argv[], void *userdata) { r = sd_bus_call(bus, m, 0, &error, &reply); if (r < 0) - return log_error_errno(r, "Failed to execute operation: %s", bus_error_message(&error, r)); + return log_error_errno(r, "Failed to add dependency: %s", bus_error_message(&error, r)); r = bus_deserialize_and_dump_unit_file_changes(reply, arg_quiet, NULL, NULL); if (r < 0) return r; - if (!arg_no_reload) - r = daemon_reload(argc, argv, userdata); - else - r = 0; + if (arg_no_reload) + return 0; + return daemon_reload(argc, argv, userdata); } - - return r; } static int preset_all(int argc, char *argv[], void *userdata) { @@ -5691,13 +5644,7 @@ static int preset_all(int argc, char *argv[], void *userdata) { unsigned n_changes = 0; r = unit_file_preset_all(arg_scope, arg_runtime, arg_root, arg_preset_mode, arg_force, &changes, &n_changes); - if (r < 0) - log_error_errno(r, "Operation failed: %m"); - else { - if (!arg_quiet) - dump_unit_file_changes(changes, n_changes); - } - + unit_file_dump_changes(r, "preset", changes, n_changes, arg_quiet); unit_file_changes_free(changes, n_changes); return r; } else { @@ -5724,7 +5671,7 @@ static int preset_all(int argc, char *argv[], void *userdata) { arg_runtime, arg_force); if (r < 0) - return log_error_errno(r, "Failed to execute operation: %s", bus_error_message(&error, r)); + return log_error_errno(r, "Failed to preset all units: %s", bus_error_message(&error, r)); r = bus_deserialize_and_dump_unit_file_changes(reply, arg_quiet, NULL, NULL); if (r < 0) |