why do we need to reset device migration state at device_reset()?

[Hooollin]

I'm currently debugging a migration flow with SPDK and libvfio-user using VFIO Migration V1 and noticed that device_state is explicitly updated in the code path; could someone clarify the design rationale behind this update?

static int
device_reset(vfu_ctx_t *vfu_ctx, vfu_reset_type_t reason)
{
    int ret;

    ret = call_reset_cb(vfu_ctx, reason);
    if (ret < 0) {
        return ret;
    }
    
    // reset device migration state here
    if (vfu_ctx->migration != NULL) {
        return handle_device_state(vfu_ctx, vfu_ctx->migration,
                                   VFIO_DEVICE_STATE_RUNNING, false);
    }
    return 0;
}

In my testing, this operation appears to inadvertently clear the high bit of the SAVING state, which risks disrupting the migration state machine, so I'm wondering whether this is intentional per the V1 spec (and if so, how the flag should be properly preserved) or if it might be an oversight that requires masking/workaround.

Any insights into the intended behavior or references to relevant documentation would be greatly appreciated.

[tmakatos]

Any insights into the intended behavior or references to relevant documentation would be greatly appreciated.

Sadly I didn't document this well enough, but I think I did it this way because that's what the kernel did (and I belive it's what QEMU expected). If it's in SAVING state the guest isn't running so it can't be the guest triggering it, most likely it's the VMM resetting the device after a failed migration. Have you actually seen an error because of this?

I'm currently debugging a migration flow with SPDK and libvfio-user using VFIO Migration V1

How come? I presume you know this is eventually going away?

[tmakatos]

The guest could also be performing a reboot, which would trigger device_reset().

If the guest is in SAVING state it's stopped, it can't initiate any such actions, so the only component that could reboot the guest is the higher-level control plane, and since there's an outstanding migration this operation should be queued. Have you observed an actual error because of this?

[Hooollin]

Sorry for the confusion. What I actually meant is that if the guest reboots during the pre-copy phase, it triggers reset_device(), which attempts to clear the VFIO_DEVICE_STATE_SAVING flag.

Looking at the state machine:

    [VFIO_DEVICE_STATE_RUNNING | VFIO_DEVICE_STATE_SAVING] = {
        .state =
            (1 << VFIO_DEVICE_STATE_STOP) |
            (1 << VFIO_DEVICE_STATE_SAVING) |
            (1 << VFIO_DEVICE_STATE_RUNNING | VFIO_DEVICE_STATE_SAVING) |
            (1 << VFIO_DEVICE_STATE_ERROR),
        .name = "pre-copy"
    },

Based on the state machine definition, transitioning from pre-copy back to running shouldn't be allowed. However, in my test environment, I actually observed the following sequence:

RUNNING (1) → PRE-COPY (3) → after reset_device() → RUNNING (1) → STOP (0) → STOP-AND-COPY (2).

[tmakatos]

Based on the state machine definition, transitioning from pre-copy back to running shouldn't be allowed.

It shouldn't be allowed by the VMM.

[Hooollin]

I believe the transition to STOP occurs because the reset clears the VFIO_DEVICE_STATE_SAVING flag, which causes QEMU to trigger a state transition from RUNNING to STOP (whereas it originally intended to transition from pre-copy to saving).

[Hooollin]

Based on the state machine definition, transitioning from pre-copy back to running shouldn't be allowed.

It shouldn't be allowed by the VMM.

To clarify: does this mean QEMU is expected to prevent device_reset() from being triggered when the guest OS reboots during migration? Or should the reset be handled in a way that respects the current pre-copy state?

[tmakatos]

I believe the transition to STOP occurs because the reset clears the VFIO_DEVICE_STATE_SAVING flag, which causes QEMU to trigger a state transition from RUNNING to STOP

Ah sorry, I didn't pay enough attention to what happens after reset_device() → RUNNING (1).

To clarify: does this mean QEMU is expected to prevent device_reset() from being triggered when the guest OS reboots during migration? Or should the reset be handled in a way that respects the current pre-copy state?

If the guest reboots then the device state is reset, so any pre-copy state that has been captured so far must be discarded. So the current transition looks fine, however what the actual implications of this aren't clear. What's the problem this causes?

[Hooollin]

From my understanding, QEMU follows a fixed live migration workflow:
a) Migration begins with device_state = RUNNING (1).
b) The pre-copy phase is triggered.
c) QEMU iteratively tracks and copies dirty pages.
d) The stop-and-copy phase is triggered.
e) QEMU writes VFIO_DEVICE_STATE_SAVING (2) to device_state and fetches pending data.

If reset_device() is invoked after step (b), SPDK discards any pre-captured migration state and resets device_state back to RUNNING (1). However, QEMU's internal migration FSM remains unaware of this asynchronous reset and still assumes the device is in PRE_COPY (3).

When QEMU reaches step (d), it reads device_state as RUNNING (1) from the migration register. Following its internal logic, it masks out the low-order RUNNING flag bit and writes STOPPED (0) to device_state (note that QEMU actually expects to transition directly from PRE_COPY (3) to STOP_AND_COPY (2)).

When QEMU reaches step (e), it attempts to write STOP_AND_COPY (2). SPDK validates the state transition, finds old_state is STOPPED (0), and rejects the STOPPED → STOP_AND_COPY transition as invalid per the state machine specification. This causes the migration to abort and roll back.

Root Cause: The issue stems from a state desynchronization between QEMU's migration controller and SPDK's device state machine after an in-flight reset. QEMU assumes a linear state progression, while the reset forces the device back to RUNNING, breaking the expected transition chain and triggering a migration failure.

[tmakatos]

Thanks for explaining the problem in such great detail, it's now obvious what's broken. My earlier statement:

If the guest reboots then the device state is reset, so any pre-copy state that has been captured so far must be discarded.

should be corrected to:

any pre-copy state that has been captured so far must be discarded but the migration state should remain the same".

@Hooollin would you be interested in fixing this?

[Hooollin]

Thanks for the clarification! That makes sense, I’d be happy to take this on.

[jlevon]

some features that depend on the V1 interface.
The implementation or the design? If there is stuff missing from the v2 spec, could you please file separate issue(s) for that? Thanks.

[Hooollin]

We haven't modified any code. We're intentionally continuing to use the V1 migration code as-is.