Merge pull request #3596 from poettering/machine-clean

make "machinectl clean" asynchronous, and open it up via PolicyKit

src/basic/fileio.c

@@ -1354,3 +1354,44 @@ int link_tmpfile(int fd, const char *path, const char *target) {
 
         return 0;
 }
+
+int read_nul_string(FILE *f, char **ret) {
+        _cleanup_free_ char *x = NULL;
+        size_t allocated = 0, n = 0;
+
+        assert(f);
+        assert(ret);
+
+        /* Reads a NUL-terminated string from the specified file. */
+
+        for (;;) {
+                int c;
+
+                if (!GREEDY_REALLOC(x, allocated, n+2))
+                        return -ENOMEM;
+
+                c = fgetc(f);
+                if (c == 0) /* Terminate at NUL byte */
+                        break;
+                if (c == EOF) {
+                        if (ferror(f))
+                                return -errno;
+                        break; /* Terminate at EOF */
+                }
+
+                x[n++] = (char) c;
+        }
+
+        if (x)
+                x[n] = 0;
+        else {
+                x = new0(char, 1);
+                if (!x)
+                        return -ENOMEM;
+        }
+
+        *ret = x;
+        x = NULL;
+
+        return 0;
+}

src/basic/fileio.h

@@ -86,3 +86,5 @@ int open_tmpfile_unlinkable(const char *directory, int flags);
 int open_tmpfile_linkable(const char *target, int flags, char **ret_path);
 
 int link_tmpfile(int fd, const char *path, const char *target);
+
+int read_nul_string(FILE *f, char **ret);

src/core/cgroup.c

@@ -36,8 +36,7 @@
 
 #define CGROUP_CPU_QUOTA_PERIOD_USEC ((usec_t) 100 * USEC_PER_MSEC)
 
-static void cgroup_compat_warn(void)
-{
+static void cgroup_compat_warn(void) {
         static bool cgroup_compat_warned = false;
 
         if (cgroup_compat_warned)
@@ -50,7 +49,7 @@ static void cgroup_compat_warn(void)
 #define log_cgroup_compat(unit, fmt, ...) do {                                  \
                 cgroup_compat_warn();                                           \
                 log_unit_debug(unit, "cgroup-compat: " fmt, ##__VA_ARGS__);     \
-        } while (0)
+        } while (false)
 
 void cgroup_context_init(CGroupContext *c) {
         assert(c);

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;

src/machine/machine-dbus.c

@@ -1200,8 +1200,6 @@ int bus_machine_method_copy(sd_bus_message *message, void *userdata, sd_bus_erro
 
         child_fail:
                 (void) write(errno_pipe_fd[1], &r, sizeof(r));
-                errno_pipe_fd[1] = safe_close(errno_pipe_fd[1]);
-
                 _exit(EXIT_FAILURE);
         }
 
@@ -1209,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;

src/machine/machinectl.c

@@ -2418,7 +2418,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];
@@ -2427,15 +2427,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));
 

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;
-
-                if (mode == REMOVE_HIDDEN && !IMAGE_IS_HIDDEN(image))
-                        continue;
-
-                r = image_remove(image);
-                if (r == -EBUSY) /* keep images that are currently being used. */
-                        continue;
+                r = image_discover(images);
                 if (r < 0)
-                        return sd_bus_error_set_errnof(error, r, "Failed to remove image %s: %m", image->name);
+                        goto child_fail;
 
-                r = sd_bus_message_append(reply, "(st)", image->name, image->usage_exclusive);
-                if (r < 0)
-                        return r;
+                l = write(result_fd, &success, sizeof(success));
+                if (l < 0) {
+                        r = -errno;
+                        goto child_fail;
+                }
+
+                HASHMAP_FOREACH(image, images, i) {
+
+                        /* 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)
-                return r;
+        errno_pipe_fd[1] = safe_close(errno_pipe_fd[1]);
 
-        return sd_bus_send(NULL, reply, NULL);
+        /* 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;
+        }
+
+        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) {

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);

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);

src/machine/org.freedesktop.machine1.conf

@@ -116,6 +116,10 @@
                        send_interface="org.freedesktop.machine1.Manager"
                        send_member="SetImageLimit"/>
 
+                <allow send_destination="org.freedesktop.machine1"
+                       send_interface="org.freedesktop.machine1.Manager"
+                       send_member="CleanPool"/>
+
                 <allow send_destination="org.freedesktop.machine1"
                        send_interface="org.freedesktop.machine1.Manager"
                        send_member="MapFromMachineUser"/>