repart: Allow combining CopyBlocks= and CopyFiles=

Let's allow the combination of these two options. When used, repart
will first try to apply the CopyBlocks= behavior. If that's not possible,
it falls back to the CopyFiles= behavior.

This is a first step in being able to also use the partition definition
files shipped in the image to build the image in mkosi instead of having
a separate set of repart definition files to build the image.

diff --git a/man/repart.d.xml b/man/repart.d.xml
index a79724a93ea1c271e601cf5a9d0dda26299c27e3..752fc3b852fe86d0c12df8b193c7c9542b80d92d 100644 (file)
--- a/man/repart.d.xml
+++ b/man/repart.d.xml
@@ -373,10 +373,7 @@
         data is never overwritten. Note that the data is copied in before the partition table is updated,
         i.e. before the partition actually is persistently created. This provides robustness: it is
         guaranteed that the partition either doesn't exist or exists fully populated; it is not possible that
-        the partition exists but is not or only partially populated.</para>
-
-        <para>This option cannot be combined with <varname>Format=</varname> or
-        <varname>CopyFiles=</varname>.</para></listitem>
+        the partition exists but is not or only partially populated.</para></listitem>
       </varlistentry>
 
       <varlistentry>
@@ -395,9 +392,7 @@
 
         <para>Similarly to the behaviour of <varname>CopyBlocks=</varname>, the file system is formatted
         before the partition is created, ensuring that the partition only ever exists with a fully
-        initialized file system.</para>
-
-        <para>This option cannot be combined with <varname>CopyBlocks=</varname>.</para></listitem>
+        initialized file system.</para></listitem>
       </varlistentry>
 
       <varlistentry>
@@ -439,7 +434,10 @@
         <citerefentry project='man-pages'><refentrytitle>mkfs.xfs</refentrytitle><manvolnum>8</manvolnum></citerefentry>
         due to limitations of its protofile format.</para>
 
-        <para>This option cannot be combined with <varname>CopyBlocks=</varname>.</para>
+        <para>When this option is used in combination with <varname>CopyBlocks=</varname>,
+        <command>systemd-repart</command> will first try the <varname>CopyBlocks=</varname> logic and will
+        only fall back to the <varname>CopyFiles=</varname> logic if the <varname>CopyBlocks=</varname> logic
+        cannot be used.</para>
 
         <para>When
         <citerefentry><refentrytitle>systemd-repart</refentrytitle><manvolnum>8</manvolnum></citerefentry>

