machine: rework Operation logic to reuse in varlink interface

5 files changed, 68 insertions, 43 deletions

diff --git a/src/machine/image-dbus.c b/src/machine/image-dbus.c
index da71b9115f..2ae61f99b7 100644
--- a/src/machine/image-dbus.c
+++ b/src/machine/image-dbus.c
@@ -79,7 +79,7 @@ int bus_image_method_remove(
 
         errno_pipe_fd[1] = safe_close(errno_pipe_fd[1]);
 
-        r = operation_new(m, NULL, child, message, errno_pipe_fd[0], NULL);
+        r = operation_new_with_bus_reply(m, /* machine= */ NULL, child, message, errno_pipe_fd[0], /* ret= */ NULL);
         if (r < 0) {
                 (void) sigkill_wait(child);
                 return r;
@@ -196,7 +196,7 @@ int bus_image_method_clone(
 
         errno_pipe_fd[1] = safe_close(errno_pipe_fd[1]);
 
-        r = operation_new(m, NULL, child, message, errno_pipe_fd[0], NULL);
+        r = operation_new_with_bus_reply(m, /* machine= */ NULL, child, message, errno_pipe_fd[0], /* ret= */ NULL);
         if (r < 0) {
                 (void) sigkill_wait(child);
                 return r;
diff --git a/src/machine/machine-dbus.c b/src/machine/machine-dbus.c
index fc7c9a8303..30abc659f8 100644
--- a/src/machine/machine-dbus.c
+++ b/src/machine/machine-dbus.c
@@ -856,7 +856,7 @@ int bus_machine_method_copy(sd_bus_message *message, void *userdata, sd_bus_erro
 
         /* Copying might take a while, hence install a watch on the child, and return */
 
-        r = operation_new(m->manager, m, child, message, errno_pipe_fd[0], NULL);
+        r = operation_new_with_bus_reply(m->manager, m, child, message, errno_pipe_fd[0], /* ret= */ NULL);
         if (r < 0) {
                 (void) sigkill_wait(child);
                 return r;
diff --git a/src/machine/machined-dbus.c b/src/machine/machined-dbus.c
index 730b436545..f4915f67da 100644
--- a/src/machine/machined-dbus.c
+++ b/src/machine/machined-dbus.c
@@ -813,7 +813,7 @@ static int method_clean_pool(sd_bus_message *message, void *userdata, sd_bus_err
 
         /* The clean-up might take a while, hence install a watch on the child and return */
 
-        r = operation_new(m, NULL, child, message, errno_pipe_fd[0], &operation);
+        r = operation_new_with_bus_reply(m, /* machine= */ NULL, child, message, errno_pipe_fd[0], &operation);
         if (r < 0) {
                 (void) sigkill_wait(child);
                 return r;
diff --git a/src/machine/operation.c b/src/machine/operation.c
index 87c62a977b..4d3939496f 100644
--- a/src/machine/operation.c
+++ b/src/machine/operation.c
@@ -8,8 +8,38 @@
 #include "operation.h"
 #include "process-util.h"
 
+static int operation_done_internal(const siginfo_t *si, Operation *o, sd_bus_error *error) {
+        int r;
+
+        assert(si);
+        assert(o);
+
+        if (si->si_code != CLD_EXITED)
+                return log_debug_errno(SYNTHETIC_ERRNO(ESHUTDOWN), "Child died abnormally");
+
+        if (si->si_status == EXIT_SUCCESS)
+                r = 0;
+        else {
+                ssize_t n = read(o->errno_fd, &r, sizeof(r));
+                if (n < 0)
+                        return log_debug_errno(errno, "Failed to read operation's errno: %m");
+                if (n != sizeof(r))
+                        return log_debug_errno(SYNTHETIC_ERRNO(EIO), "Received unexpectedly short message when reading operation's errno");
+        }
+
+        if (o->done)
+                /* A completion routine is set for this operation, call it. */
+                return o->done(o, r, error);
+
+        /* The default operation when done is to simply return an error on failure or an empty success
+         * message on success. */
+        if (r < 0)
+                log_debug_errno(r, "Operation failed: %m");
+
+        return r;
+}
+
 static int operation_done(sd_event_source *s, const siginfo_t *si, void *userdata) {
-        _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL;
         Operation *o = ASSERT_PTR(userdata);
         int r;
 
@@ -21,61 +51,44 @@ static int operation_done(sd_event_source *s, const siginfo_t *si, void *userdat
 
         o->pid = 0;
 
-        if (si->si_code != CLD_EXITED) {
-                r = sd_bus_error_set(&error, SD_BUS_ERROR_FAILED, "Child died abnormally.");
-                goto fail;
-        }
+        if (o->message) {
+                _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL;
 
-        if (si->si_status == EXIT_SUCCESS)
-                r = 0;
-        else if (read(o->errno_fd, &r, sizeof(r)) != sizeof(r)) { /* Try to acquire error code for failed operation */
-                r = sd_bus_error_set(&error, SD_BUS_ERROR_FAILED, "Child failed.");
-                goto fail;
-        }
-
-        if (o->done) {
-                /* A completion routine is set for this operation, call it. */
-                r = o->done(o, r, &error);
+                r = operation_done_internal(si, o, &error);
                 if (r < 0) {
                         if (!sd_bus_error_is_set(&error))
                                 sd_bus_error_set_errno(&error, r);
 
-                        goto fail;
-                }
-
-        } else {
-                /* The default operation when done is to simply return an error on failure or an empty success
-                 * message on success. */
-                if (r < 0) {
-                        sd_bus_error_set_errno(&error, r);
-                        goto fail;
+                        r = sd_bus_reply_method_error(o->message, &error);
+                        if (r < 0)
+                                log_error_errno(r, "Failed to reply to dbus message: %m");
+                } else {
+                        r = sd_bus_reply_method_return(o->message, NULL);
+                        if (r < 0)
+                                log_error_errno(r, "Failed to reply to dbus message: %m");
                 }
-
-                r = sd_bus_reply_method_return(o->message, NULL);
+        } else if (o->link) {
+                r = operation_done_internal(si, o, /* error = */ NULL);
                 if (r < 0)
-                        log_error_errno(r, "Failed to reply to message: %m");
-        }
-
-        operation_free(o);
-        return 0;
-
-fail:
-        r = sd_bus_reply_method_error(o->message, &error);
-        if (r < 0)
-                log_error_errno(r, "Failed to reply to message: %m");
+                        (void) sd_varlink_error_errno(o->link, r);
+                else
+                        (void) sd_varlink_reply(o->link, NULL);
+        } else
+                assert_not_reached();
 
         operation_free(o);
         return 0;
 }
 
-int operation_new(Manager *manager, Machine *machine, pid_t child, sd_bus_message *message, int errno_fd, Operation **ret) {
+int operation_new(Manager *manager, Machine *machine, pid_t child, sd_bus_message *message, sd_varlink *link, int errno_fd, Operation **ret) {
         Operation *o;
         int r;
 
         assert(manager);
         assert(child > 1);
-        assert(message);
         assert(errno_fd >= 0);
+        assert(message || link);
+        assert(!(message && link));
 
         o = new0(Operation, 1);
         if (!o)
@@ -91,6 +104,7 @@ int operation_new(Manager *manager, Machine *machine, pid_t child, sd_bus_messag
 
         o->pid = child;
         o->message = sd_bus_message_ref(message);
+        o->link = sd_varlink_ref(link);
         o->errno_fd = errno_fd;
 
         LIST_PREPEND(operations, manager->operations, o);
@@ -125,6 +139,7 @@ Operation *operation_free(Operation *o) {
                 (void) sigkill_wait(o->pid);
 
         sd_bus_message_unref(o->message);
+        sd_varlink_unref(o->link);
 
         if (o->manager) {
                 LIST_REMOVE(operations, o->manager->operations, o);
diff --git a/src/machine/operation.h b/src/machine/operation.h
index fd4828878d..75bf918c2b 100644
--- a/src/machine/operation.h
+++ b/src/machine/operation.h
@@ -18,7 +18,11 @@ struct Operation {
         Manager *manager;
         Machine *machine;
         pid_t pid;
+
+        /* only one of these two fields should be set */
+        sd_varlink *link;
         sd_bus_message *message;
+
         int errno_fd;
         int extra_fd;
         sd_event_source *event_source;
@@ -27,5 +31,11 @@ struct Operation {
         LIST_FIELDS(Operation, operations_by_machine);
 };
 
-int operation_new(Manager *manager, Machine *machine, pid_t child, sd_bus_message *message, int errno_fd, Operation **ret);
+int operation_new(Manager *manager, Machine *machine, pid_t child, sd_bus_message *message, sd_varlink *link, int errno_fd, Operation **ret);
 Operation *operation_free(Operation *o);
+static inline int operation_new_with_bus_reply(Manager *manager, Machine *machine, pid_t child, sd_bus_message *message, int errno_fd, Operation **ret) {
+        return operation_new(manager, machine, child, message, /* link = */ NULL, errno_fd, ret);
+}
+static inline int operation_new_with_varlink_reply(Manager *manager, Machine *machine, pid_t child, sd_varlink *link, int errno_fd, Operation **ret) {
+        return operation_new(manager, machine, child, /* message = */ NULL, link, errno_fd, ret);
+}