core: a more informative error when SetProperties/StartTransientUnit fails

I was changing how some properties are appended to the StartTransientUnit call
and messed up the message contents. When something is wrong with how the
message is structed, we would return a very generic
"Failed to start transient service unit: No such device or address".

Mention that it was property setting that failed, and translate ENXIO to a
different message. bus_unit_set_properties() or any of the children it calls
may also return other errors, in particular EBADMSG or ENOMEM, but the error
message that is generated for those is understandable, so we don't need to
"translate" them explicitly.

bus_unit_set_properties() is called from two places, so it seems nicer to
generate the message internally, rather than ask the caller to do that. Also,
now bus_unit_set_properties() always sets <error>, which is nicer for the
callers.

diff --git a/src/core/dbus-unit.c b/src/core/dbus-unit.c
index f01e7c7604a3dbf357709431e8d765a009fb4007..3f083a8174550bc98ffb4a495ee2a88a13dd6301 100644 (file)
--- a/src/core/dbus-unit.c
+++ b/src/core/dbus-unit.c
@@ -2413,7 +2413,7 @@ int bus_unit_set_properties(
 
         r = sd_bus_message_enter_container(message, 'a', "(sv)");
         if (r < 0)
-                return r;
+                goto error;
 
         for (;;) {
                 const char *name;
@@ -2421,7 +2421,7 @@ int bus_unit_set_properties(
 
                 r = sd_bus_message_enter_container(message, 'r', "sv");
                 if (r < 0)
-                        return r;
+                        goto error;
                 if (r == 0) {
                         if (for_real || UNIT_WRITE_FLAGS_NOOP(flags))
                                 break;
@@ -2429,7 +2429,7 @@ int bus_unit_set_properties(
                         /* Reached EOF. Let's try again, and this time for realz... */
                         r = sd_bus_message_rewind(message, false);
                         if (r < 0)
-                                return r;
+                                goto error;
 
                         for_real = true;
                         continue;
@@ -2437,11 +2437,11 @@ int bus_unit_set_properties(
 
                 r = sd_bus_message_read(message, "s", &name);
                 if (r < 0)
-                        return r;
+                        goto error;
 
                 r = sd_bus_message_enter_container(message, 'v', NULL);
                 if (r < 0)
-                        return r;
+                        goto error;
 
                 /* If not for real, then mask out the two target flags */
                 f = for_real ? flags : (flags & ~(UNIT_RUNTIME|UNIT_PERSISTENT));
@@ -2455,7 +2455,7 @@ int bus_unit_set_properties(
                 if (r == 0)
                         r = bus_unit_set_live_property(u, name, message, f, error);
                 if (r < 0)
-                        return r;
+                        goto error;
 
                 if (r == 0)
                         return sd_bus_error_setf(error, SD_BUS_ERROR_PROPERTY_READ_ONLY,
@@ -2463,23 +2463,32 @@ int bus_unit_set_properties(
 
                 r = sd_bus_message_exit_container(message);
                 if (r < 0)
-                        return r;
+                        goto error;
 
                 r = sd_bus_message_exit_container(message);
                 if (r < 0)
-                        return r;
+                        goto error;
 
                 n += for_real;
         }
 
         r = sd_bus_message_exit_container(message);
         if (r < 0)
-                return r;
+                goto error;
 
         if (commit && n > 0 && UNIT_VTABLE(u)->bus_commit_properties)
                 UNIT_VTABLE(u)->bus_commit_properties(u);
 
         return n;
+
+ error:
+        /* Pretty much any of the calls above can fail if the message is not formed properly
+         * or if it has unexpected contents. Fill in a more informative error message here. */
+        if (sd_bus_error_is_set(error))
+                return r;
+        return sd_bus_error_set_errnof(error, r,
+                                       r == -ENXIO ? "Failed to set unit properties: Unexpected message contents"
+                                                   : "Failed to set unit properties: %m");
 }
 
 int bus_unit_validate_load_state(Unit *u, sd_bus_error *error) {