[systemd-commits] 13 commits - TODO src/libsystemd src/login src/machine src/nspawn src/shared src/sysv-generator

Lennart Poettering lennart at kemper.freedesktop.org
Wed Jan 14 14:18:41 PST 2015


 TODO                                |    4 
 src/libsystemd/sd-bus/bus-dump.c    |    8 -
 src/login/loginctl.c                |   13 ++
 src/machine/machinectl.c            |    4 
 src/nspawn/nspawn.c                 |   62 +++++++++---
 src/shared/copy.c                   |    4 
 src/shared/copy.h                   |    4 
 src/shared/machine-image.c          |  174 +++++++++++++++++++++++++++++++++--
 src/shared/machine-image.h          |    5 +
 src/shared/pty.c                    |    9 -
 src/shared/ptyfwd.c                 |    4 
 src/shared/util.c                   |  178 +++++++++++++++++++++++++++++++-----
 src/shared/util.h                   |   22 +++-
 src/sysv-generator/sysv-generator.c |   30 +++---
 14 files changed, 440 insertions(+), 81 deletions(-)

New commits:
commit 435fdcef52087b99a2bef745eb07a1228ed6d993
Author: Lennart Poettering <lennart at poettering.net>
Date:   Wed Jan 14 23:18:24 2015 +0100

    update TODO

diff --git a/TODO b/TODO
index d59c5a2..c56579a 100644
--- a/TODO
+++ b/TODO
@@ -47,7 +47,7 @@ Release 219 preparations:
 
 Features:
 
-* machine: we should fake a read-only flag for simple directory images via a flag file
+* rename "gpt" image type to "raw"
 
 * import: support import from local files, and export to local files
 
@@ -65,8 +65,6 @@ Features:
 
 * introduce systemd-nspawn-ephemeral at .service, and hook it into "machinectl start" with a new --ephemeral switch
 
-* nspawn should lock container images while running off them
-
 * logind,machined: add generic catch-all polkit verbs for most priviliged operations, similar to systemd itself
 
 * "machinectl status" should also show internal logs of the container in question

commit 657bdca9e4c0f0163b8c1f139e3f6734d2e0d69a
Author: Lennart Poettering <lennart at poettering.net>
Date:   Wed Jan 14 23:17:07 2015 +0100

    nspawn: fix an incorrect assert comparison

diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c
index 7f87e37..f502500 100644
--- a/src/nspawn/nspawn.c
+++ b/src/nspawn/nspawn.c
@@ -1655,7 +1655,7 @@ static int watch_rtnl(sd_event *event, int recv_fd, union in_addr_union *exposed
         cmsg = CMSG_FIRSTHDR(&mh);
         assert(cmsg->cmsg_level == SOL_SOCKET);
         assert(cmsg->cmsg_type == SCM_RIGHTS);
-        assert(cmsg->cmsg_len = CMSG_LEN(sizeof(int)));
+        assert(cmsg->cmsg_len == CMSG_LEN(sizeof(int)));
         memcpy(&fd, CMSG_DATA(cmsg), sizeof(int));
 
         r = sd_rtnl_open_fd(&rtnl, fd, 1, RTNLGRP_IPV4_IFADDR);

commit 2fbcde7402a26d365b6a8091b912154e6d187ee4
Author: Lennart Poettering <lennart at poettering.net>
Date:   Wed Jan 14 23:16:28 2015 +0100

    loginctl: fix misuse compound literals
    
    The lifetime of compound literals is bound to the local scope, we hence
    cannot refernce them outside of it.

diff --git a/src/login/loginctl.c b/src/login/loginctl.c
index 064411e..b0eede9 100644
--- a/src/login/loginctl.c
+++ b/src/login/loginctl.c
@@ -846,6 +846,7 @@ static int show_seat(int argc, char *argv[], void *userdata) {
 static int activate(int argc, char *argv[], void *userdata) {
         _cleanup_bus_error_free_ sd_bus_error error = SD_BUS_ERROR_NULL;
         sd_bus *bus = userdata;
+        char *short_argv[3];
         int r, i;
 
         assert(bus);
@@ -858,7 +859,11 @@ static int activate(int argc, char *argv[], void *userdata) {
                  * session name, which the calls will then resolve to
                  * the caller's session. */
 
-                argv = STRV_MAKE(argv[0], "");
+                short_argv[0] = argv[0];
+                short_argv[1] = (char*) "";
+                short_argv[2] = NULL;
+
+                argv = short_argv;
                 argc = 2;
         }
 
@@ -919,6 +924,7 @@ static int kill_session(int argc, char *argv[], void *userdata) {
 static int enable_linger(int argc, char *argv[], void *userdata) {
         _cleanup_bus_error_free_ sd_bus_error error = SD_BUS_ERROR_NULL;
         sd_bus *bus = userdata;
+        char* short_argv[3];
         bool b;
         int r, i;
 
@@ -930,7 +936,10 @@ static int enable_linger(int argc, char *argv[], void *userdata) {
         b = streq(argv[0], "enable-linger");
 
         if (argc < 2) {
-                argv = STRV_MAKE(argv[0], "");
+                short_argv[0] = argv[0];
+                short_argv[1] = (char*) "";
+                short_argv[2] = NULL;
+                argv = short_argv;
                 argc = 2;
         }
 

commit c7fbd99660363b74ccb3c75d280c158518bc78b3
Author: Lennart Poettering <lennart at poettering.net>
Date:   Wed Jan 14 23:16:11 2015 +0100

    sd-bus: tell Coverity that it's OK not to care for return values in some cases

diff --git a/src/libsystemd/sd-bus/bus-dump.c b/src/libsystemd/sd-bus/bus-dump.c
index 33d0ed2..afdf52f 100644
--- a/src/libsystemd/sd-bus/bus-dump.c
+++ b/src/libsystemd/sd-bus/bus-dump.c
@@ -419,16 +419,16 @@ int bus_creds_dump(sd_bus_creds *c, FILE *f, bool terse) {
 
         if (c->mask & SD_BUS_CREDS_CGROUP)
                 fprintf(f, "%sCGroup=%s%s%s", prefix, color, c->cgroup, suffix);
-        sd_bus_creds_get_unit(c, &u);
+        (void) sd_bus_creds_get_unit(c, &u);
         if (u)
                 fprintf(f, "%sUnit=%s%s%s", prefix, color, u, suffix);
-        sd_bus_creds_get_user_unit(c, &uu);
+        (void) sd_bus_creds_get_user_unit(c, &uu);
         if (uu)
                 fprintf(f, "%sUserUnit=%s%s%s", prefix, color, uu, suffix);
-        sd_bus_creds_get_slice(c, &sl);
+        (void) sd_bus_creds_get_slice(c, &sl);
         if (sl)
                 fprintf(f, "%sSlice=%s%s%s", prefix, color, sl, suffix);
-        sd_bus_creds_get_session(c, &s);
+        (void) sd_bus_creds_get_session(c, &s);
         if (s)
                 fprintf(f, "%sSession=%s%s%s", prefix, color, s, suffix);
 

commit 30535c16924a3da7b47ea87190d929d617d95c5a
Author: Lennart Poettering <lennart at poettering.net>
Date:   Wed Jan 14 23:09:02 2015 +0100

    nspawn: add file system locks for controlling access to container images
    
    This adds three kinds of file system locks for container images:
    
    a) a file system lock next to the actual image, in a .lck file in the
       same directory the image is located. This lock has the benefit of
       usually being located on the same NFS share as the image itself, and
       thus allows locking container images across NFS shares.
    
    b) a file system lock in /run, named after st_dev and st_ino of the
       root of the image. This lock has the advantage that it is unique even
       if the same image is bind mounted to two different places at the same
       time, as the ino/dev stays constant for them.
    
    c) a file system lock that is only taken when a new disk image is about
       to be created, that ensures that checking whether the name is already
       used across the search path, and actually placing the image is not
       interrupted by other code taking the name.
    
    a + b are read-write locks. When a container is booted in read-only mode
    a read lock is taken, otherwise a write lock.
    
    Lock b is always taken after a, to avoid ABBA problems.
    
    Lock c is mostly relevant when renaming or cloning images.

diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c
index 1247146..7f87e37 100644
--- a/src/nspawn/nspawn.c
+++ b/src/nspawn/nspawn.c
@@ -3337,6 +3337,7 @@ int main(int argc, char *argv[]) {
         pid_t pid = 0;
         int ret = EXIT_SUCCESS;
         union in_addr_union exposed = {};
+        _cleanup_release_lock_file_ LockFile tree_global_lock = LOCK_FILE_INIT, tree_local_lock = LOCK_FILE_INIT;
 
         log_parse_environment();
         log_open();
@@ -3382,20 +3383,8 @@ int main(int argc, char *argv[]) {
                         goto finish;
                 }
 
-                if (arg_template) {
-                        r = btrfs_subvol_snapshot(arg_template, arg_directory, arg_read_only, true);
-                        if (r == -EEXIST) {
-                                if (!arg_quiet)
-                                        log_info("Directory %s already exists, not populating from template %s.", arg_directory, arg_template);
-                        } else if (r < 0) {
-                                log_error_errno(r, "Couldn't create snapshort %s from %s: %m", arg_directory, arg_template);
-                                goto finish;
-                        } else {
-                                if (!arg_quiet)
-                                        log_info("Populated %s from template %s.", arg_directory, arg_template);
-                        }
-
-                } else if (arg_ephemeral) {
+                if (arg_ephemeral) {
+                        _cleanup_release_lock_file_ LockFile original_lock = LOCK_FILE_INIT;
                         char *np;
 
                         /* If the specified path is a mount point we
@@ -3418,6 +3407,12 @@ int main(int argc, char *argv[]) {
                                 goto finish;
                         }
 
+                        r = image_path_lock(np, (arg_read_only ? LOCK_SH : LOCK_EX) | LOCK_NB, &tree_global_lock, &tree_local_lock);
+                        if (r < 0) {
+                                log_error_errno(r, "Failed to lock %s: %m", np);
+                                goto finish;
+                        }
+
                         r = btrfs_subvol_snapshot(arg_directory, np, arg_read_only, true);
                         if (r < 0) {
                                 free(np);
@@ -3429,6 +3424,31 @@ int main(int argc, char *argv[]) {
                         arg_directory = np;
 
                         remove_subvol = true;
+
+                } else {
+                        r = image_path_lock(arg_directory, (arg_read_only ? LOCK_SH : LOCK_EX) | LOCK_NB, &tree_global_lock, &tree_local_lock);
+                        if (r == -EBUSY) {
+                                log_error_errno(r, "Directory tree %s is currently busy.", arg_directory);
+                                goto finish;
+                        }
+                        if (r < 0) {
+                                log_error_errno(r, "Failed to lock %s: %m", arg_directory);
+                                return r;
+                        }
+
+                        if (arg_template) {
+                                r = btrfs_subvol_snapshot(arg_template, arg_directory, arg_read_only, true);
+                                if (r == -EEXIST) {
+                                        if (!arg_quiet)
+                                                log_info("Directory %s already exists, not populating from template %s.", arg_directory, arg_template);
+                                } else if (r < 0) {
+                                        log_error_errno(r, "Couldn't create snapshort %s from %s: %m", arg_directory, arg_template);
+                                        goto finish;
+                                } else {
+                                        if (!arg_quiet)
+                                                log_info("Populated %s from template %s.", arg_directory, arg_template);
+                                }
+                        }
                 }
 
                 if (arg_boot) {
@@ -3455,6 +3475,16 @@ int main(int argc, char *argv[]) {
                 assert(arg_image);
                 assert(!arg_template);
 
+                r = image_path_lock(arg_image, (arg_read_only ? LOCK_SH : LOCK_EX) | LOCK_NB, &tree_global_lock, &tree_local_lock);
+                if (r == -EBUSY) {
+                        r = log_error_errno(r, "Disk image %s is currently busy.", arg_image);
+                        goto finish;
+                }
+                if (r < 0) {
+                        r = log_error_errno(r, "Failed to create image lock: %m");
+                        goto finish;
+                }
+
                 if (!mkdtemp(template)) {
                         log_error_errno(errno, "Failed to create temporary directory: %m");
                         r = -errno;
diff --git a/src/shared/machine-image.c b/src/shared/machine-image.c
index 117994d..752d658 100644
--- a/src/shared/machine-image.c
+++ b/src/shared/machine-image.c
@@ -28,6 +28,7 @@
 #include "btrfs-util.h"
 #include "path-util.h"
 #include "copy.h"
+#include "mkdir.h"
 #include "machine-image.h"
 
 static const char image_search_path[] =
@@ -340,12 +341,20 @@ void image_hashmap_free(Hashmap *map) {
 }
 
 int image_remove(Image *i) {
+        _cleanup_release_lock_file_ LockFile global_lock = LOCK_FILE_INIT, local_lock = LOCK_FILE_INIT;
+        int r;
+
         assert(i);
 
         if (path_equal(i->path, "/") ||
             path_startswith(i->path, "/usr"))
                 return -EROFS;
 
+        /* Make sure we don't interfere with a running nspawn */
+        r = image_path_lock(i->path, LOCK_EX|LOCK_NB, &global_lock, &local_lock);
+        if (r < 0)
+                return r;
+
         switch (i->type) {
 
         case IMAGE_SUBVOLUME:
@@ -366,6 +375,7 @@ int image_remove(Image *i) {
 }
 
 int image_rename(Image *i, const char *new_name) {
+        _cleanup_release_lock_file_ LockFile global_lock = LOCK_FILE_INIT, local_lock = LOCK_FILE_INIT, name_lock = LOCK_FILE_INIT;
         _cleanup_free_ char *new_path = NULL, *nn = NULL;
         unsigned file_attr = 0;
         int r;
@@ -379,6 +389,18 @@ int image_rename(Image *i, const char *new_name) {
             path_startswith(i->path, "/usr"))
                 return -EROFS;
 
+        /* Make sure we don't interfere with a running nspawn */
+        r = image_path_lock(i->path, LOCK_EX|LOCK_NB, &global_lock, &local_lock);
+        if (r < 0)
+                return r;
+
+        /* Make sure nobody takes the new name, between the time we
+         * checked it is currently unused in all search paths, and the
+         * time we take possesion of it */
+        r = image_name_lock(new_name, LOCK_EX|LOCK_NB, &name_lock);
+        if (r < 0)
+                return r;
+
         r = image_find(new_name, NULL);
         if (r < 0)
                 return r;
@@ -438,6 +460,7 @@ int image_rename(Image *i, const char *new_name) {
 }
 
 int image_clone(Image *i, const char *new_name, bool read_only) {
+        _cleanup_release_lock_file_ LockFile name_lock = LOCK_FILE_INIT;
         const char *new_path;
         int r;
 
@@ -446,6 +469,13 @@ int image_clone(Image *i, const char *new_name, bool read_only) {
         if (!image_name_is_valid(new_name))
                 return -EINVAL;
 
+        /* Make sure nobody takes the new name, between the time we
+         * checked it is currently unused in all search paths, and the
+         * time we take possesion of it */
+        r = image_name_lock(new_name, LOCK_EX|LOCK_NB, &name_lock);
+        if (r < 0)
+                return r;
+
         r = image_find(new_name, NULL);
         if (r < 0)
                 return r;
@@ -478,6 +508,7 @@ int image_clone(Image *i, const char *new_name, bool read_only) {
 }
 
 int image_read_only(Image *i, bool b) {
+        _cleanup_release_lock_file_ LockFile global_lock = LOCK_FILE_INIT, local_lock = LOCK_FILE_INIT;
         int r;
         assert(i);
 
@@ -485,6 +516,11 @@ int image_read_only(Image *i, bool b) {
             path_startswith(i->path, "/usr"))
                 return -EROFS;
 
+        /* Make sure we don't interfere with a running nspawn */
+        r = image_path_lock(i->path, LOCK_EX|LOCK_NB, &global_lock, &local_lock);
+        if (r < 0)
+                return r;
+
         switch (i->type) {
 
         case IMAGE_SUBVOLUME:
@@ -533,6 +569,88 @@ int image_read_only(Image *i, bool b) {
         return 0;
 }
 
+int image_path_lock(const char *path, int operation, LockFile *global, LockFile *local) {
+        _cleanup_free_ char *p = NULL;
+        LockFile t = LOCK_FILE_INIT;
+        struct stat st;
+        int r;
+
+        assert(path);
+        assert(global);
+        assert(local);
+
+        /* Locks an image path. This actually creates two locks: one
+         * "local" one, next to the image path itself, which might be
+         * shared via NFS. And another "global" one, in /run, that
+         * uses the device/inode number. This has the benefit that we
+         * can even lock a tree that is a mount point, correctly. */
+
+        if (path_equal(path, "/"))
+                return -EBUSY;
+
+        if (!path_is_absolute(path))
+                return -EINVAL;
+
+        if (stat(path, &st) >= 0) {
+                if (asprintf(&p, "/run/systemd/nspawn/locks/inode-%lu:%lu", (unsigned long) st.st_dev, (unsigned long) st.st_ino) < 0)
+                        return -ENOMEM;
+        }
+
+        r = make_lock_file_for(path, operation, &t);
+        if (r < 0)
+                return r;
+
+        if (p) {
+                mkdir_p("/run/systemd/nspawn/locks", 0600);
+
+                r = make_lock_file(p, operation, global);
+                if (r < 0) {
+                        release_lock_file(&t);
+                        return r;
+                }
+        }
+
+        *local = t;
+        return 0;
+}
+
+int image_name_lock(const char *name, int operation, LockFile *ret) {
+        const char *p;
+
+        assert(name);
+        assert(ret);
+
+        /* Locks an image name, regardless of the precise path used. */
+
+        if (!image_name_is_valid(name))
+                return -EINVAL;
+
+        if (streq(name, ".host"))
+                return -EBUSY;
+
+        mkdir_p("/run/systemd/nspawn/locks", 0600);
+        p = strappenda("/run/systemd/nspawn/locks/name-", name);
+
+        return make_lock_file(p, operation, ret);
+}
+
+bool image_name_is_valid(const char *s) {
+        if (!filename_is_valid(s))
+                return false;
+
+        if (string_has_cc(s, NULL))
+                return false;
+
+        if (!utf8_is_valid(s))
+                return false;
+
+        /* Temporary files for atomically creating new files */
+        if (startswith(s, ".#"))
+                return false;
+
+        return true;
+}
+
 static const char* const image_type_table[_IMAGE_TYPE_MAX] = {
         [IMAGE_DIRECTORY] = "directory",
         [IMAGE_SUBVOLUME] = "subvolume",
diff --git a/src/shared/machine-image.h b/src/shared/machine-image.h
index 10e5d0a..4f41b4f 100644
--- a/src/shared/machine-image.h
+++ b/src/shared/machine-image.h
@@ -63,3 +63,8 @@ int image_read_only(Image *i, bool b);
 
 const char* image_type_to_string(ImageType t) _const_;
 ImageType image_type_from_string(const char *s) _pure_;
+
+bool image_name_is_valid(const char *s) _pure_;
+
+int image_path_lock(const char *path, int operation, LockFile *global, LockFile *local);
+int image_name_lock(const char *name, int operation, LockFile *ret);
diff --git a/src/shared/util.c b/src/shared/util.c
index 857bb1b..884e782 100644
--- a/src/shared/util.c
+++ b/src/shared/util.c
@@ -62,6 +62,7 @@
 #include <sys/xattr.h>
 #include <libgen.h>
 #include <sys/statvfs.h>
+#include <sys/file.h>
 #include <linux/fs.h>
 #undef basename
 
@@ -4303,23 +4304,6 @@ bool machine_name_is_valid(const char *s) {
         return true;
 }
 
-bool image_name_is_valid(const char *s) {
-        if (!filename_is_valid(s))
-                return false;
-
-        if (string_has_cc(s, NULL))
-                return false;
-
-        if (!utf8_is_valid(s))
-                return false;
-
-        /* Temporary files for atomically creating new files */
-        if (startswith(s, ".#"))
-                return false;
-
-        return true;
-}
-
 int pipe_eof(int fd) {
         struct pollfd pollfd = {
                 .fd = fd,
@@ -7819,3 +7803,125 @@ int read_attr_path(const char *p, unsigned *ret) {
 
         return read_attr_fd(fd, ret);
 }
+
+int make_lock_file(const char *p, int operation, LockFile *ret) {
+        _cleanup_close_ int fd = -1;
+        _cleanup_free_ char *t = NULL;
+        int r;
+
+        /*
+         * We use UNPOSIX locks if they are available. They have nice
+         * semantics, and are mostly compatible with NFS. However,
+         * they are only available on new kernels. When we detect we
+         * are running on an older kernel, then we fall back to good
+         * old BSD locks. They also have nice semantics, but are
+         * slightly problematic on NFS, where they are upgraded to
+         * POSIX locks, even though locally they are orthogonal to
+         * POSIX locks.
+         */
+
+        t = strdup(p);
+        if (!t)
+                return -ENOMEM;
+
+        for (;;) {
+                struct flock fl = {
+                        .l_type = (operation & ~LOCK_NB) == LOCK_EX ? F_WRLCK : F_RDLCK,
+                        .l_whence = SEEK_SET,
+                };
+                struct stat st;
+
+                fd = open(p, O_CREAT|O_RDWR|O_NOFOLLOW|O_CLOEXEC|O_NOCTTY, 0600);
+                if (fd < 0)
+                        return -errno;
+
+                r = fcntl(fd, (operation & LOCK_NB) ? F_OFD_SETLK : F_OFD_SETLKW, &fl);
+                if (r < 0) {
+
+                        /* If the kernel is too old, use good old BSD locks */
+                        if (errno == EINVAL)
+                                r = flock(fd, operation);
+
+                        if (r < 0)
+                                return errno == EAGAIN ? -EBUSY : -errno;
+                }
+
+                /* If we acquired the lock, let's check if the file
+                 * still exists in the file system. If not, then the
+                 * previous exclusive owner removed it and then closed
+                 * it. In such a case our acquired lock is worthless,
+                 * hence try again. */
+
+                r = fstat(fd, &st);
+                if (r < 0)
+                        return -errno;
+                if (st.st_nlink > 0)
+                        break;
+
+                fd = safe_close(fd);
+        }
+
+        ret->path = t;
+        ret->fd = fd;
+        ret->operation = operation;
+
+        fd = -1;
+        t = NULL;
+
+        return r;
+}
+
+int make_lock_file_for(const char *p, int operation, LockFile *ret) {
+        const char *fn;
+        char *t;
+
+        assert(p);
+        assert(ret);
+
+        fn = basename(p);
+        if (!filename_is_valid(fn))
+                return -EINVAL;
+
+        t = newa(char, strlen(p) + 2 + 4 + 1);
+        stpcpy(stpcpy(stpcpy(mempcpy(t, p, fn - p), ".#"), fn), ".lck");
+
+        return make_lock_file(t, operation, ret);
+}
+
+void release_lock_file(LockFile *f) {
+        int r;
+
+        if (!f)
+                return;
+
+        if (f->path) {
+
+                /* If we are the exclusive owner we can safely delete
+                 * the lock file itself. If we are not the exclusive
+                 * owner, we can try becoming it. */
+
+                if (f->fd >= 0 &&
+                    (f->operation & ~LOCK_NB) == LOCK_SH) {
+                        static const struct flock fl = {
+                                .l_type = F_WRLCK,
+                                .l_whence = SEEK_SET,
+                        };
+
+                        r = fcntl(f->fd, F_OFD_SETLK, &fl);
+                        if (r < 0 && errno == EINVAL)
+                                r = flock(f->fd, LOCK_EX|LOCK_NB);
+
+                        if (r >= 0)
+                                f->operation = LOCK_EX|LOCK_NB;
+                }
+
+                if ((f->operation & ~LOCK_NB) == LOCK_EX)
+                        unlink_noerrno(f->path);
+
+                free(f->path);
+                f->path = NULL;
+        }
+
+        f->fd = safe_close(f->fd);
+        f->operation = 0;
+}
diff --git a/src/shared/util.h b/src/shared/util.h
index 5d9637e..fdb9fb6 100644
--- a/src/shared/util.h
+++ b/src/shared/util.h
@@ -550,7 +550,6 @@ bool hostname_is_valid(const char *s) _pure_;
 char* hostname_cleanup(char *s, bool lowercase);
 
 bool machine_name_is_valid(const char *s) _pure_;
-bool image_name_is_valid(const char *s) _pure_;
 
 char* strshorten(char *s, size_t l);
 
@@ -1080,4 +1079,18 @@ int chattr_path(const char *p, bool b, unsigned mask);
 int read_attr_fd(int fd, unsigned *ret);
 int read_attr_path(const char *p, unsigned *ret);
 
+typedef struct LockFile {
+        char *path;
+        int fd;
+        int operation;
+} LockFile;
+
+int make_lock_file(const char *p, int operation, LockFile *ret);
+int make_lock_file_for(const char *p, int operation, LockFile *ret);
+void release_lock_file(LockFile *f);
+
+#define _cleanup_release_lock_file_ _cleanup_(release_lock_file)
+
+#define LOCK_FILE_INIT { .fd = -1, .path = NULL }
+
 #define RLIMIT_MAKE_CONST(lim) ((struct rlimit) { lim, lim })

commit 805e5dda0a01c99d231824e1a9c4a208418bf342
Author: Lennart Poettering <lennart at poettering.net>
Date:   Wed Jan 14 22:37:56 2015 +0100

    sysv-generator: always use fstatat() if we can

diff --git a/src/sysv-generator/sysv-generator.c b/src/sysv-generator/sysv-generator.c
index 89c0e7c..4774981 100644
--- a/src/sysv-generator/sysv-generator.c
+++ b/src/sysv-generator/sysv-generator.c
@@ -722,28 +722,33 @@ static int enumerate_sysv(LookupPaths lp, Hashmap *all_services) {
                 }
 
                 while ((de = readdir(d))) {
-                        SysvStub *service;
-                        struct stat st;
                         _cleanup_free_ char *fpath = NULL, *name = NULL;
+                        _cleanup_free_ SysvStub *service = NULL;
+                        struct stat st;
                         int r;
 
                         if (hidden_file(de->d_name))
                                 continue;
 
-                        fpath = strjoin(*path, "/", de->d_name, NULL);
-                        if (!fpath)
-                                return log_oom();
-
-                        if (stat(fpath, &st) < 0)
+                        if (fstatat(dirfd(d), de->d_name, &st, 0) < 0) {
+                                log_warning_errno(errno, "stat() failed on %s/%s: %m", *path, de->d_name);
                                 continue;
+                        }
 
                         if (!(st.st_mode & S_IXUSR))
                                 continue;
 
+                        if (!S_ISREG(st.st_mode))
+                                continue;
+
                         name = sysv_translate_name(de->d_name);
                         if (!name)
                                 return log_oom();
 
+                        fpath = strjoin(*path, "/", de->d_name, NULL);
+                        if (!fpath)
+                                return log_oom();
+
                         if (hashmap_contains(all_services, name))
                                 continue;
 
@@ -756,12 +761,11 @@ static int enumerate_sysv(LookupPaths lp, Hashmap *all_services) {
                         service->path = fpath;
 
                         r = hashmap_put(all_services, service->name, service);
-                        if (r < 0) {
-                                free(service);
+                        if (r < 0)
                                 return log_oom();
-                        }
 
                         name = fpath = NULL;
+                        service = NULL;
                 }
         }
 

commit b3fae863ef548add2d01c3956ce7720f4eeeca7e
Author: Lennart Poettering <lennart at poettering.net>
Date:   Wed Jan 14 22:31:03 2015 +0100

    sysv-generator: fix memory leak on failure
    
    This fixes a memory leak introduced by
    1ed0c19f81fd13cdf283c6def0168ce122a853a9

diff --git a/src/sysv-generator/sysv-generator.c b/src/sysv-generator/sysv-generator.c
index 2f24ef2..89c0e7c 100644
--- a/src/sysv-generator/sysv-generator.c
+++ b/src/sysv-generator/sysv-generator.c
@@ -755,13 +755,11 @@ static int enumerate_sysv(LookupPaths lp, Hashmap *all_services) {
                         service->name = name;
                         service->path = fpath;
 
-                        r = load_sysv(service);
-                        if (r < 0)
-                                continue;
-
                         r = hashmap_put(all_services, service->name, service);
-                        if (r < 0)
+                        if (r < 0) {
+                                free(service);
                                 return log_oom();
+                        }
 
                         name = fpath = NULL;
                 }
@@ -943,6 +941,12 @@ int main(int argc, char *argv[]) {
         }
 
         HASHMAP_FOREACH(service, all_services, j) {
+                q = load_sysv(service);
+                if (q < 0)
+                        continue;
+        }
+
+        HASHMAP_FOREACH(service, all_services, j) {
                 q = fix_order(service, all_services);
                 if (q < 0)
                         continue;

commit bb4a228207815df88cdf68acd9e46ec19e0d3e30
Author: Lennart Poettering <lennart at poettering.net>
Date:   Wed Jan 14 22:30:43 2015 +0100

    machinectl: fix minor memory leak

diff --git a/src/machine/machinectl.c b/src/machine/machinectl.c
index 980fba1..7ea991a 100644
--- a/src/machine/machinectl.c
+++ b/src/machine/machinectl.c
@@ -1065,11 +1065,11 @@ static int copy_files(int argc, char *argv[], void *userdata) {
                 return -EINVAL;
         }
 
-        t = strdup(host_path);
+        t = strdupa(host_path);
         host_basename = basename(t);
         host_dirname = dirname(host_path);
 
-        t = strdup(container_path);
+        t = strdupa(container_path);
         container_basename = basename(t);
         container_dirname = dirname(container_path);
 

commit aa0fff7f9c9fce89210ec764dd892afbdc28c571
Author: Lennart Poettering <lennart at poettering.net>
Date:   Wed Jan 14 02:22:27 2015 +0100

    pty: minor modernization
    
    We initialize structs during declartion if possible

diff --git a/src/shared/pty.c b/src/shared/pty.c
index 6863be6..fbe6295 100644
--- a/src/shared/pty.c
+++ b/src/shared/pty.c
@@ -550,16 +550,15 @@ int pty_signal(Pty *pty, int sig) {
 }
 
 int pty_resize(Pty *pty, unsigned short term_width, unsigned short term_height) {
-        struct winsize ws;
+        struct winsize ws = {
+                .ws_col = term_width,
+                .ws_row = term_height,
+        };
 
         assert_return(pty, -EINVAL);
         assert_return(pty_is_open(pty), -ENODEV);
         assert_return(pty_is_parent(pty), -ENODEV);
 
-        zero(ws);
-        ws.ws_col = term_width;
-        ws.ws_row = term_height;
-
         /*
          * This will send SIGWINCH to the pty slave foreground process group.
          * We will also get one, but we don't need it.

commit 01b725684f6ba54f0db815669e4e07eb0e02fedb
Author: Lennart Poettering <lennart at poettering.net>
Date:   Wed Jan 14 02:21:51 2015 +0100

    machined: use the FS_IMMUTABLE_FL file flag, if available, to implement a "read-only" concept for raw disk images, too

diff --git a/src/shared/machine-image.c b/src/shared/machine-image.c
index 25689ca..117994d 100644
--- a/src/shared/machine-image.c
+++ b/src/shared/machine-image.c
@@ -120,6 +120,8 @@ static int image_make(
                 (faccessat(dfd, filename, W_OK, AT_EACCESS) < 0 && errno == EROFS);
 
         if (S_ISDIR(st.st_mode)) {
+                _cleanup_close_ int fd = -1;
+                unsigned file_attr = 0;
 
                 if (!ret)
                         return 1;
@@ -127,15 +129,14 @@ static int image_make(
                 if (!pretty)
                         pretty = filename;
 
+                fd = openat(dfd, filename, O_CLOEXEC|O_NOCTTY|O_DIRECTORY);
+                if (fd < 0)
+                        return -errno;
+
                 /* btrfs subvolumes have inode 256 */
                 if (st.st_ino == 256) {
-                        _cleanup_close_ int fd = -1;
                         struct statfs sfs;
 
-                        fd = openat(dfd, filename, O_CLOEXEC|O_NOCTTY|O_DIRECTORY);
-                        if (fd < 0)
-                                return -errno;
-
                         if (fstatfs(fd, &sfs) < 0)
                                 return -errno;
 
@@ -173,13 +174,17 @@ static int image_make(
                         }
                 }
 
-                /* It's just a normal directory. */
+                /* If the IMMUTABLE bit is set, we consider the
+                 * directory read-only. Since the ioctl is not
+                 * supported everywhere we ignore failures. */
+                (void) read_attr_fd(fd, &file_attr);
 
+                /* It's just a normal directory. */
                 r = image_new(IMAGE_DIRECTORY,
                               pretty,
                               path,
                               filename,
-                              read_only,
+                              read_only || (file_attr & FS_IMMUTABLE_FL),
                               0,
                               0,
                               ret);
@@ -347,6 +352,11 @@ int image_remove(Image *i) {
                 return btrfs_subvol_remove(i->path);
 
         case IMAGE_DIRECTORY:
+                /* Allow deletion of read-only directories */
+                (void) chattr_path(i->path, false, FS_IMMUTABLE_FL);
+
+                /* fall through */
+
         case IMAGE_GPT:
                 return rm_rf_dangerous(i->path, false, true, false);
 
@@ -357,6 +367,7 @@ int image_remove(Image *i) {
 
 int image_rename(Image *i, const char *new_name) {
         _cleanup_free_ char *new_path = NULL, *nn = NULL;
+        unsigned file_attr = 0;
         int r;
 
         assert(i);
@@ -376,8 +387,16 @@ int image_rename(Image *i, const char *new_name) {
 
         switch (i->type) {
 
-        case IMAGE_SUBVOLUME:
         case IMAGE_DIRECTORY:
+                /* Turn of the immutable bit while we rename the image, so that we can rename it */
+                (void) read_attr_path(i->path, &file_attr);
+
+                if (file_attr & FS_IMMUTABLE_FL)
+                        (void) chattr_path(i->path, false, FS_IMMUTABLE_FL);
+
+                /* fall through */
+
+        case IMAGE_SUBVOLUME:
                 new_path = file_in_same_dir(i->path, new_name);
                 break;
 
@@ -403,6 +422,10 @@ int image_rename(Image *i, const char *new_name) {
         if (renameat2(AT_FDCWD, i->path, AT_FDCWD, new_path, RENAME_NOREPLACE) < 0)
                 return -errno;
 
+        /* Restore the immutable bit, if it was set before */
+        if (file_attr & FS_IMMUTABLE_FL)
+                (void) chattr_path(new_path, true, FS_IMMUTABLE_FL);
+
         free(i->path);
         i->path = new_path;
         new_path = NULL;
@@ -468,6 +491,22 @@ int image_read_only(Image *i, bool b) {
                 r = btrfs_subvol_set_read_only(i->path, b);
                 if (r < 0)
                         return r;
+
+                break;
+
+        case IMAGE_DIRECTORY:
+                /* For simple directory trees we cannot use the access
+                   mode of the top-level directory, since it has an
+                   effect on the container itself.  However, we can
+                   use the "immutable" flag, to at least make the
+                   top-level directory read-only. It's not as good as
+                   a read-only subvolume, but at least something, and
+                   we can read the value back.*/
+
+                r = chattr_path(i->path, b, FS_IMMUTABLE_FL);
+                if (r < 0)
+                        return r;
+
                 break;
 
         case IMAGE_GPT: {
@@ -487,7 +526,6 @@ int image_read_only(Image *i, bool b) {
                 break;
         }
 
-        case IMAGE_DIRECTORY:
         default:
                 return -ENOTSUP;
         }
diff --git a/src/shared/util.c b/src/shared/util.c
index 9fd2d89..857bb1b 100644
--- a/src/shared/util.c
+++ b/src/shared/util.c
@@ -7791,9 +7791,31 @@ int chattr_path(const char *p, bool b, unsigned mask) {
         if (mask == 0)
                 return 0;
 
-        fd = open(p, O_RDWR|O_CLOEXEC|O_NOCTTY|O_NOFOLLOW);
+        fd = open(p, O_RDONLY|O_CLOEXEC|O_NOCTTY|O_NOFOLLOW);
         if (fd < 0)
                 return -errno;
 
         return chattr_fd(fd, b, mask);
 }
+
+int read_attr_fd(int fd, unsigned *ret) {
+        assert(fd >= 0);
+
+        if (ioctl(fd, FS_IOC_GETFLAGS, ret) < 0)
+                return -errno;
+
+        return 0;
+}
+
+int read_attr_path(const char *p, unsigned *ret) {
+        _cleanup_close_ int fd = -1;
+
+        assert(p);
+        assert(ret);
+
+        fd = open(p, O_RDONLY|O_CLOEXEC|O_NOCTTY|O_NOFOLLOW);
+        if (fd < 0)
+                return -errno;
+
+        return read_attr_fd(fd, ret);
+}
diff --git a/src/shared/util.h b/src/shared/util.h
index e58a17a..5d9637e 100644
--- a/src/shared/util.h
+++ b/src/shared/util.h
@@ -1077,4 +1077,7 @@ int same_fd(int a, int b);
 int chattr_fd(int fd, bool b, unsigned mask);
 int chattr_path(const char *p, bool b, unsigned mask);
 
+int read_attr_fd(int fd, unsigned *ret);
+int read_attr_path(const char *p, unsigned *ret);
+
 #define RLIMIT_MAKE_CONST(lim) ((struct rlimit) { lim, lim })

commit 45030287af1e8e76b0feb1cfc3011a0ef2b37d0d
Author: Lennart Poettering <lennart at poettering.net>
Date:   Wed Jan 14 02:04:17 2015 +0100

    util: the chattr flags field is actually unsigned, judging by kernel sources
    
    Unlike some client code suggests...

diff --git a/src/shared/copy.c b/src/shared/copy.c
index b681f6f..0239a58 100644
--- a/src/shared/copy.c
+++ b/src/shared/copy.c
@@ -359,7 +359,7 @@ int copy_file_fd(const char *from, int fdt, bool try_reflink) {
         return r;
 }
 
-int copy_file(const char *from, const char *to, int flags, mode_t mode, int chattr_flags) {
+int copy_file(const char *from, const char *to, int flags, mode_t mode, unsigned chattr_flags) {
         int fdt, r;
 
         assert(from);
@@ -389,7 +389,7 @@ int copy_file(const char *from, const char *to, int flags, mode_t mode, int chat
         return 0;
 }
 
-int copy_file_atomic(const char *from, const char *to, mode_t mode, bool replace, int chattr_flags) {
+int copy_file_atomic(const char *from, const char *to, mode_t mode, bool replace, unsigned chattr_flags) {
         _cleanup_free_ char *t;
         int r;
 
diff --git a/src/shared/copy.h b/src/shared/copy.h
index e4e3079..8de0cfb 100644
--- a/src/shared/copy.h
+++ b/src/shared/copy.h
@@ -25,8 +25,8 @@
 #include <sys/types.h>
 
 int copy_file_fd(const char *from, int to, bool try_reflink);
-int copy_file(const char *from, const char *to, int flags, mode_t mode, int chattr_flags);
-int copy_file_atomic(const char *from, const char *to, mode_t mode, bool replace, int chattr_flags);
+int copy_file(const char *from, const char *to, int flags, mode_t mode, unsigned chattr_flags);
+int copy_file_atomic(const char *from, const char *to, mode_t mode, bool replace, unsigned chattr_flags);
 int copy_tree(const char *from, const char *to, bool merge);
 int copy_tree_at(int fdf, const char *from, int fdt, const char *to, bool merge);
 int copy_directory_fd(int dirfd, const char *to, bool merge);
diff --git a/src/shared/util.c b/src/shared/util.c
index 1210900..9fd2d89 100644
--- a/src/shared/util.c
+++ b/src/shared/util.c
@@ -7758,11 +7758,14 @@ int same_fd(int a, int b) {
         return fa == fb;
 }
 
-int chattr_fd(int fd, bool b, int mask) {
-        int old_attr, new_attr;
+int chattr_fd(int fd, bool b, unsigned mask) {
+        unsigned old_attr, new_attr;
 
         assert(fd >= 0);
 
+        if (mask == 0)
+                return 0;
+
         if (ioctl(fd, FS_IOC_GETFLAGS, &old_attr) < 0)
                 return -errno;
 
@@ -7780,9 +7783,14 @@ int chattr_fd(int fd, bool b, int mask) {
         return 0;
 }
 
-int chattr_path(const char *p, bool b, int mask) {
+int chattr_path(const char *p, bool b, unsigned mask) {
         _cleanup_close_ int fd = -1;
 
+        assert(p);
+
+        if (mask == 0)
+                return 0;
+
         fd = open(p, O_RDWR|O_CLOEXEC|O_NOCTTY|O_NOFOLLOW);
         if (fd < 0)
                 return -errno;
diff --git a/src/shared/util.h b/src/shared/util.h
index 850019a..e58a17a 100644
--- a/src/shared/util.h
+++ b/src/shared/util.h
@@ -1074,7 +1074,7 @@ int fd_getcrtime_at(int dirfd, const char *name, usec_t *usec, int flags);
 
 int same_fd(int a, int b);
 
-int chattr_fd(int fd, bool b, int mask);
-int chattr_path(const char *p, bool b, int mask);
+int chattr_fd(int fd, bool b, unsigned mask);
+int chattr_path(const char *p, bool b, unsigned mask);
 
 #define RLIMIT_MAKE_CONST(lim) ((struct rlimit) { lim, lim })

commit 679bc6cb9016715339aac4ae6b2d5371c6262935
Author: Lennart Poettering <lennart at poettering.net>
Date:   Wed Jan 14 02:01:42 2015 +0100

    ptyfw: add missing error check

diff --git a/src/shared/ptyfwd.c b/src/shared/ptyfwd.c
index a780f7d..31274a1 100644
--- a/src/shared/ptyfwd.c
+++ b/src/shared/ptyfwd.c
@@ -293,7 +293,7 @@ static int on_sigwinch_event(sd_event_source *e, const struct signalfd_siginfo *
 
         /* The window size changed, let's forward that. */
         if (ioctl(STDOUT_FILENO, TIOCGWINSZ, &ws) >= 0)
-                (void)ioctl(f->master, TIOCSWINSZ, &ws);
+                (void) ioctl(f->master, TIOCSWINSZ, &ws);
 
         return 0;
 }
@@ -373,6 +373,8 @@ int pty_forward_new(sd_event *event, int master, bool ignore_vhangup, PTYForward
                 return r;
 
         r = sd_event_add_signal(f->event, &f->sigwinch_event_source, SIGWINCH, on_sigwinch_event, f);
+        if (r < 0)
+                return r;
 
         *ret = f;
         f = NULL;

commit 8937422f3b8c4a163ffa4df697ef2939161f4f53
Author: Lennart Poettering <lennart at poettering.net>
Date:   Wed Jan 14 02:01:11 2015 +0100

    nspawn: remove the right propagation directory

diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c
index 3fce3ad..1247146 100644
--- a/src/nspawn/nspawn.c
+++ b/src/nspawn/nspawn.c
@@ -4034,7 +4034,7 @@ finish:
         if (arg_machine) {
                 const char *p;
 
-                p = strappenda("/run/systemd/nspawn/propagate", arg_machine);
+                p = strappenda("/run/systemd/nspawn/propagate/", arg_machine);
                 (void) rm_rf(p, false, true, false);
         }
 



More information about the systemd-commits mailing list