From 5b6299ee8b5b3d2bd3f05b84b3e05f05f29dc7ce Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 10 Oct 2018 16:47:13 +0200 Subject: [PATCH 01/23] import: downgrade log message to LOG_WARNING, because we ignore the failure --- src/import/importd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/import/importd.c b/src/import/importd.c index 2c88d353da..29951d2d27 100644 --- a/src/import/importd.c +++ b/src/import/importd.c @@ -196,7 +196,7 @@ static void transfer_send_log_line(Transfer *t, const char *line) { priority, line); if (r < 0) - log_error_errno(r, "Cannot emit message: %m"); + log_warning_errno(r, "Cannot emit log message signal, ignoring: %m"); } static void transfer_send_logs(Transfer *t, bool flush) { From 4b71c27b427a612df7f677f2d20585c845451149 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 10 Oct 2018 16:47:38 +0200 Subject: [PATCH 02/23] import: use generic word 'transfer' instead of 'import' After all, these lines are also printed for exports/pulls... --- src/import/importd.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/import/importd.c b/src/import/importd.c index 29951d2d27..37b6859af9 100644 --- a/src/import/importd.c +++ b/src/import/importd.c @@ -301,17 +301,16 @@ static int transfer_on_pid(sd_event_source *s, const siginfo_t *si, void *userda if (si->si_code == CLD_EXITED) { if (si->si_status != 0) - log_error("Import process failed with exit code %i.", si->si_status); + log_error("Transfer process failed with exit code %i.", si->si_status); else { - log_debug("Import process succeeded."); + log_debug("Transfer process succeeded."); success = true; } } else if (IN_SET(si->si_code, CLD_KILLED, CLD_DUMPED)) - - log_error("Import process terminated by signal %s.", signal_to_string(si->si_status)); + log_error("Transfer process terminated by signal %s.", signal_to_string(si->si_status)); else - log_error("Import process failed due to unknown reason."); + log_error("Transfer process failed due to unknown reason."); t->pid = 0; From b0325c991e23a1be38359a38edb1cc2130c0d341 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 10 Oct 2018 16:49:01 +0200 Subject: [PATCH 03/23] import: tiny modernization/simplification Less indentation and fewer {} are always a good thing. --- src/import/importd.c | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/src/import/importd.c b/src/import/importd.c index 37b6859af9..8416d78e7d 100644 --- a/src/import/importd.c +++ b/src/import/importd.c @@ -325,14 +325,12 @@ static int transfer_on_log(sd_event_source *s, int fd, uint32_t revents, void *u assert(t); l = read(fd, t->log_message + t->log_message_size, sizeof(t->log_message) - t->log_message_size); + if (l < 0) + log_error_errno(errno, "Failed to read log message: %m"); if (l <= 0) { /* EOF/read error. We just close the pipe here, and * close the watch, waiting for the SIGCHLD to arrive, * before we do anything else. */ - - if (l < 0) - log_error_errno(errno, "Failed to read log message: %m"); - t->log_event_source = sd_event_source_unref(t->log_event_source); return 0; } @@ -635,12 +633,9 @@ static Transfer *manager_find(Manager *m, TransferType type, const char *remote) assert(type >= 0); assert(type < _TRANSFER_TYPE_MAX); - HASHMAP_FOREACH(t, m->transfers, i) { - - if (t->type == type && - streq_ptr(t->remote, remote)) + HASHMAP_FOREACH(t, m->transfers, i) + if (t->type == type && streq_ptr(t->remote, remote)) return t; - } return NULL; } From fd67de0152ca63ac47c954d37da6579be3b4ee20 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 10 Oct 2018 17:44:09 +0200 Subject: [PATCH 04/23] tree-wide: fix a few missing includes --- src/import/import-common.h | 2 ++ src/shared/machine-image.h | 2 ++ src/shared/verbs.h | 2 ++ 3 files changed, 6 insertions(+) diff --git a/src/import/import-common.h b/src/import/import-common.h index 99ac5fe970..34ed829de0 100644 --- a/src/import/import-common.h +++ b/src/import/import-common.h @@ -1,6 +1,8 @@ /* SPDX-License-Identifier: LGPL-2.1+ */ #pragma once +#include + int import_make_read_only_fd(int fd); int import_make_read_only(const char *path); diff --git a/src/shared/machine-image.h b/src/shared/machine-image.h index 2e38522acd..de06147e7b 100644 --- a/src/shared/machine-image.h +++ b/src/shared/machine-image.h @@ -4,6 +4,8 @@ #include #include +#include "sd-id128.h" + #include "hashmap.h" #include "lockfile-util.h" #include "macro.h" diff --git a/src/shared/verbs.h b/src/shared/verbs.h index e174255a76..010c0df3fd 100644 --- a/src/shared/verbs.h +++ b/src/shared/verbs.h @@ -1,6 +1,8 @@ /* SPDX-License-Identifier: LGPL-2.1+ */ #pragma once +#include + #define VERB_ANY ((unsigned) -1) typedef enum VerbFlags { From 1295c9066a490bee60c2921b798911326a468eb8 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 10 Oct 2018 17:44:43 +0200 Subject: [PATCH 05/23] import: prefer fd_get_path() over readlink_malloc() where applicable --- src/import/export.c | 4 ++-- src/import/import.c | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/import/export.c b/src/import/export.c index f81485bbc7..490710678f 100644 --- a/src/import/export.c +++ b/src/import/export.c @@ -95,7 +95,7 @@ static int export_tar(int argc, char *argv[], void *userdata) { fd = STDOUT_FILENO; - (void) readlink_malloc("/proc/self/fd/1", &pretty); + (void) fd_get_path(fd, &pretty); log_info("Exporting '%s', saving to '%s' with compression '%s'.", local, strna(pretty), import_compress_type_to_string(arg_compress)); } @@ -172,7 +172,7 @@ static int export_raw(int argc, char *argv[], void *userdata) { fd = STDOUT_FILENO; - (void) readlink_malloc("/proc/self/fd/1", &pretty); + (void) fd_get_path(fd, &pretty); log_info("Exporting '%s', saving to '%s' with compression '%s'.", local, strna(pretty), import_compress_type_to_string(arg_compress)); } diff --git a/src/import/import.c b/src/import/import.c index 9dca2f3d41..f34244acff 100644 --- a/src/import/import.c +++ b/src/import/import.c @@ -96,7 +96,7 @@ static int import_tar(int argc, char *argv[], void *userdata) { fd = STDIN_FILENO; - (void) readlink_malloc("/proc/self/fd/0", &pretty); + (void) fd_get_path(fd, &pretty); log_info("Importing '%s', saving as '%s'.", strna(pretty), local); } @@ -192,7 +192,7 @@ static int import_raw(int argc, char *argv[], void *userdata) { fd = STDIN_FILENO; - (void) readlink_malloc("/proc/self/fd/0", &pretty); + (void) fd_get_path(fd, &pretty); log_info("Importing '%s', saving as '%s'.", strempty(pretty), local); } From b11591af2712d154ad3a083730650c6567e6e7c3 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 10 Oct 2018 20:58:03 +0200 Subject: [PATCH 06/23] import-util: downgrade log message about quota to LOG_DEBUG We invoke this usually on a temporary path before renaming it into place. This means the log message is quite suprising as it mentions a weird path with random characters in it. Hence, let's downgrade the message in order not to confuse the user. --- src/shared/import-util.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/shared/import-util.c b/src/shared/import-util.c index d53b90796f..bcd6c0c5ea 100644 --- a/src/shared/import-util.c +++ b/src/shared/import-util.c @@ -160,7 +160,7 @@ int import_assign_pool_quota_and_warn(const char *path) { if (r < 0) return log_error_errno(r, "Failed to set up default quota hierarchy for %s: %m", path); if (r > 0) - log_info("Set up default quota hierarchy for %s.", path); + log_debug("Set up default quota hierarchy for %s.", path); return 0; } From 5a8582fbffa12e8b9cefffd1c634c5af5cce334d Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 10 Oct 2018 21:00:03 +0200 Subject: [PATCH 07/23] import: use parse_percent() where appropriate --- src/import/importd.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/import/importd.c b/src/import/importd.c index 8416d78e7d..260705bcad 100644 --- a/src/import/importd.c +++ b/src/import/importd.c @@ -510,7 +510,6 @@ static int manager_on_notify(sd_event_source *s, int fd, uint32_t revents, void struct ucred *ucred = NULL; Manager *m = userdata; struct cmsghdr *cmsg; - unsigned percent; char *p, *e; Transfer *t; Iterator i; @@ -566,15 +565,15 @@ static int manager_on_notify(sd_event_source *s, int fd, uint32_t revents, void e = strchrnul(p, '\n'); *e = 0; - r = safe_atou(p, &percent); - if (r < 0 || percent > 100) { + r = parse_percent(p); + if (r < 0) { log_warning("Got invalid percent value, ignoring."); return 0; } - t->progress_percent = percent; + t->progress_percent = (unsigned) r; - log_debug("Got percentage from client: %u%%", percent); + log_debug("Got percentage from client: %u%%", t->progress_percent); return 0; } From 9cb0509c5ffcc60c99e4c613acffa18dd794db6a Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 10 Oct 2018 21:00:57 +0200 Subject: [PATCH 08/23] polkit: add missing method calls These were missing, hence the polkit hook-up for them never worked... --- src/import/org.freedesktop.import1.conf | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/import/org.freedesktop.import1.conf b/src/import/org.freedesktop.import1.conf index 9889cd6b10..74c21f6410 100644 --- a/src/import/org.freedesktop.import1.conf +++ b/src/import/org.freedesktop.import1.conf @@ -46,6 +46,22 @@ send_interface="org.freedesktop.import1.Manager" send_member="CancelTransfer"/> + + + + + + + + From 576cf244a491a76bae9f6ece57f60a9e3b4ad53f Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 10 Oct 2018 21:25:01 +0200 Subject: [PATCH 09/23] man: document that machinectl is also happy with block devices these days --- man/machinectl.xml | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/man/machinectl.xml b/man/machinectl.xml index 670205033b..fc61613fb6 100644 --- a/man/machinectl.xml +++ b/man/machinectl.xml @@ -70,11 +70,12 @@ top-level directories /usr, /etc, and so on. - btrfs subvolumes containing OS trees, similar to - normal directory trees. + btrfs subvolumes containing OS trees, similar to regular directory trees. - Binary "raw" disk images containing MBR or GPT - partition tables and Linux file system partitions. + Binary "raw" disk image files containing MBR or GPT partition tables and Linux file + systems. + + Similarly, block devices containing MBR or GPT partition tables and file systems. The file system tree of the host OS itself. From b3cade0c2792298d6cce0a0b2c8c4974ed981b00 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 10 Oct 2018 21:03:30 +0200 Subject: [PATCH 10/23] copy: support getting progress feedback from the various copy functions This adds two optional functions that may be passed to the various copy functions. One is invoked whenever we start copying a new file object, the other while we copy file payload in each loop iteration. When the caller passes one or both they can get notifications about copy progress, for example to log where things are. --- src/basic/btrfs-util.c | 29 +++++++++-- src/basic/btrfs-util.h | 12 ++++- src/basic/copy.c | 116 +++++++++++++++++++++++++++++++++-------- src/basic/copy.h | 52 ++++++++++++++---- src/basic/fd-util.c | 2 +- 5 files changed, 171 insertions(+), 40 deletions(-) diff --git a/src/basic/btrfs-util.c b/src/basic/btrfs-util.c index 48e819a7cb..3252b75802 100644 --- a/src/basic/btrfs-util.c +++ b/src/basic/btrfs-util.c @@ -1503,7 +1503,12 @@ static int copy_subtree_quota_limits(int fd, uint64_t old_subvol, uint64_t new_s return changed; } -static int subvol_snapshot_children(int old_fd, int new_fd, const char *subvolume, uint64_t old_subvol_id, BtrfsSnapshotFlags flags) { +static int subvol_snapshot_children( + int old_fd, + int new_fd, + const char *subvolume, + uint64_t old_subvol_id, + BtrfsSnapshotFlags flags) { struct btrfs_ioctl_search_args args = { .key.tree_id = BTRFS_ROOT_TREE_OBJECTID, @@ -1683,7 +1688,14 @@ static int subvol_snapshot_children(int old_fd, int new_fd, const char *subvolum return 0; } -int btrfs_subvol_snapshot_fd(int old_fd, const char *new_path, BtrfsSnapshotFlags flags) { +int btrfs_subvol_snapshot_fd_full( + int old_fd, + const char *new_path, + BtrfsSnapshotFlags flags, + copy_progress_path_t progress_path, + copy_progress_bytes_t progress_bytes, + void *userdata) { + _cleanup_close_ int new_fd = -1; const char *subvolume; int r; @@ -1711,7 +1723,7 @@ int btrfs_subvol_snapshot_fd(int old_fd, const char *new_path, BtrfsSnapshotFlag } else if (r < 0) return r; - r = copy_directory_fd(old_fd, new_path, COPY_MERGE|COPY_REFLINK); + r = copy_directory_fd_full(old_fd, new_path, COPY_MERGE|COPY_REFLINK, progress_path, progress_bytes, userdata); if (r < 0) goto fallback_fail; @@ -1748,7 +1760,14 @@ int btrfs_subvol_snapshot_fd(int old_fd, const char *new_path, BtrfsSnapshotFlag return subvol_snapshot_children(old_fd, new_fd, subvolume, 0, flags); } -int btrfs_subvol_snapshot(const char *old_path, const char *new_path, BtrfsSnapshotFlags flags) { +int btrfs_subvol_snapshot_full( + const char *old_path, + const char *new_path, + BtrfsSnapshotFlags flags, + copy_progress_path_t progress_path, + copy_progress_bytes_t progress_bytes, + void *userdata) { + _cleanup_close_ int old_fd = -1; assert(old_path); @@ -1758,7 +1777,7 @@ int btrfs_subvol_snapshot(const char *old_path, const char *new_path, BtrfsSnaps if (old_fd < 0) return -errno; - return btrfs_subvol_snapshot_fd(old_fd, new_path, flags); + return btrfs_subvol_snapshot_fd_full(old_fd, new_path, flags, progress_path, progress_bytes, userdata); } int btrfs_qgroup_find_parents(int fd, uint64_t qgroupid, uint64_t **ret) { diff --git a/src/basic/btrfs-util.h b/src/basic/btrfs-util.h index b0cf6739f7..3adb51e02d 100644 --- a/src/basic/btrfs-util.h +++ b/src/basic/btrfs-util.h @@ -7,6 +7,7 @@ #include "sd-id128.h" +#include "copy.h" #include "time-util.h" typedef struct BtrfsSubvolInfo { @@ -67,8 +68,15 @@ int btrfs_resize_loopback(const char *path, uint64_t size, bool grow_only); int btrfs_subvol_make(const char *path); int btrfs_subvol_make_fd(int fd, const char *subvolume); -int btrfs_subvol_snapshot_fd(int old_fd, const char *new_path, BtrfsSnapshotFlags flags); -int btrfs_subvol_snapshot(const char *old_path, const char *new_path, BtrfsSnapshotFlags flags); +int btrfs_subvol_snapshot_fd_full(int old_fd, const char *new_path, BtrfsSnapshotFlags flags, copy_progress_path_t progress_path, copy_progress_bytes_t progress_bytes, void *userdata); +static inline int btrfs_subvol_snapshot_fd(int old_fd, const char *new_path, BtrfsSnapshotFlags flags) { + return btrfs_subvol_snapshot_fd_full(old_fd, new_path, flags, NULL, NULL, NULL); +} + +int btrfs_subvol_snapshot_full(const char *old_path, const char *new_path, BtrfsSnapshotFlags flags, copy_progress_path_t progress_path, copy_progress_bytes_t progress_bytes, void *userdata); +static inline int btrfs_subvol_snapshot(const char *old_path, const char *new_path, BtrfsSnapshotFlags flags) { + return btrfs_subvol_snapshot_full(old_path, new_path, flags, NULL, NULL, NULL); +} int btrfs_subvol_remove(const char *path, BtrfsRemoveFlags flags); int btrfs_subvol_remove_fd(int fd, const char *subvolume, BtrfsRemoveFlags flags); diff --git a/src/basic/copy.c b/src/basic/copy.c index 3efd9041c0..5cc3a9572b 100644 --- a/src/basic/copy.c +++ b/src/basic/copy.c @@ -90,7 +90,9 @@ int copy_bytes_full( uint64_t max_bytes, CopyFlags copy_flags, void **ret_remains, - size_t *ret_remains_size) { + size_t *ret_remains_size, + copy_progress_bytes_t progress, + void *userdata) { bool try_cfr = true, try_sendfile = true, try_splice = true; int r, nonblock_pipe = -1; @@ -308,10 +310,17 @@ int copy_bytes_full( } next: + if (progress) { + r = progress(n, userdata); + if (r < 0) + return r; + } + if (max_bytes != (uint64_t) -1) { assert(max_bytes >= (uint64_t) n); max_bytes -= n; } + /* sendfile accepts at most SSIZE_MAX-offset bytes to copy, * so reduce our maximum by the amount we already copied, * but don't go below our copy buffer size, unless we are @@ -363,7 +372,9 @@ static int fd_copy_regular( const char *to, uid_t override_uid, gid_t override_gid, - CopyFlags copy_flags) { + CopyFlags copy_flags, + copy_progress_bytes_t progress, + void *userdata) { _cleanup_close_ int fdf = -1, fdt = -1; struct timespec ts[2]; @@ -381,7 +392,7 @@ static int fd_copy_regular( if (fdt < 0) return -errno; - r = copy_bytes(fdf, fdt, (uint64_t) -1, copy_flags); + r = copy_bytes_full(fdf, fdt, (uint64_t) -1, copy_flags, NULL, NULL, progress, userdata); if (r < 0) { (void) unlinkat(dt, to, 0); return r; @@ -483,7 +494,11 @@ static int fd_copy_directory( unsigned depth_left, uid_t override_uid, gid_t override_gid, - CopyFlags copy_flags) { + CopyFlags copy_flags, + const char *display_path, + copy_progress_path_t progress_path, + copy_progress_bytes_t progress_bytes, + void *userdata) { _cleanup_close_ int fdf = -1, fdt = -1; _cleanup_closedir_ DIR *d = NULL; @@ -524,6 +539,8 @@ static int fd_copy_directory( r = 0; FOREACH_DIRENT_ALL(de, d, return -errno) { + const char *child_display_path = NULL; + _cleanup_free_ char *dp = NULL; struct stat buf; int q; @@ -535,6 +552,17 @@ static int fd_copy_directory( continue; } + if (progress_path) { + if (display_path) + child_display_path = dp = strjoin(display_path, "/", de->d_name); + else + child_display_path = de->d_name; + + r = progress_path(child_display_path, &buf, userdata); + if (r < 0) + return r; + } + if (S_ISDIR(buf.st_mode)) { /* * Don't descend into directories on other file systems, if this is requested. We do a simple @@ -566,9 +594,9 @@ static int fd_copy_directory( continue; } - q = fd_copy_directory(dirfd(d), de->d_name, &buf, fdt, de->d_name, original_device, depth_left-1, override_uid, override_gid, copy_flags); + q = fd_copy_directory(dirfd(d), de->d_name, &buf, fdt, de->d_name, original_device, depth_left-1, override_uid, override_gid, copy_flags, child_display_path, progress_path, progress_bytes, userdata); } else if (S_ISREG(buf.st_mode)) - q = fd_copy_regular(dirfd(d), de->d_name, &buf, fdt, de->d_name, override_uid, override_gid, copy_flags); + q = fd_copy_regular(dirfd(d), de->d_name, &buf, fdt, de->d_name, override_uid, override_gid, copy_flags, progress_bytes, userdata); else if (S_ISLNK(buf.st_mode)) q = fd_copy_symlink(dirfd(d), de->d_name, &buf, fdt, de->d_name, override_uid, override_gid, copy_flags); else if (S_ISFIFO(buf.st_mode)) @@ -606,7 +634,18 @@ static int fd_copy_directory( return r; } -int copy_tree_at(int fdf, const char *from, int fdt, const char *to, uid_t override_uid, gid_t override_gid, CopyFlags copy_flags) { +int copy_tree_at_full( + int fdf, + const char *from, + int fdt, + const char *to, + uid_t override_uid, + gid_t override_gid, + CopyFlags copy_flags, + copy_progress_path_t progress_path, + copy_progress_bytes_t progress_bytes, + void *userdata) { + struct stat st; assert(from); @@ -616,9 +655,9 @@ int copy_tree_at(int fdf, const char *from, int fdt, const char *to, uid_t overr return -errno; if (S_ISREG(st.st_mode)) - return fd_copy_regular(fdf, from, &st, fdt, to, override_uid, override_gid, copy_flags); + return fd_copy_regular(fdf, from, &st, fdt, to, override_uid, override_gid, copy_flags, progress_bytes, userdata); else if (S_ISDIR(st.st_mode)) - return fd_copy_directory(fdf, from, &st, fdt, to, st.st_dev, COPY_DEPTH_MAX, override_uid, override_gid, copy_flags); + return fd_copy_directory(fdf, from, &st, fdt, to, st.st_dev, COPY_DEPTH_MAX, override_uid, override_gid, copy_flags, NULL, progress_path, progress_bytes, userdata); else if (S_ISLNK(st.st_mode)) return fd_copy_symlink(fdf, from, &st, fdt, to, override_uid, override_gid, copy_flags); else if (S_ISFIFO(st.st_mode)) @@ -629,11 +668,14 @@ int copy_tree_at(int fdf, const char *from, int fdt, const char *to, uid_t overr return -EOPNOTSUPP; } -int copy_tree(const char *from, const char *to, uid_t override_uid, gid_t override_gid, CopyFlags copy_flags) { - return copy_tree_at(AT_FDCWD, from, AT_FDCWD, to, override_uid, override_gid, copy_flags); -} +int copy_directory_fd_full( + int dirfd, + const char *to, + CopyFlags copy_flags, + copy_progress_path_t progress_path, + copy_progress_bytes_t progress_bytes, + void *userdata) { -int copy_directory_fd(int dirfd, const char *to, CopyFlags copy_flags) { struct stat st; assert(dirfd >= 0); @@ -645,10 +687,17 @@ int copy_directory_fd(int dirfd, const char *to, CopyFlags copy_flags) { if (!S_ISDIR(st.st_mode)) return -ENOTDIR; - return fd_copy_directory(dirfd, NULL, &st, AT_FDCWD, to, st.st_dev, COPY_DEPTH_MAX, UID_INVALID, GID_INVALID, copy_flags); + return fd_copy_directory(dirfd, NULL, &st, AT_FDCWD, to, st.st_dev, COPY_DEPTH_MAX, UID_INVALID, GID_INVALID, copy_flags, NULL, progress_path, progress_bytes, userdata); } -int copy_directory(const char *from, const char *to, CopyFlags copy_flags) { +int copy_directory_full( + const char *from, + const char *to, + CopyFlags copy_flags, + copy_progress_path_t progress_path, + copy_progress_bytes_t progress_bytes, + void *userdata) { + struct stat st; assert(from); @@ -660,10 +709,16 @@ int copy_directory(const char *from, const char *to, CopyFlags copy_flags) { if (!S_ISDIR(st.st_mode)) return -ENOTDIR; - return fd_copy_directory(AT_FDCWD, from, &st, AT_FDCWD, to, st.st_dev, COPY_DEPTH_MAX, UID_INVALID, GID_INVALID, copy_flags); + return fd_copy_directory(AT_FDCWD, from, &st, AT_FDCWD, to, st.st_dev, COPY_DEPTH_MAX, UID_INVALID, GID_INVALID, copy_flags, NULL, progress_path, progress_bytes, userdata); } -int copy_file_fd(const char *from, int fdt, CopyFlags copy_flags) { +int copy_file_fd_full( + const char *from, + int fdt, + CopyFlags copy_flags, + copy_progress_bytes_t progress_bytes, + void *userdata) { + _cleanup_close_ int fdf = -1; int r; @@ -674,7 +729,7 @@ int copy_file_fd(const char *from, int fdt, CopyFlags copy_flags) { if (fdf < 0) return -errno; - r = copy_bytes(fdf, fdt, (uint64_t) -1, copy_flags); + r = copy_bytes_full(fdf, fdt, (uint64_t) -1, copy_flags, NULL, NULL, progress_bytes, userdata); (void) copy_times(fdf, fdt); (void) copy_xattr(fdf, fdt); @@ -682,7 +737,16 @@ int copy_file_fd(const char *from, int fdt, CopyFlags copy_flags) { return r; } -int copy_file(const char *from, const char *to, int flags, mode_t mode, unsigned chattr_flags, CopyFlags copy_flags) { +int copy_file_full( + const char *from, + const char *to, + int flags, + mode_t mode, + unsigned chattr_flags, + CopyFlags copy_flags, + copy_progress_bytes_t progress_bytes, + void *userdata) { + int fdt = -1, r; assert(from); @@ -697,7 +761,7 @@ int copy_file(const char *from, const char *to, int flags, mode_t mode, unsigned if (chattr_flags != 0) (void) chattr_fd(fdt, chattr_flags, (unsigned) -1, NULL); - r = copy_file_fd(from, fdt, copy_flags); + r = copy_file_fd_full(from, fdt, copy_flags, progress_bytes, userdata); if (r < 0) { close(fdt); (void) unlink(to); @@ -712,7 +776,15 @@ int copy_file(const char *from, const char *to, int flags, mode_t mode, unsigned return 0; } -int copy_file_atomic(const char *from, const char *to, mode_t mode, unsigned chattr_flags, CopyFlags copy_flags) { +int copy_file_atomic_full( + const char *from, + const char *to, + mode_t mode, + unsigned chattr_flags, + CopyFlags copy_flags, + copy_progress_bytes_t progress_bytes, + void *userdata) { + _cleanup_(unlink_and_freep) char *t = NULL; _cleanup_close_ int fdt = -1; int r; @@ -745,7 +817,7 @@ int copy_file_atomic(const char *from, const char *to, mode_t mode, unsigned cha if (chattr_flags != 0) (void) chattr_fd(fdt, chattr_flags, (unsigned) -1, NULL); - r = copy_file_fd(from, fdt, copy_flags); + r = copy_file_fd_full(from, fdt, copy_flags, progress_bytes, userdata); if (r < 0) return r; diff --git a/src/basic/copy.h b/src/basic/copy.h index 6a0a6bc9b3..a41b44c70a 100644 --- a/src/basic/copy.h +++ b/src/basic/copy.h @@ -1,9 +1,11 @@ /* SPDX-License-Identifier: LGPL-2.1+ */ #pragma once +#include #include #include #include +#include #include typedef enum CopyFlags { @@ -13,16 +15,46 @@ typedef enum CopyFlags { COPY_SAME_MOUNT = 1 << 3, /* Don't descend recursively into other file systems, across mount point boundaries */ } CopyFlags; -int copy_file_fd(const char *from, int to, CopyFlags copy_flags); -int copy_file(const char *from, const char *to, int open_flags, mode_t mode, unsigned chattr_flags, CopyFlags copy_flags); -int copy_file_atomic(const char *from, const char *to, mode_t mode, unsigned chattr_flags, CopyFlags copy_flags); -int copy_tree(const char *from, const char *to, uid_t override_uid, gid_t override_gid, CopyFlags copy_flags); -int copy_tree_at(int fdf, const char *from, int fdt, const char *to, uid_t override_uid, gid_t override_gid, CopyFlags copy_flags); -int copy_directory_fd(int dirfd, const char *to, CopyFlags copy_flags); -int copy_directory(const char *from, const char *to, CopyFlags copy_flags); -int copy_bytes_full(int fdf, int fdt, uint64_t max_bytes, CopyFlags copy_flags, void **ret_remains, size_t *ret_remains_size); -static inline int copy_bytes(int fdf, int fdt, uint64_t max_bytes, CopyFlags copy_flags) { - return copy_bytes_full(fdf, fdt, max_bytes, copy_flags, NULL, NULL); +typedef int (*copy_progress_bytes_t)(uint64_t n_bytes, void *userdata); +typedef int (*copy_progress_path_t)(const char *path, const struct stat *st, void *userdata); + +int copy_file_fd_full(const char *from, int to, CopyFlags copy_flags, copy_progress_bytes_t progress, void *userdata); +static inline int copy_file_fd(const char *from, int to, CopyFlags copy_flags) { + return copy_file_fd_full(from, to, copy_flags, NULL, NULL); } + +int copy_file_full(const char *from, const char *to, int open_flags, mode_t mode, unsigned chattr_flags, CopyFlags copy_flags, copy_progress_bytes_t progress, void *userdata); +static inline int copy_file(const char *from, const char *to, int open_flags, mode_t mode, unsigned chattr_flags, CopyFlags copy_flags) { + return copy_file_full(from, to, open_flags, mode, chattr_flags, copy_flags, NULL, NULL); +} + +int copy_file_atomic_full(const char *from, const char *to, mode_t mode, unsigned chattr_flags, CopyFlags copy_flags, copy_progress_bytes_t progress, void *userdata); +static inline int copy_file_atomic(const char *from, const char *to, mode_t mode, unsigned chattr_flags, CopyFlags copy_flags) { + return copy_file_atomic_full(from, to, mode, chattr_flags, copy_flags, NULL, NULL); +} + +int copy_tree_at_full(int fdf, const char *from, int fdt, const char *to, uid_t override_uid, gid_t override_gid, CopyFlags copy_flags, copy_progress_path_t progress_path, copy_progress_bytes_t progress_bytes, void *userdata); +static inline int copy_tree_at(int fdf, const char *from, int fdt, const char *to, uid_t override_uid, gid_t override_gid, CopyFlags copy_flags) { + return copy_tree_at_full(fdf, from, fdt, to, override_uid, override_gid, copy_flags, NULL, NULL, NULL); +} +static inline int copy_tree(const char *from, const char *to, uid_t override_uid, gid_t override_gid, CopyFlags copy_flags) { + return copy_tree_at_full(AT_FDCWD, from, AT_FDCWD, to, override_uid, override_gid, copy_flags, NULL, NULL, NULL); +} + +int copy_directory_fd_full(int dirfd, const char *to, CopyFlags copy_flags, copy_progress_path_t progress_path, copy_progress_bytes_t progress_bytes, void *userdata); +static inline int copy_directory_fd(int dirfd, const char *to, CopyFlags copy_flags) { + return copy_directory_fd_full(dirfd, to, copy_flags, NULL, NULL, NULL); +} + +int copy_directory_full(const char *from, const char *to, CopyFlags copy_flags, copy_progress_path_t progress_path, copy_progress_bytes_t progress_bytes, void *userdata); +static inline int copy_directory(const char *from, const char *to, CopyFlags copy_flags) { + return copy_directory_full(from, to, copy_flags, NULL, NULL, NULL); +} + +int copy_bytes_full(int fdf, int fdt, uint64_t max_bytes, CopyFlags copy_flags, void **ret_remains, size_t *ret_remains_size, copy_progress_bytes_t progress, void *userdata); +static inline int copy_bytes(int fdf, int fdt, uint64_t max_bytes, CopyFlags copy_flags) { + return copy_bytes_full(fdf, fdt, max_bytes, copy_flags, NULL, NULL, NULL, NULL); +} + int copy_times(int fdf, int fdt); int copy_xattr(int fdf, int fdt); diff --git a/src/basic/fd-util.c b/src/basic/fd-util.c index 5775a13e3e..97dc40357c 100644 --- a/src/basic/fd-util.c +++ b/src/basic/fd-util.c @@ -648,7 +648,7 @@ int fd_duplicate_data_fd(int fd) { if ((size_t) isz >= DATA_FD_MEMORY_LIMIT) { - r = copy_bytes_full(fd, pipefds[1], DATA_FD_MEMORY_LIMIT, 0, &remains, &remains_size); + r = copy_bytes_full(fd, pipefds[1], DATA_FD_MEMORY_LIMIT, 0, &remains, &remains_size, NULL, NULL); if (r < 0 && r != -EAGAIN) return r; /* If we get EAGAIN it could be because of the source or because of * the destination fd, we can't know, as sendfile() and friends won't From 1d7579c473dd665df213fbc8a30f032994546246 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 10 Oct 2018 21:20:08 +0200 Subject: [PATCH 11/23] machine: add support for importing containers from plain directories Fixes: #2728 This is also supposed to be preparation for doing #10234 eventually, where a very similar operation is requested: instead of importing a tree to /var/lib/machines it would need to be imported into /var/lib/portables/. --- man/machinectl.xml | 9 + meson.build | 12 +- src/basic/rm-rf.h | 8 + src/import/import-fs.c | 323 ++++++++++++++++++++++++ src/import/importd.c | 133 +++++++++- src/import/meson.build | 6 + src/import/org.freedesktop.import1.conf | 4 + src/machine/machinectl.c | 84 +++++- 8 files changed, 561 insertions(+), 18 deletions(-) create mode 100644 src/import/import-fs.c diff --git a/man/machinectl.xml b/man/machinectl.xml index fc61613fb6..e403c51e28 100644 --- a/man/machinectl.xml +++ b/man/machinectl.xml @@ -815,6 +815,15 @@ cancel-transfer. + + import-fs DIRECTORY [NAME] + + Imports a container image stored in a local directory into + /var/lib/machines/, operates similar to import-tar or + import-raw, but the first argument is the source directory. If supported, this command will + create btrfs snapshot or subvolume for the new image. + + export-tar NAME [FILE] export-raw NAME [FILE] diff --git a/meson.build b/meson.build index 5dc25d03dc..1f25955736 100644 --- a/meson.build +++ b/meson.build @@ -227,6 +227,7 @@ conf.set_quoted('ROOTLIBEXECDIR', rootlibexecdir) conf.set_quoted('BOOTLIBDIR', bootlibdir) conf.set_quoted('SYSTEMD_PULL_PATH', join_paths(rootlibexecdir, 'systemd-pull')) conf.set_quoted('SYSTEMD_IMPORT_PATH', join_paths(rootlibexecdir, 'systemd-import')) +conf.set_quoted('SYSTEMD_IMPORT_FS_PATH', join_paths(rootlibexecdir, 'systemd-import-fs')) conf.set_quoted('SYSTEMD_EXPORT_PATH', join_paths(rootlibexecdir, 'systemd-export')) conf.set_quoted('VENDOR_KEYRING_PATH', join_paths(rootlibexecdir, 'import-pubring.gpg')) conf.set_quoted('USER_KEYRING_PATH', join_paths(pkgsysconfdir, 'import-pubring.gpg')) @@ -2126,6 +2127,14 @@ if conf.get('ENABLE_IMPORTD') == 1 install : true, install_dir : rootlibexecdir) + systemd_import_fs = executable('systemd-import-fs', + systemd_import_fs_sources, + include_directories : includes, + link_with : [libshared], + install_rpath : rootlibexecdir, + install : true, + install_dir : rootlibexecdir) + systemd_export = executable('systemd-export', systemd_export_sources, include_directories : includes, @@ -2137,7 +2146,8 @@ if conf.get('ENABLE_IMPORTD') == 1 install_rpath : rootlibexecdir, install : true, install_dir : rootlibexecdir) - public_programs += [systemd_pull, systemd_import, systemd_export] + + public_programs += [systemd_pull, systemd_import, systemd_import_fs, systemd_export] endif if conf.get('ENABLE_REMOTE') == 1 and conf.get('HAVE_LIBCURL') == 1 diff --git a/src/basic/rm-rf.h b/src/basic/rm-rf.h index 0a2d7f0358..3ee2b97e37 100644 --- a/src/basic/rm-rf.h +++ b/src/basic/rm-rf.h @@ -22,3 +22,11 @@ static inline void rm_rf_physical_and_free(char *p) { free(p); } DEFINE_TRIVIAL_CLEANUP_FUNC(char*, rm_rf_physical_and_free); + +/* Similar as above, but also has magic btrfs subvolume powers */ +static inline void rm_rf_subvolume_and_free(char *p) { + PROTECT_ERRNO; + (void) rm_rf(p, REMOVE_ROOT|REMOVE_PHYSICAL|REMOVE_SUBVOLUME); + free(p); +} +DEFINE_TRIVIAL_CLEANUP_FUNC(char*, rm_rf_subvolume_and_free); diff --git a/src/import/import-fs.c b/src/import/import-fs.c new file mode 100644 index 0000000000..3836f87570 --- /dev/null +++ b/src/import/import-fs.c @@ -0,0 +1,323 @@ +/* SPDX-License-Identifier: LGPL-2.1+ */ + +#include + +#include "alloc-util.h" +#include "btrfs-util.h" +#include "fd-util.h" +#include "fileio.h" +#include "fs-util.h" +#include "hostname-util.h" +#include "import-common.h" +#include "import-util.h" +#include "machine-image.h" +#include "mkdir.h" +#include "ratelimit.h" +#include "rm-rf.h" +#include "string-util.h" +#include "verbs.h" +#include "parse-util.h" + +static bool arg_force = false; +static bool arg_read_only = false; +static const char *arg_image_root = "/var/lib/machines"; + +typedef struct ProgressInfo { + RateLimit limit; + char *path; + uint64_t size; + bool started; + bool logged_incomplete; +} ProgressInfo; + +static volatile sig_atomic_t cancelled = false; + +static void sigterm_sigint(int sig) { + cancelled = true; +} + +static void progress_info_free(ProgressInfo *p) { + free(p->path); +} + +static void progress_show(ProgressInfo *p) { + assert(p); + + /* Show progress only every now and then. */ + if (!ratelimit_below(&p->limit)) + return; + + /* Suppress the first message, start with the second one */ + if (!p->started) { + p->started = true; + return; + } + + /* Mention the list is incomplete before showing first output. */ + if (!p->logged_incomplete) { + log_notice("(Note, file list shown below is incomplete, and is intended as sporadic progress report only.)"); + p->logged_incomplete = true; + } + + if (p->size == 0) + log_info("Copying tree, currently at '%s'...", p->path); + else { + char buffer[FORMAT_BYTES_MAX]; + + log_info("Copying tree, currently at '%s' (@%s)...", p->path, format_bytes(buffer, sizeof(buffer), p->size)); + } +} + +static int progress_path(const char *path, const struct stat *st, void *userdata) { + ProgressInfo *p = userdata; + int r; + + assert(p); + + if (cancelled) + return -EOWNERDEAD; + + r = free_and_strdup(&p->path, path); + if (r < 0) + return r; + + p->size = 0; + + progress_show(p); + return 0; +} + +static int progress_bytes(uint64_t nbytes, void *userdata) { + ProgressInfo *p = userdata; + + assert(p); + assert(p->size != UINT64_MAX); + + if (cancelled) + return -EOWNERDEAD; + + p->size += nbytes; + + progress_show(p); + return 0; +} + +static int import_fs(int argc, char *argv[], void *userdata) { + _cleanup_(rm_rf_subvolume_and_freep) char *temp_path = NULL; + _cleanup_(progress_info_free) ProgressInfo progress = {}; + const char *path = NULL, *local = NULL, *final_path; + _cleanup_close_ int open_fd = -1; + struct sigaction old_sigint_sa, old_sigterm_sa; + static const struct sigaction sa = { + .sa_handler = sigterm_sigint, + .sa_flags = SA_RESTART, + }; + int r, fd; + + if (argc >= 2) + path = argv[1]; + if (isempty(path) || streq(path, "-")) + path = NULL; + + if (argc >= 3) + local = argv[2]; + else if (path) + local = basename(path); + if (isempty(local) || streq(local, "-")) + local = NULL; + + if (local) { + if (!machine_name_is_valid(local)) { + log_error("Local image name '%s' is not valid.", local); + return -EINVAL; + } + + if (!arg_force) { + r = image_find(IMAGE_MACHINE, local, NULL); + if (r < 0) { + if (r != -ENOENT) + return log_error_errno(r, "Failed to check whether image '%s' exists: %m", local); + } else { + log_error("Image '%s' already exists.", local); + return -EEXIST; + } + } + } else + local = "imported"; + + if (path) { + open_fd = open(path, O_DIRECTORY|O_RDONLY|O_CLOEXEC); + if (open_fd < 0) + return log_error_errno(errno, "Failed to open directory to import: %m"); + + fd = open_fd; + + log_info("Importing '%s', saving as '%s'.", path, local); + } else { + _cleanup_free_ char *pretty = NULL; + + fd = STDIN_FILENO; + + (void) fd_get_path(fd, &pretty); + log_info("Importing '%s', saving as '%s'.", strempty(pretty), local); + } + + final_path = strjoina(arg_image_root, "/", local); + + r = tempfn_random(final_path, NULL, &temp_path); + if (r < 0) + return log_oom(); + + (void) mkdir_parents_label(temp_path, 0700); + + RATELIMIT_INIT(progress.limit, 200*USEC_PER_MSEC, 1); + + /* Hook into SIGINT/SIGTERM, so that we can cancel things then */ + assert(sigaction(SIGINT, &sa, &old_sigint_sa) >= 0); + assert(sigaction(SIGTERM, &sa, &old_sigterm_sa) >= 0); + + r = btrfs_subvol_snapshot_fd_full( + fd, + temp_path, + BTRFS_SNAPSHOT_FALLBACK_COPY|BTRFS_SNAPSHOT_RECURSIVE|BTRFS_SNAPSHOT_FALLBACK_DIRECTORY|BTRFS_SNAPSHOT_QUOTA, + progress_path, + progress_bytes, + &progress); + if (r == -EOWNERDEAD) { /* SIGINT + SIGTERM cause this, see signal handler above */ + log_error("Copy cancelled."); + goto finish; + } + if (r < 0) { + log_error_errno(r, "Failed to copy directory: %m"); + goto finish; + } + + (void) import_assign_pool_quota_and_warn(temp_path); + + if (arg_read_only) { + r = import_make_read_only(temp_path); + if (r < 0) { + log_error_errno(r, "Failed to make directory read-only: %m"); + goto finish; + } + } + + if (arg_force) + (void) rm_rf(final_path, REMOVE_ROOT|REMOVE_PHYSICAL|REMOVE_SUBVOLUME); + + r = rename_noreplace(AT_FDCWD, temp_path, AT_FDCWD, final_path); + if (r < 0) { + log_error_errno(r, "Failed to move image into place: %m"); + goto finish; + } + + temp_path = mfree(temp_path); + + log_info("Exiting."); + +finish: + /* Put old signal handlers into place */ + assert(sigaction(SIGINT, &old_sigint_sa, NULL) >= 0); + assert(sigaction(SIGTERM, &old_sigterm_sa, NULL) >= 0); + + return 0; +} + +static int help(int argc, char *argv[], void *userdata) { + + printf("%s [OPTIONS...] {COMMAND} ...\n\n" + "Import container images from a file system.\n\n" + " -h --help Show this help\n" + " --version Show package version\n" + " --force Force creation of image\n" + " --image-root=PATH Image root directory\n" + " --read-only Create a read-only image\n\n" + "Commands:\n" + " run DIRECTORY [NAME] Import a directory\n", + program_invocation_short_name); + + return 0; +} + +static int parse_argv(int argc, char *argv[]) { + + enum { + ARG_VERSION = 0x100, + ARG_FORCE, + ARG_IMAGE_ROOT, + ARG_READ_ONLY, + }; + + static const struct option options[] = { + { "help", no_argument, NULL, 'h' }, + { "version", no_argument, NULL, ARG_VERSION }, + { "force", no_argument, NULL, ARG_FORCE }, + { "image-root", required_argument, NULL, ARG_IMAGE_ROOT }, + { "read-only", no_argument, NULL, ARG_READ_ONLY }, + {} + }; + + int c; + + assert(argc >= 0); + assert(argv); + + while ((c = getopt_long(argc, argv, "h", options, NULL)) >= 0) + + switch (c) { + + case 'h': + return help(0, NULL, NULL); + + case ARG_VERSION: + return version(); + + case ARG_FORCE: + arg_force = true; + break; + + case ARG_IMAGE_ROOT: + arg_image_root = optarg; + break; + + case ARG_READ_ONLY: + arg_read_only = true; + break; + + case '?': + return -EINVAL; + + default: + assert_not_reached("Unhandled option"); + } + + return 1; +} + +static int import_fs_main(int argc, char *argv[]) { + + static const Verb verbs[] = { + { "help", VERB_ANY, VERB_ANY, 0, help }, + { "run", 2, 3, 0, import_fs }, + {} + }; + + return dispatch_verb(argc, argv, verbs, NULL); +} + +int main(int argc, char *argv[]) { + int r; + + setlocale(LC_ALL, ""); + log_parse_environment(); + log_open(); + + r = parse_argv(argc, argv); + if (r <= 0) + goto finish; + + r = import_fs_main(argc, argv); + +finish: + return r < 0 ? EXIT_FAILURE : EXIT_SUCCESS; +} diff --git a/src/import/importd.c b/src/import/importd.c index 260705bcad..6fa8342172 100644 --- a/src/import/importd.c +++ b/src/import/importd.c @@ -10,6 +10,7 @@ #include "bus-util.h" #include "def.h" #include "fd-util.h" +#include "float.h" #include "hostname-util.h" #include "import-util.h" #include "machine-pool.h" @@ -34,6 +35,7 @@ typedef struct Manager Manager; typedef enum TransferType { TRANSFER_IMPORT_TAR, TRANSFER_IMPORT_RAW, + TRANSFER_IMPORT_FS, TRANSFER_EXPORT_TAR, TRANSFER_EXPORT_RAW, TRANSFER_PULL_TAR, @@ -94,6 +96,7 @@ struct Manager { static const char* const transfer_type_table[_TRANSFER_TYPE_MAX] = { [TRANSFER_IMPORT_TAR] = "import-tar", [TRANSFER_IMPORT_RAW] = "import-raw", + [TRANSFER_IMPORT_FS] = "import-fs", [TRANSFER_EXPORT_TAR] = "export-tar", [TRANSFER_EXPORT_RAW] = "export-raw", [TRANSFER_PULL_TAR] = "pull-tar", @@ -156,6 +159,7 @@ static int transfer_new(Manager *m, Transfer **ret) { .stdin_fd = -1, .stdout_fd = -1, .verify = _IMPORT_VERIFY_INVALID, + .progress_percent= (unsigned) -1, }; id = m->current_transfer_id + 1; @@ -177,6 +181,15 @@ static int transfer_new(Manager *m, Transfer **ret) { return 0; } +static double transfer_percent_as_double(Transfer *t) { + assert(t); + + if (t->progress_percent == (unsigned) -1) + return -DBL_MAX; + + return (double) t->progress_percent / 100.0; +} + static void transfer_send_log_line(Transfer *t, const char *line) { int r, priority = LOG_INFO; @@ -357,7 +370,7 @@ static int transfer_start(Transfer *t) { return r; if (r == 0) { const char *cmd[] = { - NULL, /* systemd-import, systemd-export or systemd-pull */ + NULL, /* systemd-import, systemd-import-fs, systemd-export or systemd-pull */ NULL, /* tar, raw */ NULL, /* --verify= */ NULL, /* verify argument */ @@ -390,17 +403,52 @@ static int transfer_start(Transfer *t) { _exit(EXIT_FAILURE); } - if (IN_SET(t->type, TRANSFER_IMPORT_TAR, TRANSFER_IMPORT_RAW)) - cmd[k++] = SYSTEMD_IMPORT_PATH; - else if (IN_SET(t->type, TRANSFER_EXPORT_TAR, TRANSFER_EXPORT_RAW)) - cmd[k++] = SYSTEMD_EXPORT_PATH; - else - cmd[k++] = SYSTEMD_PULL_PATH; + switch (t->type) { - if (IN_SET(t->type, TRANSFER_IMPORT_TAR, TRANSFER_EXPORT_TAR, TRANSFER_PULL_TAR)) + case TRANSFER_IMPORT_TAR: + case TRANSFER_IMPORT_RAW: + cmd[k++] = SYSTEMD_IMPORT_PATH; + break; + + case TRANSFER_IMPORT_FS: + cmd[k++] = SYSTEMD_IMPORT_FS_PATH; + break; + + case TRANSFER_EXPORT_TAR: + case TRANSFER_EXPORT_RAW: + cmd[k++] = SYSTEMD_EXPORT_PATH; + break; + + case TRANSFER_PULL_TAR: + case TRANSFER_PULL_RAW: + cmd[k++] = SYSTEMD_PULL_PATH; + break; + + default: + assert_not_reached("Unexpected transfer type"); + } + + switch (t->type) { + + case TRANSFER_IMPORT_TAR: + case TRANSFER_EXPORT_TAR: + case TRANSFER_PULL_TAR: cmd[k++] = "tar"; - else + break; + + case TRANSFER_IMPORT_RAW: + case TRANSFER_EXPORT_RAW: + case TRANSFER_PULL_RAW: cmd[k++] = "raw"; + break; + + case TRANSFER_IMPORT_FS: + cmd[k++] = "run"; + break; + + default: + break; + } if (t->verify != _IMPORT_VERIFY_INVALID) { cmd[k++] = "--verify"; @@ -704,6 +752,68 @@ static int method_import_tar_or_raw(sd_bus_message *msg, void *userdata, sd_bus_ return sd_bus_reply_method_return(msg, "uo", id, object); } +static int method_import_fs(sd_bus_message *msg, void *userdata, sd_bus_error *error) { + _cleanup_(transfer_unrefp) Transfer *t = NULL; + int fd, force, read_only, r; + const char *local, *object; + Manager *m = userdata; + uint32_t id; + + assert(msg); + assert(m); + + r = bus_verify_polkit_async( + msg, + CAP_SYS_ADMIN, + "org.freedesktop.import1.import", + NULL, + false, + UID_INVALID, + &m->polkit_registry, + error); + if (r < 0) + return r; + if (r == 0) + return 1; /* Will call us back */ + + r = sd_bus_message_read(msg, "hsbb", &fd, &local, &force, &read_only); + if (r < 0) + return r; + + if (!machine_name_is_valid(local)) + return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Local name %s is invalid", local); + + r = setup_machine_directory((uint64_t) -1, error); + if (r < 0) + return r; + + r = transfer_new(m, &t); + if (r < 0) + return r; + + t->type = TRANSFER_IMPORT_FS; + t->force_local = force; + t->read_only = read_only; + + t->local = strdup(local); + if (!t->local) + return -ENOMEM; + + t->stdin_fd = fcntl(fd, F_DUPFD_CLOEXEC, 3); + if (t->stdin_fd < 0) + return -errno; + + r = transfer_start(t); + if (r < 0) + return r; + + object = t->object_path; + id = t->id; + t = NULL; + + return sd_bus_reply_method_return(msg, "uo", id, object); +} + static int method_export_tar_or_raw(sd_bus_message *msg, void *userdata, sd_bus_error *error) { _cleanup_(transfer_unrefp) Transfer *t = NULL; int fd, r; @@ -879,7 +989,7 @@ static int method_list_transfers(sd_bus_message *msg, void *userdata, sd_bus_err transfer_type_to_string(t->type), t->remote, t->local, - (double) t->progress_percent / 100.0, + transfer_percent_as_double(t), t->object_path); if (r < 0) return r; @@ -975,7 +1085,7 @@ static int property_get_progress( assert(reply); assert(t); - return sd_bus_message_append(reply, "d", (double) t->progress_percent / 100.0); + return sd_bus_message_append(reply, "d", transfer_percent_as_double(t)); } static BUS_DEFINE_PROPERTY_GET_ENUM(property_get_type, transfer_type, TransferType); @@ -998,6 +1108,7 @@ static const sd_bus_vtable manager_vtable[] = { SD_BUS_VTABLE_START(0), SD_BUS_METHOD("ImportTar", "hsbb", "uo", method_import_tar_or_raw, SD_BUS_VTABLE_UNPRIVILEGED), SD_BUS_METHOD("ImportRaw", "hsbb", "uo", method_import_tar_or_raw, SD_BUS_VTABLE_UNPRIVILEGED), + SD_BUS_METHOD("ImportFileSystem", "hsbb", "uo", method_import_fs, SD_BUS_VTABLE_UNPRIVILEGED), SD_BUS_METHOD("ExportTar", "shs", "uo", method_export_tar_or_raw, SD_BUS_VTABLE_UNPRIVILEGED), SD_BUS_METHOD("ExportRaw", "shs", "uo", method_export_tar_or_raw, SD_BUS_VTABLE_UNPRIVILEGED), SD_BUS_METHOD("PullTar", "sssb", "uo", method_pull_tar_or_raw, SD_BUS_VTABLE_UNPRIVILEGED), diff --git a/src/import/meson.build b/src/import/meson.build index 283ba08c67..1c15fd883f 100644 --- a/src/import/meson.build +++ b/src/import/meson.build @@ -38,6 +38,12 @@ systemd_import_sources = files(''' qcow2-util.h '''.split()) +systemd_import_fs_sources = files(''' + import-fs.c + import-common.c + import-common.h +'''.split()) + systemd_export_sources = files(''' export.c export-tar.c diff --git a/src/import/org.freedesktop.import1.conf b/src/import/org.freedesktop.import1.conf index 74c21f6410..2fdb2ba77c 100644 --- a/src/import/org.freedesktop.import1.conf +++ b/src/import/org.freedesktop.import1.conf @@ -54,6 +54,10 @@ send_interface="org.freedesktop.import1.Manager" send_member="ImportRaw"/> + + diff --git a/src/machine/machinectl.c b/src/machine/machinectl.c index 094ee9d360..158cf73c28 100644 --- a/src/machine/machinectl.c +++ b/src/machine/machinectl.c @@ -5,6 +5,7 @@ #include #include #include +#include #include #include #include @@ -2133,6 +2134,66 @@ static int import_raw(int argc, char *argv[], void *userdata) { return transfer_image_common(bus, m); } +static int import_fs(int argc, char *argv[], void *userdata) { + _cleanup_(sd_bus_message_unrefp) sd_bus_message *m = NULL; + _cleanup_close_ int fd = -1; + const char *local = NULL, *path = NULL; + sd_bus *bus = userdata; + int r; + + assert(bus); + + if (argc >= 2) + path = argv[1]; + if (isempty(path) || streq(path, "-")) + path = NULL; + + if (argc >= 3) + local = argv[2]; + else if (path) + local = basename(path); + if (isempty(local) || streq(local, "-")) + local = NULL; + + if (!local) { + log_error("Need either path or local name."); + return -EINVAL; + } + + if (!machine_name_is_valid(local)) { + log_error("Local name %s is not a suitable machine name.", local); + return -EINVAL; + } + + if (path) { + fd = open(path, O_DIRECTORY|O_RDONLY|O_CLOEXEC); + if (fd < 0) + return log_error_errno(errno, "Failed to open directory '%s': %m", path); + } + + r = sd_bus_message_new_method_call( + bus, + &m, + "org.freedesktop.import1", + "/org/freedesktop/import1", + "org.freedesktop.import1.Manager", + "ImportFileSystem"); + if (r < 0) + return bus_log_create_error(r); + + r = sd_bus_message_append( + m, + "hsbb", + fd >= 0 ? fd : STDIN_FILENO, + local, + arg_force, + arg_read_only); + if (r < 0) + return bus_log_create_error(r); + + return transfer_image_common(bus, m); +} + static void determine_compression_from_filename(const char *p) { if (arg_format) return; @@ -2464,12 +2525,21 @@ static int list_transfers(int argc, char *argv[], void *userdata) { (int) max_remote, "REMOTE"); for (j = 0; j < n_transfers; j++) - printf("%*" PRIu32 " %*u%% %-*s %-*s %-*s\n", - (int) MAX(2U, DECIMAL_STR_WIDTH(max_id)), transfers[j].id, - (int) 6, (unsigned) (transfers[j].progress * 100), - (int) max_type, transfers[j].type, - (int) max_local, transfers[j].local, - (int) max_remote, transfers[j].remote); + + if (transfers[j].progress < 0) + printf("%*" PRIu32 " %*s %-*s %-*s %-*s\n", + (int) MAX(2U, DECIMAL_STR_WIDTH(max_id)), transfers[j].id, + (int) 7, "n/a", + (int) max_type, transfers[j].type, + (int) max_local, transfers[j].local, + (int) max_remote, transfers[j].remote); + else + printf("%*" PRIu32 " %*u%% %-*s %-*s %-*s\n", + (int) MAX(2U, DECIMAL_STR_WIDTH(max_id)), transfers[j].id, + (int) 6, (unsigned) (transfers[j].progress * 100), + (int) max_type, transfers[j].type, + (int) max_local, transfers[j].local, + (int) max_remote, transfers[j].remote); if (arg_legend) { if (n_transfers > 0) @@ -2687,6 +2757,7 @@ static int help(int argc, char *argv[], void *userdata) { " pull-raw URL [NAME] Download a RAW container or VM image\n" " import-tar FILE [NAME] Import a local TAR container image\n" " import-raw FILE [NAME] Import a local RAW container or VM image\n" + " import-fs DIRECTORY [NAME] Import a local directory container image\n" " export-tar NAME [FILE] Export a TAR container image locally\n" " export-raw NAME [FILE] Export a RAW container or VM image locally\n" " list-transfers Show list of downloads in progress\n" @@ -3008,6 +3079,7 @@ static int machinectl_main(int argc, char *argv[], sd_bus *bus) { { "disable", 2, VERB_ANY, 0, enable_machine }, { "import-tar", 2, 3, 0, import_tar }, { "import-raw", 2, 3, 0, import_raw }, + { "import-fs", 2, 3, 0, import_fs }, { "export-tar", 2, 3, 0, export_tar }, { "export-raw", 2, 3, 0, export_raw }, { "pull-tar", 2, 3, 0, pull_tar }, From e21b7229ff776891443fe3200ebf4648f3f2940b Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 11 Oct 2018 10:01:41 +0200 Subject: [PATCH 12/23] import: rerrange tarball/fs imports that have a single top-level directory containing the OS tree Let's handle tarball imports nicer that have a single top-level directory containing the OS tree: let's move everything down during import, so that the OS tree is ready to use automatically. Fixes: #2116 --- src/import/import-common.c | 109 +++++++++++++++++++++++++++++++++++++ src/import/import-common.h | 2 + src/import/import-fs.c | 4 ++ src/import/import-tar.c | 4 ++ 4 files changed, 119 insertions(+) diff --git a/src/import/import-common.c b/src/import/import-common.c index e2de2c2dd0..24cab484a2 100644 --- a/src/import/import-common.c +++ b/src/import/import-common.c @@ -5,10 +5,15 @@ #include #include +#include "alloc-util.h" #include "btrfs-util.h" #include "capability-util.h" +#include "dirent-util.h" #include "fd-util.h" +#include "fileio.h" +#include "fs-util.h" #include "import-common.h" +#include "os-util.h" #include "process-util.h" #include "signal-util.h" #include "util.h" @@ -147,3 +152,107 @@ int import_fork_tar_c(const char *path, pid_t *ret) { return TAKE_FD(pipefd[0]); } + +int import_mangle_os_tree(const char *path) { + _cleanup_closedir_ DIR *d = NULL, *cd = NULL; + _cleanup_free_ char *child = NULL, *t = NULL; + const char *joined; + struct dirent *de; + int r; + + assert(path); + + /* Some tarballs contain a single top-level directory that contains the actual OS directory tree. Try to + * recognize this, and move the tree one level up. */ + + r = path_is_os_tree(path); + if (r < 0) + return log_error_errno(r, "Failed to determine whether '%s' is an OS tree: %m", path); + if (r > 0) { + log_debug("Directory tree '%s' is a valid OS tree.", path); + return 0; + } + + log_debug("Directory tree '%s' is not recognizable as OS tree, checking whether to rearrange it.", path); + + d = opendir(path); + if (!d) + return log_error_errno(r, "Failed to open directory '%s': %m", path); + + errno = 0; + de = readdir_no_dot(d); + if (!de) { + if (errno != 0) + return log_error_errno(errno, "Failed to iterate through directory '%s': %m", path); + + log_debug("Directory '%s' is empty, leaving it as it is.", path); + return 0; + } + + child = strdup(de->d_name); + if (!child) + return log_oom(); + + errno = 0; + de = readdir_no_dot(d); + if (de) { + if (errno != 0) + return log_error_errno(errno, "Failed to iterate through directory '%s': %m", path); + + log_debug("Directory '%s' does not look like a directory tree, and has multiple children, leaving as it is.", path); + return 0; + } + + joined = strjoina(path, "/", child); + r = path_is_os_tree(joined); + if (r == -ENOTDIR) { + log_debug("Directory '%s' does not look like a directory tree, and contains a single regular file only, leaving as it is.", path); + return 0; + } + if (r < 0) + return log_error_errno(r, "Failed to determine whether '%s' is an OS tree: %m", joined); + if (r == 0) { + log_debug("Neither '%s' nor '%s' is a valid OS tree, leaving them as they are.", path, joined); + return 0; + } + + /* Nice, we have checked now: + * + * 1. The top-level directory does not qualify as OS tree + * 1. The top-level directory only contains one item + * 2. That item is a directory + * 3. And that directory qualifies as OS tree + * + * Let's now rearrange things, moving everything in the inner directory one level up */ + + cd = xopendirat(dirfd(d), child, O_NOFOLLOW); + if (!cd) + return log_error_errno(errno, "Can't open directory '%s': %m", joined); + + log_info("Rearranging '%s', moving OS tree one directory up.", joined); + + /* Let's rename the child to an unguessable name so that we can be sure all files contained in it can be + * safely moved up and won't collide with the name. */ + r = tempfn_random(child, NULL, &t); + if (r < 0) + return log_oom(); + r = rename_noreplace(dirfd(d), child, dirfd(d), t); + if (r < 0) + return log_error_errno(r, "Unable to rename '%s' to '%s/%s': %m", joined, path, t); + + FOREACH_DIRENT_ALL(de, cd, return log_error_errno(errno, "Failed to iterate through directory '%s': %m", joined)) { + if (dot_or_dot_dot(de->d_name)) + continue; + + r = rename_noreplace(dirfd(cd), de->d_name, dirfd(d), de->d_name); + if (r < 0) + return log_error_errno(r, "Unable to move '%s/%s/%s' to '%s/%s': %m", path, t, de->d_name, path, de->d_name); + } + + if (unlinkat(dirfd(d), t, AT_REMOVEDIR) < 0) + return log_error_errno(errno, "Failed to remove temporary directory '%s/%s': %m", path, t); + + log_info("Successfully rearranged OS tree."); + + return 0; +} diff --git a/src/import/import-common.h b/src/import/import-common.h index 34ed829de0..94d224f412 100644 --- a/src/import/import-common.h +++ b/src/import/import-common.h @@ -8,3 +8,5 @@ int import_make_read_only(const char *path); int import_fork_tar_c(const char *path, pid_t *ret); int import_fork_tar_x(const char *path, pid_t *ret); + +int import_mangle_os_tree(const char *path); diff --git a/src/import/import-fs.c b/src/import/import-fs.c index 3836f87570..bc2e631e7b 100644 --- a/src/import/import-fs.c +++ b/src/import/import-fs.c @@ -192,6 +192,10 @@ static int import_fs(int argc, char *argv[], void *userdata) { goto finish; } + r = import_mangle_os_tree(temp_path); + if (r < 0) + goto finish; + (void) import_assign_pool_quota_and_warn(temp_path); if (arg_read_only) { diff --git a/src/import/import-tar.c b/src/import/import-tar.c index 0399b03747..a164417de0 100644 --- a/src/import/import-tar.c +++ b/src/import/import-tar.c @@ -184,6 +184,10 @@ static int tar_import_finish(TarImport *i) { return r; } + r = import_mangle_os_tree(i->temp_path); + if (r < 0) + return r; + if (i->read_only) { r = import_make_read_only(i->temp_path); if (r < 0) From 5f7ecd610c7918bb89468ce068bfcf68114dc53e Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 11 Oct 2018 10:27:59 +0200 Subject: [PATCH 13/23] import: drop logic of setting up /var/lib/machines as btrfs loopback mount Let's simplify things and drop the logic that /var/lib/machines is setup as auto-growing btrfs loopback file /var/lib/machines.raw. THis was done in order to make quota available for machine management, but quite frankly never really worked properly, as we couldn't grow the file system in sync with its use properly. Moreover philosophically it's problematic overriding the admin's choice of file system like this. Let's hence drop this, and simplify things. Deleting code is a good feeling. Now that regular file systems provide project quota we could probably add per-machine quota support based on that, hence the btrfs quota argument is not that interesting anymore (though btrfs quota is a bit more powerful as it allows recursive quota, i.e. that the machine pool gets an overall quota in addition to per-machine quota). --- TODO | 6 +- man/machinectl.xml | 38 +--- src/basic/btrfs-util.c | 90 --------- src/basic/btrfs-util.h | 3 - src/import/import-raw.c | 13 -- src/import/import-tar.c | 13 -- src/import/importd.c | 6 +- src/import/pull-job.c | 10 - src/import/pull-job.h | 3 - src/import/pull-raw.c | 6 - src/import/pull-tar.c | 6 - src/machine/machined-dbus.c | 31 +-- src/shared/machine-pool.c | 369 +---------------------------------- src/shared/machine-pool.h | 6 +- units/var-lib-machines.mount | 7 +- 15 files changed, 24 insertions(+), 583 deletions(-) diff --git a/TODO b/TODO index ae1bedcd29..d8d120d66a 100644 --- a/TODO +++ b/TODO @@ -235,8 +235,7 @@ Features: the runtime dir as we maintain for the fdstore: i.e. keep it around as long as the unit is running or has a job queued. -* support projid-based quota in machinectl for containers, and then drop - implicit btrfs loopback magic in machined +* support projid-based quota in machinectl for containers * Add NetworkNamespacePath= to specify a path to a network namespace @@ -883,9 +882,6 @@ Features: - "machinectl commit" that takes a writable snapshot of a tree, invokes a shell in it, and marks it read-only after use -* importd: - - generate a nice warning if mkfs.btrfs is missing - * cryptsetup: - cryptsetup-generator: allow specification of passwords in crypttab itself - support rd.luks.allow-discards= kernel cmdline params in cryptsetup generator diff --git a/man/machinectl.xml b/man/machinectl.xml index e403c51e28..95823eb413 100644 --- a/man/machinectl.xml +++ b/man/machinectl.xml @@ -650,22 +650,7 @@ units. If the size limit shall be disabled, specify - as size. - Note that per-container size limits are only supported - on btrfs file systems. Also note that, if - set-limit is invoked without an image - parameter, and /var/lib/machines is - empty, and the directory is not located on btrfs, a btrfs - loopback file is implicitly created as - /var/lib/machines.raw with the given - size, and mounted to - /var/lib/machines. The size of the - loopback may later be readjusted with - set-limit, as well. If such a - loopback-mounted /var/lib/machines - directory is used, set-limit without an image - name alters both the quota setting within the file system as - well as the loopback file and file system size - itself. + Note that per-container size limits are only supported on btrfs file systems. @@ -803,12 +788,8 @@ image is read from standard input, in which case the second argument is mandatory. - Both pull-tar and pull-raw - will resize /var/lib/machines.raw and the - filesystem therein as necessary. Optionally, the - switch may be used to create a - read-only container or VM image. No cryptographic validation - is done when importing the images. + Optionally, the switch may be used to create a read-only container or VM + image. No cryptographic validation is done when importing the images. Much like image downloads, ongoing imports may be listed with list-transfers and aborted with @@ -920,18 +901,7 @@ /var/lib/machines/ to make them available for control with machinectl. - Note that some image operations are only supported, - efficient or atomic on btrfs file systems. Due to this, if the - pull-tar, pull-raw, - import-tar, import-raw and - set-limit commands notice that - /var/lib/machines is empty and not located on - btrfs, they will implicitly set up a loopback file - /var/lib/machines.raw containing a btrfs file - system that is mounted to - /var/lib/machines. The size of this loopback - file may be controlled dynamically with - set-limit. + Note that some image operations are only supported, efficient or atomic on btrfs file systems. Disk images are understood by systemd-nspawn1 diff --git a/src/basic/btrfs-util.c b/src/basic/btrfs-util.c index 3252b75802..cedce84e9c 100644 --- a/src/basic/btrfs-util.c +++ b/src/basic/btrfs-util.c @@ -870,96 +870,6 @@ int btrfs_subvol_set_subtree_quota_limit(const char *path, uint64_t subvol_id, u return btrfs_subvol_set_subtree_quota_limit_fd(fd, subvol_id, referenced_max); } -int btrfs_resize_loopback_fd(int fd, uint64_t new_size, bool grow_only) { - struct btrfs_ioctl_vol_args args = {}; - char p[SYS_BLOCK_PATH_MAX("/loop/backing_file")], q[DEV_NUM_PATH_MAX]; - _cleanup_free_ char *backing = NULL; - _cleanup_close_ int loop_fd = -1, backing_fd = -1; - struct stat st; - dev_t dev = 0; - int r; - - /* In contrast to btrfs quota ioctls ftruncate() cannot make sense of "infinity" or file sizes > 2^31 */ - if (!FILE_SIZE_VALID(new_size)) - return -EINVAL; - - /* btrfs cannot handle file systems < 16M, hence use this as minimum */ - if (new_size < 16*1024*1024) - new_size = 16*1024*1024; - - r = btrfs_get_block_device_fd(fd, &dev); - if (r < 0) - return r; - if (r == 0) - return -ENODEV; - - xsprintf_sys_block_path(p, "/loop/backing_file", dev); - r = read_one_line_file(p, &backing); - if (r == -ENOENT) - return -ENODEV; - if (r < 0) - return r; - if (isempty(backing) || !path_is_absolute(backing)) - return -ENODEV; - - backing_fd = open(backing, O_RDWR|O_CLOEXEC|O_NOCTTY); - if (backing_fd < 0) - return -errno; - - if (fstat(backing_fd, &st) < 0) - return -errno; - if (!S_ISREG(st.st_mode)) - return -ENODEV; - - if (new_size == (uint64_t) st.st_size) - return 0; - - if (grow_only && new_size < (uint64_t) st.st_size) - return -EINVAL; - - xsprintf_dev_num_path(q, "block", dev); - loop_fd = open(q, O_RDWR|O_CLOEXEC|O_NOCTTY); - if (loop_fd < 0) - return -errno; - - if (snprintf(args.name, sizeof(args.name), "%" PRIu64, new_size) >= (int) sizeof(args.name)) - return -EINVAL; - - if (new_size < (uint64_t) st.st_size) { - /* Decrease size: first decrease btrfs size, then shorten loopback */ - if (ioctl(fd, BTRFS_IOC_RESIZE, &args) < 0) - return -errno; - } - - if (ftruncate(backing_fd, new_size) < 0) - return -errno; - - if (ioctl(loop_fd, LOOP_SET_CAPACITY, 0) < 0) - return -errno; - - if (new_size > (uint64_t) st.st_size) { - /* Increase size: first enlarge loopback, then increase btrfs size */ - if (ioctl(fd, BTRFS_IOC_RESIZE, &args) < 0) - return -errno; - } - - /* Make sure the free disk space is correctly updated for both file systems */ - (void) fsync(fd); - (void) fsync(backing_fd); - - return 1; -} - -int btrfs_resize_loopback(const char *p, uint64_t new_size, bool grow_only) { - _cleanup_close_ int fd = -1; - - fd = open(p, O_RDONLY|O_NOCTTY|O_CLOEXEC); - if (fd < 0) - return -errno; - - return btrfs_resize_loopback_fd(fd, new_size, grow_only); -} - int btrfs_qgroupid_make(uint64_t level, uint64_t id, uint64_t *ret) { assert(ret); diff --git a/src/basic/btrfs-util.h b/src/basic/btrfs-util.h index 3adb51e02d..085aca4dbc 100644 --- a/src/basic/btrfs-util.h +++ b/src/basic/btrfs-util.h @@ -62,9 +62,6 @@ int btrfs_quota_scan_start(int fd); int btrfs_quota_scan_wait(int fd); int btrfs_quota_scan_ongoing(int fd); -int btrfs_resize_loopback_fd(int fd, uint64_t size, bool grow_only); -int btrfs_resize_loopback(const char *path, uint64_t size, bool grow_only); - int btrfs_subvol_make(const char *path); int btrfs_subvol_make_fd(int fd, const char *subvolume); diff --git a/src/import/import-raw.c b/src/import/import-raw.c index e8adaae740..2b96330c14 100644 --- a/src/import/import-raw.c +++ b/src/import/import-raw.c @@ -37,7 +37,6 @@ struct RawImport { char *local; bool force_local; bool read_only; - bool grow_machine_directory; char *temp_path; char *final_path; @@ -47,8 +46,6 @@ struct RawImport { ImportCompress compress; - uint64_t written_since_last_grow; - sd_event_source *input_event_source; uint8_t buffer[16*1024]; @@ -95,7 +92,6 @@ int raw_import_new( _cleanup_(raw_import_unrefp) RawImport *i = NULL; _cleanup_free_ char *root = NULL; - bool grow; int r; assert(ret); @@ -104,8 +100,6 @@ int raw_import_new( if (!root) return -ENOMEM; - grow = path_startswith(root, "/var/lib/machines"); - i = new(RawImport, 1); if (!i) return -ENOMEM; @@ -117,7 +111,6 @@ int raw_import_new( .userdata = userdata, .last_percent = (unsigned) -1, .image_root = TAKE_PTR(root), - .grow_machine_directory = grow, }; RATELIMIT_INIT(i->progress_rate_limit, 100 * USEC_PER_MSEC, 1); @@ -307,11 +300,6 @@ static int raw_import_write(const void *p, size_t sz, void *userdata) { RawImport *i = userdata; ssize_t n; - if (i->grow_machine_directory && i->written_since_last_grow >= GROW_INTERVAL_BYTES) { - i->written_since_last_grow = 0; - grow_machine_directory(); - } - n = sparse_write(i->output_fd, p, sz, 64); if (n < 0) return (int) n; @@ -319,7 +307,6 @@ static int raw_import_write(const void *p, size_t sz, void *userdata) { return -EIO; i->written_uncompressed += sz; - i->written_since_last_grow += sz; return 0; } diff --git a/src/import/import-tar.c b/src/import/import-tar.c index a164417de0..896effab77 100644 --- a/src/import/import-tar.c +++ b/src/import/import-tar.c @@ -37,7 +37,6 @@ struct TarImport { char *local; bool force_local; bool read_only; - bool grow_machine_directory; char *temp_path; char *final_path; @@ -47,8 +46,6 @@ struct TarImport { ImportCompress compress; - uint64_t written_since_last_grow; - sd_event_source *input_event_source; uint8_t buffer[16*1024]; @@ -102,7 +99,6 @@ int tar_import_new( _cleanup_(tar_import_unrefp) TarImport *i = NULL; _cleanup_free_ char *root = NULL; - bool grow; int r; assert(ret); @@ -111,8 +107,6 @@ int tar_import_new( if (!root) return -ENOMEM; - grow = path_startswith(root, "/var/lib/machines"); - i = new(TarImport, 1); if (!i) return -ENOMEM; @@ -124,7 +118,6 @@ int tar_import_new( .userdata = userdata, .last_percent = (unsigned) -1, .image_root = TAKE_PTR(root), - .grow_machine_directory = grow, }; RATELIMIT_INIT(i->progress_rate_limit, 100 * USEC_PER_MSEC, 1); @@ -245,17 +238,11 @@ static int tar_import_write(const void *p, size_t sz, void *userdata) { TarImport *i = userdata; int r; - if (i->grow_machine_directory && i->written_since_last_grow >= GROW_INTERVAL_BYTES) { - i->written_since_last_grow = 0; - grow_machine_directory(); - } - r = loop_write(i->tar_fd, p, sz, false); if (r < 0) return r; i->written_uncompressed += sz; - i->written_since_last_grow += sz; return 0; } diff --git a/src/import/importd.c b/src/import/importd.c index 6fa8342172..83f186f66e 100644 --- a/src/import/importd.c +++ b/src/import/importd.c @@ -719,7 +719,7 @@ static int method_import_tar_or_raw(sd_bus_message *msg, void *userdata, sd_bus_ if (!machine_name_is_valid(local)) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Local name %s is invalid", local); - r = setup_machine_directory((uint64_t) -1, error); + r = setup_machine_directory(error); if (r < 0) return r; @@ -783,7 +783,7 @@ static int method_import_fs(sd_bus_message *msg, void *userdata, sd_bus_error *e if (!machine_name_is_valid(local)) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Local name %s is invalid", local); - r = setup_machine_directory((uint64_t) -1, error); + r = setup_machine_directory(error); if (r < 0) return r; @@ -924,7 +924,7 @@ static int method_pull_tar_or_raw(sd_bus_message *msg, void *userdata, sd_bus_er if (v < 0) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Unknown verification mode %s", verify); - r = setup_machine_directory((uint64_t) -1, error); + r = setup_machine_directory(error); if (r < 0) return r; diff --git a/src/import/pull-job.c b/src/import/pull-job.c index 0b7d9df32d..a44e0a7eda 100644 --- a/src/import/pull-job.c +++ b/src/import/pull-job.c @@ -74,7 +74,6 @@ static int pull_job_restart(PullJob *j) { j->payload_allocated = 0; j->written_compressed = 0; j->written_uncompressed = 0; - j->written_since_last_grow = 0; r = pull_job_begin(j); if (r < 0) @@ -224,11 +223,6 @@ static int pull_job_write_uncompressed(const void *p, size_t sz, void *userdata) if (j->disk_fd >= 0) { - if (j->grow_machine_directory && j->written_since_last_grow >= GROW_INTERVAL_BYTES) { - j->written_since_last_grow = 0; - grow_machine_directory(); - } - if (j->allow_sparse) n = sparse_write(j->disk_fd, p, sz, 64); else { @@ -250,7 +244,6 @@ static int pull_job_write_uncompressed(const void *p, size_t sz, void *userdata) } j->written_uncompressed += sz; - j->written_since_last_grow += sz; return 0; } @@ -577,9 +570,6 @@ int pull_job_begin(PullJob *j) { if (j->state != PULL_JOB_INIT) return -EBUSY; - if (j->grow_machine_directory) - grow_machine_directory(); - r = curl_glue_make(&j->curl, j->url, j); if (r < 0) return r; diff --git a/src/import/pull-job.h b/src/import/pull-job.h index 0c104af758..c907e74060 100644 --- a/src/import/pull-job.h +++ b/src/import/pull-job.h @@ -80,9 +80,6 @@ struct PullJob { char *checksum; - bool grow_machine_directory; - uint64_t written_since_last_grow; - VerificationStyle style; }; diff --git a/src/import/pull-raw.c b/src/import/pull-raw.c index 2ceca68c07..39d1747f7d 100644 --- a/src/import/pull-raw.c +++ b/src/import/pull-raw.c @@ -56,7 +56,6 @@ struct RawPull { char *local; bool force_local; - bool grow_machine_directory; bool settings; bool roothash; @@ -119,7 +118,6 @@ int raw_pull_new( _cleanup_(sd_event_unrefp) sd_event *e = NULL; _cleanup_(raw_pull_unrefp) RawPull *i = NULL; _cleanup_free_ char *root = NULL; - bool grow; int r; assert(ret); @@ -128,8 +126,6 @@ int raw_pull_new( if (!root) return -ENOMEM; - grow = path_startswith(root, "/var/lib/machines"); - if (event) e = sd_event_ref(event); else { @@ -150,7 +146,6 @@ int raw_pull_new( .on_finished = on_finished, .userdata = userdata, .image_root = TAKE_PTR(root), - .grow_machine_directory = grow, .event = TAKE_PTR(e), .glue = TAKE_PTR(g), }; @@ -689,7 +684,6 @@ int raw_pull_start( i->raw_job->on_open_disk = raw_pull_job_on_open_disk_raw; i->raw_job->on_progress = raw_pull_job_on_progress; i->raw_job->calc_checksum = verify != IMPORT_VERIFY_NO; - i->raw_job->grow_machine_directory = i->grow_machine_directory; r = pull_find_old_etags(url, i->image_root, DT_REG, ".raw-", ".raw", &i->raw_job->old_etags); if (r < 0) diff --git a/src/import/pull-tar.c b/src/import/pull-tar.c index 67ea8813e1..b19d4ea391 100644 --- a/src/import/pull-tar.c +++ b/src/import/pull-tar.c @@ -52,7 +52,6 @@ struct TarPull { char *local; bool force_local; - bool grow_machine_directory; bool settings; pid_t tar_pid; @@ -112,7 +111,6 @@ int tar_pull_new( _cleanup_(sd_event_unrefp) sd_event *e = NULL; _cleanup_(tar_pull_unrefp) TarPull *i = NULL; _cleanup_free_ char *root = NULL; - bool grow; int r; assert(ret); @@ -121,8 +119,6 @@ int tar_pull_new( if (!root) return -ENOMEM; - grow = path_startswith(root, "/var/lib/machines"); - if (event) e = sd_event_ref(event); else { @@ -143,7 +139,6 @@ int tar_pull_new( .on_finished = on_finished, .userdata = userdata, .image_root = TAKE_PTR(root), - .grow_machine_directory = grow, .event = TAKE_PTR(e), .glue = TAKE_PTR(g), }; @@ -512,7 +507,6 @@ int tar_pull_start( i->tar_job->on_open_disk = tar_pull_job_on_open_disk_tar; i->tar_job->on_progress = tar_pull_job_on_progress; i->tar_job->calc_checksum = verify != IMPORT_VERIFY_NO; - i->tar_job->grow_machine_directory = i->grow_machine_directory; r = pull_find_old_etags(url, i->image_root, DT_DIR, ".tar-", NULL, &i->tar_job->old_etags); if (r < 0) diff --git a/src/machine/machined-dbus.c b/src/machine/machined-dbus.c index 87e6298c78..453ca30b64 100644 --- a/src/machine/machined-dbus.c +++ b/src/machine/machined-dbus.c @@ -41,15 +41,10 @@ static int property_get_pool_usage( _cleanup_close_ int fd = -1; uint64_t usage = (uint64_t) -1; - struct stat st; assert(bus); assert(reply); - /* We try to read the quota info from /var/lib/machines, as - * well as the usage of the loopback file - * /var/lib/machines.raw, and pick the larger value. */ - fd = open("/var/lib/machines", O_RDONLY|O_CLOEXEC|O_DIRECTORY); if (fd >= 0) { BtrfsQuotaInfo q; @@ -58,11 +53,6 @@ static int property_get_pool_usage( usage = q.referenced; } - if (stat("/var/lib/machines.raw", &st) >= 0) { - if (usage == (uint64_t) -1 || st.st_blocks * 512ULL > usage) - usage = st.st_blocks * 512ULL; - } - return sd_bus_message_append(reply, "t", usage); } @@ -77,15 +67,10 @@ static int property_get_pool_limit( _cleanup_close_ int fd = -1; uint64_t size = (uint64_t) -1; - struct stat st; assert(bus); assert(reply); - /* We try to read the quota limit from /var/lib/machines, as - * well as the size of the loopback file - * /var/lib/machines.raw, and pick the smaller value. */ - fd = open("/var/lib/machines", O_RDONLY|O_CLOEXEC|O_DIRECTORY); if (fd >= 0) { BtrfsQuotaInfo q; @@ -94,11 +79,6 @@ static int property_get_pool_limit( size = q.referenced_max; } - if (stat("/var/lib/machines.raw", &st) >= 0) { - if (size == (uint64_t) -1 || (uint64_t) st.st_size < size) - size = st.st_size; - } - return sd_bus_message_append(reply, "t", size); } @@ -877,19 +857,10 @@ static int method_set_pool_limit(sd_bus_message *message, void *userdata, sd_bus return 1; /* Will call us back */ /* Set up the machine directory if necessary */ - r = setup_machine_directory(limit, error); + r = setup_machine_directory(error); if (r < 0) return r; - /* Resize the backing loopback device, if there is one, except if we asked to drop any limit */ - if (limit != (uint64_t) -1) { - r = btrfs_resize_loopback("/var/lib/machines", limit, false); - if (r == -ENOTTY) - return sd_bus_error_setf(error, SD_BUS_ERROR_NOT_SUPPORTED, "Quota is only supported on btrfs."); - if (r < 0 && r != -ENODEV) /* ignore ENODEV, as that's what is returned if the file system is not on loopback */ - return sd_bus_error_set_errnof(error, r, "Failed to adjust loopback limit: %m"); - } - (void) btrfs_qgroup_set_limit("/var/lib/machines", 0, limit); r = btrfs_subvol_set_subtree_quota_limit("/var/lib/machines", 0, limit); diff --git a/src/shared/machine-pool.c b/src/shared/machine-pool.c index df56492e7b..de4f704252 100644 --- a/src/shared/machine-pool.c +++ b/src/shared/machine-pool.c @@ -1,46 +1,13 @@ /* SPDX-License-Identifier: LGPL-2.1+ */ #include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include #include -#include -#include -#include "sd-bus-protocol.h" -#include "sd-bus.h" - -#include "alloc-util.h" #include "btrfs-util.h" -#include "fd-util.h" -#include "fileio.h" -#include "fs-util.h" #include "label.h" -#include "lockfile-util.h" -#include "log.h" #include "machine-pool.h" -#include "macro.h" #include "missing.h" -#include "mkdir.h" -#include "mount-util.h" -#include "parse-util.h" -#include "path-util.h" -#include "process-util.h" -#include "signal-util.h" #include "stat-util.h" -#include "string-util.h" - -#define VAR_LIB_MACHINES_SIZE_START (1024UL*1024UL*500UL) -#define VAR_LIB_MACHINES_FREE_MIN (1024UL*1024UL*750UL) static int check_btrfs(void) { struct statfs sfs; @@ -56,344 +23,24 @@ static int check_btrfs(void) { return F_TYPE_EQUAL(sfs.f_type, BTRFS_SUPER_MAGIC); } -static int setup_machine_raw(uint64_t size, sd_bus_error *error) { - _cleanup_free_ char *tmp = NULL; - _cleanup_close_ int fd = -1; - struct statvfs ss; - pid_t pid = 0; +int setup_machine_directory(sd_bus_error *error) { int r; - /* We want to be able to make use of btrfs-specific file - * system features, in particular subvolumes, reflinks and - * quota. Hence, if we detect that /var/lib/machines.raw is - * not located on btrfs, let's create a loopback file, place a - * btrfs file system into it, and mount it to - * /var/lib/machines. */ - - fd = open("/var/lib/machines.raw", O_RDWR|O_CLOEXEC|O_NONBLOCK|O_NOCTTY); - if (fd >= 0) - return TAKE_FD(fd); - - if (errno != ENOENT) - return sd_bus_error_set_errnof(error, errno, "Failed to open /var/lib/machines.raw: %m"); - - r = tempfn_xxxxxx("/var/lib/machines.raw", NULL, &tmp); - if (r < 0) - return r; - - (void) mkdir_p_label("/var/lib", 0755); - fd = open(tmp, O_RDWR|O_CREAT|O_EXCL|O_NOCTTY|O_CLOEXEC, 0600); - if (fd < 0) - return sd_bus_error_set_errnof(error, errno, "Failed to create /var/lib/machines.raw: %m"); - - if (fstatvfs(fd, &ss) < 0) { - r = sd_bus_error_set_errnof(error, errno, "Failed to determine free space on /var/lib/machines.raw: %m"); - goto fail; - } - - if (ss.f_bsize * ss.f_bavail < VAR_LIB_MACHINES_FREE_MIN) { - r = sd_bus_error_setf(error, SD_BUS_ERROR_FAILED, "Not enough free disk space to set up /var/lib/machines."); - goto fail; - } - - if (ftruncate(fd, size) < 0) { - r = sd_bus_error_set_errnof(error, errno, "Failed to enlarge /var/lib/machines.raw: %m"); - goto fail; - } - - r = safe_fork("(mkfs)", FORK_RESET_SIGNALS|FORK_DEATHSIG, &pid); - if (r < 0) { - sd_bus_error_set_errnof(error, r, "Failed to fork mkfs.btrfs: %m"); - goto fail; - } - if (r == 0) { - - /* Child */ - - fd = safe_close(fd); - - execlp("mkfs.btrfs", "-Lvar-lib-machines", tmp, NULL); - if (errno == ENOENT) - _exit(99); - - _exit(EXIT_FAILURE); - } - - r = wait_for_terminate_and_check("mkfs", pid, 0); - pid = 0; - - if (r < 0) { - sd_bus_error_set_errnof(error, r, "Failed to wait for mkfs.btrfs: %m"); - goto fail; - } - if (r == 99) { - r = sd_bus_error_set_errnof(error, ENOENT, "Cannot set up /var/lib/machines, mkfs.btrfs is missing"); - goto fail; - } - if (r != EXIT_SUCCESS) { - r = sd_bus_error_setf(error, SD_BUS_ERROR_FAILED, "mkfs.btrfs failed with error code %i", r); - goto fail; - } - - r = rename_noreplace(AT_FDCWD, tmp, AT_FDCWD, "/var/lib/machines.raw"); - if (r < 0) { - sd_bus_error_set_errnof(error, r, "Failed to move /var/lib/machines.raw into place: %m"); - goto fail; - } - - return TAKE_FD(fd); - -fail: - unlink_noerrno(tmp); - - if (pid > 1) - kill_and_sigcont(pid, SIGKILL); - - return r; -} - -int setup_machine_directory(uint64_t size, sd_bus_error *error) { - _cleanup_(release_lock_file) LockFile lock_file = LOCK_FILE_INIT; - struct loop_info64 info = { - .lo_flags = LO_FLAGS_AUTOCLEAR, - }; - _cleanup_close_ int fd = -1, control = -1, loop = -1; - _cleanup_free_ char* loopdev = NULL; - char tmpdir[] = "/tmp/machine-pool.XXXXXX", *mntdir = NULL; - bool tmpdir_made = false, mntdir_made = false, mntdir_mounted = false; - char buf[FORMAT_BYTES_MAX]; - int r, nr = -1; - - /* btrfs cannot handle file systems < 16M, hence use this as minimum */ - if (size == (uint64_t) -1) - size = VAR_LIB_MACHINES_SIZE_START; - else if (size < 16*1024*1024) - size = 16*1024*1024; - - /* Make sure we only set the directory up once at a time */ - r = make_lock_file("/run/systemd/machines.lock", LOCK_EX, &lock_file); - if (r < 0) - return r; - r = check_btrfs(); if (r < 0) return sd_bus_error_set_errnof(error, r, "Failed to determine whether /var/lib/machines is located on btrfs: %m"); - if (r > 0) { - (void) btrfs_subvol_make_label("/var/lib/machines"); - - r = btrfs_quota_enable("/var/lib/machines", true); - if (r < 0) - log_warning_errno(r, "Failed to enable quota for /var/lib/machines, ignoring: %m"); - - r = btrfs_subvol_auto_qgroup("/var/lib/machines", 0, true); - if (r < 0) - log_warning_errno(r, "Failed to set up default quota hierarchy for /var/lib/machines, ignoring: %m"); - - return 1; - } - - if (path_is_mount_point("/var/lib/machines", NULL, AT_SYMLINK_FOLLOW) > 0) { - log_debug("/var/lib/machines is already a mount point, not creating loopback file for it."); - return 0; - } - - r = dir_is_populated("/var/lib/machines"); - if (r < 0 && r != -ENOENT) - return r; - if (r > 0) { - log_debug("/var/log/machines is already populated, not creating loopback file for it."); - return 0; - } - - r = mkfs_exists("btrfs"); - if (r == 0) - return sd_bus_error_set_errnof(error, ENOENT, "Cannot set up /var/lib/machines, mkfs.btrfs is missing"); - if (r < 0) - return r; - - fd = setup_machine_raw(size, error); - if (fd < 0) - return fd; - - control = open("/dev/loop-control", O_RDWR|O_CLOEXEC|O_NOCTTY|O_NONBLOCK); - if (control < 0) - return sd_bus_error_set_errnof(error, errno, "Failed to open /dev/loop-control: %m"); - - nr = ioctl(control, LOOP_CTL_GET_FREE); - if (nr < 0) - return sd_bus_error_set_errnof(error, errno, "Failed to allocate loop device: %m"); - - if (asprintf(&loopdev, "/dev/loop%i", nr) < 0) { - r = -ENOMEM; - goto fail; - } - - loop = open(loopdev, O_CLOEXEC|O_RDWR|O_NOCTTY|O_NONBLOCK); - if (loop < 0) { - r = sd_bus_error_set_errnof(error, errno, "Failed to open loopback device: %m"); - goto fail; - } - - if (ioctl(loop, LOOP_SET_FD, fd) < 0) { - r = sd_bus_error_set_errnof(error, errno, "Failed to bind loopback device: %m"); - goto fail; - } - - if (ioctl(loop, LOOP_SET_STATUS64, &info) < 0) { - r = sd_bus_error_set_errnof(error, errno, "Failed to enable auto-clear for loopback device: %m"); - goto fail; - } - - /* We need to make sure the new /var/lib/machines directory - * has an access mode of 0700 at the time it is first made - * available. mkfs will create it with 0755 however. Hence, - * let's mount the directory into an inaccessible directory - * below /tmp first, fix the access mode, and move it to the - * public place then. */ - - if (!mkdtemp(tmpdir)) { - r = sd_bus_error_set_errnof(error, errno, "Failed to create temporary mount parent directory: %m"); - goto fail; - } - tmpdir_made = true; - - mntdir = strjoina(tmpdir, "/mnt"); - if (mkdir(mntdir, 0700) < 0) { - r = sd_bus_error_set_errnof(error, errno, "Failed to create temporary mount directory: %m"); - goto fail; - } - mntdir_made = true; - - if (mount(loopdev, mntdir, "btrfs", 0, NULL) < 0) { - r = sd_bus_error_set_errnof(error, errno, "Failed to mount loopback device: %m"); - goto fail; - } - mntdir_mounted = true; - - r = btrfs_quota_enable(mntdir, true); - if (r < 0) - log_warning_errno(r, "Failed to enable quota, ignoring: %m"); - - r = btrfs_subvol_auto_qgroup(mntdir, 0, true); - if (r < 0) - log_warning_errno(r, "Failed to set up default quota hierarchy, ignoring: %m"); - - if (chmod(mntdir, 0700) < 0) { - r = sd_bus_error_set_errnof(error, errno, "Failed to fix owner: %m"); - goto fail; - } - - (void) mkdir_p_label("/var/lib/machines", 0700); - - if (mount(mntdir, "/var/lib/machines", NULL, MS_BIND, NULL) < 0) { - r = sd_bus_error_set_errnof(error, errno, "Failed to mount directory into right place: %m"); - goto fail; - } - - (void) syncfs(fd); - - log_info("Set up /var/lib/machines as btrfs loopback file system of size %s mounted on /var/lib/machines.raw.", format_bytes(buf, sizeof(buf), size)); - - (void) umount2(mntdir, MNT_DETACH); - (void) rmdir(mntdir); - (void) rmdir(tmpdir); - - return 1; - -fail: - if (mntdir_mounted) - (void) umount2(mntdir, MNT_DETACH); - - if (mntdir_made) - (void) rmdir(mntdir); - if (tmpdir_made) - (void) rmdir(tmpdir); - - if (loop >= 0) { - (void) ioctl(loop, LOOP_CLR_FD); - loop = safe_close(loop); - } - - (void) ioctl(control, LOOP_CTL_REMOVE, nr); - - return r; -} - -static int sync_path(const char *p) { - _cleanup_close_ int fd = -1; - - fd = open(p, O_RDONLY|O_CLOEXEC|O_NOCTTY); - if (fd < 0) - return -errno; - - if (syncfs(fd) < 0) - return -errno; - - return 0; -} - -int grow_machine_directory(void) { - char buf[FORMAT_BYTES_MAX]; - struct statvfs a, b; - uint64_t old_size, new_size, max_add; - int r; - - /* Ensure the disk space data is accurate */ - sync_path("/var/lib/machines"); - sync_path("/var/lib/machines.raw"); - - if (statvfs("/var/lib/machines.raw", &a) < 0) - return -errno; - - if (statvfs("/var/lib/machines", &b) < 0) - return -errno; - - /* Don't grow if not enough disk space is available on the host */ - if (((uint64_t) a.f_bavail * (uint64_t) a.f_bsize) <= VAR_LIB_MACHINES_FREE_MIN) - return 0; - - /* Don't grow if at least 1/3th of the fs is still free */ - if (b.f_bavail > b.f_blocks / 3) - return 0; - - /* Calculate how much we are willing to add at most */ - max_add = ((uint64_t) a.f_bavail * (uint64_t) a.f_bsize) - VAR_LIB_MACHINES_FREE_MIN; - - /* Calculate the old size */ - old_size = (uint64_t) b.f_blocks * (uint64_t) b.f_bsize; - - /* Calculate the new size as three times the size of what is used right now */ - new_size = ((uint64_t) b.f_blocks - (uint64_t) b.f_bavail) * (uint64_t) b.f_bsize * 3; - - /* Always, grow at least to the start size */ - if (new_size < VAR_LIB_MACHINES_SIZE_START) - new_size = VAR_LIB_MACHINES_SIZE_START; - - /* If the new size is smaller than the old size, don't grow */ - if (new_size < old_size) - return 0; - - /* Ensure we never add more than the maximum */ - if (new_size > old_size + max_add) - new_size = old_size + max_add; - - r = btrfs_resize_loopback("/var/lib/machines", new_size, true); - if (r < 0) - return log_debug_errno(r, "Failed to resize loopback: %m"); if (r == 0) return 0; - /* Also bump the quota, of both the subvolume leaf qgroup, as - * well as of any subtree quota group by the same id but a - * higher level, if it exists. */ - r = btrfs_qgroup_set_limit("/var/lib/machines", 0, new_size); - if (r < 0) - log_debug_errno(r, "Failed to set btrfs limit: %m"); + (void) btrfs_subvol_make_label("/var/lib/machines"); - r = btrfs_subvol_set_subtree_quota_limit("/var/lib/machines", 0, new_size); + r = btrfs_quota_enable("/var/lib/machines", true); if (r < 0) - log_debug_errno(r, "Failed to set btrfs subtree limit: %m"); + log_warning_errno(r, "Failed to enable quota for /var/lib/machines, ignoring: %m"); + + r = btrfs_subvol_auto_qgroup("/var/lib/machines", 0, true); + if (r < 0) + log_warning_errno(r, "Failed to set up default quota hierarchy for /var/lib/machines, ignoring: %m"); - log_info("Grew /var/lib/machines btrfs loopback file system to %s.", format_bytes(buf, sizeof(buf), new_size)); return 1; } diff --git a/src/shared/machine-pool.h b/src/shared/machine-pool.h index fd09296f23..6f59a18fb6 100644 --- a/src/shared/machine-pool.h +++ b/src/shared/machine-pool.h @@ -5,8 +5,4 @@ #include "sd-bus.h" -/* Grow the /var/lib/machines directory after each 10MiB written */ -#define GROW_INTERVAL_BYTES (UINT64_C(10) * UINT64_C(1024) * UINT64_C(1024)) - -int setup_machine_directory(uint64_t size, sd_bus_error *error); -int grow_machine_directory(void); +int setup_machine_directory(sd_bus_error *error); diff --git a/units/var-lib-machines.mount b/units/var-lib-machines.mount index 5da0c6fa32..3658199cc7 100644 --- a/units/var-lib-machines.mount +++ b/units/var-lib-machines.mount @@ -7,8 +7,13 @@ # the Free Software Foundation; either version 2.1 of the License, or # (at your option) any later version. +# This unit is required for pre-240 versions of systemd that automatically set +# up /var/lib/machines.raw as loopback-mounted btrfs file system. Later +# versions don't do that anymore, but let's keep minimal compatibility by +# mounting the image still, if it exists. + [Unit] -Description=Virtual Machine and Container Storage +Description=Virtual Machine and Container Storage (Compatibility) ConditionPathExists=/var/lib/machines.raw [Mount] From 77e0a1b5e09d782a32f5144319f344cc77f5ab45 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 26 Oct 2018 16:06:54 +0200 Subject: [PATCH 14/23] path-util: handle NULL inputs in last_path_component() --- src/basic/path-util.c | 3 +++ src/test/test-path-util.c | 1 + 2 files changed, 4 insertions(+) diff --git a/src/basic/path-util.c b/src/basic/path-util.c index eb64c886e6..243771d9e4 100644 --- a/src/basic/path-util.c +++ b/src/basic/path-util.c @@ -750,6 +750,9 @@ const char *last_path_component(const char *path) { unsigned l, k; + if (!path) + return NULL; + l = k = strlen(path); if (l == 0) /* special case — an empty string */ return path; diff --git a/src/test/test-path-util.c b/src/test/test-path-util.c index b5030ea494..dd00f13905 100644 --- a/src/test/test-path-util.c +++ b/src/test/test-path-util.c @@ -406,6 +406,7 @@ static void test_file_in_same_dir(void) { } static void test_last_path_component(void) { + assert_se(last_path_component(NULL) == NULL); assert_se(streq(last_path_component("a/b/c"), "c")); assert_se(streq(last_path_component("a/b/c/"), "c/")); assert_se(streq(last_path_component("/"), "/")); From a60c8eee108c3ac226e340c6218ad0c7576a225f Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 26 Oct 2018 16:07:35 +0200 Subject: [PATCH 15/23] path-util: add helper for extracting last filename from path --- src/basic/path-util.c | 31 ++++++++++++++++++++++++++++++ src/basic/path-util.h | 1 + src/test/test-path-util.c | 40 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 72 insertions(+) diff --git a/src/basic/path-util.c b/src/basic/path-util.c index 243771d9e4..b7f91ee3ae 100644 --- a/src/basic/path-util.c +++ b/src/basic/path-util.c @@ -769,6 +769,37 @@ const char *last_path_component(const char *path) { return path + k; } +int path_extract_filename(const char *p, char **ret) { + _cleanup_free_ char *a = NULL; + const char *c, *e = NULL, *q; + + /* Extracts the filename part (i.e. right-most component) from a path, i.e. string that passes + * filename_is_valid(). A wrapper around last_path_component(), but eats up trailing slashes. */ + + if (!p) + return -EINVAL; + + c = last_path_component(p); + + for (q = c; *q != 0; q++) + if (*q != '/') + e = q + 1; + + if (!e) /* no valid character? */ + return -EINVAL; + + a = strndup(c, e - c); + if (!a) + return -ENOMEM; + + if (!filename_is_valid(a)) + return -EINVAL; + + *ret = TAKE_PTR(a); + + return 0; +} + bool filename_is_valid(const char *p) { const char *e; diff --git a/src/basic/path-util.h b/src/basic/path-util.h index e2a51ff33a..53b980d3c1 100644 --- a/src/basic/path-util.h +++ b/src/basic/path-util.h @@ -133,6 +133,7 @@ int parse_path_argument_and_warn(const char *path, bool suppress_root, char **ar char* dirname_malloc(const char *path); const char *last_path_component(const char *path); +int path_extract_filename(const char *p, char **ret); bool filename_is_valid(const char *p) _pure_; bool path_is_valid(const char *p) _pure_; diff --git a/src/test/test-path-util.c b/src/test/test-path-util.c index dd00f13905..7feae54068 100644 --- a/src/test/test-path-util.c +++ b/src/test/test-path-util.c @@ -425,6 +425,45 @@ static void test_last_path_component(void) { assert_se(streq(last_path_component("/a/"), "a/")); } +static void test_path_extract_filename_one(const char *input, const char *output, int ret) { + _cleanup_free_ char *k = NULL; + int r; + + r = path_extract_filename(input, &k); + log_info("%s → %s/%s [expected: %s/%s]", strnull(input), strnull(k), strerror(-r), strnull(output), strerror(-ret)); + assert_se(streq_ptr(k, output)); + assert_se(r == ret); +} + +static void test_path_extract_filename(void) { + test_path_extract_filename_one(NULL, NULL, -EINVAL); + test_path_extract_filename_one("a/b/c", "c", 0); + test_path_extract_filename_one("a/b/c/", "c", 0); + test_path_extract_filename_one("/", NULL, -EINVAL); + test_path_extract_filename_one("//", NULL, -EINVAL); + test_path_extract_filename_one("///", NULL, -EINVAL); + test_path_extract_filename_one(".", NULL, -EINVAL); + test_path_extract_filename_one("./.", NULL, -EINVAL); + test_path_extract_filename_one("././", NULL, -EINVAL); + test_path_extract_filename_one("././/", NULL, -EINVAL); + test_path_extract_filename_one("/foo/a", "a", 0); + test_path_extract_filename_one("/foo/a/", "a", 0); + test_path_extract_filename_one("", NULL, -EINVAL); + test_path_extract_filename_one("a", "a", 0); + test_path_extract_filename_one("a/", "a", 0); + test_path_extract_filename_one("/a", "a", 0); + test_path_extract_filename_one("/a/", "a", 0); + test_path_extract_filename_one("/////////////a/////////////", "a", 0); + test_path_extract_filename_one("xx/.", NULL, -EINVAL); + test_path_extract_filename_one("xx/..", NULL, -EINVAL); + test_path_extract_filename_one("..", NULL, -EINVAL); + test_path_extract_filename_one("/..", NULL, -EINVAL); + test_path_extract_filename_one("../", NULL, -EINVAL); + test_path_extract_filename_one(".", NULL, -EINVAL); + test_path_extract_filename_one("/.", NULL, -EINVAL); + test_path_extract_filename_one("./", NULL, -EINVAL); +} + static void test_filename_is_valid(void) { char foo[FILENAME_MAX+2]; int i; @@ -543,6 +582,7 @@ int main(int argc, char **argv) { test_prefix_root(); test_file_in_same_dir(); test_last_path_component(); + test_path_extract_filename(); test_filename_is_valid(); test_hidden_or_backup_file(); test_skip_dev_prefix(); From 0c9448fa8201ad2e2a2dde57f4d4b7e48a34c522 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 26 Oct 2018 16:13:07 +0200 Subject: [PATCH 16/23] copy: remove reflinking debug message It pretty much ends up in an inner loop most of the time, hence let's just remove it. This is pretty easily debuggable with strace anyway... --- src/basic/copy.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/basic/copy.c b/src/basic/copy.c index 5cc3a9572b..7a4db459e3 100644 --- a/src/basic/copy.c +++ b/src/basic/copy.c @@ -163,8 +163,6 @@ int copy_bytes_full( return 1; /* we copied only some number of bytes, which worked, but this means we didn't hit EOF, return 1 */ } } - - log_debug_errno(r, "Reflinking didn't work, falling back to non-reflink copying: %m"); } } } From 844416b64c15f7b417d79bf64df9f475cf4d6047 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 26 Oct 2018 16:51:00 +0200 Subject: [PATCH 17/23] stat-util: add fd_verify_directory(), similar to fd_verify_regular() --- src/basic/stat-util.c | 23 +++++++++++++++++++++++ src/basic/stat-util.h | 3 +++ 2 files changed, 26 insertions(+) diff --git a/src/basic/stat-util.c b/src/basic/stat-util.c index 3bef0dfe44..8b63eb360b 100644 --- a/src/basic/stat-util.c +++ b/src/basic/stat-util.c @@ -296,3 +296,26 @@ int fd_verify_regular(int fd) { return stat_verify_regular(&st); } + +int stat_verify_directory(const struct stat *st) { + assert(st); + + if (S_ISLNK(st->st_mode)) + return -ELOOP; + + if (!S_ISDIR(st->st_mode)) + return -ENOTDIR; + + return 0; +} + +int fd_verify_directory(int fd) { + struct stat st; + + assert(fd >= 0); + + if (fstat(fd, &st) < 0) + return -errno; + + return stat_verify_directory(&st); +} diff --git a/src/basic/stat-util.h b/src/basic/stat-util.h index 1a725f1da0..84400a6083 100644 --- a/src/basic/stat-util.h +++ b/src/basic/stat-util.h @@ -59,3 +59,6 @@ int path_is_temporary_fs(const char *path); int stat_verify_regular(const struct stat *st); int fd_verify_regular(int fd); + +int stat_verify_directory(const struct stat *st); +int fd_verify_directory(int fd); From 176a05c24be1168cd0a0e39f5e7084112861d3ed Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 26 Oct 2018 16:51:34 +0200 Subject: [PATCH 18/23] importd: immediately validate whether fd is of right type when receiving it Just some paranoia. --- src/import/importd.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/import/importd.c b/src/import/importd.c index 83f186f66e..2426933558 100644 --- a/src/import/importd.c +++ b/src/import/importd.c @@ -22,6 +22,7 @@ #include "process-util.h" #include "signal-util.h" #include "socket-util.h" +#include "stat-util.h" #include "string-table.h" #include "strv.h" #include "syslog-util.h" @@ -716,6 +717,10 @@ static int method_import_tar_or_raw(sd_bus_message *msg, void *userdata, sd_bus_ if (r < 0) return r; + r = fd_verify_regular(fd); + if (r < 0) + return r; + if (!machine_name_is_valid(local)) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Local name %s is invalid", local); @@ -780,6 +785,10 @@ static int method_import_fs(sd_bus_message *msg, void *userdata, sd_bus_error *e if (r < 0) return r; + r = fd_verify_directory(fd); + if (r < 0) + return r; + if (!machine_name_is_valid(local)) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Local name %s is invalid", local); @@ -846,6 +855,10 @@ static int method_export_tar_or_raw(sd_bus_message *msg, void *userdata, sd_bus_ if (!machine_name_is_valid(local)) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Local name %s is invalid", local); + r = fd_verify_regular(fd); + if (r < 0) + return r; + type = streq_ptr(sd_bus_message_get_member(msg), "ExportTar") ? TRANSFER_EXPORT_TAR : TRANSFER_EXPORT_RAW; r = transfer_new(m, &t); From e288e0599ea7cf192c923aa3b05a829263a2f3ba Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 26 Oct 2018 16:53:43 +0200 Subject: [PATCH 19/23] machinectl: be more careful when generating machine name from path name Let's use our new path_extract_filename() for extracting the basename from a path in a safe fashion. Let's do this for all our three local import calls. --- src/machine/machinectl.c | 69 +++++++++++++++++++++++----------------- 1 file changed, 40 insertions(+), 29 deletions(-) diff --git a/src/machine/machinectl.c b/src/machine/machinectl.c index 158cf73c28..a49b11e7b0 100644 --- a/src/machine/machinectl.c +++ b/src/machine/machinectl.c @@ -2000,28 +2000,38 @@ static int transfer_image_common(sd_bus *bus, sd_bus_message *m) { return -r; } +static const char *nullify_dash(const char *p) { + if (isempty(p)) + return NULL; + + if (streq(p, "-")) + return NULL; + + return p; +} + static int import_tar(int argc, char *argv[], void *userdata) { _cleanup_(sd_bus_message_unrefp) sd_bus_message *m = NULL; - _cleanup_free_ char *ll = NULL; - _cleanup_close_ int fd = -1; + _cleanup_free_ char *ll = NULL, *fn = NULL; const char *local = NULL, *path = NULL; + _cleanup_close_ int fd = -1; sd_bus *bus = userdata; int r; assert(bus); if (argc >= 2) - path = argv[1]; - if (isempty(path) || streq(path, "-")) - path = NULL; + path = nullify_dash(argv[1]); if (argc >= 3) - local = argv[2]; - else if (path) - local = basename(path); - if (isempty(local) || streq(local, "-")) - local = NULL; + local = nullify_dash(argv[2]); + else if (path) { + r = path_extract_filename(path, &fn); + if (r < 0) + return log_error_errno(r, "Cannot extract container name from filename: %m"); + local = fn; + } if (!local) { log_error("Need either path or local name."); return -EINVAL; @@ -2069,26 +2079,26 @@ static int import_tar(int argc, char *argv[], void *userdata) { static int import_raw(int argc, char *argv[], void *userdata) { _cleanup_(sd_bus_message_unrefp) sd_bus_message *m = NULL; - _cleanup_free_ char *ll = NULL; - _cleanup_close_ int fd = -1; + _cleanup_free_ char *ll = NULL, *fn = NULL; const char *local = NULL, *path = NULL; + _cleanup_close_ int fd = -1; sd_bus *bus = userdata; int r; assert(bus); if (argc >= 2) - path = argv[1]; - if (isempty(path) || streq(path, "-")) - path = NULL; + path = nullify_dash(argv[1]); if (argc >= 3) - local = argv[2]; - else if (path) - local = basename(path); - if (isempty(local) || streq(local, "-")) - local = NULL; + local = nullify_dash(argv[2]); + else if (path) { + r = path_extract_filename(path, &fn); + if (r < 0) + return log_error_errno(r, "Cannot extract container name from filename: %m"); + local = fn; + } if (!local) { log_error("Need either path or local name."); return -EINVAL; @@ -2136,25 +2146,26 @@ static int import_raw(int argc, char *argv[], void *userdata) { static int import_fs(int argc, char *argv[], void *userdata) { _cleanup_(sd_bus_message_unrefp) sd_bus_message *m = NULL; - _cleanup_close_ int fd = -1; const char *local = NULL, *path = NULL; + _cleanup_free_ char *fn = NULL; + _cleanup_close_ int fd = -1; sd_bus *bus = userdata; int r; assert(bus); if (argc >= 2) - path = argv[1]; - if (isempty(path) || streq(path, "-")) - path = NULL; + path = nullify_dash(argv[1]); if (argc >= 3) - local = argv[2]; - else if (path) - local = basename(path); - if (isempty(local) || streq(local, "-")) - local = NULL; + local = nullify_dash(argv[2]); + else if (path) { + r = path_extract_filename(path, &fn); + if (r < 0) + return log_error_errno(r, "Cannot extract container name from filename: %m"); + local = fn; + } if (!local) { log_error("Need either path or local name."); return -EINVAL; From 82f299e74e19d32b6c99faf3bbb6be0f89d0a8d5 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 26 Oct 2018 18:02:09 +0200 Subject: [PATCH 20/23] export-tar: check tar result as well Let's make sure we properly notice when tar doesn't actually work: let's propagate the error. --- src/import/export-tar.c | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/src/import/export-tar.c b/src/import/export-tar.c index 5acb09432a..3aab390637 100644 --- a/src/import/export-tar.c +++ b/src/import/export-tar.c @@ -141,6 +141,26 @@ static void tar_export_report_progress(TarExport *e) { e->last_percent = percent; } +static int tar_export_finish(TarExport *e) { + int r; + + assert(e); + assert(e->tar_fd >= 0); + + if (e->tar_pid > 0) { + r = wait_for_terminate_and_check("tar", e->tar_pid, WAIT_LOG); + e->tar_pid = 0; + if (r < 0) + return r; + if (r != EXIT_SUCCESS) + return -EPROTO; + } + + e->tar_fd = safe_close(e->tar_fd); + + return 0; +} + static int tar_export_process(TarExport *e) { ssize_t l; int r; @@ -156,7 +176,7 @@ static int tar_export_process(TarExport *e) { e->tried_splice = true; } else if (l == 0) { - r = 0; + r = tar_export_finish(e); goto finish; } else { e->written_uncompressed += l; @@ -172,7 +192,7 @@ static int tar_export_process(TarExport *e) { uint8_t input[COPY_BUFFER_SIZE]; if (e->eof) { - r = 0; + r = tar_export_finish(e); goto finish; } From d02bfa50f6ca1bb0a4761a0adda1558b87474922 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 26 Oct 2018 18:09:18 +0200 Subject: [PATCH 21/23] import: properly fail if tar exited non-zero, not just when it segfaulted or such --- src/import/import-tar.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/import/import-tar.c b/src/import/import-tar.c index 896effab77..0bb086f142 100644 --- a/src/import/import-tar.c +++ b/src/import/import-tar.c @@ -175,6 +175,8 @@ static int tar_import_finish(TarImport *i) { i->tar_pid = 0; if (r < 0) return r; + if (r != EXIT_SUCCESS) + return -EPROTO; } r = import_mangle_os_tree(i->temp_path); From f5095a6af064001c94d633123dabe2a59c841bf3 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 26 Oct 2018 18:18:36 +0200 Subject: [PATCH 22/23] test: add simple test for importd It tests importing and exporting, and a few other machinectl commands. It currently does not test pulling (i.e. http downloads), but we might want to add that later on. --- test/TEST-25-IMPORT/Makefile | 1 + test/TEST-25-IMPORT/test.sh | 43 +++++++++++ test/TEST-25-IMPORT/testsuite.sh | 128 +++++++++++++++++++++++++++++++ 3 files changed, 172 insertions(+) create mode 120000 test/TEST-25-IMPORT/Makefile create mode 100755 test/TEST-25-IMPORT/test.sh create mode 100755 test/TEST-25-IMPORT/testsuite.sh diff --git a/test/TEST-25-IMPORT/Makefile b/test/TEST-25-IMPORT/Makefile new file mode 120000 index 0000000000..e9f93b1104 --- /dev/null +++ b/test/TEST-25-IMPORT/Makefile @@ -0,0 +1 @@ +../TEST-01-BASIC/Makefile \ No newline at end of file diff --git a/test/TEST-25-IMPORT/test.sh b/test/TEST-25-IMPORT/test.sh new file mode 100755 index 0000000000..188e7233bb --- /dev/null +++ b/test/TEST-25-IMPORT/test.sh @@ -0,0 +1,43 @@ +#!/bin/bash +# -*- mode: shell-script; indent-tabs-mode: nil; sh-basic-offset: 4; -*- +# ex: ts=8 sw=4 sts=4 et filetype=sh +set -e +TEST_DESCRIPTION="test importd" + +. $TEST_BASE_DIR/test-functions + +test_setup() { + create_empty_image + mkdir -p $TESTDIR/root + mount ${LOOPDEV}p1 $TESTDIR/root + + ( + LOG_LEVEL=5 + eval $(udevadm info --export --query=env --name=${LOOPDEV}p2) + + setup_basic_environment + dracut_install dd gunzip mv tar diff + + # setup the testsuite service + cat >$initdir/etc/systemd/system/testsuite.service < /var/tmp/scratch/adirectory/athirdfile + +# Test import-fs +machinectl import-fs /var/tmp/scratch/ +test -d /var/lib/machines/scratch +machinectl image-status scratch + +# Test export-tar +machinectl export-tar scratch /var/tmp/scratch.tar.gz +test -f /var/tmp/scratch.tar.gz +mkdir /var/tmp/extract +(cd /var/tmp/extract ; tar xzf /var/tmp/scratch.tar.gz) +diff -r /var/tmp/scratch/ /var/tmp/extract/ +rm -rf /var/tmp/extract + +# Test import-tar +machinectl import-tar /var/tmp/scratch.tar.gz scratch2 +test -d /var/lib/machines/scratch2 +machinectl image-status scratch2 +diff -r /var/tmp/scratch/ /var/lib/machines/scratch2 + +# Test removal +machinectl remove scratch +! test -f /var/lib/machines/scratch +! machinectl image-status scratch + +# Test clone +machinectl clone scratch2 scratch3 +test -d /var/lib/machines/scratch2 +machinectl image-status scratch2 +test -d /var/lib/machines/scratch3 +machinectl image-status scratch3 +diff -r /var/tmp/scratch/ /var/lib/machines/scratch3 + +# Test removal +machinectl remove scratch2 +! test -f /var/lib/machines/scratch2 +! machinectl image-status scratch2 + +# Test rename +machinectl rename scratch3 scratch4 +test -d /var/lib/machines/scratch4 +machinectl image-status scratch4 +! test -f /var/lib/machines/scratch3 +! machinectl image-status scratch3 +diff -r /var/tmp/scratch/ /var/lib/machines/scratch4 + +# Test removal +machinectl remove scratch4 +! test -f /var/lib/machines/scratch4 +! machinectl image-status scratch4 + +rm -rf /var/tmp/scratch + +echo OK > /testok + +exit 0 From fe1bff158e66bd4cc05e435ab29af830b8f3b3ac Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 11 Oct 2018 10:15:21 +0200 Subject: [PATCH 23/23] update TODO --- TODO | 3 +++ 1 file changed, 3 insertions(+) diff --git a/TODO b/TODO index d8d120d66a..847661a62a 100644 --- a/TODO +++ b/TODO @@ -41,6 +41,9 @@ Features: * bootctl,sd-boot: actually honour the "architecture" key +* when a socket unit is spawned with an AF_UNIX path in /var/run, complain and + patch it to use /run instead + * consider splitting out all temporary file creation APIs (we have so many in fileio.h and elsewhere!) into a new util file of its own.