On Tue, Aug 16, 2022 at 09:03:55PM +0000, Miod Vallat wrote:
> > after a prompt from stsp@ and florian@, reporting that newfs_msdos
> > fails when given an $duid.i argument, I set down to see what could be
> > going on. My test using an USB stick failed to reprdouce the problem.
> > Then I started using a vnd, and that shows the issue (once in a
> > while). The feeling is that any disk devcied created on the fly might
> > show this issue.
>
> Moments ago kn@ stumbled upon this as well.
>
> It turns out that, at least in the vnd case, there is indeed a race
> between the scheduling of the task running disk_attach_callback and the
> first access to the vnd device, which will cause the label to get read.
>
> While vnd(4) has logic to read a label on-demand, disk_attach_callback
> assumes it runs at a point where the label can safely be obtained, and
> will not retry (because it sets DKF_OPENED, which never gets unset).
>
> The following shell script will demonstrate the issue:
>
> cat << EOF > test.sh
> #! /bin/sh
> set -e
>
> dd if=/dev/zero of=img.bin bs=1m count=4 >/dev/null
> iter=1
> while [ $iter -lt 1000 ]; do
> vnconfig vnd0 img.bin
> fdisk -iy vnd0 > /dev/null
> disklabel -Aw vnd0 > /dev/null
> duid=$(sysctl hw.disknames|sed 's,.*vnd0:,,')
> disklabel $duid > /dev/null
> vnconfig -u vnd0
> iter=$((iter + 1))
> done
> EOF
>
> (don't forget to vnconfig -u vnd0 if it fails).
Here's the improved version of a new regress test I initially wrote to
compare real disks with vnd and block devices with duids.
I ran into this issue when trying to test installboot(8) changes on
vnd(4) rather than real hardware first.
The new regress spins up a vnd disk, creates a label and accesses the
disk through both its disklabel and block device... former is broken.
This could easily be adapted or copied into initial installboot(8)
tests which I wanted to do anyway.
regress/sys/kern/disk seems specific and yet broad enough; putting it
under lib/libutil/ due to opendev(3) makes little sense since that's
just the common entry point to what is actually a kernel bug and this
is not really vnd specific either as we're dealing with a
race condition/design flaw.
Feedback? Objections? OK?
Index: regress/sys/kern/disk/Makefile
===================================================================
RCS file: regress/sys/kern/disk/Makefile
diff -N regress/sys/kern/disk/Makefile
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ regress/sys/kern/disk/Makefile 18 Aug 2022 10:06:49 -0000
@@ -0,0 +1,56 @@
+# $OpenBSD: $
+
+# anything calling opendev(3) with "sd0a" or "fc2aa4672193310c.a", e.g.
+# disklabel(8), newfs(8) or newfs_msdos(8)
+OPENDEV ?= /sbin/disklabel
+# write default MBR
+FORMAT ?= /sbin/fdisk -iy
+# write standard disklabel to get a valid DUID
+NEWLABEL ?= /sbin/disklabel -Aw
+
+# whether to trace diskmap(4)'s DIOCMAP handling DUIDs/block devices
+DO_TRACE ?= No
+
+IMGFILE = disk.img
+DEVFILE = vnd.txt
+DUIDFILE = duid.txt
+PART = a
+
+
+REGRESS_SETUP_ONCE = format-disk
+
+create-new-disk: create-new-disk
+ dd if=/dev/zero of=${IMGFILE} bs=1m count=0 seek=32
+ ${SUDO} vnconfig -- ${IMGFILE} > ${DEVFILE}
+format-disk: create-new-disk
+ ${SUDO} ${FORMAT} -- "$$(<${DEVFILE})"
+ ${SUDO} ${NEWLABEL} -- "$$(<${DEVFILE})"
+
+
+REGRESS_TARGETS = opendev-use-block \
+ opendev-use-duid
+
+opendev-use-block:
+ ${SUDO} ${OPENDEV} -- "$$(<${DEVFILE})${PART}"
+
+opendev-use-duid:
+ ${SUDO} disklabel -- "$$(<${DEVFILE})" | \
+ awk '$$1 == "duid:" { print $$2 }' > ${DUIDFILE}
+.if ${DO_TRACE:L} == "yes"
+ @# always run kdump regardless of newfs exit code
+ sh -uc '${SUDO} ktrace -- ${OPENDEV} -- "$$(<${DUIDFILE}).${PART}" ;\
+ ret=$$? ;\
+ ${SUDO} kdump -tcn | grep -FwA2 DIOCMAP ;\
+ exit $$ret'
+.else
+ ${SUDO} ${OPENDEV} -- "$$(<${DUIDFILE}).${PART}"
+.endif
+
+
+CLEANFILES = ${IMGFILE} ${DEVFILE} ${DUIDFILE} ktrace.out
+REGRESS_CLEANUP = cleanup
+
+cleanup:
+ ${SUDO} vnconfig -vu -- "$$(<${DEVFILE})"
+
+.include <bsd.regress.mk>
