On Sat, Sep 10, 2022 at 02:10:22AM +0000, Klemens Nanni wrote:
> It does not have the prettiest signature, but nicely folds identical
> copies into MI softraid.c, which then allows us to
> - avoid further diverging MD code
> - implement the keydisk fix on tech@ once instead of thrice
> - reuse sr_open_chunk() in an upcoming diff to make -p softraid aware
>
> The last point is especially useful, otherwise I'd have to copy what
> sr_open_chunk() does yet another three times (each *_softraid.c file),
> turning it a much simpler and smaller diff.
>
> This conflicts with the keydisk fix on tech@
> "Softraid crypto with keydisk and installboot, skip on the same disk".
>
> It does not really matter which one we land first as the actual keydisk
> fix won't change, it just gets added either once or thrice.
>
> I've successfully tested the diff below together with the keydisk fix
> as well as the upcoming -p/softraid diff on amd64, arm64 and sparc64.
>
> Feedback? OK?
Full diff to ease testing/review for those interested. It includes all
outstanding work, which at this point is messy to keep apart:
- merge code into sr_open_chunk()
- make -p softraid aware
* add purely MI softraid.c sr_prepareboot() + sr_prepare_chunk()
which reuses md_prepareboot(0
- zap regress -p/softraid workaround
- skip keydisks
I'm sure this can be polished a bit more, there are might be better
function names, and verbose logging could probably be improved, but this
works and fixes all currently known issues (except for the kernel race).
With this in arm64, amd64, i386 and sparc64 should work exactly the same
without any manual fixes required during root on softraid install or odd
MD/raid-level/number-of-chunk specific bugs on all three architectures.
Index: regress/usr.sbin/installboot/Makefile
===================================================================
RCS file: /cvs/src/regress/usr.sbin/installboot/Makefile,v
retrieving revision 1.26
diff -u -p -r1.26 Makefile
--- regress/usr.sbin/installboot/Makefile 9 Sep 2022 12:55:43 -0000
1.26
+++ regress/usr.sbin/installboot/Makefile 10 Sep 2022 02:38:43 -0000
@@ -116,12 +116,6 @@ REGRESS_TARGETS = prepare
prepare:
${SUDO} ${REAL_RUN} -p -- "$$(<${ROOTDEVFILE})"
-.if ${USE_SOFTRAID:L} == "yes"
- # XXX -p is not yet softraid(4) aware, need to prepare chunks manually
-. for devfile in ${DISKDEVFILES}
- ${SUDO} ${REAL_RUN} -p -- "$$(<${devfile})"
-. endfor
-.endif
REGRESS_TARGETS += dry-prepare \
dry-default \
Index: usr.sbin/installboot/efi_softraid.c
===================================================================
RCS file: /cvs/src/usr.sbin/installboot/efi_softraid.c,v
retrieving revision 1.2
diff -u -p -r1.2 efi_softraid.c
--- usr.sbin/installboot/efi_softraid.c 29 Aug 2022 18:54:43 -0000 1.2
+++ usr.sbin/installboot/efi_softraid.c 10 Sep 2022 01:52:49 -0000
@@ -16,18 +16,9 @@
* OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
*/
-#include <sys/types.h>
-#include <sys/disklabel.h>
-#include <sys/ioctl.h>
-
#include <dev/biovar.h>
-#include <dev/softraidvar.h>
-#include <err.h>
-#include <fcntl.h>
#include <stdio.h>
-#include <string.h>
-#include <util.h>
#include <unistd.h>
#include "installboot.h"
@@ -40,31 +31,9 @@ sr_install_bootblk(int devfd, int vol, i
int diskfd;
char part;
- /* Get device name for this disk/chunk. */
- memset(&bd, 0, sizeof(bd));
- bd.bd_volid = vol;
- bd.bd_diskid = disk;
- if (ioctl(devfd, BIOCDISK, &bd) == -1)
- err(1, "BIOCDISK");
-
- /* Check disk status. */
- if (bd.bd_status != BIOC_SDONLINE && bd.bd_status != BIOC_SDREBUILD) {
- fprintf(stderr, "softraid chunk %u not online - skipping...\n",
- disk);
+ diskfd = sr_open_chunk(devfd, vol, disk, &bd, &realdev, &part);
+ if (diskfd == -1)
return;
- }
-
- if (strlen(bd.bd_vendor) < 1)
- errx(1, "invalid disk name");
- part = bd.bd_vendor[strlen(bd.bd_vendor) - 1];
- if (part < 'a' || part >= 'a' + MAXPARTITIONS)
- errx(1, "invalid partition %c\n", part);
- bd.bd_vendor[strlen(bd.bd_vendor) - 1] = '\0';
-
- /* Open device. */
- if ((diskfd = opendev(bd.bd_vendor, (nowrite ? O_RDONLY : O_RDWR),
- OPENDEV_PART, &realdev)) == -1)
- err(1, "open: %s", realdev);
if (verbose)
fprintf(stderr, "%s%c: %s boot blocks on %s\n", bd.bd_vendor,
Index: usr.sbin/installboot/i386_softraid.c
===================================================================
RCS file: /cvs/src/usr.sbin/installboot/i386_softraid.c,v
retrieving revision 1.19
diff -u -p -r1.19 i386_softraid.c
--- usr.sbin/installboot/i386_softraid.c 29 Aug 2022 18:54:43 -0000
1.19
+++ usr.sbin/installboot/i386_softraid.c 10 Sep 2022 02:59:56 -0000
@@ -32,7 +32,6 @@
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
-#include <util.h>
#include "installboot.h"
#include "i386_installboot.h"
@@ -51,31 +50,9 @@ sr_install_bootblk(int devfd, int vol, i
char part, efipart;
int diskfd;
- /* Get device name for this disk/chunk. */
- memset(&bd, 0, sizeof(bd));
- bd.bd_volid = vol;
- bd.bd_diskid = disk;
- if (ioctl(devfd, BIOCDISK, &bd) == -1)
- err(1, "BIOCDISK");
-
- /* Check disk status. */
- if (bd.bd_status != BIOC_SDONLINE && bd.bd_status != BIOC_SDREBUILD) {
- fprintf(stderr, "softraid chunk %u not online - skipping...\n",
- disk);
+ diskfd = sr_open_chunk(devfd, vol, disk, &bd, &dev, &part);
+ if (diskfd == -1)
return;
- }
-
- if (strlen(bd.bd_vendor) < 1)
- errx(1, "invalid disk name");
- part = bd.bd_vendor[strlen(bd.bd_vendor) - 1];
- if (part < 'a' || part >= 'a' + MAXPARTITIONS)
- errx(1, "invalid partition %c\n", part);
- bd.bd_vendor[strlen(bd.bd_vendor) - 1] = '\0';
-
- /* Open this device and check its disklabel. */
- if ((diskfd = opendev(bd.bd_vendor, (nowrite? O_RDONLY:O_RDWR),
- OPENDEV_PART, &dev)) == -1)
- err(1, "open: %s", dev);
/* Get and check disklabel. */
if (ioctl(diskfd, DIOCGDINFO, &dl) == -1)
Index: usr.sbin/installboot/installboot.c
===================================================================
RCS file: /cvs/src/usr.sbin/installboot/installboot.c,v
retrieving revision 1.15
diff -u -p -r1.15 installboot.c
--- usr.sbin/installboot/installboot.c 19 Aug 2022 08:27:48 -0000 1.15
+++ usr.sbin/installboot/installboot.c 10 Sep 2022 02:40:20 -0000
@@ -91,7 +91,11 @@ main(int argc, char **argv)
err(1, "open: %s", realdev);
if (prepare) {
+#if SOFTRAID
+ sr_prepareboot(devfd, dev);
+#else
md_prepareboot(devfd, realdev);
+#endif
return 0;
}
Index: usr.sbin/installboot/installboot.h
===================================================================
RCS file: /cvs/src/usr.sbin/installboot/installboot.h,v
retrieving revision 1.14
diff -u -p -r1.14 installboot.h
--- usr.sbin/installboot/installboot.h 3 Feb 2022 10:25:14 -0000 1.14
+++ usr.sbin/installboot/installboot.h 10 Sep 2022 03:03:03 -0000
@@ -43,6 +43,8 @@ void md_prepareboot(int, char *);
void md_installboot(int, char *);
#ifdef SOFTRAID
+int sr_open_chunk(int, int, int, struct bioc_disk *, char **, char *);
+void sr_prepareboot(int, char *);
void sr_installboot(int, char *);
void sr_install_bootblk(int, int, int);
void sr_install_bootldr(int, char *);
Index: usr.sbin/installboot/softraid.c
===================================================================
RCS file: /cvs/src/usr.sbin/installboot/softraid.c,v
retrieving revision 1.5
diff -u -p -r1.5 softraid.c
--- usr.sbin/installboot/softraid.c 8 Jun 2020 19:17:12 -0000 1.5
+++ usr.sbin/installboot/softraid.c 10 Sep 2022 03:03:02 -0000
@@ -15,19 +15,60 @@
* OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
*/
+#include <sys/types.h>
+#include <sys/disklabel.h>
#include <sys/dkio.h>
#include <sys/ioctl.h>
#include <dev/biovar.h>
#include <err.h>
+#include <fcntl.h>
#include <stdio.h>
#include <string.h>
+#include <unistd.h>
+#include <util.h>
#include "installboot.h"
static int sr_volume(int, char *, int *, int *);
+static void
+sr_prepare_chunk(int devfd, int vol, int disk)
+{
+ struct bioc_disk bd;
+ char *realdev;
+ char part;
+ int diskfd;
+
+ diskfd = sr_open_chunk(devfd, vol, disk, &bd, &realdev, &part);
+ if (diskfd == -1)
+ return;
+
+ warnx("prepping %s", realdev);
+
+ /* Prepare file system on device. */
+ md_prepareboot(diskfd, realdev);
+
+ close(diskfd);
+}
+
+void
+sr_prepareboot(int devfd, char *dev)
+{
+ int vol = -1, ndisks = 0, disk;
+
+ /* Use the normal process if this is not a softraid volume. */
+ if (!sr_volume(devfd, dev, &vol, &ndisks)) {
+ md_prepareboot(devfd, dev);
+ return;
+ }
+
+ /* Prepare file system on each disk that is part of this volume. */
+ for (disk = 0; disk < ndisks; disk++)
+ sr_prepare_chunk(devfd, vol, disk);
+}
+
void
sr_installboot(int devfd, char *dev)
{
@@ -107,4 +148,47 @@ sr_status(struct bio_status *bs)
else
exit(1);
}
+}
+
+int
+sr_open_chunk(int devfd, int vol, int disk, struct bioc_disk *bd,
+ char **realdev, char *part)
+{
+ int diskfd;
+
+ /* Get device name for this disk/chunk. */
+ memset(bd, 0, sizeof(*bd));
+ bd->bd_volid = vol;
+ bd->bd_diskid = disk;
+ if (ioctl(devfd, BIOCDISK, bd) == -1)
+ err(1, "BIOCDISK");
+
+ /* Check disk status. */
+ if (bd->bd_status != BIOC_SDONLINE &&
+ bd->bd_status != BIOC_SDREBUILD) {
+ fprintf(stderr, "softraid chunk %u not online - skipping...\n",
+ disk);
+ return -1;
+ }
+
+ /* Keydisks always has as size of zero. */
+ if (bd->bd_size == 0) {
+ fprintf(stderr, "softraid chunk %u is keydisk - skipping...\n",
+ disk);
+ return -1;
+ }
+
+ if (strlen(bd->bd_vendor) < 1)
+ errx(1, "invalid disk name");
+ *part = bd->bd_vendor[strlen(bd->bd_vendor) - 1];
+ if (*part < 'a' || *part >= 'a' + MAXPARTITIONS)
+ errx(1, "invalid partition %c\n", *part);
+ bd->bd_vendor[strlen(bd->bd_vendor) - 1] = '\0';
+
+ /* Open device. */
+ if ((diskfd = opendev(bd->bd_vendor, (nowrite ? O_RDONLY : O_RDWR),
+ OPENDEV_PART, realdev)) == -1)
+ err(1, "open: %s", *realdev);
+
+ return diskfd;
}
Index: usr.sbin/installboot/sparc64_softraid.c
===================================================================
RCS file: /cvs/src/usr.sbin/installboot/sparc64_softraid.c,v
retrieving revision 1.6
diff -u -p -r1.6 sparc64_softraid.c
--- usr.sbin/installboot/sparc64_softraid.c 29 Aug 2022 18:54:43 -0000
1.6
+++ usr.sbin/installboot/sparc64_softraid.c 10 Sep 2022 03:00:05 -0000
@@ -15,19 +15,14 @@
* OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
*/
-#include <sys/types.h>
-#include <sys/disklabel.h>
#include <sys/ioctl.h>
#include <sys/stat.h>
#include <dev/biovar.h>
-#include <dev/softraidvar.h>
#include <err.h>
-#include <fcntl.h>
#include <stdio.h>
#include <string.h>
-#include <util.h>
#include <unistd.h>
#include "installboot.h"
@@ -41,31 +36,9 @@ sr_install_bootblk(int devfd, int vol, i
int diskfd;
char part;
- /* Get device name for this disk/chunk. */
- memset(&bd, 0, sizeof(bd));
- bd.bd_volid = vol;
- bd.bd_diskid = disk;
- if (ioctl(devfd, BIOCDISK, &bd) == -1)
- err(1, "BIOCDISK");
-
- /* Check disk status. */
- if (bd.bd_status != BIOC_SDONLINE && bd.bd_status != BIOC_SDREBUILD) {
- fprintf(stderr, "softraid chunk %u not online - skipping...\n",
- disk);
+ diskfd = sr_open_chunk(devfd, vol, disk, &bd, &realdev, &part);
+ if (diskfd == -1)
return;
- }
-
- if (strlen(bd.bd_vendor) < 1)
- errx(1, "invalid disk name");
- part = bd.bd_vendor[strlen(bd.bd_vendor) - 1];
- if (part < 'a' || part >= 'a' + MAXPARTITIONS)
- errx(1, "invalid partition %c\n", part);
- bd.bd_vendor[strlen(bd.bd_vendor) - 1] = '\0';
-
- /* Open device. */
- if ((diskfd = opendev(bd.bd_vendor, (nowrite ? O_RDONLY : O_RDWR),
- OPENDEV_PART, &realdev)) == -1)
- err(1, "open: %s", realdev);
if (verbose)
fprintf(stderr, "%s%c: %s boot blocks on %s\n", bd.bd_vendor,
