Revert abort-on-panic in 'dev' cargo profile

Since proper unwinding is required for handling `should_panic` tests
cases, the existing default of abort-on-panic was rather inconvenient.
This returns the 'dev' profile to using unwind-on-panic, and cleans up
the 'phd' profile, which can use 'dev' as well for its unwinding.

.github/buildomat/jobs/image.sh

@@ -30,6 +30,9 @@ banner build
 
 # Enable the "omicron-build" feature to indicate this is an artifact destined
 # for production use on an appropriately configured Oxide machine
+#
+# The 'release' profile is configured for abort-on-panic, so we get an
+# immediate coredump rather than unwinding in the case of an error.
 ptime -m cargo build --release --verbose -p propolis-server --features omicron-build
 
 banner image

.github/buildomat/jobs/phd-build.sh

@@ -17,15 +17,15 @@ outdir="/out"
 cargo --version
 rustc --version
 
-# Build the Propolis server binary in debug mode to enable assertions
-# that should fire during tests.
+# Build the Propolis server binary with 'dev' profile to enable assertions that
+# should fire during tests.
 banner build-propolis
 ptime -m cargo build --verbose -p propolis-server
 
-# Build the PHD runner with the phd profile to enable unwind on panic,
-# which the framework requires to catch certain test failures.
+# The PHD runner requires unwind-on-panic to catch certain test failures, so
+# build it with the 'dev' profile which is so configured.
 banner build-phd
-ptime -m cargo build --verbose -p phd-runner --profile=phd
+ptime -m cargo build --verbose -p phd-runner
 
 banner contents
 tar -czvf target/debug/propolis-server-debug.tar.gz \

Cargo.toml

@@ -27,17 +27,18 @@ exclude = [
   "phd-tests/buildomat",
 ]
 
-[profile.dev]
+# If one wants the 'dev' profile, but with "panic = abort" semantics, they
+# should opt in with this profile.  Unwinding is required by PHD and
+# should_abort cargo tests, and so remains the default for the 'dev' profile.
+[profile.dev-abort]
+inherits = "dev"
 panic = "abort"
+
+# Building for 'release' implies running on a real illumos system, where we
+# certainly want (rust) panics to cause an immediate abort and coredump.
 [profile.release]
 panic = "abort"
 
-# The PHD test runner needs to use unwinding to catch panics that occur during
-# tests (e.g. due to a failed `assert!` in a test case).
-[profile.phd]
-inherits = "dev"
-panic = "unwind"
-
 [workspace.dependencies]
 # Internal crates
 bhyve_api = { path = "crates/bhyve-api" }

phd-tests/README.md

@@ -34,15 +34,15 @@ against them.
 
 To build:
 
-`cargo build -p phd-runner --profile=phd`
+`cargo build -p phd-runner
 
-Note that `--profile=phd` is required to allow the runner to catch assertions
-from test cases (the default Propolis profile aborts on panic instead of
-unwinding).
+PHD requires the unwinding of stacks in order to properly catch assertions in
+test cases, so building with a profile which sets `panic = "abort"` is not
+supported.  This precludes the use of the `release` or `dev-abort` profiles.
 
 To run:
 
-`pfexec cargo run -p phd-runner --profile=phd -- [OPTIONS]`
+`pfexec cargo run -p phd-runner -- [OPTIONS]`
 
 Running under pfexec is required to allow PHD to ensure the host system is
 correctly configured to run live migration tests.

phd-tests/quickstart.sh

@@ -10,7 +10,7 @@ if [ ! -d "$PHD_QUICKSTART_DIR" ]; then
 	mkdir $PHD_QUICKSTART_DIR
 fi
 
-pfexec cargo run --profile=phd -p phd-runner -- \
+pfexec cargo run -p phd-runner -- \
 	run \
 	--artifact-toml-path ./artifacts.toml \
 	--tmp-directory $PHD_QUICKSTART_DIR \