Merge pull request #3153 from poettering/async-clone

machined: make "clone" asynchronous, and support copy-based fall-back

9 files changed, 313 insertions, 131 deletions

diff --git a/src/machine/image-dbus.c b/src/machine/image-dbus.c
index b764bc43a0..0eed9b81bb 100644
--- a/src/machine/image-dbus.c
+++ b/src/machine/image-dbus.c
@@ -20,9 +20,11 @@
 #include "alloc-util.h"
 #include "bus-label.h"
 #include "bus-util.h"
+#include "fd-util.h"
 #include "image-dbus.h"
 #include "io-util.h"
 #include "machine-image.h"
+#include "process-util.h"
 #include "strv.h"
 #include "user-util.h"
 
@@ -33,13 +35,18 @@ int bus_image_method_remove(
                 void *userdata,
                 sd_bus_error *error) {
 
+        _cleanup_close_pair_ int errno_pipe_fd[2] = { -1, -1 };
         Image *image = userdata;
         Manager *m = image->userdata;
+        pid_t child;
         int r;
 
         assert(message);
         assert(image);
 
+        if (m->n_operations >= OPERATIONS_MAX)
+                return sd_bus_error_setf(error, SD_BUS_ERROR_LIMITS_EXCEEDED, "Too many ongoing operations.");
+
         r = bus_verify_polkit_async(
                         message,
                         CAP_SYS_ADMIN,
@@ -54,11 +61,35 @@ int bus_image_method_remove(
         if (r == 0)
                 return 1; /* Will call us back */
 
-        r = image_remove(image);
-        if (r < 0)
+        if (pipe2(errno_pipe_fd, O_CLOEXEC|O_NONBLOCK) < 0)
+                return sd_bus_error_set_errnof(error, errno, "Failed to create pipe: %m");
+
+        child = fork();
+        if (child < 0)
+                return sd_bus_error_set_errnof(error, errno, "Failed to fork(): %m");
+        if (child == 0) {
+                errno_pipe_fd[0] = safe_close(errno_pipe_fd[0]);
+
+                r = image_remove(image);
+                if (r < 0) {
+                        (void) write(errno_pipe_fd[1], &r, sizeof(r));
+                        _exit(EXIT_FAILURE);
+                }
+
+                _exit(EXIT_SUCCESS);
+        }
+
+        errno_pipe_fd[1] = safe_close(errno_pipe_fd[1]);
+
+        r = operation_new(m, NULL, child, message, errno_pipe_fd[0]);
+        if (r < 0) {
+                (void) sigkill_wait(child);
                 return r;
+        }
 
-        return sd_bus_reply_method_return(message, NULL);
+        errno_pipe_fd[0] = -1;
+
+        return 1;
 }
 
 int bus_image_method_rename(
@@ -107,13 +138,19 @@ int bus_image_method_clone(
                 void *userdata,
                 sd_bus_error *error) {
 
+        _cleanup_close_pair_ int errno_pipe_fd[2] = { -1, -1 };
         Image *image = userdata;
         Manager *m = image->userdata;
         const char *new_name;
         int r, read_only;
+        pid_t child;
 
         assert(message);
         assert(image);
+        assert(m);
+
+        if (m->n_operations >= OPERATIONS_MAX)
+                return sd_bus_error_setf(error, SD_BUS_ERROR_LIMITS_EXCEEDED, "Too many ongoing operations.");
 
         r = sd_bus_message_read(message, "sb", &new_name, &read_only);
         if (r < 0)
@@ -136,13 +173,35 @@ int bus_image_method_clone(
         if (r == 0)
                 return 1; /* Will call us back */
 
-        r = image_clone(image, new_name, read_only);
-        if (r == -EOPNOTSUPP)
-                return sd_bus_reply_method_errnof(message, r, "Image cloning is currently only supported on btrfs file systems.");
-        if (r < 0)
+        if (pipe2(errno_pipe_fd, O_CLOEXEC|O_NONBLOCK) < 0)
+                return sd_bus_error_set_errnof(error, errno, "Failed to create pipe: %m");
+
+        child = fork();
+        if (child < 0)
+                return sd_bus_error_set_errnof(error, errno, "Failed to fork(): %m");
+        if (child == 0) {
+                errno_pipe_fd[0] = safe_close(errno_pipe_fd[0]);
+
+                r = image_clone(image, new_name, read_only);
+                if (r < 0) {
+                        (void) write(errno_pipe_fd[1], &r, sizeof(r));
+                        _exit(EXIT_FAILURE);
+                }
+
+                _exit(EXIT_SUCCESS);
+        }
+
+        errno_pipe_fd[1] = safe_close(errno_pipe_fd[1]);
+
+        r = operation_new(m, NULL, child, message, errno_pipe_fd[0]);
+        if (r < 0) {
+                (void) sigkill_wait(child);
                 return r;
+        }
 
-        return sd_bus_reply_method_return(message, NULL);
+        errno_pipe_fd[0] = -1;
+
+        return 1;
 }
 
 int bus_image_method_mark_read_only(
diff --git a/src/machine/machine-dbus.c b/src/machine/machine-dbus.c
index 5121bfdd18..7b9aa66d63 100644
--- 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 7d4270a8ff..c1fae57084 100644
--- 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 1d8cc5911a..e5d75361a9 100644
--- 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/machinectl.c b/src/machine/machinectl.c
index 5a68c4ceb2..1165ab5afa 100644
--- a/src/machine/machinectl.c
+++ b/src/machine/machinectl.c
@@ -1076,6 +1076,7 @@ static int terminate_machine(int argc, char *argv[], void *userdata) {
 
 static int copy_files(int argc, char *argv[], void *userdata) {
         _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL;
+        _cleanup_(sd_bus_message_unrefp) sd_bus_message *m = NULL;
         _cleanup_free_ char *abs_host_path = NULL;
         char *dest, *host_path, *container_path;
         sd_bus *bus = userdata;
@@ -1099,19 +1100,28 @@ static int copy_files(int argc, char *argv[], void *userdata) {
                 host_path = abs_host_path;
         }
 
-        r = sd_bus_call_method(
+        r = sd_bus_message_new_method_call(
                         bus,
+                        &m,
                         "org.freedesktop.machine1",
                         "/org/freedesktop/machine1",
                         "org.freedesktop.machine1.Manager",
-                        copy_from ? "CopyFromMachine" : "CopyToMachine",
-                        &error,
-                        NULL,
+                        copy_from ? "CopyFromMachine" : "CopyToMachine");
+        if (r < 0)
+                return bus_log_create_error(r);
+
+        r = sd_bus_message_append(
+                        m,
                         "sss",
                         argv[1],
                         copy_from ? container_path : host_path,
                         copy_from ? host_path : container_path);
         if (r < 0)
+                return bus_log_create_error(r);
+
+        /* This is a slow operation, hence turn off any method call timeouts */
+        r = sd_bus_call(bus, m, USEC_INFINITY, &error, NULL);
+        if (r < 0)
                 return log_error_errno(r, "Failed to copy: %s", bus_error_message(&error, r));
 
         return 0;
@@ -1393,7 +1403,6 @@ static int shell_machine(int argc, char *argv[], void *userdata) {
 }
 
 static int remove_image(int argc, char *argv[], void *userdata) {
-        _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL;
         sd_bus *bus = userdata;
         int r, i;
 
@@ -1402,19 +1411,27 @@ static int remove_image(int argc, char *argv[], void *userdata) {
         polkit_agent_open_if_enabled();
 
         for (i = 1; i < argc; i++) {
-                r = sd_bus_call_method(
+                _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL;
+                _cleanup_(sd_bus_message_unrefp) sd_bus_message *m = NULL;
+
+                r = sd_bus_message_new_method_call(
                                 bus,
+                                &m,
                                 "org.freedesktop.machine1",
                                 "/org/freedesktop/machine1",
                                 "org.freedesktop.machine1.Manager",
-                                "RemoveImage",
-                                &error,
-                                NULL,
-                                "s", argv[i]);
-                if (r < 0) {
-                        log_error("Could not remove image: %s", bus_error_message(&error, -r));
-                        return r;
-                }
+                                "RemoveImage");
+                if (r < 0)
+                        return bus_log_create_error(r);
+
+                r = sd_bus_message_append(m, "s", argv[i]);
+                if (r < 0)
+                        return bus_log_create_error(r);
+
+                /* This is a slow operation, hence turn off any method call timeouts */
+                r = sd_bus_call(bus, m, USEC_INFINITY, &error, NULL);
+                if (r < 0)
+                        return log_error_errno(r, "Could not remove image: %s", bus_error_message(&error, r));
         }
 
         return 0;
@@ -1446,24 +1463,30 @@ static int rename_image(int argc, char *argv[], void *userdata) {
 
 static int clone_image(int argc, char *argv[], void *userdata) {
         _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL;
+        _cleanup_(sd_bus_message_unrefp) sd_bus_message *m = NULL;
         sd_bus *bus = userdata;
         int r;
 
         polkit_agent_open_if_enabled();
 
-        r = sd_bus_call_method(
+        r = sd_bus_message_new_method_call(
                         bus,
+                        &m,
                         "org.freedesktop.machine1",
                         "/org/freedesktop/machine1",
                         "org.freedesktop.machine1.Manager",
-                        "CloneImage",
-                        &error,
-                        NULL,
-                        "ssb", argv[1], argv[2], arg_read_only);
-        if (r < 0) {
-                log_error("Could not clone image: %s", bus_error_message(&error, -r));
-                return r;
-        }
+                        "CloneImage");
+        if (r < 0)
+                return bus_log_create_error(r);
+
+        r = sd_bus_message_append(m, "ssb", argv[1], argv[2], arg_read_only);
+        if (r < 0)
+                return bus_log_create_error(r);
+
+        /* This is a slow operation, hence turn off any method call timeouts */
+        r = sd_bus_call(bus, m, USEC_INFINITY, &error, NULL);
+        if (r < 0)
+                return log_error_errno(r, "Could not clone image: %s", bus_error_message(&error, r));
 
         return 0;
 }
diff --git a/src/machine/machined.c b/src/machine/machined.c
index f2c1966a6b..f7ceb5e603 100644
--- a/src/machine/machined.c
+++ b/src/machine/machined.c
@@ -70,6 +70,11 @@ void manager_free(Manager *m) {
 
         assert(m);
 
+        while (m->operations)
+                operation_free(m->operations);
+
+        assert(m->n_operations == 0);
+
         while ((machine = hashmap_first(m->machines)))
                 machine_free(machine);
 
@@ -336,6 +341,9 @@ int manager_startup(Manager *m) {
 static bool check_idle(void *userdata) {
         Manager *m = userdata;
 
+        if (m->operations)
+                return false;
+
         manager_gc(m, true);
 
         return hashmap_isempty(m->machines);
diff --git a/src/machine/machined.h b/src/machine/machined.h
index e7d7dfdceb..7b9b148044 100644
--- a/src/machine/machined.h
+++ b/src/machine/machined.h
@@ -32,6 +32,7 @@ typedef struct Manager Manager;
 #include "image-dbus.h"
 #include "machine-dbus.h"
 #include "machine.h"
+#include "operation.h"
 
 struct Manager {
         sd_event *event;
@@ -49,6 +50,9 @@ struct Manager {
         LIST_HEAD(Machine, machine_gc_queue);
 
         Machine *host_machine;
+
+        LIST_HEAD(Operation, operations);
+        unsigned n_operations;
 };
 
 Manager *manager_new(void);
diff --git a/src/machine/operation.c b/src/machine/operation.c
new file mode 100644
index 0000000000..e6ddc41a55
--- /dev/null
+++ b/src/machine/operation.c
@@ -0,0 +1,131 @@
+/***
+  This file is part of systemd.
+
+  Copyright 2016 Lennart Poettering
+
+  systemd is free software; you can redistribute it and/or modify it
+  under the terms of the GNU Lesser General Public License as published by
+  the Free Software Foundation; either version 2.1 of the License, or
+  (at your option) any later version.
+
+  systemd is distributed in the hope that it will be useful, but
+  WITHOUT ANY WARRANTY; without even the implied warranty of
+  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+  Lesser General Public License for more details.
+
+  You should have received a copy of the GNU Lesser General Public License
+  along with systemd; If not, see <http://www.gnu.org/licenses/>.
+***/
+
+#include "alloc-util.h"
+#include "fd-util.h"
+#include "operation.h"
+#include "process-util.h"
+
+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 = userdata;
+        int r;
+
+        assert(o);
+        assert(si);
+
+        log_debug("Operating " PID_FMT " is now complete with with code=%s status=%i",
+                  o->pid,
+                  sigchld_code_to_string(si->si_code), si->si_status);
+
+        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");
+
+        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");
+
+        operation_free(o);
+        return 0;
+}
+
+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(manager->event, &o->event_source, child, WEXITED, operation_done, o);
+        if (r < 0) {
+                free(o);
+                return r;
+        }
+
+        o->pid = child;
+        o->message = sd_bus_message_ref(message);
+        o->errno_fd = errno_fd;
+
+        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);
+
+        /* At this point we took ownership of both the child and the errno file descriptor! */
+
+        return 0;
+}
+
+Operation *operation_free(Operation *o) {
+        if (!o)
+                return NULL;
+
+        sd_event_source_unref(o->event_source);
+
+        safe_close(o->errno_fd);
+
+        if (o->pid > 1)
+                (void) sigkill_wait(o->pid);
+
+        sd_bus_message_unref(o->message);
+
+        if (o->manager) {
+                LIST_REMOVE(operations, o->manager->operations, 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
new file mode 100644
index 0000000000..7ca47bc3af
--- /dev/null
+++ b/src/machine/operation.h
@@ -0,0 +1,47 @@
+#pragma once
+
+/***
+  This file is part of systemd.
+
+  Copyright 2016 Lennart Poettering
+
+  systemd is free software; you can redistribute it and/or modify it
+  under the terms of the GNU Lesser General Public License as published by
+  the Free Software Foundation; either version 2.1 of the License, or
+  (at your option) any later version.
+
+  systemd is distributed in the hope that it will be useful, but
+  WITHOUT ANY WARRANTY; without even the implied warranty of
+  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+  Lesser General Public License for more details.
+
+  You should have received a copy of the GNU Lesser General Public License
+  along with systemd; If not, see <http://www.gnu.org/licenses/>.
+***/
+
+#include <sys/types.h>
+
+#include "sd-bus.h"
+#include "sd-event.h"
+
+#include "list.h"
+
+typedef struct Operation Operation;
+
+#include "machined.h"
+
+#define OPERATIONS_MAX 64
+
+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 *manager, Machine *machine, pid_t child, sd_bus_message *message, int errno_fd);
+Operation *operation_free(Operation *o);