Change Summary:
Update testing notes, add bug IDs.
Testing Done: |
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Bugs: |
|
Review Request #1189 — Created Aug. 29, 2018 and submitted
Information | |
---|---|
citrus | |
illumos-gate | |
master | |
9627, 9628, 9721 | |
|
|
5e9492b... | |
Reviewers | |
general | |
9627 No longer need 32-bit boot_archive 9628 UFS boot archives are too large 9721 cmd/boot: support cpio boot archive
This has been tested on OmniOS, creating HSFS and UFS archives with both
old and new code and comparing contents, and test booting. Also testing with CPIO.
For the size calculation problem, there is more testing information in the issue -
1024x too much space was being reserved for directories.With this change I'm adding a new property on the
system/boot-archive:default
service to specify the desired archive format; this defaults to cpio. This property is honoured by both the Cbootadm
code, which is responsible for building the HSFS format archives, and by the script that handles UFS and CPIO.
If the property is not present then the default is HSFS if/usr/bin/mkisofs
is present, and CPIO otherwise. If the property is set to an unknown value then CPIO will be used.The man page for
bootadm
has been updated to include information on the new option and also to show the-f
option forbootadm update-archive
(force archive creation) which was previously undocumented.Testing
On my test system a boot archive contains 1056 files, 92 directories and 216 hardlinks (mostly CPU firmware).
During system boot, 2953 file opens are performed of which 2274 result in ENOENT; the total data read from the archive is 46MiB.
Testing with each of the four archive formats below shows that processing time during boot is in the range 3-9 seconds for all formats, with different boots giving different results. On average archive processing takes around 5 seconds regardless of format.
bloody% svcprop -p config/format boot-archive cpiobloody% time pfexec bootadm update-archive -f updating platform/i86pc/amd64/boot_archive (CPIO) pfexec bootadm update-archive -f 5.56s user 0.15s system 98% cpu 5.826 total bloody% file /platform/i86pc/amd64/boot_archive /platform/i86pc/amd64/boot_archive: gzip compressed data - deflate method , original file name bloody% gzip -dc /platform/i86pc/amd64/boot_archive > /tmp/l bloody% file /tmp/l /tmp/l: ASCII cpio archive - CHR (-c) header bloody% ls -lh /platform/i86pc/amd64/boot_archive /tmp/l -rw-r--r-- 1 root root 31.6M Jul 5 15:45 /platform/i86pc/amd64/boot_archive -rw-r--r-- 1 af other 83.1M Jul 5 15:45 /tmp/lbloody% svccfg -s boot-archive setprop config/format = ufs bloody% time pfexec bootadm update-archive -f updating platform/i86pc/amd64/boot_archive (UFS) pfexec bootadm update-archive -f 3.86s user 0.92s system 6% cpu 1:15.96 total bloody% file /platform/i86pc/amd64/boot_archive /platform/i86pc/amd64/boot_archive: English text bloody% ls -l /platform/i86pc/amd64/boot_archive -rw-r--r-- 1 root root 45.8M Jul 5 15:48 /platform/i86pc/amd64/boot_archivebloody% svccfg -s boot-archive setprop config/format = hsfs bloody% time pfexec bootadm update-archive -f updating //platform/i86pc/amd64/boot_archive (HSFS) pfexec bootadm update-archive -f 0.17s user 0.12s system 1% cpu 21.938 total bloody% file /platform/i86pc/amd64/boot_archive /platform/i86pc/amd64/boot_archive: ISO 9660 filesystem image bloody% ls -lh /platform/i86pc/amd64/boot_archive -rw-r--r-- 1 root root 36.4M Jul 5 15:46 /platform/i86pc/amd64/boot_archivebloody% svccfg -s boot-archive setprop config/format = ufs-nocompress bloody% time pfexec bootadm update-archive -f updating platform/i86pc/amd64/boot_archive (UFS-nocompress) pfexec bootadm update-archive -f 5.32s user 1.54s system 5% cpu 1:56.01 total bloody% file /platform/i86pc/amd64/boot_archive /platform/i86pc/amd64/boot_archive: gzip compressed data - deflate method , original file name bloody% gzip -dc /platform/i86pc/amd64/boot_archive >| /tmp/l bloody% file /tmp/l /tmp/l: English text bloody% ls -lh /platform/i86pc/amd64/boot_archive /tmp/l -rw-r--r-- 1 root root 27.5M Jul 5 15:43 /platform/i86pc/amd64/boot_archive -rw-r--r-- 1 af other 91.7M Jul 5 15:44 /tmp/lAlso, testing with multiple mounted boot environments with differing properties, including unset (i.e. an old BE)
omni# beadm list | awk '$3 ~ /\// {print $3}' | while read mp; do echo "$mp: \\c" SVCCFG_REPOSITORY=$mp/etc/svc/repository.db svccfg -s boot-archive listprop config/format done /: config/format astring cpio /hsfs: config/format astring hsfs /ufs: config/format astring ufs /ufs-nocompress: config/format astring ufs-nocompress /cpio: config/format astring cpio /unset: zsh: exit 1 omni# bootadm -a update_all -f updating /platform/i86pc/amd64/boot_archive (CPIO) Creating boot_archive for /hsfs updating /hsfs//platform/i86pc/amd64/boot_archive (HSFS) Creating boot_archive for /ufs updating /ufs/platform/i86pc/amd64/boot_archive (UFS) Creating boot_archive for /ufs-nocompress updating /ufs-nocompress/platform/i86pc/amd64/boot_archive (UFS-nocompress) Creating boot_archive for /cpio updating /cpio/platform/i86pc/amd64/boot_archive (CPIO) Creating boot_archive for /unset updating /unset/platform/i86pc/amd64/boot_archive (UFS)
Update testing notes, add bug IDs.
Testing Done: |
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Bugs: |
|
usr/src/cmd/boot/bootadm/bootadm.c (Diff revision 1) |
---|
Old code called dircache_updatefile() in default case. Should the new code do the same?
usr/src/cmd/boot/bootadm/bootadm.c (Diff revision 1) |
---|
better switch the return type to bool - we only return 0 or 1 values there.
usr/src/cmd/boot/bootadm/bootadm.c (Diff revision 1) |
---|
I'd like to have verbose log there to track what is going on, please add:
if (bam_verbose) bam_print("mkisofs not found\n");
usr/src/cmd/boot/bootadm/bootadm.c (Diff revision 1) |
---|
also for verbose tracking:
if (bam_verbose) { bam_print("%s does not have %s/%s property, using " "mkisofs\n", BOOT_ARCHIVE_FMRI, SCF_PG_CONFIG, SCF_PROPERTY_FORMAT); }
usr/src/cmd/boot/bootadm/bootadm.c (Diff revision 1) |
---|
and here:
if (bam_verbose) { bam_print("creating %s boot archive\n", format != NULL? format : "hsfs"); }
usr/src/cmd/boot/bootadm/bootadm.c (Diff revision 1) |
---|
this printout can now be removed as we just did announce in use_mkisofs(), also the message is wrong anyhow.
usr/src/cmd/boot/scripts/boot-archive-update.ksh (Diff revision 1) |
---|
"on x86 get rid of transient boot configuration"
both grub and loader use transient config.
usr/src/cmd/boot/scripts/create_ramdisk.ksh (Diff revision 1) |
---|
actually for sparc we should default to compress=yes, because it will compress individual files in the archive (as opposed to compress the entire boot archive). loader and grub can gunzip the boot_archive while loading it, but sparc can not, which is why we gzip individual files there.
I am not entirely sure about whole archive versus files compression on x86 - whole archive compression will save us a bit more loading bandwith, individual compressed files will save us memory needed for boot archive.
Also there is a question, should we still need uncompressed kernel on sparc, because x86 seems to be able to cope with compressed kernel just fine...
usr/src/cmd/boot/scripts/create_ramdisk.ksh (Diff revision 1) |
---|
BOOT_ARCHIVE is set as relative path (without starting '/'), should we set archive with "/$ALT_ROOT" here? The message below is confusing..
usr/src/cmd/boot/scripts/create_ramdisk.ksh (Diff revision 1) |
---|
There we do have one small issue - we always use mkfile below, therefore we always will create new archive there.
boot after pkg update, 32-bit archive is gone:
tsoome@uefi-oi:~$ ls -l /platform/i86pc/ total 10 drwxr-xr-x 4 root sys 6 aug 30 12:10 amd64 drwxr-xr-x 10 root sys 10 sept 26 2017 kernel drwxr-xr-x 4 root sys 8 jaan 19 2018 ucode drwxr-xr-x 2 root root 2 aug 22 11:10 updates tsoome@uefi-oi:~$ tsoome@uefi-oi:~$ sudo -s Password: root@uefi-oi:/home/tsoome# svcprop -p config/format boot-archive cpio root@uefi-oi:/home/tsoome# svccfg -s boot-archive setprop config/format = ufs root@uefi-oi:/home/tsoome# svccfg -s boot-archive:default refresh root@uefi-oi:/home/tsoome# svcprop -p config/format boot-archive ufs root@uefi-oi:/home/tsoome# bootadm update-archive -fv forced update of archive requested cannot find: /etc/cluster/nodeid: No such file or directory cannot find: /etc/devices/mdi_ib_cache: No such file or directory cannot find: /etc/devices/retire_store: No such file or directory creating ufs boot archive updating platform/i86pc/amd64/boot_archive (UFS) root@uefi-oi:/home/tsoome# root@uefi-oi:/home/tsoome# rebootsystem booted OK with cpio/ufs/ufs-nocompress/hsfs, however, there is one change missing:
diff --git a/usr/src/cmd/halt/halt.c b/usr/src/cmd/halt/halt.c index 166e5a163b..657f207a85 100644 --- a/usr/src/cmd/halt/halt.c +++ b/usr/src/cmd/halt/halt.c @@ -1584,12 +1584,12 @@ main(int argc, char *argv[]) if (cmd != A_DUMP) { int start, end, delta; - (void) kill(-1, SIGTERM); - start = time(NULL); - if (zoneid == GLOBAL_ZONEID && !nosync) do_archives_update(fast_reboot); + (void) kill(-1, SIGTERM); + start = time(NULL); + end = time(NULL); delta = end - start; if (delta < 5)it is needed to make sure we can get format from SMF, otherwise we will have format ping-pong...
Commit: |
|
||||
---|---|---|---|---|---|
Diff: |
Revision 3 (+551 -543) |
usr/src/cmd/boot/bootadm/bootadm.c (Diff revision 3) |
---|
gcc7:
bootadm.c:3332:5: error: suggest explicit braces to avoid ambiguous 'else' [-Werror=dangling-else]
Commit: |
|
||||
---|---|---|---|---|---|
Diff: |
Revision 4 (+553 -543) |
usr/src/cmd/boot/scripts/create_ramdisk.ksh (Diff revision 4) |
---|
Should that not be "Unsupported archive format"?
usr/src/cmd/boot/scripts/create_ramdisk.ksh (Diff revision 4) |
---|
Should ufs-nocompress be shown in the usage output?
Is the logic correct in the following situation: An updated system running this code running bootadm -R against an OS instance that doesn't have cpio support in the kernel?
Will the system still generate a cpio archive in this case, because that's the default? Then the system won't boot.
This is one case of a general issue, of what happens when it can't contact SMF? If SMF isn't running, then I believe the error path is safe in the simplest case of that. But with an alternate root, using SMF seems entirely unsafe.
Adding an SMF dependency seems undesirable in any case.
How about setting a property on the zfs root filesystem for the BE instead?
I think you have to be absolutely sure that the target instance you're generating the archive for understands the format you're generating. So you can only use cpio if you can confirm it's valid, else the old hsfs/ufs has to be the default.
Commit: |
|
||||
---|---|---|---|---|---|
Diff: |
Revision 5 (+553 -543) |
Add proper handling for alternate root archives.
Commit: |
|
||||
---|---|---|---|---|---|
Diff: |
Revision 6 (+660 -550) |
Testing Done: |
|
---|
usr/src/cmd/boot/scripts/create_ramdisk.ksh (Diff revision 6) |
---|
First, shouldn't that be $FORMAT, not $1, in the error message?
Second, what if the admin sets the archive property to mkisofs, but that doesn't work because mkisofs is missing (for whatever reason)? We then fall through to running this script. In the old implementation, that was fine because we then tried ufs. But in this case we error out.
I believe we shouldn't error out on invalid input, but choose a sensible format here. Whether we do or we don't, we need to produce a more helpful error or warning: both for completely invalid formats, and separately for the case where mkisofs is asked for but not available.
(And yes, there's the possibility that an a;ternate BE specifies mkisofs but the current system doesn't have it.)
Commit: |
|
||||
---|---|---|---|---|---|
Diff: |
Revision 7 (+659 -550) |
Added mkdir as /boot/amd64 may not exist (reported by tsoome)
Commit: |
|
||||
---|---|---|---|---|---|
Diff: |
Revision 8 (+660 -550) |