machined: "machinectl clean" can take a while, do it asynchronously from a background process

This is a follow-up to 5d2036b5f3506bd0ff07042aee8d69c26db32298, and also makes
the "machinectl clean" verb asynchronous, after all it's little more than a
series of image removals.

The changes required to make this happen are a bit more comprehensive as we
need to pass information about deleted images back to the client, as well as
information about the image we failed on if we failed on one. Hence, create a
temporary file in /tmp, serialize that data into, and read it from the parent
after the operation is complete.

6 files changed, 229 insertions, 55 deletions

diff --git a/src/machine/image-dbus.c b/src/machine/image-dbus.c
index 0eed9b81bb..867bbc467b 100644
--- 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, NULL, child, message, errno_pipe_fd[0]);
+        r = operation_new(m, NULL, child, message, errno_pipe_fd[0], NULL);
         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, NULL, child, message, errno_pipe_fd[0]);
+        r = operation_new(m, NULL, child, message, errno_pipe_fd[0], 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 f50f363ba3..ba7ac04b56 100644
--- a/src/machine/machine-dbus.c
+++ b/src/machine/machine-dbus.c
@@ -1207,7 +1207,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]);
+        r = operation_new(m->manager, m, child, message, errno_pipe_fd[0], NULL);
         if (r < 0) {
                 (void) sigkill_wait(child);
                 return r;
diff --git a/src/machine/machinectl.c b/src/machine/machinectl.c
index 5ca557abbf..d68c50b203 100644
--- a/src/machine/machinectl.c
+++ b/src/machine/machinectl.c
@@ -2375,7 +2375,7 @@ static int set_limit(int argc, char *argv[], void *userdata) {
 }
 
 static int clean_images(int argc, char *argv[], void *userdata) {
-        _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL;
+        _cleanup_(sd_bus_message_unrefp) sd_bus_message *m = NULL, *reply = NULL;
         _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL;
         uint64_t usage, total = 0;
         char fb[FORMAT_BYTES_MAX];
@@ -2384,15 +2384,22 @@ static int clean_images(int argc, char *argv[], void *userdata) {
         unsigned c = 0;
         int r;
 
-        r = sd_bus_call_method(
+        r = sd_bus_message_new_method_call(
                         bus,
+                        &m,
                         "org.freedesktop.machine1",
                         "/org/freedesktop/machine1",
                         "org.freedesktop.machine1.Manager",
-                        "CleanPool",
-                        &error,
-                        &reply,
-                        "s", arg_all ? "all" : "hidden");
+                        "CleanPool");
+        if (r < 0)
+                return bus_log_create_error(r);
+
+        r = sd_bus_message_append(m, "s", arg_all ? "all" : "hidden");
+        if (r < 0)
+                return bus_log_create_error(r);
+
+        /* This is a slow operation, hence permit a longer time for completion. */
+        r = sd_bus_call(bus, m, USEC_INFINITY, &error, &reply);
         if (r < 0)
                 return log_error_errno(r, "Could not clean pool: %s", bus_error_message(&error, r));
 
diff --git a/src/machine/machined-dbus.c b/src/machine/machined-dbus.c
index 31efa3695b..52ce83a185 100644
--- a/src/machine/machined-dbus.c
+++ b/src/machine/machined-dbus.c
@@ -29,6 +29,7 @@
 #include "bus-util.h"
 #include "cgroup-util.h"
 #include "fd-util.h"
+#include "fileio.h"
 #include "formats-util.h"
 #include "hostname-util.h"
 #include "image-dbus.h"
@@ -822,22 +823,106 @@ static int method_mark_image_read_only(sd_bus_message *message, void *userdata,
         return bus_image_method_mark_read_only(message, i, error);
 }
 
+static int clean_pool_done(Operation *operation, int ret, sd_bus_error *error) {
+        _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL;
+        _cleanup_fclose_ FILE *f = NULL;
+        bool success;
+        size_t n;
+        int r;
+
+        assert(operation);
+        assert(operation->extra_fd >= 0);
+
+        if (lseek(operation->extra_fd, 0, SEEK_SET) == (off_t) -1)
+                return -errno;
+
+        f = fdopen(operation->extra_fd, "re");
+        if (!f)
+                return -errno;
+
+        operation->extra_fd = -1;
+
+        /* The resulting temporary file starts with a boolean value that indicates success or not. */
+        errno = 0;
+        n = fread(&success, 1, sizeof(success), f);
+        if (n != sizeof(success))
+                return ret < 0 ? ret : (errno != 0 ? -errno : -EIO);
+
+        if (ret < 0) {
+                _cleanup_free_ char *name = NULL;
+
+                /* The clean-up operation failed. In this case the resulting temporary file should contain a boolean
+                 * set to false followed by the name of the failed image. Let's try to read this and use it for the
+                 * error message. If we can't read it, don't mind, and return the naked error. */
+
+                if (success) /* The resulting temporary file could not be updated, ignore it. */
+                        return ret;
+
+                r = read_nul_string(f, &name);
+                if (r < 0 || isempty(name)) /* Same here... */
+                        return ret;
+
+                return sd_bus_error_set_errnof(error, ret, "Failed to remove image %s: %m", name);
+        }
+
+        assert(success);
+
+        r = sd_bus_message_new_method_return(operation->message, &reply);
+        if (r < 0)
+                return r;
+
+        r = sd_bus_message_open_container(reply, 'a', "(st)");
+        if (r < 0)
+                return r;
+
+        /* On success the resulting temporary file will contain a list of image names that were removed followed by
+         * their size on disk. Let's read that and turn it into a bus message. */
+        for (;;) {
+                _cleanup_free_ char *name = NULL;
+                uint64_t size;
+
+                r = read_nul_string(f, &name);
+                if (r < 0)
+                        return r;
+                if (isempty(name)) /* reached the end */
+                        break;
+
+                errno = 0;
+                n = fread(&size, 1, sizeof(size), f);
+                if (n != sizeof(size))
+                        return errno != 0 ? -errno : -EIO;
+
+                r = sd_bus_message_append(reply, "(st)", name, size);
+                if (r < 0)
+                        return r;
+        }
+
+        r = sd_bus_message_close_container(reply);
+        if (r < 0)
+                return r;
+
+        return sd_bus_send(NULL, reply, NULL);
+}
+
 static int method_clean_pool(sd_bus_message *message, void *userdata, sd_bus_error *error) {
         enum {
                 REMOVE_ALL,
                 REMOVE_HIDDEN,
         } mode;
 
-        _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL;
-        _cleanup_(image_hashmap_freep) Hashmap *images = NULL;
+        _cleanup_close_pair_ int errno_pipe_fd[2] = { -1, -1 };
+        _cleanup_close_ int result_fd = -1;
         Manager *m = userdata;
-        Image *image;
+        Operation *operation;
         const char *mm;
-        Iterator i;
+        pid_t child;
         int r;
 
         assert(message);
 
+        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, "s", &mm);
         if (r < 0)
                 return r;
@@ -863,50 +948,109 @@ static int method_clean_pool(sd_bus_message *message, void *userdata, sd_bus_err
         if (r == 0)
                 return 1; /* Will call us back */
 
-        images = hashmap_new(&string_hash_ops);
-        if (!images)
-                return -ENOMEM;
+        if (pipe2(errno_pipe_fd, O_CLOEXEC|O_NONBLOCK) < 0)
+                return sd_bus_error_set_errnof(error, errno, "Failed to create pipe: %m");
 
-        r = image_discover(images);
-        if (r < 0)
-                return r;
+        /* Create a temporary file we can dump information about deleted images into. We use a temporary file for this
+         * instead of a pipe or so, since this might grow quit large in theory and we don't want to process this
+         * continously */
+        result_fd = open_tmpfile_unlinkable("/tmp/", O_RDWR|O_CLOEXEC);
+        if (result_fd < 0)
+                return -errno;
 
-        r = sd_bus_message_new_method_return(message, &reply);
-        if (r < 0)
-                return r;
+        /* This might be a slow operation, run it asynchronously in a background process */
+        child = fork();
+        if (child < 0)
+                return sd_bus_error_set_errnof(error, errno, "Failed to fork(): %m");
 
-        r = sd_bus_message_open_container(reply, 'a', "(st)");
-        if (r < 0)
-                return r;
+        if (child == 0) {
+                _cleanup_(image_hashmap_freep) Hashmap *images = NULL;
+                bool success = true;
+                Image *image;
+                Iterator i;
+                ssize_t l;
 
-        HASHMAP_FOREACH(image, images, i) {
+                errno_pipe_fd[0] = safe_close(errno_pipe_fd[0]);
 
-                /* We can't remove vendor images (i.e. those in /usr) */
-                if (IMAGE_IS_VENDOR(image))
-                        continue;
+                images = hashmap_new(&string_hash_ops);
+                if (!images) {
+                        r = -ENOMEM;
+                        goto child_fail;
+                }
 
-                if (IMAGE_IS_HOST(image))
-                        continue;
+                r = image_discover(images);
+                if (r < 0)
+                        goto child_fail;
 
-                if (mode == REMOVE_HIDDEN && !IMAGE_IS_HIDDEN(image))
-                        continue;
+                l = write(result_fd, &success, sizeof(success));
+                if (l < 0) {
+                        r = -errno;
+                        goto child_fail;
+                }
 
-                r = image_remove(image);
-                if (r == -EBUSY) /* keep images that are currently being used. */
-                        continue;
-                if (r < 0)
-                        return sd_bus_error_set_errnof(error, r, "Failed to remove image %s: %m", image->name);
+                HASHMAP_FOREACH(image, images, i) {
 
-                r = sd_bus_message_append(reply, "(st)", image->name, image->usage_exclusive);
-                if (r < 0)
-                        return r;
+                        /* We can't remove vendor images (i.e. those in /usr) */
+                        if (IMAGE_IS_VENDOR(image))
+                                continue;
+
+                        if (IMAGE_IS_HOST(image))
+                                continue;
+
+                        if (mode == REMOVE_HIDDEN && !IMAGE_IS_HIDDEN(image))
+                                continue;
+
+                        r = image_remove(image);
+                        if (r == -EBUSY) /* keep images that are currently being used. */
+                                continue;
+                        if (r < 0) {
+                                /* If the operation failed, let's override everything we wrote, and instead write there at which image we failed. */
+                                success = false;
+                                (void) ftruncate(result_fd, 0);
+                                (void) lseek(result_fd, 0, SEEK_SET);
+                                (void) write(result_fd, &success, sizeof(success));
+                                (void) write(result_fd, image->name, strlen(image->name)+1);
+                                goto child_fail;
+                        }
+
+                        l = write(result_fd, image->name, strlen(image->name)+1);
+                        if (l < 0) {
+                                r = -errno;
+                                goto child_fail;
+                        }
+
+                        l = write(result_fd, &image->usage_exclusive, sizeof(image->usage_exclusive));
+                        if (l < 0) {
+                                r = -errno;
+                                goto child_fail;
+                        }
+                }
+
+                result_fd = safe_close(result_fd);
+                _exit(EXIT_SUCCESS);
+
+        child_fail:
+                (void) write(errno_pipe_fd[1], &r, sizeof(r));
+                _exit(EXIT_FAILURE);
         }
 
-        r = sd_bus_message_close_container(reply);
-        if (r < 0)
+        errno_pipe_fd[1] = safe_close(errno_pipe_fd[1]);
+
+        /* 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);
+        if (r < 0) {
+                (void) sigkill_wait(child);
                 return r;
+        }
 
-        return sd_bus_send(NULL, reply, NULL);
+        operation->extra_fd = result_fd;
+        operation->done = clean_pool_done;
+
+        result_fd = -1;
+        errno_pipe_fd[0] = -1;
+
+        return 1;
 }
 
 static int method_set_pool_limit(sd_bus_message *message, void *userdata, sd_bus_error *error) {
diff --git a/src/machine/operation.c b/src/machine/operation.c
index e6ddc41a55..8f8321a8b3 100644
--- a/src/machine/operation.c
+++ b/src/machine/operation.c
@@ -41,18 +41,33 @@ static int operation_done(sd_event_source *s, const siginfo_t *si, void *userdat
                 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.");
-
+        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_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");
+        if (o->done) {
+                /* A completion routine is set for this operation, call it. */
+                r = o->done(o, r, &error);
+                if (r < 0) {
+                        if (!sd_bus_error_is_set(&error))
+                                sd_bus_error_set_errno(&error, r);
+
+                        goto fail;
+                }
+
+        } else {
+                /* The default default operaton when done is to simply return an error on failure or an empty success
+                 * message on success. */
+                if (r < 0)
+                        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;
@@ -66,7 +81,7 @@ fail:
         return 0;
 }
 
-int operation_new(Manager *manager, Machine *machine, 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 **ret) {
         Operation *o;
         int r;
 
@@ -79,6 +94,8 @@ int operation_new(Manager *manager, Machine *machine, pid_t child, sd_bus_messag
         if (!o)
                 return -ENOMEM;
 
+        o->extra_fd = -1;
+
         r = sd_event_add_child(manager->event, &o->event_source, child, WEXITED, operation_done, o);
         if (r < 0) {
                 free(o);
@@ -102,6 +119,9 @@ int operation_new(Manager *manager, Machine *machine, pid_t child, sd_bus_messag
 
         /* At this point we took ownership of both the child and the errno file descriptor! */
 
+        if (ret)
+                *ret = o;
+
         return 0;
 }
 
@@ -112,6 +132,7 @@ Operation *operation_free(Operation *o) {
         sd_event_source_unref(o->event_source);
 
         safe_close(o->errno_fd);
+        safe_close(o->extra_fd);
 
         if (o->pid > 1)
                 (void) sigkill_wait(o->pid);
diff --git a/src/machine/operation.h b/src/machine/operation.h
index 7ca47bc3af..9831b123d7 100644
--- a/src/machine/operation.h
+++ b/src/machine/operation.h
@@ -38,10 +38,12 @@ struct Operation {
         pid_t pid;
         sd_bus_message *message;
         int errno_fd;
+        int extra_fd;
         sd_event_source *event_source;
+        int (*done)(Operation *o, int ret, sd_bus_error *error);
         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);
+int operation_new(Manager *manager, Machine *machine, pid_t child, sd_bus_message *message, int errno_fd, Operation **ret);
 Operation *operation_free(Operation *o);