Merge pull request #1595 from poettering/proxy-fixes

bus proxy fixes, and more

11 files changed, 119 insertions, 53 deletions

diff --git a/src/basic/btrfs-util.c b/src/basic/btrfs-util.c
index 074deeccda..ec7e00986b 100644
--- a/src/basic/btrfs-util.c
+++ b/src/basic/btrfs-util.c
@@ -799,6 +799,45 @@ int btrfs_resize_loopback(const char *p, uint64_t new_size, bool grow_only) {
         return btrfs_resize_loopback_fd(fd, new_size, grow_only);
 }
 
+static int make_qgroup_id(uint64_t level, uint64_t id, uint64_t *ret) {
+        assert(ret);
+
+        if (level >= (UINT64_C(1) << (64 - BTRFS_QGROUP_LEVEL_SHIFT)))
+                return -EINVAL;
+
+        if (id >= (UINT64_C(1) << BTRFS_QGROUP_LEVEL_SHIFT))
+                return -EINVAL;
+
+        *ret = (level << BTRFS_QGROUP_LEVEL_SHIFT) | id;
+        return 0;
+}
+
+static int qgroup_create_or_destroy(int fd, bool b, uint64_t level, uint64_t id) {
+
+        struct btrfs_ioctl_qgroup_create_args args = {
+                .create = b,
+        };
+
+        int r;
+
+        r = make_qgroup_id(level, id, (uint64_t*) &args.qgroupid);
+        if (r < 0)
+                return r;
+
+        if (ioctl(fd, BTRFS_IOC_QGROUP_CREATE, &args) < 0)
+                return -errno;
+
+        return 0;
+}
+
+int btrfs_qgroup_create(int fd, uint64_t level, uint64_t id) {
+        return qgroup_create_or_destroy(fd, true, level, id);
+}
+
+int btrfs_qgroup_destroy(int fd, uint64_t level, uint64_t id) {
+        return qgroup_create_or_destroy(fd, false, level, id);
+}
+
 static int subvol_remove_children(int fd, const char *subvolume, uint64_t subvol_id, bool recursive) {
         struct btrfs_ioctl_search_args args = {
                 .key.tree_id = BTRFS_ROOT_TREE_OBJECTID,
@@ -828,16 +867,6 @@ static int subvol_remove_children(int fd, const char *subvolume, uint64_t subvol
         if (!S_ISDIR(st.st_mode))
                 return -EINVAL;
 
-        /* First, try to remove the subvolume. If it happens to be
-         * already empty, this will just work. */
-        strncpy(vol_args.name, subvolume, sizeof(vol_args.name)-1);
-        if (ioctl(fd, BTRFS_IOC_SNAP_DESTROY, &vol_args) >= 0)
-                return 0;
-        if (!recursive || errno != ENOTEMPTY)
-                return -errno;
-
-        /* OK, the subvolume is not empty, let's look for child
-         * subvolumes, and remove them, first */
         subvol_fd = openat(fd, subvolume, O_RDONLY|O_NOCTTY|O_CLOEXEC|O_DIRECTORY);
         if (subvol_fd < 0)
                 return -errno;
@@ -848,6 +877,19 @@ static int subvol_remove_children(int fd, const char *subvolume, uint64_t subvol
                         return r;
         }
 
+        /* First, try to remove the subvolume. If it happens to be
+         * already empty, this will just work. */
+        strncpy(vol_args.name, subvolume, sizeof(vol_args.name)-1);
+        if (ioctl(fd, BTRFS_IOC_SNAP_DESTROY, &vol_args) >= 0) {
+                (void) btrfs_qgroup_destroy(fd, 0, subvol_id);
+                return 0;
+        }
+        if (!recursive || errno != ENOTEMPTY)
+                return -errno;
+
+        /* OK, the subvolume is not empty, let's look for child
+         * subvolumes, and remove them, first */
+
         args.key.min_offset = args.key.max_offset = subvol_id;
 
         while (btrfs_ioctl_search_args_compare(&args) <= 0) {
@@ -925,6 +967,7 @@ static int subvol_remove_children(int fd, const char *subvolume, uint64_t subvol
         if (ioctl(fd, BTRFS_IOC_SNAP_DESTROY, &vol_args) < 0)
                 return -errno;
 
+        (void) btrfs_qgroup_destroy(fd, 0, subvol_id);
         return 0;
 }
 
diff --git a/src/basic/btrfs-util.h b/src/basic/btrfs-util.h
index 8632c3638c..ad7c7009ab 100644
--- a/src/basic/btrfs-util.h
+++ b/src/basic/btrfs-util.h
@@ -86,3 +86,6 @@ int btrfs_resize_loopback(const char *path, uint64_t size, bool grow_only);
 
 int btrfs_subvol_remove(const char *path, bool recursive);
 int btrfs_subvol_remove_fd(int fd, const char *subvolume, bool recursive);
+
+int btrfs_qgroup_create(int fd, uint64_t level, uint64_t id);
+int btrfs_qgroup_destroy(int fd, uint64_t level, uint64_t id);
diff --git a/src/basic/missing.h b/src/basic/missing.h
index 5fb9951a84..70d6c8308e 100644
--- a/src/basic/missing.h
+++ b/src/basic/missing.h
@@ -252,6 +252,10 @@ static inline int getrandom(void *buffer, size_t count, unsigned flags) {
 #define BTRFS_SEARCH_ARGS_BUFSIZE (4096 - sizeof(struct btrfs_ioctl_search_key))
 #endif
 
+#ifndef BTRFS_QGROUP_LEVEL_SHIFT
+#define BTRFS_QGROUP_LEVEL_SHIFT 48
+#endif
+
 #ifndef HAVE_LINUX_BTRFS_H
 struct btrfs_ioctl_vol_args {
         int64_t fd;
diff --git a/src/bus-proxyd/bus-proxyd.c b/src/bus-proxyd/bus-proxyd.c
index 2bc265d9b4..64d1c5231f 100644
--- a/src/bus-proxyd/bus-proxyd.c
+++ b/src/bus-proxyd/bus-proxyd.c
@@ -85,11 +85,11 @@ static void *run_client(void *userdata) {
         int r;
 
         r = proxy_new(&p, c->fd, c->fd, arg_address);
+        c->fd = -1;
+
         if (r < 0)
                 goto exit;
 
-        c->fd = -1;
-
         /* set comm to "p$PIDu$UID" and suffix with '*' if truncated */
         r = snprintf(comm, sizeof(comm), "p" PID_FMT "u" UID_FMT, p->local_creds.pid, p->local_creds.uid);
         if (r >= (ssize_t)sizeof(comm))
@@ -116,13 +116,12 @@ static int loop_clients(int accept_fd, uid_t bus_uid) {
         int r;
 
         r = pthread_attr_init(&attr);
-        if (r < 0) {
-                return log_error_errno(errno, "Cannot initialize pthread attributes: %m");
-        }
+        if (r != 0)
+                return log_error_errno(r, "Cannot initialize pthread attributes: %m");
 
         r = pthread_attr_setdetachstate(&attr, PTHREAD_CREATE_DETACHED);
-        if (r < 0) {
-                r = log_error_errno(errno, "Cannot mark pthread attributes as detached: %m");
+        if (r != 0) {
+                r = log_error_errno(r, "Cannot mark pthread attributes as detached: %m");
                 goto finish;
         }
 
@@ -156,8 +155,8 @@ static int loop_clients(int accept_fd, uid_t bus_uid) {
                 c->bus_uid = bus_uid;
 
                 r = pthread_create(&tid, &attr, run_client, c);
-                if (r < 0) {
-                        log_error("Cannot spawn thread: %m");
+                if (r != 0) {
+                        log_warning_errno(r, "Cannot spawn thread, ignoring: %m");
                         client_context_free(c);
                         continue;
                 }
diff --git a/src/bus-proxyd/bus-xml-policy.c b/src/bus-proxyd/bus-xml-policy.c
index 9a3b451c56..91717653c2 100644
--- a/src/bus-proxyd/bus-xml-policy.c
+++ b/src/bus-proxyd/bus-xml-policy.c
@@ -1186,14 +1186,14 @@ int shared_policy_new(SharedPolicy **out) {
                 return log_oom();
 
         r = pthread_mutex_init(&sp->lock, NULL);
-        if (r < 0) {
-                log_error_errno(r, "Cannot initialize shared policy mutex: %m");
+        if (r != 0) {
+                r = log_error_errno(r, "Cannot initialize shared policy mutex: %m");
                 goto exit_free;
         }
 
         r = pthread_rwlock_init(&sp->rwlock, NULL);
-        if (r < 0) {
-                log_error_errno(r, "Cannot initialize shared policy rwlock: %m");
+        if (r != 0) {
+                r = log_error_errno(r, "Cannot initialize shared policy rwlock: %m");
                 goto exit_mutex;
         }
 
diff --git a/src/bus-proxyd/proxy.c b/src/bus-proxyd/proxy.c
index 88800f5e7f..bc8516f5c6 100644
--- a/src/bus-proxyd/proxy.c
+++ b/src/bus-proxyd/proxy.c
@@ -100,18 +100,24 @@ static int proxy_create_destination(Proxy *p, const char *destination, const cha
         return 0;
 }
 
-static int proxy_create_local(Proxy *p, int in_fd, int out_fd, bool negotiate_fds) {
-        _cleanup_bus_flush_close_unref_ sd_bus *b = NULL;
+static int proxy_create_local(Proxy *p, bool negotiate_fds) {
         sd_id128_t server_id;
+        sd_bus *b;
         int r;
 
         r = sd_bus_new(&b);
         if (r < 0)
                 return log_error_errno(r, "Failed to allocate bus: %m");
 
-        r = sd_bus_set_fd(b, in_fd, out_fd);
-        if (r < 0)
+        r = sd_bus_set_fd(b, p->local_in, p->local_out);
+        if (r < 0) {
+                sd_bus_unref(b);
                 return log_error_errno(r, "Failed to set fds: %m");
+        }
+
+        /* The fds are now owned by the bus, and we indicate that by
+         * storing the bus object in the proxy object. */
+        p->local_bus = b;
 
         r = sd_bus_get_bus_id(p->destination_bus, &server_id);
         if (r < 0)
@@ -139,8 +145,6 @@ static int proxy_create_local(Proxy *p, int in_fd, int out_fd, bool negotiate_fd
         if (r < 0)
                 return log_error_errno(r, "Failed to start bus client: %m");
 
-        p->local_bus = b;
-        b = NULL;
         return 0;
 }
 
@@ -224,9 +228,17 @@ int proxy_new(Proxy **out, int in_fd, int out_fd, const char *destination) {
         bool is_unix;
         int r;
 
+        /* This takes possession/destroys the file descriptors passed
+         * in even on failure. The caller should hence forget about
+         * the fds in all cases after calling this function and not
+         * close them. */
+
         p = new0(Proxy, 1);
-        if (!p)
+        if (!p) {
+                safe_close(in_fd);
+                safe_close(out_fd);
                 return log_oom();
+        }
 
         p->local_in = in_fd;
         p->local_out = out_fd;
@@ -247,7 +259,7 @@ int proxy_new(Proxy **out, int in_fd, int out_fd, const char *destination) {
         if (r < 0)
                 return r;
 
-        r = proxy_create_local(p, in_fd, out_fd, is_unix);
+        r = proxy_create_local(p, is_unix);
         if (r < 0)
                 return r;
 
@@ -257,6 +269,7 @@ int proxy_new(Proxy **out, int in_fd, int out_fd, const char *destination) {
 
         *out = p;
         p = NULL;
+
         return 0;
 }
 
@@ -273,7 +286,14 @@ Proxy *proxy_free(Proxy *p) {
                 free(activation);
         }
 
-        sd_bus_flush_close_unref(p->local_bus);
+        if (p->local_bus)
+                sd_bus_flush_close_unref(p->local_bus);
+        else {
+                safe_close(p->local_in);
+                if (p->local_out != p->local_in)
+                        safe_close(p->local_out);
+        }
+
         sd_bus_flush_close_unref(p->destination_bus);
         set_free_free(p->owned_names);
         free(p);
diff --git a/src/core/dbus.c b/src/core/dbus.c
index 2d6a1ff836..d8891d49d8 100644
--- a/src/core/dbus.c
+++ b/src/core/dbus.c
@@ -777,7 +777,7 @@ static int bus_setup_api(Manager *m, sd_bus *bus) {
                 return r;
 
         HASHMAP_FOREACH_KEY(u, name, m->watch_bus, i) {
-                r = unit_install_bus_match(bus, u, name);
+                r = unit_install_bus_match(u, bus, name);
                 if (r < 0)
                         log_error_errno(r, "Failed to subscribe to NameOwnerChanged signal: %m");
         }
diff --git a/src/core/unit.c b/src/core/unit.c
index 39cd89f1e3..d8f0eb8111 100644
--- a/src/core/unit.c
+++ b/src/core/unit.c
@@ -2508,26 +2508,23 @@ static int signal_name_owner_changed(sd_bus_message *message, void *userdata, sd
         return 0;
 }
 
-int unit_install_bus_match(sd_bus *bus, Unit *u, const char *name) {
-        _cleanup_free_ char *match = NULL;
-        Manager *m = u->manager;
+int unit_install_bus_match(Unit *u, sd_bus *bus, const char *name) {
+        const char *match;
 
-        assert(m);
+        assert(u);
+        assert(bus);
+        assert(name);
 
         if (u->match_bus_slot)
                 return -EBUSY;
 
-        match = strjoin("type='signal',"
+        match = strjoina("type='signal',"
                         "sender='org.freedesktop.DBus',"
                         "path='/org/freedesktop/DBus',"
                         "interface='org.freedesktop.DBus',"
                         "member='NameOwnerChanged',"
-                        "arg0='",
-                        name,
-                        "'",
+                        "arg0='", name, "'",
                         NULL);
-        if (!match)
-                return -ENOMEM;
 
         return sd_bus_add_match(bus, &u->match_bus_slot, match, signal_name_owner_changed, u);
 }
@@ -2544,7 +2541,7 @@ int unit_watch_bus_name(Unit *u, const char *name) {
         if (u->manager->api_bus) {
                 /* If the bus is already available, install the match directly.
                  * Otherwise, just put the name in the list. bus_setup_api() will take care later. */
-                r = unit_install_bus_match(u->manager->api_bus, u, name);
+                r = unit_install_bus_match(u, u->manager->api_bus, name);
                 if (r < 0)
                         return log_warning_errno(r, "Failed to subscribe to NameOwnerChanged signal: %m");
         }
diff --git a/src/core/unit.h b/src/core/unit.h
index a4a1b011fc..9f8518c720 100644
--- a/src/core/unit.h
+++ b/src/core/unit.h
@@ -520,7 +520,7 @@ void unit_unwatch_all_pids(Unit *u);
 
 void unit_tidy_watch_pids(Unit *u, pid_t except1, pid_t except2);
 
-int unit_install_bus_match(sd_bus *bus, Unit *u, const char *name);
+int unit_install_bus_match(Unit *u, sd_bus *bus, const char *name);
 int unit_watch_bus_name(Unit *u, const char *name);
 void unit_unwatch_bus_name(Unit *u, const char *name);
 
diff --git a/src/libsystemd/sd-bus/bus-kernel.c b/src/libsystemd/sd-bus/bus-kernel.c
index 577a8b44c3..570d35c7ad 100644
--- a/src/libsystemd/sd-bus/bus-kernel.c
+++ b/src/libsystemd/sd-bus/bus-kernel.c
@@ -1433,12 +1433,12 @@ int bus_kernel_pop_memfd(sd_bus *bus, void **address, size_t *mapped, size_t *al
         if (!bus || !bus->is_kernel)
                 return -EOPNOTSUPP;
 
-        assert_se(pthread_mutex_lock(&bus->memfd_cache_mutex) >= 0);
+        assert_se(pthread_mutex_lock(&bus->memfd_cache_mutex) == 0);
 
         if (bus->n_memfd_cache <= 0) {
                 int r;
 
-                assert_se(pthread_mutex_unlock(&bus->memfd_cache_mutex) >= 0);
+                assert_se(pthread_mutex_unlock(&bus->memfd_cache_mutex) == 0);
 
                 r = memfd_new(bus->description);
                 if (r < 0)
@@ -1460,7 +1460,7 @@ int bus_kernel_pop_memfd(sd_bus *bus, void **address, size_t *mapped, size_t *al
         *allocated = c->allocated;
         fd = c->fd;
 
-        assert_se(pthread_mutex_unlock(&bus->memfd_cache_mutex) >= 0);
+        assert_se(pthread_mutex_unlock(&bus->memfd_cache_mutex) == 0);
 
         return fd;
 }
@@ -1484,10 +1484,10 @@ void bus_kernel_push_memfd(sd_bus *bus, int fd, void *address, size_t mapped, si
                 return;
         }
 
-        assert_se(pthread_mutex_lock(&bus->memfd_cache_mutex) >= 0);
+        assert_se(pthread_mutex_lock(&bus->memfd_cache_mutex) == 0);
 
         if (bus->n_memfd_cache >= ELEMENTSOF(bus->memfd_cache)) {
-                assert_se(pthread_mutex_unlock(&bus->memfd_cache_mutex) >= 0);
+                assert_se(pthread_mutex_unlock(&bus->memfd_cache_mutex) == 0);
 
                 close_and_munmap(fd, address, mapped);
                 return;
@@ -1507,7 +1507,7 @@ void bus_kernel_push_memfd(sd_bus *bus, int fd, void *address, size_t mapped, si
                 c->allocated = allocated;
         }
 
-        assert_se(pthread_mutex_unlock(&bus->memfd_cache_mutex) >= 0);
+        assert_se(pthread_mutex_unlock(&bus->memfd_cache_mutex) == 0);
 }
 
 void bus_kernel_flush_memfd(sd_bus *b) {
diff --git a/src/libsystemd/sd-event/sd-event.c b/src/libsystemd/sd-event/sd-event.c
index 1a82c4c940..1905ebfc73 100644
--- a/src/libsystemd/sd-event/sd-event.c
+++ b/src/libsystemd/sd-event/sd-event.c
@@ -1123,8 +1123,8 @@ _public_ int sd_event_add_signal(
                 callback = signal_exit_callback;
 
         r = pthread_sigmask(SIG_SETMASK, NULL, &ss);
-        if (r < 0)
-                return -errno;
+        if (r != 0)
+                return -r;
 
         if (!sigismember(&ss, sig))
                 return -EBUSY;