Closed tuan-hoang1 closed 4 years ago
Quick talk with Stefan today, we decided I would create a patch for this feature. Doing now.
Here is an alternative suggestion, which fits in the CoreOS model and avoid hardcoding configuration in code. I don't know a lot about zipl though, so feel free to adapt to your case.
The lookup logic for zipl.conf
could be augmented to fall through these directories, in order of priority, and select the existing path with the highest priority:
/run/zipl/zipl.conf
/etc/zipl.conf
/usr/lib/zipl/zipl.conf
In the normal case, this won't change the current behavior, and it will keep reading /etc/zipl.conf
.
However distributions can now start shipping a /usr/lib/zipl/zipl.conf
to be used as a default if (and only if) the file under /etc
does not exist.
This allows distributions to ship (and update) the default settings, while allowing users to keep their customizations under /etc
(persistent) and /run
(volatile, only for the current boot).
(Path naming is just an example, feel free to pick what fits for you)
@tuan-hoang1, please also set the secure=auto in the default config.
Using /lib/s390-tools/zipl.conf
as default is good idea too.
@stefan-haberland , @martinezjavier : Sorry, was preempted by other tasks ... How does the very simple patch below look ?:
diff --git a/zipl/Makefile b/zipl/Makefile
index a02e997..931b113 100644
--- a/zipl/Makefile
+++ b/zipl/Makefile
@@ -8,6 +8,7 @@ all:
install: all
$(MAKE) -C src install
$(MAKE) -C man install
+ $(INSTALL) -g $(GROUP) -o $(OWNER) -m 644 doc/zipl.conf.minimal $(DESTDIR)$(TOOLS_LIBDIR)/zipl.conf
clean:
$(MAKE) -C src clean
diff --git a/zipl/doc/zipl.conf.minimal b/zipl/doc/zipl.conf.minimal
new file mode 100644
index 0000000..2766c31
--- /dev/null
+++ b/zipl/doc/zipl.conf.minimal
@@ -0,0 +1,6 @@
+[defaultboot]
+defaultauto
+prompt=1
+timeout=5
+target=/boot
+secure=auto
diff --git a/zipl/include/zipl.h b/zipl/include/zipl.h
index 02827e2..f20da74 100644
--- a/zipl/include/zipl.h
+++ b/zipl/include/zipl.h
@@ -48,6 +48,7 @@
#define ZIPL_CONF_VAR "ZIPLCONF"
#define ZIPL_DEFAULT_CONF TOOLS_SYSCONFDIR "/zipl.conf"
+#define ZIPL_FALLBACK_CONF TOOLS_LIBDIR "/zipl.conf"
#define ZIPL_DEFAULT_BLSDIR "/boot/loader/entries"
#define ZIPL_STAGE3_PATH TOOLS_LIBDIR "/stage3.bin"
#define ZIPL_SIPL_PATH "/sys/firmware/ipl/has_secure"
diff --git a/zipl/src/job.c b/zipl/src/job.c
index 1178a7d..73b61a1 100644
--- a/zipl/src/job.c
+++ b/zipl/src/job.c
@@ -1796,8 +1796,12 @@ get_job_from_config_file(struct command_line* cmdline, struct job_data* job)
source = " (from environment variable "
ZIPL_CONF_VAR ")";
} else {
- /* Use default config file */
+ /* Use default config file if it is readable else use fallback config file */
filename = ZIPL_DEFAULT_CONF;
+ rc = misc_check_readable_file(ZIPL_DEFAULT_CONF);
+ if (rc) {
+ filename = ZIPL_FALLBACK_CONF;
+ }
source = "";
}
printf("Using config file '%s'%s\n", filename, source);
I like it :-)
@tuan-hoang1 looks good to me.
> --- /dev/null
> +++ b/zipl/doc/zipl.conf.minimal
> @@ -0,0 +1,6 @@
> +[defaultboot]
> +defaultauto
> +prompt=1
> +timeout=5
> +target=/boot
> +secure=auto
I would add a comment header to this example explaining that is a minimal config that could be used when the IPL sections are defined in BLS snippets, something like the following:
# This is an example of a minimal zipl.conf file that can be
# used when the sections are defined in BootLoaderSpec
# fragments files.
#
# See the zipl and zipl.conf man page for more details.
And should also document in both man pages the behavior that you are introducing here.
> } else {
> - /* Use default config file */
> + /* Use default config file if it is readable else use fallback config file */
> filename = ZIPL_DEFAULT_CONF;
> + rc = misc_check_readable_file(ZIPL_DEFAULT_CONF);
> + if (rc) {
> + filename = ZIPL_FALLBACK_CONF;
> + }
I would also add support for /run/zipl/zipl.conf
as @lucab suggested. That could be done as a follow-up but I think is worth it since you are already working on this.
Many thanks for super helpful comments ! TIL how to write a man page. Heck, it feels good ! Since we are getting close to a final patch, I create this PR: https://github.com/ibm-s390-tools/s390-tools/pull/71
Hi Stefan @stefan-haberland,
If BLS conf files are used, a
/etc/zipl.conf
might be as simple as below from Fedora 30:The code that generated this zipl.conf file is at : https://github.com/rhinstaller/anaconda/blob/master/pyanaconda/bootloader/zipl.py#L132
@martinezjavier suggested if we could have a way to not to have this simple zipl.conf file existed.
My suggestion is to have a cmdline option like
--dummy-conf
or--simple-conf
, which assume above 5 fields without generating an actual/etc/zipl.conf
file (please correct me Javier if I'm wrong here). One thing on top of my mind is, those options must be only applied if BLS is used (i.e.--blsdir
is provided or/boot/loader/entries
exists).