Running installboot(8) on softraid(4) volumes means installing stages on
every softraid chunk.

The overall idea is the same, but MD implementations differ.

sparc64_softraid.c's sr_install_bootblk() reuses sparc64_installboot.c's
md_installboot() for this.

For sparc64, md_installboot() does the copy of stage 2, usually
/usr/mdec/ofwboot to /ofwboot, so when `-r root' is passed, it prefixes
the file path with "root".

For single-disk installations (plain-disk and single-chunk softraid)
this is fine, but as soon as multiple chunks are used, md_installboot()
currently prefixes the path each time, obviously resulting in invalid
paths starting with the second run.

Other architectures do reuse md_installboot() as well but either don't
do such a copy or implement the prefixing differently -- plus they must
support softraid in the firt place to be able to hit this type of bug.


The fix is not super pretty and makes sparc64's code look a little of,
but this seems to be the simplest and most straight forward fix.

Alternatives could perhaps be reworking the fileprefix() function to be
resilient against reuse, but that doesn't seem like a sane idea and
would involves testing on all architectures.

With this fixed, installboot regress finally passes on sparc64 and
installboot no longer fails at the end of a fresh installation onto
softraid with multiple chunks.

Feedback? OK?


Index: regress/usr.sbin/installboot/Makefile
===================================================================
RCS file: /cvs/src/regress/usr.sbin/installboot/Makefile,v
retrieving revision 1.17
diff -u -p -r1.17 Makefile
--- regress/usr.sbin/installboot/Makefile       1 Sep 2022 17:23:36 -0000       
1.17
+++ regress/usr.sbin/installboot/Makefile       1 Sep 2022 19:46:09 -0000
@@ -43,7 +43,6 @@ STAGEDIR =            /usr/mdec
 STAGEFILES =           ${STAGENAMES:=${STAGEDIR}/%}
 
 .if ${USE_SOFTRAID:L} == "yes"
-# installboot(8) behaviour for multi-chunk softraid(4) differs across platforms
 NDISKS ?=              1 2
 .else
 NDISKS =               1
@@ -121,7 +120,6 @@ dry-default:
 dry-root:
        ${SUDO} ${DRY_RUN} -r/ -- "$$(<${ROOTDEVFILE})"
 
-# XXX fails with NDISKS > 1 on sparc64, 1 <= NDISKS <= 4 works on amd64
 root:
        ${SUDO} ${REAL_RUN} -r ${MOUNTPOINT} "$$(<${ROOTDEVFILE})"
 root-stages:
Index: usr.sbin/installboot/sparc64_installboot.c
===================================================================
RCS file: /cvs/src/usr.sbin/installboot/sparc64_installboot.c,v
retrieving revision 1.10
diff -u -p -r1.10 sparc64_installboot.c
--- usr.sbin/installboot/sparc64_installboot.c  31 Aug 2022 19:40:37 -0000      
1.10
+++ usr.sbin/installboot/sparc64_installboot.c  1 Sep 2022 19:46:09 -0000
@@ -97,10 +97,17 @@ md_prepareboot(int devfd, char *dev)
 void
 md_installboot(int devfd, char *dev)
 {
+       static int prefixed = 0;
+
        /* XXX - is this necessary? */
        sync();
 
-       bootldr = fileprefix(root, bootldr);
+       /*
+        * sr_install_bootblk() calls md_installboot() for every softraid chunk
+        * but the path must be prefixed only once.
+        */
+       if (!prefixed++)
+               bootldr = fileprefix(root, bootldr);
        if (bootldr == NULL)
                exit(1);
        if (verbose)

Reply via email to