tmpfiles: create subvolumes for "v", "q", and "Q" only if / is a subvolume

It's not a good idea to create subvolumes for parts of the OS tree (such
as /home, or /var) if the root directory is not a subvolume too. We
shouldn't assume control of "heavier" objects such as subvolumes, if the
originating object (the root directory) is a "light-weight" object, i.e.
a plain directory.

Effectively this means that chroot() environments that are run on a
plain directory do not have to deal with problems around systemd
creating subvolumes that cannot be removed with a simple "rm" anymore.
However, if the chroot manager creates a proper subvolume for such an
environment it will also get further subvolumes placed in there, under
the assumption that the manager understands the concept of subvolumes in
that case.

diff --git a/man/tmpfiles.d.xml b/man/tmpfiles.d.xml
index 3f6128cb5b34f498e38545bd75877ec7a66e235c..5bf1f2956bb30e53736113cf922545689d74518c 100644 (file)
--- a/man/tmpfiles.d.xml
+++ b/man/tmpfiles.d.xml
@@ -169,13 +169,15 @@
         <varlistentry>
           <term><varname>v</varname></term>
           <listitem><para>Create a subvolume if the path does not
-          exist yet and the file system supports this
-          (btrfs). Otherwise, create a normal directory, in the same
-          way as <varname>d</varname>. A subvolume created with this
-          line type is not assigned to any higher-level quota
-          group. For that, use <varname>q</varname> or
-          <varname>Q</varname>, which allow creating simple quota group
-          hierarchies, see below.</para></listitem>
+          exist yet, the file system supports subvolumes (btrfs), and
+          the system itself is installed into a subvolume
+          (specifically: the root directory <filename>/</filename> is
+          itself a subvolume). Otherwise, create a normal directory, in
+          the same way as <varname>d</varname>. A subvolume created
+          with this line type is not assigned to any higher-level
+          quota group. For that, use <varname>q</varname> or
+          <varname>Q</varname>, which allow creating simple quota
+          group hierarchies, see below.</para></listitem>
         </varlistentry>
 
         <varlistentry>

diff --git a/src/basic/btrfs-util.c b/src/basic/btrfs-util.c
index 290fabdeffef111a62ffd39aed13cec1fecbdad7..8c90e1e4a216053fd7fdb94590fa28817d5d3c9d 100644 (file)
--- a/src/basic/btrfs-util.c
+++ b/src/basic/btrfs-util.c
@@ -105,7 +105,7 @@ int btrfs_is_filesystem(int fd) {
         return F_TYPE_EQUAL(sfs.f_type, BTRFS_SUPER_MAGIC);
 }
 
-int btrfs_is_subvol(int fd) {
+int btrfs_is_subvol_fd(int fd) {
         struct stat st;
 
         assert(fd >= 0);
@@ -121,6 +121,18 @@ int btrfs_is_subvol(int fd) {
         return btrfs_is_filesystem(fd);
 }
 
+int btrfs_is_subvol(const char *path) {
+        _cleanup_close_ int fd = -1;
+
+        assert(path);
+
+        fd = open(path, O_RDONLY|O_NOCTTY|O_CLOEXEC|O_DIRECTORY);
+        if (fd < 0)
+                return -errno;
+
+        return btrfs_is_subvol_fd(fd);
+}
+
 int btrfs_subvol_make(const char *path) {
         struct btrfs_ioctl_vol_args args = {};
         _cleanup_close_ int fd = -1;
@@ -1682,7 +1694,7 @@ int btrfs_subvol_snapshot_fd(int old_fd, const char *new_path, BtrfsSnapshotFlag
         assert(old_fd >= 0);
         assert(new_path);
 
-        r = btrfs_is_subvol(old_fd);
+        r = btrfs_is_subvol_fd(old_fd);
         if (r < 0)
                 return r;
         if (r == 0) {
@@ -1868,7 +1880,7 @@ int btrfs_subvol_auto_qgroup_fd(int fd, uint64_t subvol_id, bool insert_intermed
          */
 
         if (subvol_id == 0) {
-                r = btrfs_is_subvol(fd);
+                r = btrfs_is_subvol_fd(fd);
                 if (r < 0)
                         return r;
                 if (!r)

diff --git a/src/basic/btrfs-util.h b/src/basic/btrfs-util.h
index fc9efd72d5268805d526b3a591b8bbe1d964fc93..8c11ce35d26ff18591835f00ef2ec574ed80375e 100644 (file)
--- a/src/basic/btrfs-util.h
+++ b/src/basic/btrfs-util.h
@@ -56,7 +56,9 @@ typedef enum BtrfsRemoveFlags {
 } BtrfsRemoveFlags;
 
 int btrfs_is_filesystem(int fd);
-int btrfs_is_subvol(int fd);
+
+int btrfs_is_subvol_fd(int fd);
+int btrfs_is_subvol(const char *path);
 
 int btrfs_reflink(int infd, int outfd);
 int btrfs_clone_range(int infd, uint64_t in_offset, int ofd, uint64_t out_offset, uint64_t sz);

diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c
index 74b6b91593d7b9b2fdfa48b553162b4da344116a..f9a759e2232f914919cc440d8304bcc625f4bf0e 100644 (file)
--- a/src/tmpfiles/tmpfiles.c
+++ b/src/tmpfiles/tmpfiles.c
@@ -1226,8 +1226,28 @@ static int create_item(Item *i) {
                         mkdir_parents_label(i->path, 0755);
 
                 if (IN_SET(i->type, CREATE_SUBVOLUME, CREATE_SUBVOLUME_INHERIT_QUOTA, CREATE_SUBVOLUME_NEW_QUOTA)) {
-                        RUN_WITH_UMASK((~i->mode) & 0777)
-                                r = btrfs_subvol_make(i->path);
+
+                        if (btrfs_is_subvol(isempty(arg_root) ? "/" : arg_root) <= 0)
+
+                                /* Don't create a subvolume unless the
+                                 * root directory is one, too. We do
+                                 * this under the assumption that if
+                                 * the root directory is just a plain
+                                 * directory (i.e. very light-weight),
+                                 * we shouldn't try to split it up
+                                 * into subvolumes (i.e. more
+                                 * heavy-weight). Thus, chroot()
+                                 * environments and suchlike will get
+                                 * a full brtfs subvolume set up below
+                                 * their tree only if they
+                                 * specifically set up a btrfs
+                                 * subvolume for the root dir too. */
+
+                                r = -ENOTTY;
+                        else {
+                                RUN_WITH_UMASK((~i->mode) & 0777)
+                                        r = btrfs_subvol_make(i->path);
+                        }
                 } else
                         r = 0;