lib/server: allow migration of in-memory block backends #826
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
pfmooney
approved these changes
Dec 12, 2024
iximeow
approved these changes
Dec 12, 2024
gjcolombo
force-pushed
the
gjcolombo/spec-key
branch
from
d6dd7c7 to
9a291d3
Compare
This test launches a VM with a disk backed by an in-memory backend, modifies the disk, migrates the VM, and ensures that the changes are still present on the migration target. Tune a few things up along the way: - Add a missing `with_context` in the artifact store. - Add an affordance to set the minimum size of a PHD FAT volume (previously the size was determined solely by the number of files a test added to the volume). - Tweak the Ubuntu 22.04 guest adapter's startup sequence so that tests run as root (so that they don't need to elevate with `sudo` to run commands that Just Work on other guest OSes that run as a superuser).
gjcolombo
force-pushed
the
gjcolombo/migrate-in-memory-backends
branch
from
ef7998c to
488bc6a
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
(N.B. Stacked on #816.)
Register in-memory block backends for lifecycle callouts and teach them to export and import their contents.
Truthfully, this may be somewhat overengineered. Today, in-memory storage backends are only used for the cloud-init drive, which Nexus always marks as read-only. Since the guest can never change the drive's contents, and #807 ensures that propolis-server migration targets always inherit their initial in-memory backend contents from their sources, it is technically legal for the backend just to return
Migrator::Emptyif it's read-only (and returnMigrator::NonMigratableif it's writable). However, this makes the Propolis lib assume things about the way propolis-server drives migrations, which seemed like a layering violation, so this change just does what's needed to migrate the backend's current data in all cases.One downside of this approach is that the serialized backend contents show up in migration logs (as part of the giant blob logged in the destination's
device_statefunction). It may make sense to adjust this logging so that the serialized device payloads are removed from this log message. See #825 for more thoughts on this. I think this change would be large enough that if pursued it should be in its own PR.Add a PHD test for the new migration behavior. This test works on Debian 11, Ubuntu 22.04, and Windows Server 2022. It's skipped on Alpine guests because (for reasons I don't yet understand) the test's use of
iflag=directon that OS isn't sufficient to get reads to hit the actual in-memory backend.Finally, tweak a few PHD bits:
Tests: new PHD variation with Alpine, Debian 11, Ubuntu, and WS2022 guests.
Fixes #301.