On Tue, Sep 06, 2022 at 09:06:41PM +0000, Klemens Nanni wrote:
> On Sun, Sep 04, 2022 at 07:08:51PM +0000, Mikolaj Kucharski wrote:
> > Hi,
> >
> > I have strange setup on some of my machines, when I want to encrypt disk
> > where OpenBSD is installed, but still be able to boot them up without
> > any user interaction, like passphrase entry for CRYPTO softraid(4). I
> > have this so I can with little time spent lock out access to the disk,
> > by wiping beginning of the disk, instead of entire disk. I do recognise
> > magnitute of limitations of this. I still try to wipe entire disk, when
> > it's time for a machine decommission, but first I break CRYPTO softraid
> > by wiping beginning and then switch to proper full disk wipe.
> >
> > All in all that brings me to the below diff. I was only able to test on
> > amd64, as this is the only type of machine which I have.
>
> Thanks, although the setup seems a bit strange, your diff makes sense
> and works as advertised on amd64, arm64 and sparc64.
>
> I have adjusted our installboot regress tests to install onto softraid
> RAID 1C with a keydisk so it must a) iterate over multiple chunks and
> b) ignore the key-disk, which is a nice combined exercise.
>
> Here is your diff with tweaked wording so it is clearer; this also
> nicely aligns the "- skipping..." for both offline and keydisk cases.
>
> With this diff, regress/usr.sbin/installboot passes on amd64, arm64 and
> sparc64 using the above mentioned softraid.
>
> regress uses a separate device for the keydisk but that does not effect
> the skip logic.
>
> Feedback? OK?
Thank you! One question about source code comment and English language.
> >
> >
> > Index: i386_softraid.c
> > ===================================================================
> > RCS file: /cvs/src/usr.sbin/installboot/i386_softraid.c,v
> > retrieving revision 1.19
> > diff -u -p -u -r1.19 i386_softraid.c
> > --- i386_softraid.c 29 Aug 2022 18:54:43 -0000 1.19
> > +++ i386_softraid.c 3 Sep 2022 11:28:55 -0000
> > @@ -65,6 +65,13 @@ sr_install_bootblk(int devfd, int vol, i
> > return;
> > }
> >
> > + /* Key disk has size of zero */
> > + if (bd.bd_size == 0) {
> > + fprintf(stderr, "softraid chunk %u looks like key disk - "
> > + "skipping...\n", disk);
> > + return;
> > + }
> > +
> > if (strlen(bd.bd_vendor) < 1)
> > errx(1, "invalid disk name");
> > part = bd.bd_vendor[strlen(bd.bd_vendor) - 1];
> >
> >
> > Below follows my test and comments what happens without the diff
> > and with the diff.
> >
> > First without the diff machine doesn't boot when I use keydisk on the
> > same disk which has the OpenBSD operaring system, wd0a and wd0d:
> >
> > Booting from Hard Disk...
> > Using drive 0, partition 3.
> > Loading......
> > ERR M
> >
> >
> > To keep it short, it is because of installboot(8) installs boot blocks
> > on both wd0a and wd0d:
> >
> > ramdisk# bioctl sd0
> > Volume Status Size Device
> > softraid0 0 Online 268426960384 sd0 CRYPTO
> > 0 Online 268426960384 0:0.0 noencl <wd0d>
> > 1 Online key disk 0:1.0 noencl <wd0a>
>
>
> Index: 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
> --- efi_softraid.c 29 Aug 2022 18:54:43 -0000 1.2
> +++ efi_softraid.c 6 Sep 2022 20:47:16 -0000
> @@ -54,6 +54,13 @@ sr_install_bootblk(int devfd, int vol, i
> return;
> }
>
> + /* Keydisks always has as size of zero. */
I'm not good with words, but is this correct grammar?
> + if (bd.bd_size == 0) {
> + fprintf(stderr, "softraid chunk %u is keydisk - skipping...\n",
> + disk);
> + return;
> + }
> +
> if (strlen(bd.bd_vendor) < 1)
> errx(1, "invalid disk name");
> part = bd.bd_vendor[strlen(bd.bd_vendor) - 1];
> Index: 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
> --- i386_softraid.c 29 Aug 2022 18:54:43 -0000 1.19
> +++ i386_softraid.c 6 Sep 2022 20:47:19 -0000
> @@ -65,6 +65,13 @@ sr_install_bootblk(int devfd, int vol, i
> return;
> }
>
> + /* Keydisks always has as size of zero. */
> + if (bd.bd_size == 0) {
> + fprintf(stderr, "softraid chunk %u is keydisk - skipping...\n",
> + disk);
> + return;
> + }
> +
> if (strlen(bd.bd_vendor) < 1)
> errx(1, "invalid disk name");
> part = bd.bd_vendor[strlen(bd.bd_vendor) - 1];
> Index: 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
> --- sparc64_softraid.c 29 Aug 2022 18:54:43 -0000 1.6
> +++ sparc64_softraid.c 6 Sep 2022 20:47:57 -0000
> @@ -55,6 +55,13 @@ sr_install_bootblk(int devfd, int vol, i
> return;
> }
>
> + /* Keydisks always has as size of zero. */
> + if (bd.bd_size == 0) {
> + fprintf(stderr, "softraid chunk %u is keydisk - skipping...\n",
> + disk);
> + return;
> + }
> +
> if (strlen(bd.bd_vendor) < 1)
> errx(1, "invalid disk name");
> part = bd.bd_vendor[strlen(bd.bd_vendor) - 1];
>
--
Regards,
Mikolaj
