summaryrefslogtreecommitdiff
path: root/src/core/dbus-manager.c
diff options
context:
space:
mode:
authorZbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>2016-04-17 10:16:44 -0400
committerZbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>2016-04-19 08:58:00 -0400
commitaf3d811352094f3f1304bdf8ba9cdd2b4b03b55c (patch)
treeed4ded67db03941683d54251bfa6805280128e7c /src/core/dbus-manager.c
parent795ff6d5d827eb5743d9e37c4acaee4bdeff58b4 (diff)
shared/install,systemctl,core: report offending file on installation error
Fixes #2191: $ systemctl --root=/ enable sddm Created symlink /etc/systemd/system/display-manager.service, pointing to /usr/lib/systemd/system/sddm.service. $ sudo build/systemctl --root=/ enable gdm Failed to enable unit, file /etc/systemd/system/display-manager.service already exists and is a symlink to /usr/lib/systemd/system/sddm.service. $ sudo build/systemctl --root= enable sddm $ sudo build/systemctl --root= enable gdm Failed to enable unit: File /etc/systemd/system/display-manager.service already exists and is a symlink to /usr/lib/systemd/system/sddm.service. (I tried a few different approaches to pass the error information back to the caller. Adding a new parameter to hold the error results in a gigantic patch and a lot of hassle to pass the args arounds. Adding this information to the changes array is straightforward and can be more easily extended in the future.) In case local installation is performed, the full set of errors can be reported and we do that. When running over dbus, only the first error is reported.
Diffstat (limited to 'src/core/dbus-manager.c')
-rw-r--r--src/core/dbus-manager.c119
1 files changed, 74 insertions, 45 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);
}