nvme_driver: no longer drops create io queue request when inspect is called + test

[gurasinghMS]

Previously the run loop for the DriverWorkerTask would abandon work in progress when TaskControl cancelled the task to service an inspect call. This would cause the create_nvme_io_queue span to exit before the queues were properly set up. With this change the task should only be cancelled when waiting on next() . However, if active work is being done this won't abandon current work in order to service an inspect call.
There are downsides to this approach, however. If a device is being really slow to process a create command, we won't be able to service saves AND (with this change) won't be able to inspect system state either.

Added a vmm_test as well that verifies such behavior by calling inspect on nvme driver with a delayed CREATE_IO_COMPLETION_QUEUE and then verifies that save was not serviced before create completion.

Why inspect causes this task to be canceled/stopped?

Looking at the implementation of TaskControl / StopTask / StopTaskInner, we see that TaskControl, once started, invokes run() on the underlying task type (in our case DriverWorkerTask) and passes in two parameters:

• Task state (provided by the task user via task.insert())
• StopTask

Focusing on StopTask::until_stopped(), we see that it accepts a future that can be stopped using TaskControl::stop(). This function works by polling two futures:

• The future provided to until_stopped, which in our case is an async loop (or whatever future the user wants to run)
• StopTaskInner

Zooming in further to StopTaskInner::poll, we can see that it can return Poll::Ready() for two reasons:

• !shared.calls.is_empty() ← This becomes important
• shared.stop == true

The shared calls are important because they allow TaskControl to update existing task state while the task is running. This is done by updating the Vec<> of function calls and invoking the waker associated with the StopTask, which is polling both the user function and StopTaskInner. This is, in my understanding, the same waker that the user function f would invoke when more work is available.

When this happens, StopTaskInner::poll returns Poll::Ready(Err(Cancelled)), at which point the entire task exits and yields execution back to the internal components of TaskControl. TaskControl then executes all the calls accumulated in shared.calls and invokes run() on the underlying task again, provided the task was not stopped by the user.

The side effect of all of this is that the future the user was awaiting inside until_stopped gets discarded. Crucially, this is also the same pathway leveraged by Inspect and InspectMut to inspect the underlying task. As a result, when the nvme_driver is inspected, it causes the task to be cancelled and re-run. In our case, this would discard the create_nvme_io_queue future being awaited, putting the system into a bad state.

A simple fix here is to only wrap the recv.next() in until_stopped, which is also the pattern used elsewhere in the codebase. The downside here is that we can't stop stuck create_nvme_io_queue or save commands anymore ... but we were not doing that previously either. If either of those are stuck, we need to tear everything down anyways.

The relationships between these objects can be mapped out as:

[Copilot]

Pull request overview

This PR addresses an NVMe driver servicing bug where TaskControl cancellation (triggered by inspect) could abandon in-progress work (notably create_io_queue), and adds a VMM integration test intended to validate the corrected behavior during servicing/save/restore.

Changes:

• Adjust DriverWorkerTask’s run loop to only be cancellable while awaiting the next command, avoiding mid-request cancellation.
• Add a new OpenHCL servicing VMM test that issues inspect calls during a delayed IO-queue-creation fault and then performs save/restore validation.
• Add the inspect crate as a dependency for vmm_tests.

Reviewed changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated 4 comments.

File	Description
vm/devices/storage/disk_nvme/nvme_driver/src/driver.rs	Moves the until_stopped cancellation boundary to only cover recv.next() in the worker loop.
vmm_tests/vmm_tests/tests/tests/multiarch/openhcl_servicing.rs	Adds a new integration test exercising inspect during delayed IO queue creation + subsequent save/restore assertions.
vmm_tests/vmm_tests/Cargo.toml	Adds inspect dependency required by the new test’s inspect-node matching.
Cargo.lock	Records the new inspect dependency in the lockfile.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 4 changed files in this pull request and generated 4 comments.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 4 changed files in this pull request and generated 1 comment.

[mattkur]

I think this is strictly better behavior, but I'm a little concerned about not being able to inspect while something is wedged.

[github-actions]

1 Petri tests failed (0 unstable)

[gurasinghMS]

I think this is strictly better behavior, but I'm a little concerned about not being able to inspect while something is wedged.

I agree with ya! I was thinking about this and to keep inspect availabillity high we could save the future and when run() is called we either await on the saved future or try to get more work using next(). Either way we always keep track of what we are working on and continue working even if the task was cancelled and restarted. Ofcourse this is easier said than done.

[mattkur]

Do you mind elaborating further on why inspect causes this task to be canceled/stopped?

[gurasinghMS]

Do you mind elaborating further on why inspect causes this task to be canceled/stopped?

Added to the PR description.

[gurasinghMS]

Adding a doc comment to TaskControl in #3414

[mattkur]

Thanks for this! I think you identified a similar problem in the ioq as well. Mind taking that fix here as well?