On 11/08/2025 19:34, Cédric Le Goater wrote:
External email: Use caution opening links or attachments
Hello,
+ Avihai
On 8/11/25 18:02, Kunkun Jiang wrote:
Hi all,
While testing VFIO migration, I encountered an corner scenario case:
VFIO migration will not be aborted when the vfio device of dst-vm fails to
transition from RESUMING to RUNNING state in vfio_vmstate_change.
I saw the comments in the vfio_vmstate_change but I don't understand why no
action is taken for this situation.
There is error handling in vfio_vmstate_change() :
/*
* Migration should be aborted in this case, but vm_state_notify()
* currently does not support reporting failures.
*/
migration_file_set_error(ret, local_err);
Hmm, I think this only sets the error on src. On dst we don't have
MigrationState->to_dst_file, so we end up just reporting the error.
But even if we did set it, no one is checking if there is a migration error
after vm_start() is called in process_incoming_migration_bh().
Allowing the live migration process to continue could cause unrecoverable
damage to the VM.
What do you mean by unrecoverable damage to the VM?
If RESUMING->RUNNING transition fails, would a VFIO reset recover the device
and allow the VM to continue operation with damage limited only to the VFIO device?
In this case, can we directly exit the dst-vm? Through the return-path
mechanism, the src-vm can continue to run.
Looking forward to your reply.
The straightforward solution, as you suggested, is to exit dst upon error in
RESUMING->RUNNING transition and notify about it to src through the return-path.
However, I am not sure if failing the migration after vm_start() on dst is a
bit late (as we start vCPUs and do migration_block_activate, etc.).
But I can think of another way to solve this, hopefully simpler.
According to VFIO migration uAPI [1]:
* RESUMING -> STOP
* Leaving RESUMING terminates a data transfer session and indicates the
* device should complete processing of the data delivered by write(). The
* kernel migration driver should complete the incorporation of data written
* to the data transfer FD into the device internal state and perform
* final validity and consistency checking of the new device state. If the
* user provided data is found to be incomplete, inconsistent, or otherwise
* invalid, the migration driver must fail the SET_STATE ioctl and
* optionally go to the ERROR state as described below.
So, IIUC, we can add an explicit RESUMING->STOP transition [2] after the device
config is loaded (which is the last data the device is expected to receive).
If this transition fails, it means something was wrong with migration, and we
can send src an error msg via return-path (and not continue to vm_start()).
Maybe this approach is less complicated than the first one, and it will also
work if src VM was paused prior migration.
I already tested some POC and it seems to be working (at least with an artificial
error i injected in RESUMING->STOP transition).
Kunkun, can you apply the following diff [3] and check if this solves the issue?
And in general, what do you think? Should we go with this approach or do you
have other ideas?
Thanks.
[1]
https://elixir.bootlin.com/linux/v6.16/source/include/uapi/linux/vfio.h#L1099
[2] Today RESUMING->STOP is done implicitly by the VFIO driver as part of
RESUMING->RUNNING transition.
[3]
diff --git a/hw/vfio/migration-multifd.c b/hw/vfio/migration-multifd.c
index e4785031a7..66f8461f02 100644
--- a/hw/vfio/migration-multifd.c
+++ b/hw/vfio/migration-multifd.c
@@ -267,6 +267,12 @@ static bool vfio_load_bufs_thread_load_config(VFIODevice
*vbasedev,
ret = vfio_load_device_config_state(f_in, vbasedev);
bql_unlock();
+ ret = vfio_migration_set_state(vbasedev, VFIO_DEVICE_STATE_STOP,
+ VFIO_DEVICE_STATE_ERROR, errp);
+ if (ret) {
+ return false;
+ }
+
if (ret < 0) {
error_setg(errp, "%s: vfio_load_device_config_state() failed: %d",
vbasedev->name, ret);
diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
index 4c06e3db93..a707d17a5b 100644
--- a/hw/vfio/migration.c
+++ b/hw/vfio/migration.c
@@ -737,6 +737,8 @@ static int vfio_load_state(QEMUFile *f, void *opaque, int
version_id)
switch (data) {
case VFIO_MIG_FLAG_DEV_CONFIG_STATE:
{
+ Error *local_err = NULL;
+
if (vfio_multifd_transfer_enabled(vbasedev)) {
error_report("%s: got DEV_CONFIG_STATE in main migration "
"channel but doing multifd transfer",
@@ -744,7 +746,19 @@ static int vfio_load_state(QEMUFile *f, void *opaque, int
version_id)
return -EINVAL;
}
- return vfio_load_device_config_state(f, opaque);
+ ret = vfio_load_device_config_state(f, opaque);
+ if (ret) {
+ return ret;
+ }
+
+ ret = vfio_migration_set_state_or_reset(
+ vbasedev, VFIO_DEVICE_STATE_STOP, &local_err);
+ if (ret) {
+ error_report_err(local_err);
+ return ret;
+ }
+
+ return 0;
}
case VFIO_MIG_FLAG_DEV_SETUP_STATE:
{
diff --git a/migration/migration.c b/migration/migration.c
index 10c216d25d..fd498c864d 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -91,6 +91,7 @@ enum mig_rp_message_type {
MIG_RP_MSG_RECV_BITMAP, /* send recved_bitmap back to source */
MIG_RP_MSG_RESUME_ACK, /* tell source that we are ready to resume */
MIG_RP_MSG_SWITCHOVER_ACK, /* Tell source it's OK to do switchover */
+ MIG_RP_MSG_ERROR, /* Tell source that destination encountered an error */
MIG_RP_MSG_MAX
};
@@ -884,6 +885,11 @@ process_incoming_migration_co(void *opaque)
ret = qemu_loadvm_state(mis->from_src_file);
mis->loadvm_co = NULL;
+ if (ret) {
+ migrate_send_rp_error(mis);
+ error_report("SENT RP ERROR");
+ }
+
trace_vmstate_downtime_checkpoint("dst-precopy-loadvm-completed");
ps = postcopy_state_get();
@@ -1126,6 +1132,11 @@ bool migration_has_all_channels(void)
return true;
}
+int migrate_send_rp_error(MigrationIncomingState *mis)
+{
+ return migrate_send_rp_message(mis, MIG_RP_MSG_ERROR, 0, NULL);
+}
+
int migrate_send_rp_switchover_ack(MigrationIncomingState *mis)
{
return migrate_send_rp_message(mis, MIG_RP_MSG_SWITCHOVER_ACK, 0, NULL);
@@ -2614,6 +2625,10 @@ static void *source_return_path_thread(void *opaque)
trace_source_return_path_thread_switchover_acked();
break;
+ case MIG_RP_MSG_ERROR:
+ error_setg(&err, "DST indicated error");
+ goto out;
