zipl: relative path for BLS values

[tuan-hoang1]

Hi Javier @martinezjavier and Stefan @stefan-haberland,

Currently, zipl assumes the kernel and ramdisk paths to be absolute, both in zipl.conf and BLS conf files, or else it will print out file not found error to user.

In Fedora/RHEL CoreOS, a BLS file might look like:

initrd /ostree/fedora-coreos-<commit-id>/initramfs-5.2.11-200.fc30.x86_64.img
linux /ostree/fedora-coreos-<commit-id>/vmlinuz-5.2.11-200.fc30.x86_64

which conforms with the BLS spec, as it says the linux and initrd fields shall be a path relative to the $BOOT directory.

I suggest we support this case, by also checking the paths of the images with a prefix of /boot, and only error out when both cases (with and without /boot path prefix) cannot find the files. Below is a patch I would like propose: (https://tpaste.us/yPMy)

diff --git a/zipl/src/scan.c b/zipl/src/scan.c
index b42b5f2..3329005 100644
--- a/zipl/src/scan.c
+++ b/zipl/src/scan.c
@@ -672,7 +672,7 @@ static int
 scan_bls_field(struct misc_file_buffer *file, struct scan_token* scan,
 	       int* index)
 {
-	int current;
+	int current, rc;
 	int key_start, key_end;
 	int val_start, val_end;
 	char *val;
@@ -712,6 +712,15 @@ scan_bls_field(struct misc_file_buffer *file, struct scan_token* scan,
 
 	if (strncmp("linux", &file->buffer[key_start], key_end - key_start) == 0) {
 		misc_asprintf(&val, "%s", &file->buffer[val_start]);
+		rc = misc_check_readable_file(val);
+		if (rc) {
+			misc_asprintf(&val, "/boot%s", &file->buffer[val_start]);
+			rc = misc_check_readable_file(val);
+			if (rc) {
+				error_text("Image file '%s'", val);
+				return rc;
+			}
+		}
 		scan_append_keyword_assignment(scan, index, scan_keyword_image, val);
 		free(val);
 	}
@@ -723,6 +732,15 @@ scan_bls_field(struct misc_file_buffer *file, struct scan_token* scan,
 
 	if (strncmp("initrd", &file->buffer[key_start], key_end - key_start) == 0) {
 		misc_asprintf(&val, "%s", &file->buffer[val_start]);
+		rc = misc_check_readable_file(val);
+		if (rc) {
+			misc_asprintf(&val, "/boot%s", &file->buffer[val_start]);
+			rc = misc_check_readable_file(val);
+			if (rc) {
+				error_text("Ramdisk file '%s'", val);
+				return rc;
+			}
+		}
 		scan_append_keyword_assignment(scan, index, scan_keyword_ramdisk, val);
 		free(val);
 	}

[tuan-hoang1]

Of course we can fix this in downstream Fedora/RHEL CoreOS, it would be nice if upstream allows this behavior wrt the spec.

[martinezjavier]

@tuan-hoang1 I don't think the /boot prefix should be harcoded in zipl. It's true that the BootLoaderSpec says that the paths should be relative to the $BOOT directory, but the spec makes a lot of assumptions that are only valid for UEFI.

In the case of grub2 for example, the path is relative to the root of the partition where grub2 searches for the kernel and initramfs images. So if /boot is a mountpoint, the linux and initrd path would be relative to the boot partition. So something like your example assuming that the files are located in /boot/ostree/fedora-coreos-<commit-id>/.

But if /boot isn't a mountpoint and the images are located in the root partition, then the paths should be relative to the root of that partition, and so have the following in the BLS snippet for grub2 to find them:

linux /boot/ostree/fedora-coreos-<commit-id>/vmlinuz-5.2.11-200.fc30.s390x
initrd /boot/ostree/fedora-coreos-<commit-id>/initramfs-5.2.11-200.fc30.s390x.img

In the case of zipl, it expects that the files paths are relative to the start of the root filesystem regardless of the partition layout. In the non-BLS case when the IPL sections are defined in the zipl.conf, the file paths will be like the following:

[vmlinuz-5.2.11-200.fc30.s390x]
target=/boot
image=/boot/ostree/fedora-coreos-<commit-id>/vmlinuz-5.2.11-200.fc30.s390x
ramdisk=/boot/ostree/fedora-coreos-<commit-id>/initramfs-5.2.11-200.fc30.s390x.img

All this is to say that I think that zipl shouldn't make assumptions about the path of the images. It's up to the tool generating the BLS snippets to determine the correct path (the kernel may not even be in /boot, it could be in any other directory as long as it's in the same device than where the bootmap is).

Something that may be done is to fallback to look using the target value as prefix. That way at least the /boot won't be hardcoded but instead a value that's configurable by zipl.conf could be used.

But I'm not that familiar with that part of zipl to know if that makes sense or not.

[stefan-haberland]

Hi @tuan-hoang1 Tuan,

I have to agree with Javier. Hardcoding /boot/ does not look like a good idea.
You said it should be path relative to the $BOOT directory. But this could be anywhere mounted in the system. Taking the target information into account (as Javier suggested) could be an idea to have a more flexible way. At least zipl always need to have a target directory set to know where to write the bootmap to.

Regards,
Stefan

[tuan-hoang1]

@martinezjavier

It's up to the tool generating the BLS snippets to determine the correct path (the kernel may not even be in /boot, it could be in any other directory as long as it's in the same device than where the bootmap is).

On x86 FCOS,

/boot/grub2/grub.cfg
...
search --label boot --set boot
set root=$boot
...
blscfg

And I see that linux+initrd entries also start with /ostree. Now I have not fully read/understand the grub-core/commands/blscfg.c source, but I guess it would treat /ostree/fedora-coreos-<commit-id>/initramfs-5.2.11-200.fc30.x86_64.img as relative to grub's root (which is the partition labeled as boot it finds earlier).
So, this is somewhat similar to zipl's target=/boot.
Do I read that correctly ? If it's correct, let's agree we go on this direction.

@stefan-haberland

But this could be anywhere mounted in the system

This is the key point, thanks !

At least zipl always need to have a target directory set to know where to write the bootmap to.

I just happened to read zipl(5) more thoroughly, and figure that I agree with you, was not really paying attention to the 'bootmap' file before.

[martinezjavier]

And I see that linux+initrd entries also start with /ostree. Now I have not fully read/understand the grub-core/commands/blscfg.c source, but I guess it would treat /ostree/fedora-coreos-<commit-id>/initramfs-5.2.11-200.fc30.x86_64.img as relative to grub's root (which is the partition labeled as boot it finds earlier).

The $boot and $root are not the same than the Linux boot and root partitions. These variables are set to the device and partition that are used to store the grub config file and modules ($boot) and to store the kernel and initramfs images ($root).

On a legacy BIOS install, both the grub.cfg file, grub modules, kernel and initramfs images are in the same partition (either the Linux boot partition if it exists or the root partition). So in this case $boot == $root.

On a EFI install, the grub.cfg file and grub modules are in the EFI System Partition (ESP) but the kernel and initramfs are in a separate partition (that can either be the Linux boot partition if it exists or the root partition). So in this case $boot and $root are set to different partitions.

So the path for the linux and initrd fields are relative to the start of the partition where the kernel and initramfs images are located, which could be either the boot partition if it exists (in which case would be /ostree/...) or the root partition (in which case would be /boot/ostree...).

And this partition could be the same than $boot or $root as explained before. But that's an unrelated concept (yeah, I know it's confusing because are named the same).

Now going back to zip. I do not know if the target field has the same semantics that $boot or $root. For me it is "where the bootmap is located" rather than "where the kernel and initramfs are located". If it's the latter, then I agree that we could use that value as a prefix for the paths in linux and initrd.

I'm also OK to extend the semantics of target if it was only used to mean the place where the bootmap is located.

[tuan-hoang1]

So the path for the linux and initrd fields are relative to the start of the partition where the kernel and initramfs images are located, which could be either the boot partition if it exists (in which case would be /ostree/...) or the root partition (in which case would be /boot/ostree...).
And this partition could be the same than $boot or $root as explained before. But that's an unrelated concept (yeah, I know it's confusing because are named the same).

Okay now it seems much clearer to me.

Now going back to zip. I do not know if the target field has the same semantics that $boot or $root. For me it is "where the bootmap is located" rather than "where the kernel and initramfs are located". If it's the latter, then I agree that we could use that value as a prefix for the paths in linux and initrd.

I think zipl is aligned with $root(where kernel + initramfs are stored), because it always looks for absolute paths in root partition. Below is an example where zipl works:

...
target=/boot3
[linux]
image=/boot2/vmlinuz-vanilla
ramdisk=/boot2/initramfs-vanilla

/boot is empty, mount point for boot partition.
/boot2 and /boot3 are just directories under root partition.
/boot3 only has 1 single file /boot3/bootmap.

So, target has no similar semantics like $boot nor $root: it only stores the bootmap file, has nothing to do with kernel + initrd images (might need Stefan's help backing up this statement). Strictly speaking, we cannot make it as the absolute reference to start finding kernel + initramfs.

So, if we decide to extend the semantics of target to be the place to store kernel + initramfs, then we have to explicitly :

1. Add /path (as in target=/path) to be the second place to start finding kernel + initramfs beside /.
2. Make target=/path to be a hard requirement in minimal zipl.conf (which it already is).

[tuan-hoang1]

Okay for this I think I/we can come up with a patch soonish for this.

I just figured that even if we got this one and the issue#70 fix in s390x-tools upstream, it gotta be pretty hard to get into RHCOS in time. So, those 2 are pretty much serving FCOS in the mean time, for which I would like postpone a little bit.

We will have to work around these 2 issues in downstream FCOS/RHCOS then, I believe. Please take this into account @martinezjavier and back me up in the downstream discussions :)

[martinezjavier]

We will have to work around these 2 issues in downstream FCOS/RHCOS then, I believe. Please take this into account @martinezjavier and back me up in the downstream discussions :)

Sure, I'm not familiar with the RHCOS schedule but happy to chime in for any downstream discussion.