diff --git a/src/partition/repart.c b/src/partition/repart.c
index dadaae545f21489deed5c09cdddc2132282c2910..74e04b65abdb3bdb792202234af0a0f407a27692 100644 (file)
--- a/src/partition/repart.c
+++ b/src/partition/repart.c
@@ -1672,11 +1672,6 @@ static int partition_read_definition(Partition *p, const char *path, const char
                 return log_syntax(NULL, LOG_ERR, path, 1, SYNTHETIC_ERRNO(EINVAL),
                                   "Type= not defined, refusing.");
 
-        if ((p->copy_blocks_path || p->copy_blocks_auto) &&
-            (p->format || !strv_isempty(p->copy_files) || !strv_isempty(p->make_directories)))
-                return log_syntax(NULL, LOG_ERR, path, 1, SYNTHETIC_ERRNO(EINVAL),
-                                  "Format=/CopyFiles=/MakeDirectories= and CopyBlocks= cannot be combined, refusing.");
-
         if ((!strv_isempty(p->copy_files) || !strv_isempty(p->make_directories)) && streq_ptr(p->format, "swap"))
                 return log_syntax(NULL, LOG_ERR, path, 1, SYNTHETIC_ERRNO(EINVAL),
                                   "Format=swap and CopyFiles= cannot be combined, refusing.");
@@ -5387,19 +5382,6 @@ static int resolve_copy_blocks_auto(
         dev_t devno, found = 0;
         int r;
 
-        /* Enforce some security restrictions: CopyBlocks=auto should not be an avenue to get outside of the
-         * --root=/--image= confinement. Specifically, refuse CopyBlocks= in combination with --root= at all,
-         * and restrict block device references in the --image= case to loopback block device we set up.
-         *
-         * restrict_devno contain the dev_t of the loop back device we operate on in case of --image=, and
-         * thus declares which device (and its partition subdevices) we shall limit access to. If
-         * restrict_devno is zero no device probing access shall be allowed at all (used for --root=) and if
-         * it is (dev_t) -1 then free access shall be allowed (if neither switch is used). */
-
-        if (restrict_devno == 0)
-                return log_error_errno(SYNTHETIC_ERRNO(EPERM),
-                                       "Automatic discovery of backing block devices not permitted in --root= mode, refusing.");
-
         /* Handles CopyBlocks=auto, and finds the right source partition to copy from. We look for matching
          * partitions in the host, using the appropriate directory as key and ensuring that the partition
          * type matches. */
@@ -5492,17 +5474,13 @@ static int resolve_copy_blocks_auto(
                         found = devno;
         }
 
-        if (found == 0)
-                return log_error_errno(SYNTHETIC_ERRNO(ENXIO),
-                                       "Unable to automatically discover suitable partition to copy blocks from.");
-
         if (ret_devno)
                 *ret_devno = found;
 
         if (ret_uuid)
                 *ret_uuid = found_uuid;
 
-        return 0;
+        return found != 0;
 }
 
 static int context_open_copy_block_paths(
@@ -5544,9 +5522,35 @@ static int context_open_copy_block_paths(
                 } else if (p->copy_blocks_auto) {
                         dev_t devno = 0;  /* Fake initialization to appease gcc. */
 
+                        /* Enforce some security restrictions: CopyBlocks=auto should not be an avenue to get
+                         * outside of the --root=/--image= confinement. Specifically, refuse CopyBlocks= in
+                         * combination with --root= at all, and restrict block device references in the
+                         * --image= case to loopback block device we set up.
+                         *
+                         * restrict_devno contain the dev_t of the loop back device we operate on in case of
+                         * --image=, and thus declares which device (and its partition subdevices) we shall
+                         * limit access to. If restrict_devno is zero no device probing access shall be
+                         * allowed at all (used for --root=) and if it is (dev_t) -1 then free access shall
+                         * be allowed (if neither switch is used). */
+
+                        if (restrict_devno == 0) {
+                                if (!p->format && strv_isempty(p->copy_files) && strv_isempty(p->make_directories))
+                                        return log_error_errno(SYNTHETIC_ERRNO(EPERM),
+                                                               "Automatic discovery of backing block devices not permitted in --root= mode, refusing.");
+
+                                continue;
+                        }
+
                         r = resolve_copy_blocks_auto(p->type, p->copy_blocks_root, restrict_devno, &devno, &uuid);
                         if (r < 0)
                                 return r;
+                        if (r == 0) {
+                                if (!p->format && strv_isempty(p->copy_files) && strv_isempty(p->make_directories))
+                                        return log_error_errno(SYNTHETIC_ERRNO(ENXIO),
+                                                               "Unable to automatically discover suitable partition to copy blocks from.");
+
+                                continue;
+                        }
                         assert(devno != 0);
 
                         source_fd = r = device_open_from_devnum(S_IFBLK, devno, O_RDONLY|O_CLOEXEC|O_NONBLOCK, &opened);
@@ -5684,6 +5688,9 @@ static int context_minimize(Context *context) {
                 if (!p->format)
                         continue;
 
+                if (p->copy_blocks_fd >= 0)
+                        continue;
+
                 if (p->minimize == MINIMIZE_OFF)
                         continue;
 
@@ -5760,13 +5767,13 @@ static int context_minimize(Context *context) {
                 if (fstype_is_ro(p->format)) {
                         struct stat st;
 
-                        if (fd < 0) {
-                                fd = open(temp, O_RDONLY|O_CLOEXEC|O_NONBLOCK);
-                                if (fd < 0)
-                                        return log_error_errno(errno, "Failed to open temporary file %s: %m", temp);
-                        }
+                        assert(fd < 0);
 
-                        if (stat(temp, &st) < 0)
+                        fd = open(temp, O_RDONLY|O_CLOEXEC|O_NONBLOCK);
+                        if (fd < 0)
+                                return log_error_errno(errno, "Failed to open temporary file %s: %m", temp);
+
+                        if (fstat(fd, &st) < 0)
                                 return log_error_errno(errno, "Failed to stat temporary file: %m");
 
                         log_info("Minimal partition size of %s filesystem of partition %s is %s",
@@ -5838,6 +5845,8 @@ static int context_minimize(Context *context) {
                                 return r;
                 }
 
+                assert(fd >= 0);
+
                 p->copy_blocks_path = TAKE_PTR(temp);
                 p->copy_blocks_path_is_our_file = true;
                 p->copy_blocks_fd = TAKE_FD(fd);

diff --git a/test/units/testsuite-58.sh b/test/units/testsuite-58.sh
index 13e40bd82ab9aa6290c73d16b4dfc65f2c9f48e3..fbfc9e7b3aad6918fe6e3e00e51a988afe3ffc70 100755 (executable)
--- a/test/units/testsuite-58.sh
+++ b/test/units/testsuite-58.sh
@@ -268,6 +268,9 @@ Type=linux-generic
 Label=block-copy
 UUID=2a1d97e1d0a346cca26eadc643926617
 CopyBlocks=$imgs/block-copy
+# Test that these are ignored
+CopyFiles=abc
+Format=ext4
 EOF
 
     systemd-repart --offline="$OFFLINE" \
@@ -903,6 +906,8 @@ CopyFiles=/
 CopyFiles=/zzz:/
 CopyFiles=/:/oiu
 ExcludeFilesTarget=/oiu/usr
+# Test that this is just ignored
+CopyBlocks=auto
 EOF
 
     tee "$defs/10-usr.conf" <<EOF