From: Daan De Meyer Date: Fri, 28 Jul 2023 16:09:29 +0000 (+0200) Subject: repart: Allow combining CopyBlocks= and CopyFiles= X-Git-Tag: v255-rc1~859^2~3 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=dea0dc7ba2d779e5b65cb029395216859408931c;p=thirdparty%2Fsystemd.git 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 a79724a93ea..752fc3b852f 100644 --- 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. - - This option cannot be combined with Format= or - CopyFiles=. + the partition exists but is not or only partially populated. @@ -395,9 +392,7 @@ Similarly to the behaviour of CopyBlocks=, the file system is formatted before the partition is created, ensuring that the partition only ever exists with a fully - initialized file system. - - This option cannot be combined with CopyBlocks=. + initialized file system. @@ -439,7 +434,10 @@ mkfs.xfs8 due to limitations of its protofile format. - This option cannot be combined with CopyBlocks=. + When this option is used in combination with CopyBlocks=, + systemd-repart will first try the CopyBlocks= logic and will + only fall back to the CopyFiles= logic if the CopyBlocks= logic + cannot be used. When systemd-repart8 diff --git a/src/partition/repart.c b/src/partition/repart.c index dadaae545f2..74e04b65abd 100644 --- 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 13e40bd82ab..fbfc9e7b3aa 100755 --- 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" <