machined: rework copy-from/copy-to operation to use generic Operation object

With this all potentially slow operations are done out-of-process,
asynchronously, using the same "Operation" object.

diff --git a/src/machine/image-dbus.c b/src/machine/image-dbus.c
index e07edae6efd59ffc59322f3e506694590383e4a2..0eed9b81bb5a5b32a06c4d2f669d85875d240d47 100644 (file)
--- a/src/machine/image-dbus.c
+++ b/src/machine/image-dbus.c
@@ -81,7 +81,7 @@ int bus_image_method_remove(
 
         errno_pipe_fd[1] = safe_close(errno_pipe_fd[1]);
 
-        r = operation_new(m, child, message, errno_pipe_fd[0]);
+        r = operation_new(m, NULL, child, message, errno_pipe_fd[0]);
         if (r < 0) {
                 (void) sigkill_wait(child);
                 return r;
@@ -193,7 +193,7 @@ int bus_image_method_clone(
 
         errno_pipe_fd[1] = safe_close(errno_pipe_fd[1]);
 
-        r = operation_new(m, child, message, errno_pipe_fd[0]);
+        r = operation_new(m, NULL, child, message, errno_pipe_fd[0]);
         if (r < 0) {
                 (void) sigkill_wait(child);
                 return r;

diff --git a/src/machine/machine-dbus.c b/src/machine/machine-dbus.c
index 5121bfdd18fabd296aa80be5dc6e94af8a088499..7b9aa66d63286af74001e728746bf340e0489047 100644 (file)
--- a/src/machine/machine-dbus.c
+++ b/src/machine/machine-dbus.c
@@ -1085,52 +1085,11 @@ finish:
         return r;
 }
 
-static int machine_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;
-        MachineOperation *o = userdata;
-        int r;
-
-        assert(o);
-        assert(si);
-
-        o->pid = 0;
-
-        if (si->si_code != CLD_EXITED) {
-                r = sd_bus_error_setf(&error, SD_BUS_ERROR_FAILED, "Child died abnormally.");
-                goto fail;
-        }
-
-        if (si->si_status != EXIT_SUCCESS) {
-                if (read(o->errno_fd, &r, sizeof(r)) == sizeof(r))
-                        r = sd_bus_error_set_errnof(&error, r, "%m");
-                else
-                        r = sd_bus_error_setf(&error, SD_BUS_ERROR_FAILED, "Child failed.");
-
-                goto fail;
-        }
-
-        r = sd_bus_reply_method_return(o->message, NULL);
-        if (r < 0)
-                log_error_errno(r, "Failed to reply to message: %m");
-
-        machine_operation_unref(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");
-
-        machine_operation_unref(o);
-        return 0;
-}
-
 int bus_machine_method_copy(sd_bus_message *message, void *userdata, sd_bus_error *error) {
         const char *src, *dest, *host_path, *container_path, *host_basename, *host_dirname, *container_basename, *container_dirname;
         _cleanup_close_pair_ int errno_pipe_fd[2] = { -1, -1 };
         _cleanup_close_ int hostfd = -1;
         Machine *m = userdata;
-        MachineOperation *o;
         bool copy_from;
         pid_t child;
         char *t;
@@ -1139,7 +1098,7 @@ int bus_machine_method_copy(sd_bus_message *message, void *userdata, sd_bus_erro
         assert(message);
         assert(m);
 
-        if (m->n_operations >= MACHINE_OPERATIONS_MAX)
+        if (m->manager->n_operations >= OPERATIONS_MAX)
                 return sd_bus_error_setf(error, SD_BUS_ERROR_LIMITS_EXCEEDED, "Too many ongoing copies.");
 
         if (m->class != MACHINE_CONTAINER)
@@ -1249,27 +1208,14 @@ int bus_machine_method_copy(sd_bus_message *message, void *userdata, sd_bus_erro
 
         errno_pipe_fd[1] = safe_close(errno_pipe_fd[1]);
 
-        /* Copying might take a while, hence install a watch the
-         * child, and return */
+        /* Copying might take a while, hence install a watch on the child, and return */
 
-        o = new0(MachineOperation, 1);
-        if (!o)
-                return log_oom();
-
-        o->pid = child;
-        o->message = sd_bus_message_ref(message);
-        o->errno_fd = errno_pipe_fd[0];
-        errno_pipe_fd[0] = -1;
-
-        r = sd_event_add_child(m->manager->event, &o->event_source, child, WEXITED, machine_operation_done, o);
+        r = operation_new(m->manager, m, child, message, errno_pipe_fd[0]);
         if (r < 0) {
-                machine_operation_unref(o);
-                return log_oom();
+                (void) sigkill_wait(child);
+                return r;
         }
-
-        LIST_PREPEND(operations, m->operations, o);
-        m->n_operations++;
-        o->machine = m;
+        errno_pipe_fd[0] = -1;
 
         return 1;
 }

diff --git a/src/machine/machine.c b/src/machine/machine.c
index 7d4270a8fffe350737f398e2ce3acf701c834270..c1fae570849b7f39468c48a5cdec90d3b4be228e 100644 (file)
--- a/src/machine/machine.c
+++ b/src/machine/machine.c
@@ -89,7 +89,7 @@ void machine_free(Machine *m) {
         assert(m);
 
         while (m->operations)
-                machine_operation_unref(m->operations);
+                operation_free(m->operations);
 
         if (m->in_gc_queue)
                 LIST_REMOVE(gc_queue, m->manager->machine_gc_queue, m);
@@ -596,28 +596,6 @@ int machine_open_terminal(Machine *m, const char *path, int mode) {
         }
 }
 
-MachineOperation *machine_operation_unref(MachineOperation *o) {
-        if (!o)
-                return NULL;
-
-        sd_event_source_unref(o->event_source);
-
-        safe_close(o->errno_fd);
-
-        if (o->pid > 1)
-                (void) kill(o->pid, SIGKILL);
-
-        sd_bus_message_unref(o->message);
-
-        if (o->machine) {
-                LIST_REMOVE(operations, o->machine->operations, o);
-                o->machine->n_operations--;
-        }
-
-        free(o);
-        return NULL;
-}
-
 void machine_release_unit(Machine *m) {
         assert(m);
 

diff --git a/src/machine/machine.h b/src/machine/machine.h
index 1d8cc5911a707da20f3ee2ab02c8ac7effdcae0d..e5d75361a9a635b23db77dc448a040556197a94f 100644 (file)
--- a/src/machine/machine.h
+++ b/src/machine/machine.h
@@ -20,11 +20,11 @@
 ***/
 
 typedef struct Machine Machine;
-typedef struct MachineOperation MachineOperation;
 typedef enum KillWho KillWho;
 
 #include "list.h"
 #include "machined.h"
+#include "operation.h"
 
 typedef enum MachineState {
         MACHINE_OPENING,    /* Machine is being registered */
@@ -49,17 +49,6 @@ enum KillWho {
         _KILL_WHO_INVALID = -1
 };
 
-#define MACHINE_OPERATIONS_MAX 64
-
-struct MachineOperation {
-        Machine *machine;
-        pid_t pid;
-        sd_bus_message *message;
-        int errno_fd;
-        sd_event_source *event_source;
-        LIST_FIELDS(MachineOperation, operations);
-};
-
 struct Machine {
         Manager *manager;
 
@@ -88,10 +77,9 @@ struct Machine {
         int *netif;
         unsigned n_netif;
 
-        LIST_FIELDS(Machine, gc_queue);
+        LIST_HEAD(Operation, operations);
 
-        MachineOperation *operations;
-        unsigned n_operations;
+        LIST_FIELDS(Machine, gc_queue);
 };
 
 Machine* machine_new(Manager *manager, MachineClass class, const char *name);
@@ -109,8 +97,6 @@ void machine_release_unit(Machine *m);
 
 MachineState machine_get_state(Machine *u);
 
-MachineOperation *machine_operation_unref(MachineOperation *o);
-
 const char* machine_class_to_string(MachineClass t) _const_;
 MachineClass machine_class_from_string(const char *s) _pure_;
 

diff --git a/src/machine/operation.c b/src/machine/operation.c
index e8564c29f7159c449378e653481196e2a54877f2..e6ddc41a555067219a04410d76bef1e84e810499 100644 (file)
--- a/src/machine/operation.c
+++ b/src/machine/operation.c
@@ -66,15 +66,20 @@ fail:
         return 0;
 }
 
-int operation_new(Manager *m, pid_t child, sd_bus_message *message, int errno_fd) {
+int operation_new(Manager *manager, Machine *machine, pid_t child, sd_bus_message *message, int errno_fd) {
         Operation *o;
         int r;
 
+        assert(manager);
+        assert(child > 1);
+        assert(message);
+        assert(errno_fd >= 0);
+
         o = new0(Operation, 1);
         if (!o)
                 return -ENOMEM;
 
-        r = sd_event_add_child(m->event, &o->event_source, child, WEXITED, operation_done, o);
+        r = sd_event_add_child(manager->event, &o->event_source, child, WEXITED, operation_done, o);
         if (r < 0) {
                 free(o);
                 return r;
@@ -84,9 +89,14 @@ int operation_new(Manager *m, pid_t child, sd_bus_message *message, int errno_fd
         o->message = sd_bus_message_ref(message);
         o->errno_fd = errno_fd;
 
-        LIST_PREPEND(operations, m->operations, o);
-        m->n_operations++;
-        o->manager = m;
+        LIST_PREPEND(operations, manager->operations, o);
+        manager->n_operations++;
+        o->manager = manager;
+
+        if (machine) {
+                LIST_PREPEND(operations_by_machine, machine->operations, o);
+                o->machine = machine;
+        }
 
         log_debug("Started new operation " PID_FMT ".", child);
 
@@ -113,6 +123,9 @@ Operation *operation_free(Operation *o) {
                 o->manager->n_operations--;
         }
 
+        if (o->machine)
+                LIST_REMOVE(operations_by_machine, o->machine->operations, o);
+
         free(o);
         return NULL;
 }

diff --git a/src/machine/operation.h b/src/machine/operation.h
index 9d4c3afe4596958d3ce26000c6701225c328680b..7ca47bc3af5a011136ddd75eb7fec6a96ba7c167 100644 (file)
--- a/src/machine/operation.h
+++ b/src/machine/operation.h
@@ -34,12 +34,14 @@ typedef struct Operation Operation;
 
 struct Operation {
         Manager *manager;
+        Machine *machine;
         pid_t pid;
         sd_bus_message *message;
         int errno_fd;
         sd_event_source *event_source;
         LIST_FIELDS(Operation, operations);
+        LIST_FIELDS(Operation, operations_by_machine);
 };
 
-int operation_new(Manager *m, pid_t child, sd_bus_message *message, int errno_fd);
+int operation_new(Manager *manager, Machine *machine, pid_t child, sd_bus_message *message, int errno_fd);
 Operation *operation_free(Operation *o);