]> git.ipfire.org Git - thirdparty/systemd.git/commitdiff
machined: "machinectl clean" can take a while, do it asynchronously from a background...
authorLennart Poettering <lennart@poettering.net>
Fri, 24 Jun 2016 14:01:14 +0000 (16:01 +0200)
committerLennart Poettering <lennart@poettering.net>
Fri, 24 Jun 2016 14:01:14 +0000 (16:01 +0200)
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.

src/basic/fileio.c
src/basic/fileio.h
src/machine/image-dbus.c
src/machine/machine-dbus.c
src/machine/machinectl.c
src/machine/machined-dbus.c
src/machine/operation.c
src/machine/operation.h

index 29f53742229e53abe5eb25a7d793de296953ecf8..0360a8eab3b12dfa20b20e4bc4026bf826fb9032 100644 (file)
@@ -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;
+}
index 58dbc80c24718f9756e8cc38f43d83b399419d1b..9ac497d9ebccd8bf4cb6ea88a09c0651eaaba6b1 100644 (file)
@@ -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);
index 0eed9b81bb5a5b32a06c4d2f669d85875d240d47..867bbc467bbd978318d0b52a726bd57ee670e017 100644 (file)
@@ -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;
index f50f363ba30a18da96d0bee414554776ea4add56..ba7ac04b569c20cd4dc0428fac8a1beae580a31e 100644 (file)
@@ -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;
index 5ca557abbff3fc888c92f706767bfb6e9bf80145..d68c50b203d5c5d1b6ff92ce5f8f99d17947d260 100644 (file)
@@ -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));
 
index 31efa3695bba9cb60129fdcb9f33924d9d9f5d86..52ce83a18537e6949b4e1dc7d260c8bf228d2386 100644 (file)
@@ -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) {
index e6ddc41a555067219a04410d76bef1e84e810499..8f8321a8b3fa8b0edd738a12b1b0ef4dc73eb6b5 100644 (file)
@@ -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);
index 7ca47bc3af5a011136ddd75eb7fec6a96ba7c167..9831b123d77e790365c20d048640b137f2d99c3c 100644 (file)
@@ -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);