Merge pull request #10307 from poettering/portable-path

finishing touches to portable services, and let's move portablectl to /usr/bin to make it official

11 files changed, 159 insertions, 59 deletions

diff --git a/TODO b/TODO
index f72dd0f71..b78892a91 100644
--- a/TODO
+++ b/TODO
@@ -15,13 +15,6 @@ Janitorial Clean-ups:
 * rework mount.c and swap.c to follow proper state enumeration/deserialization
   semantics, like we do for device.c now
 
-Before v240:
-
-* move portablectl into /usr/bin
-
-* portables: introduce a new unit file directory /etc/systemd/system.attached/
-  or so, where we attach portable services to
-
 Features:
 
 * consider splitting out all temporary file creation APIs (we have so many in
diff --git a/docs/PORTABLE_SERVICES.md b/docs/PORTABLE_SERVICES.md
index 183324444..55397f463 100644
--- a/docs/PORTABLE_SERVICES.md
+++ b/docs/PORTABLE_SERVICES.md
@@ -98,16 +98,17 @@ This command does the following:
    `foobar@.{service|socket|target|timer|path}` as well as
    `foobar.*.{service|socket|target|timer|path}` and
    `foobar.{service|socket|target|timer|path}` are copied out. These unit files
-   are placed in `/etc/systemd/system/` like regular unit files. Within the
-   images the unit files are looked for at the usual locations, i.e. in
-   `/usr/lib/systemd/system/` and `/etc/systemd/system/` and so on, relative to
-   the image's root.
+   are placed in `/etc/systemd/system.attached/` (which is part of the normal
+   unit file search path of PID 1, and thus loaded exactly like regular unit
+   files). Within the images the unit files are looked for at the usual
+   locations, i.e. in `/usr/lib/systemd/system/` and `/etc/systemd/system/` and
+   so on, relative to the image's root.
 
 3. For each such unit file a drop-in file is created. Let's say
    `foobar-waldo.service` was one of the unit files copied to
-   `/etc/systemd/system/`, then a drop-in file
-   `/etc/systemd/system/foobar-waldo.service.d/20-portable.conf` is created,
-   containing a few lines of additional configuration:
+   `/etc/systemd/system.attached/`, then a drop-in file
+   `/etc/systemd/system.attached/foobar-waldo.service.d/20-portable.conf` is
+   created, containing a few lines of additional configuration:
 
    ```
    [Service]
diff --git a/man/portablectl.xml b/man/portablectl.xml
index 24b9f4ead..3a5517a4a 100644
--- a/man/portablectl.xml
+++ b/man/portablectl.xml
@@ -59,7 +59,7 @@
       <listitem><para>btrfs subvolumes containing OS trees, similar to normal directory trees.</para></listitem>
 
       <listitem><para>Binary "raw" disk images containing MBR or GPT partition tables and Linux file system
-      partitions.</para></listitem>
+      partitions. (These must be regular files, with the <filename>.raw</filename> suffix.)</para></listitem>
     </itemizedlist>
 
   </refsect1>
@@ -101,9 +101,9 @@
         <term><option>--runtime</option></term>
 
         <listitem><para>When specified the unit and drop-in files are placed in
-        <filename>/run/systemd/system/</filename> instead of <filename>/etc/systemd/system/</filename>. Images attached
-        with this option set hence remain attached only until the next reboot, while they are normally attached
-        persistently.</para></listitem>
+        <filename>/run/systemd/system.attached/</filename> instead of
+        <filename>/etc/systemd/system.attached/</filename>. Images attached with this option set hence remain attached
+        only until the next reboot, while they are normally attached persistently.</para></listitem>
       </varlistentry>
 
       <varlistentry>
@@ -167,8 +167,10 @@
           <listitem><para>All unit files of types <filename>.service</filename>, <filename>.socket</filename>,
           <filename>.target</filename>, <filename>.timer</filename> and <filename>.path</filename> which match the
           indicated unit file name prefix are copied from the image to the host's
-          <filename>/etc/systemd/system/</filename> directory (or <filename>/run/systemd/system/</filename> — depending
-          whether <option>--runtime</option> is specified, see above).</para></listitem>
+          <filename>/etc/systemd/system.attached/</filename> directory (or
+          <filename>/run/systemd/system.attached/</filename> — depending whether <option>--runtime</option> is
+          specified, see above), which is included in the built-in unit search path of the system service
+          manager.</para></listitem>
 
           <listitem><para>For unit files of type <filename>.service</filename> a drop-in is added to these copies that
           adds <varname>RootDirectory=</varname> or <varname>RootImage=</varname> settings (see
@@ -330,6 +332,10 @@
     to place image files directly in <filename>/etc/portables/</filename> or
     <filename>/run/systemd/portables/</filename> (as these are generally not suitable for storing large or non-textual
     data), but use these directories only for linking images located elsewhere into the image search path.</para>
+
+    <para>When a portable service image is attached, matching unit files are copied onto the host into the
+    <filename>/etc/systemd/system.attached/</filename> and <filename>/run/systemd/system.attached/</filename>
+    directories. When an image is detached, the unit files are removed again from these directories.</para>
   </refsect1>
 
   <refsect1>
diff --git a/man/systemd.unit.xml b/man/systemd.unit.xml
index c615739e3..352213353 100644
--- a/man/systemd.unit.xml
+++ b/man/systemd.unit.xml
@@ -39,19 +39,26 @@
     <filename><replaceable>slice</replaceable>.slice</filename>,
     <filename><replaceable>scope</replaceable>.scope</filename></para>
 
-    <para><literallayout><filename>/etc/systemd/system.control/*</filename>
+    <refsect2>
+      <title>System Unit Search Path</title>
+
+      <para><literallayout><filename>/etc/systemd/system.control/*</filename>
 <filename>/run/systemd/system.control/*</filename>
 <filename>/run/systemd/transient/*</filename>
 <filename>/run/systemd/generator.early/*</filename>
 <filename>/etc/systemd/system/*</filename>
+<filename>/etc/systemd/systemd.attached/*</filename>
 <filename>/run/systemd/system/*</filename>
+<filename>/run/systemd/systemd.attached/*</filename>
 <filename>/run/systemd/generator/*</filename>
 <filename>…</filename>
 <filename>/usr/lib/systemd/system/*</filename>
-<filename>/run/systemd/generator.late/*</filename>
-    </literallayout></para>
+<filename>/run/systemd/generator.late/*</filename></literallayout></para>
+    </refsect2>
 
-    <para><literallayout><filename>~/.config/systemd/user.control/*</filename>
+    <refsect2>
+      <title>User Unit Search Path</title>
+      <para><literallayout><filename>~/.config/systemd/user.control/*</filename>
 <filename>$XDG_RUNTIME_DIR/systemd/user.control/*</filename>
 <filename>$XDG_RUNTIME_DIR/systemd/transient/*</filename>
 <filename>$XDG_RUNTIME_DIR/systemd/generator.early/*</filename>
@@ -63,8 +70,9 @@
 <filename>~/.local/share/systemd/user/*</filename>
 <filename>…</filename>
 <filename>/usr/lib/systemd/user/*</filename>
-<filename>$XDG_RUNTIME_DIR/systemd/generator.late/*</filename>
-    </literallayout></para>
+<filename>$XDG_RUNTIME_DIR/systemd/generator.late/*</filename></literallayout></para>
+    </refsect2>
+
   </refsynopsisdiv>
 
   <refsect1>
diff --git a/meson.build b/meson.build
index 5e653b8b6..e3f06616b 100644
--- a/meson.build
+++ b/meson.build
@@ -1834,7 +1834,7 @@ if conf.get('ENABLE_PORTABLED') == 1
                          dependencies : [threads],
                          install_rpath : rootlibexecdir,
                          install : true,
-                         install_dir : rootlibexecdir)
+                         install_dir : rootbindir)
         public_programs += exe
 endif
 
diff --git a/src/libsystemd/sd-bus/bus-common-errors.c b/src/libsystemd/sd-bus/bus-common-errors.c
index b6e40ee8a..6e5fe00e0 100644
--- a/src/libsystemd/sd-bus/bus-common-errors.c
+++ b/src/libsystemd/sd-bus/bus-common-errors.c
@@ -41,6 +41,9 @@ BUS_ERROR_MAP_ELF_REGISTER const sd_bus_error_map bus_common_errors[] = {
         SD_BUS_ERROR_MAP(BUS_ERROR_NO_SUCH_USER_MAPPING,         ENXIO),
         SD_BUS_ERROR_MAP(BUS_ERROR_NO_SUCH_GROUP_MAPPING,        ENXIO),
 
+        SD_BUS_ERROR_MAP(BUS_ERROR_NO_SUCH_PORTABLE_IMAGE,       ENOENT),
+        SD_BUS_ERROR_MAP(BUS_ERROR_BAD_PORTABLE_IMAGE_TYPE,      EMEDIUMTYPE),
+
         SD_BUS_ERROR_MAP(BUS_ERROR_NO_SUCH_SESSION,              ENXIO),
         SD_BUS_ERROR_MAP(BUS_ERROR_NO_SESSION_FOR_PID,           ENXIO),
         SD_BUS_ERROR_MAP(BUS_ERROR_NO_SUCH_USER,                 ENXIO),
@@ -71,6 +74,8 @@ BUS_ERROR_MAP_ELF_REGISTER const sd_bus_error_map bus_common_errors[] = {
         SD_BUS_ERROR_MAP(BUS_ERROR_NO_SUCH_LINK,                 ENXIO),
         SD_BUS_ERROR_MAP(BUS_ERROR_LINK_BUSY,                    EBUSY),
         SD_BUS_ERROR_MAP(BUS_ERROR_NETWORK_DOWN,                 ENETDOWN),
+        SD_BUS_ERROR_MAP(BUS_ERROR_NO_SUCH_DNSSD_SERVICE,        ENOENT),
+        SD_BUS_ERROR_MAP(BUS_ERROR_DNSSD_SERVICE_EXISTS,         EEXIST),
 
         SD_BUS_ERROR_MAP(_BUS_ERROR_DNS "FORMERR",               EBADMSG),
         SD_BUS_ERROR_MAP(_BUS_ERROR_DNS "SERVFAIL",              EHOSTDOWN),
diff --git a/src/libsystemd/sd-bus/bus-common-errors.h b/src/libsystemd/sd-bus/bus-common-errors.h
index a76a93644..8339feb76 100644
--- a/src/libsystemd/sd-bus/bus-common-errors.h
+++ b/src/libsystemd/sd-bus/bus-common-errors.h
@@ -37,6 +37,7 @@
 #define BUS_ERROR_NO_SUCH_GROUP_MAPPING "org.freedesktop.machine1.NoSuchGroupMapping"
 
 #define BUS_ERROR_NO_SUCH_PORTABLE_IMAGE "org.freedesktop.portable1.NoSuchImage"
+#define BUS_ERROR_BAD_PORTABLE_IMAGE_TYPE "org.freedesktop.portable1.BadImageType"
 
 #define BUS_ERROR_NO_SUCH_SESSION "org.freedesktop.login1.NoSuchSession"
 #define BUS_ERROR_NO_SESSION_FOR_PID "org.freedesktop.login1.NoSessionForPID"
diff --git a/src/portable/portable.c b/src/portable/portable.c
index 3491723aa..ca8043b41 100644
--- a/src/portable/portable.c
+++ b/src/portable/portable.c
@@ -815,15 +815,15 @@ static int install_profile_dropin(
         return 0;
 }
 
-static const char *config_path(const LookupPaths *paths, PortableFlags flags) {
+static const char *attached_path(const LookupPaths *paths, PortableFlags flags) {
         const char *where;
 
         assert(paths);
 
         if (flags & PORTABLE_RUNTIME)
-                where = paths->runtime_config;
+                where = paths->runtime_attached;
         else
-                where = paths->persistent_config;
+                where = paths->persistent_attached;
 
         assert(where);
         return where;
@@ -849,15 +849,25 @@ static int attach_unit_file(
         assert(m);
         assert(PORTABLE_METADATA_IS_UNIT(m));
 
-        where = config_path(paths, flags);
-        path = strjoina(where, "/", m->name);
+        where = attached_path(paths, flags);
+
+        (void) mkdir_parents(where, 0755);
+        if (mkdir(where, 0755) < 0) {
+                if (errno != EEXIST)
+                        return -errno;
+        } else
+                (void) portable_changes_add(changes, n_changes, PORTABLE_MKDIR, where, NULL);
 
+        path = strjoina(where, "/", m->name);
         dropin_dir = strjoin(path, ".d");
         if (!dropin_dir)
                 return -ENOMEM;
 
-        (void) mkdir_p(dropin_dir, 0755);
-        (void) portable_changes_add(changes, n_changes, PORTABLE_MKDIR, dropin_dir, NULL);
+        if (mkdir(dropin_dir, 0755) < 0) {
+                if (errno != EEXIST)
+                        return -errno;
+        } else
+                (void) portable_changes_add(changes, n_changes, PORTABLE_MKDIR, dropin_dir, NULL);
 
         /* We install the drop-ins first, and the actual unit file last to achieve somewhat atomic behaviour if PID 1
          * is reloaded while we are creating things here: as long as only the drop-ins exist the unit doesn't exist at
@@ -1147,11 +1157,15 @@ int portable_detach(
         if (r < 0)
                 return r;
 
-        where = config_path(&paths, flags);
+        where = attached_path(&paths, flags);
 
         d = opendir(where);
-        if (!d)
+        if (!d) {
+                if (errno == ENOENT)
+                        goto not_found;
+
                 return log_debug_errno(errno, "Failed to open '%s' directory: %m", where);
+        }
 
         unit_files = set_new(&string_hash_ops);
         if (!unit_files)
@@ -1213,10 +1227,8 @@ int portable_detach(
                 }
         }
 
-        if (set_isempty(unit_files)) {
-                log_debug("No unit files associated with '%s' found. Image not attached?", name_or_path);
-                return sd_bus_error_setf(error, BUS_ERROR_NO_SUCH_UNIT, "No unit files associated with '%s' found. Image not attached?", name_or_path);
-        }
+        if (set_isempty(unit_files))
+                goto not_found;
 
         SET_FOREACH(item, unit_files, iterator) {
                 _cleanup_free_ char *md = NULL;
@@ -1289,7 +1301,15 @@ int portable_detach(
                         portable_changes_add(changes, n_changes, PORTABLE_UNLINK, sl, NULL);
         }
 
+        /* Try to remove the unit file directory, if we can */
+        if (rmdir(where) >= 0)
+                portable_changes_add(changes, n_changes, PORTABLE_UNLINK, where, NULL);
+
         return ret;
+
+not_found:
+        log_debug("No unit files associated with '%s' found. Image not attached?", name_or_path);
+        return sd_bus_error_setf(error, BUS_ERROR_NO_SUCH_UNIT, "No unit files associated with '%s' found. Image not attached?", name_or_path);
 }
 
 static int portable_get_state_internal(
@@ -1314,11 +1334,18 @@ static int portable_get_state_internal(
         if (r < 0)
                 return r;
 
-        where = config_path(&paths, flags);
+        where = attached_path(&paths, flags);
 
         d = opendir(where);
-        if (!d)
+        if (!d) {
+                if (errno == ENOENT) {
+                        /* If the 'attached' directory doesn't exist at all, then we know for sure this image isn't attached. */
+                        *ret = PORTABLE_DETACHED;
+                        return 0;
+                }
+
                 return log_debug_errno(errno, "Failed to open '%s' directory: %m", where);
+        }
 
         unit_files = set_new(&string_hash_ops);
         if (!unit_files)
diff --git a/src/portable/portabled-image-bus.c b/src/portable/portabled-image-bus.c
index 02a2db235..0e1f7c5e8 100644
--- a/src/portable/portabled-image-bus.c
+++ b/src/portable/portabled-image-bus.c
@@ -636,6 +636,10 @@ int bus_image_acquire(
 
                 r = image_from_path(name_or_path, &loaded);
         }
+        if (r == -EMEDIUMTYPE) {
+                sd_bus_error_setf(error, BUS_ERROR_BAD_PORTABLE_IMAGE_TYPE, "Typ of image '%s' not recognized; supported image types are directories/btrfs subvolumes, block devices, and raw disk image files with suffix '.raw'.", name_or_path);
+                return r;
+        }
         if (r < 0)
                 return r;
 
diff --git a/src/shared/path-lookup.c b/src/shared/path-lookup.c
index 42a5b62d5..7d7b890b4 100644
--- a/src/shared/path-lookup.c
+++ b/src/shared/path-lookup.c
@@ -142,9 +142,9 @@ int xdg_user_dirs(char ***ret_config_dirs, char ***ret_data_dirs) {
         if (!data_dirs)
                 return -ENOMEM;
 
-        *ret_config_dirs = config_dirs;
-        *ret_data_dirs = data_dirs;
-        config_dirs = data_dirs = NULL;
+        *ret_config_dirs = TAKE_PTR(config_dirs);
+        *ret_data_dirs = TAKE_PTR(data_dirs);
+
         return 0;
 }
 
@@ -421,6 +421,34 @@ static int acquire_control_dirs(UnitFileScope scope, char **persistent, char **r
         return 0;
 }
 
+static int acquire_attached_dirs(
+                UnitFileScope scope,
+                char **ret_persistent,
+                char **ret_runtime) {
+
+        _cleanup_free_ char *a = NULL, *b = NULL;
+
+        assert(ret_persistent);
+        assert(ret_runtime);
+
+        /* Portable services are not available to regular users for now. */
+        if (scope != UNIT_FILE_SYSTEM)
+                return -EOPNOTSUPP;
+
+        a = strdup("/etc/systemd/system.attached");
+        if (!a)
+                return -ENOMEM;
+
+        b = strdup("/run/systemd/system.attached");
+        if (!b)
+                return -ENOMEM;
+
+        *ret_persistent = TAKE_PTR(a);
+        *ret_runtime = TAKE_PTR(b);
+
+        return 0;
+}
+
 static int patch_root_prefix(char **p, const char *root_dir) {
         char *c;
 
@@ -468,7 +496,8 @@ int lookup_paths_init(
                 *global_persistent_config = NULL, *global_runtime_config = NULL,
                 *generator = NULL, *generator_early = NULL, *generator_late = NULL,
                 *transient = NULL,
-                *persistent_control = NULL, *runtime_control = NULL;
+                *persistent_control = NULL, *runtime_control = NULL,
+                *persistent_attached = NULL, *runtime_attached = NULL;
         bool append = false; /* Add items from SYSTEMD_UNIT_PATH before normal directories */
         _cleanup_strv_free_ char **paths = NULL;
         const char *e;
@@ -532,6 +561,10 @@ int lookup_paths_init(
         if (r < 0 && r != -EOPNOTSUPP)
                 return r;
 
+        r = acquire_attached_dirs(scope, &persistent_attached, &runtime_attached);
+        if (r < 0 && r != -EOPNOTSUPP)
+                return r;
+
         /* First priority is whatever has been passed to us via env vars */
         e = getenv("SYSTEMD_UNIT_PATH");
         if (e) {
@@ -574,8 +607,10 @@ int lookup_paths_init(
                                         persistent_config,
                                         SYSTEM_CONFIG_UNIT_PATH,
                                         "/etc/systemd/system",
+                                        STRV_IFNOTNULL(persistent_attached),
                                         runtime_config,
                                         "/run/systemd/system",
+                                        STRV_IFNOTNULL(runtime_attached),
                                         STRV_IFNOTNULL(generator),
                                         "/usr/local/lib/systemd/system",
                                         SYSTEM_DATA_UNIT_PATH,
@@ -658,33 +693,44 @@ int lookup_paths_init(
         r = patch_root_prefix(&persistent_control, root);
         if (r < 0)
                 return r;
-
         r = patch_root_prefix(&runtime_control, root);
         if (r < 0)
                 return r;
 
+        r = patch_root_prefix(&persistent_attached, root);
+        if (r < 0)
+                return r;
+        r = patch_root_prefix(&runtime_attached, root);
+        if (r < 0)
+                return r;
+
         r = patch_root_prefix_strv(paths, root);
         if (r < 0)
                 return -ENOMEM;
 
-        p->search_path = strv_uniq(paths);
-        paths = NULL;
+        *p = (LookupPaths) {
+                .search_path = strv_uniq(paths),
 
-        p->persistent_config = TAKE_PTR(persistent_config);
-        p->runtime_config = TAKE_PTR(runtime_config);
+                .persistent_config = TAKE_PTR(persistent_config),
+                .runtime_config = TAKE_PTR(runtime_config),
 
-        p->generator = TAKE_PTR(generator);
-        p->generator_early = TAKE_PTR(generator_early);
-        p->generator_late = TAKE_PTR(generator_late);
+                .generator = TAKE_PTR(generator),
+                .generator_early = TAKE_PTR(generator_early),
+                .generator_late = TAKE_PTR(generator_late),
 
-        p->transient = TAKE_PTR(transient);
+                .transient = TAKE_PTR(transient),
 
-        p->persistent_control = TAKE_PTR(persistent_control);
-        p->runtime_control = TAKE_PTR(runtime_control);
+                .persistent_control = TAKE_PTR(persistent_control),
+                .runtime_control = TAKE_PTR(runtime_control),
 
-        p->root_dir = TAKE_PTR(root);
-        p->temporary_dir = TAKE_PTR(tempdir);
+                .persistent_attached = TAKE_PTR(persistent_attached),
+                .runtime_attached = TAKE_PTR(runtime_attached),
 
+                .root_dir = TAKE_PTR(root),
+                .temporary_dir = TAKE_PTR(tempdir),
+        };
+
+        paths = NULL;
         return 0;
 }
 
@@ -697,6 +743,9 @@ void lookup_paths_free(LookupPaths *p) {
         p->persistent_config = mfree(p->persistent_config);
         p->runtime_config = mfree(p->runtime_config);
 
+        p->persistent_attached = mfree(p->persistent_attached);
+        p->runtime_attached = mfree(p->runtime_attached);
+
         p->generator = mfree(p->generator);
         p->generator_early = mfree(p->generator_early);
         p->generator_late = mfree(p->generator_late);
diff --git a/src/shared/path-lookup.h b/src/shared/path-lookup.h
index 963e09db6..cb7d4d537 100644
--- a/src/shared/path-lookup.h
+++ b/src/shared/path-lookup.h
@@ -24,6 +24,10 @@ struct LookupPaths {
         char *persistent_config;
         char *runtime_config;
 
+        /* Where units from a portable service image shall be placed. */
+        char *persistent_attached;
+        char *runtime_attached;
+
         /* Where to place generated unit files (i.e. those a "generator" tool generated). Note the special semantics of
          * this directory: the generators are flushed each time a "systemctl daemon-reload" is issued. The user should
          * not alter these directories directly. */
@@ -50,10 +54,12 @@ struct LookupPaths {
 };
 
 int lookup_paths_init(LookupPaths *p, UnitFileScope scope, LookupPathsFlags flags, const char *root_dir);
+
 int xdg_user_dirs(char ***ret_config_dirs, char ***ret_data_dirs);
 int xdg_user_runtime_dir(char **ret, const char *suffix);
 int xdg_user_config_dir(char **ret, const char *suffix);
 int xdg_user_data_dir(char **ret, const char *suffix);
+
 bool path_is_user_data_dir(const char *path);
 bool path_is_user_config_dir(const char *path);