On 9/30/21 1:09 PM, Laurent Vivier wrote:

If we want to save a snapshot of a VM to a file, we used to follow the following steps:

1- stop the VM: (qemu) stop

2- migrate the VM to a file: (qemu) migrate "exec:cat > snapshot"

3- resume the VM: (qemu) cont

After that we can restore the snapshot with: qemu-system-x86_64 ... -incoming "exec:cat snapshot" (qemu) cont

This is the basics of what libvirt does for a snapshot, and steps 1+2 are what it does for a "managedsave" (where it saves the snapshot to disk and then terminates the qemu process, for later re-animation). In those cases, it seems like this new parameter could work for us - instead of explicitly pausing the guest prior to migrating it to disk, we would set this new parameter to on, then directly migrate-to-disk (relying on qemu to do the pause). Care will need to be taken to assure that error recovery behaves the same though. There are a couple of cases when libvirt apparently *doesn't* pause the guest during the migrate-to-disk, both having to do with saving a coredump of the guest. Since I really have no idea of how common/important that is (or even if my assessment of the code is correct), I'm Cc'ing this patch to libvir-list to make sure it catches the attention of someone who knows the answers and implications.

But when failover is configured, it doesn't work anymore.

As the failover needs to ask the guest OS to unplug the card the machine cannot be paused.

This patch introduces a new migration parameter, "pause-vm", that asks the migration to pause the VM during the migration startup phase after the the card is unplugged.

Once the migration is done, we only need to resume the VM with "cont" and the card is plugged back:

1- set the parameter: (qemu) migrate_set_parameter pause-vm on

2- migrate the VM to a file: (qemu) migrate "exec:cat > snapshot"

The primary failover card (VFIO) is unplugged and the VM is paused.

3- resume the VM: (qemu) cont

The VM restarts and the primary failover card is plugged back

The VM state sent in the migration stream is "paused", it means when the snapshot is loaded or if the stream is sent to a destination QEMU, the VM needs to be resumed manually.

Signed-off-by: Laurent Vivier <lvivier@redhat.com> --- qapi/migration.json | 20 +++++++++++++++--- include/hw/virtio/virtio-net.h | 1 + hw/net/virtio-net.c | 33 ++++++++++++++++++++++++++++++ migration/migration.c | 37 +++++++++++++++++++++++++++++++++- monitor/hmp-cmds.c | 8 ++++++++ 5 files changed, 95 insertions(+), 4 deletions(-)

diff --git a/qapi/migration.json b/qapi/migration.json index 88f07baedd06..86284d96ad2a 100644 --- a/qapi/migration.json +++ b/qapi/migration.json @@ -743,6 +743,10 @@ # block device name if there is one, and to their node name # otherwise. (Since 5.2) # +# @pause-vm: Pause the virtual machine before doing the migration. +# This allows QEMU to unplug a card before doing the +# migration as it depends on the guest kernel. (since 6.2) +# # Since: 2.4 ## { 'enum': 'MigrationParameter', @@ -758,7 +762,7 @@ 'xbzrle-cache-size', 'max-postcopy-bandwidth', 'max-cpu-throttle', 'multifd-compression', 'multifd-zlib-level' ,'multifd-zstd-level', - 'block-bitmap-mapping' ] } + 'block-bitmap-mapping', 'pause-vm' ] }

## # @MigrateSetParameters: @@ -903,6 +907,10 @@ # block device name if there is one, and to their node name # otherwise. (Since 5.2) # +# @pause-vm: Pause the virtual machine before doing the migration. +# This allows QEMU to unplug a card before doing the +# migration as it depends on the guest kernel. (since 6.2) +# # Since: 2.4 ## # TODO either fuse back into MigrationParameters, or make @@ -934,7 +942,8 @@ '*multifd-compression': 'MultiFDCompression', '*multifd-zlib-level': 'uint8', '*multifd-zstd-level': 'uint8', - '*block-bitmap-mapping': [ 'BitmapMigrationNodeAlias' ] } } + '*block-bitmap-mapping': [ 'BitmapMigrationNodeAlias' ], + '*pause-vm': 'bool' } }

## # @migrate-set-parameters: @@ -1099,6 +1108,10 @@ # block device name if there is one, and to their node name # otherwise. (Since 5.2) # +# @pause-vm: Pause the virtual machine before doing the migration. +# This allows QEMU to unplug a card before doing the +# migration as it depends on the guest kernel. (since 6.2) +# # Since: 2.4 ## { 'struct': 'MigrationParameters', @@ -1128,7 +1141,8 @@ '*multifd-compression': 'MultiFDCompression', '*multifd-zlib-level': 'uint8', '*multifd-zstd-level': 'uint8', - '*block-bitmap-mapping': [ 'BitmapMigrationNodeAlias' ] } } + '*block-bitmap-mapping': [ 'BitmapMigrationNodeAlias' ], + '*pause-vm': 'bool' } }

## # @query-migrate-parameters: diff --git a/include/hw/virtio/virtio-net.h b/include/hw/virtio/virtio-net.h index 824a69c23f06..a6c186e28b45 100644 --- a/include/hw/virtio/virtio-net.h +++ b/include/hw/virtio/virtio-net.h @@ -210,6 +210,7 @@ struct VirtIONet { bool failover; DeviceListener primary_listener; Notifier migration_state; + VMChangeStateEntry *vm_state; VirtioNetRssData rss_data; struct NetRxPkt *rx_pkt; struct EBPFRSSContext ebpf_rss; diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c index f205331dcf8c..c83364eac47b 100644 --- a/hw/net/virtio-net.c +++ b/hw/net/virtio-net.c @@ -39,6 +39,7 @@ #include "migration/misc.h" #include "standard-headers/linux/ethtool.h" #include "sysemu/sysemu.h" +#include "sysemu/runstate.h" #include "trace.h" #include "monitor/qdev.h" #include "hw/pci/pci.h" @@ -3303,6 +3304,35 @@ static void virtio_net_migration_state_notifier(Notifier *notifier, void *data) virtio_net_handle_migration_primary(n, s); }

+static void virtio_net_failover_restart_cb(void *opaque, bool running, + RunState state) +{ + DeviceState *dev; + VirtIONet *n = opaque; + Error *err = NULL; + PCIDevice *pdev; + + if (!running) { + return; + } + + dev = failover_find_primary_device(n); + if (!dev) { + return; + } + + pdev = PCI_DEVICE(dev); + if (!pdev->partially_hotplugged) { + return; + } + + if (!failover_replug_primary(n, dev, &err)) { + if (err) { + error_report_err(err); + } + } +} + static bool failover_hide_primary_device(DeviceListener *listener, QemuOpts *device_opts) { @@ -3360,6 +3390,8 @@ static void virtio_net_device_realize(DeviceState *dev, Error **errp) device_listener_register(&n->primary_listener); n->migration_state.notify = virtio_net_migration_state_notifier; add_migration_state_change_notifier(&n->migration_state); + n->vm_state = qemu_add_vm_change_state_handler( + virtio_net_failover_restart_cb, n); n->host_features |= (1ULL << VIRTIO_NET_F_STANDBY); }

@@ -3508,6 +3540,7 @@ static void virtio_net_device_unrealize(DeviceState *dev) if (n->failover) { device_listener_unregister(&n->primary_listener); remove_migration_state_change_notifier(&n->migration_state); + qemu_del_vm_change_state_handler(n->vm_state); }

max_queues = n->multiqueue ? n->max_queues : 1; diff --git a/migration/migration.c b/migration/migration.c index bb909781b7f5..9c96986d4abf 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -901,6 +901,9 @@ MigrationParameters *qmp_query_migrate_parameters(Error **errp) s->parameters.block_bitmap_mapping); }

+ params->has_pause_vm = true; + params->pause_vm = s->parameters.pause_vm; + return params; }

@@ -1549,6 +1552,11 @@ static void migrate_params_test_apply(MigrateSetParameters *params, dest->has_block_bitmap_mapping = true; dest->block_bitmap_mapping = params->block_bitmap_mapping; } + + if (params->has_pause_vm) { + dest->has_pause_vm = true; + dest->pause_vm = params->pause_vm; + } }

static void migrate_params_apply(MigrateSetParameters *params, Error **errp) @@ -1671,6 +1679,10 @@ static void migrate_params_apply(MigrateSetParameters *params, Error **errp) QAPI_CLONE(BitmapMigrationNodeAliasList, params->block_bitmap_mapping); } + + if (params->has_pause_vm) { + s->parameters.pause_vm = params->pause_vm; + } }

void qmp_migrate_set_parameters(MigrateSetParameters *params, Error **errp) @@ -1718,6 +1730,12 @@ void qmp_migrate_start_postcopy(Error **errp) " started"); return; } + + if (s->parameters.pause_vm) { + error_setg(errp, "Postcopy cannot be started if pause-vm is on"); + return; + } + /* * we don't error if migration has finished since that would be racy * with issuing this command. @@ -3734,13 +3752,27 @@ static void qemu_savevm_wait_unplug(MigrationState *s, int old_state, "failure"); } } - migrate_set_state(&s->state, MIGRATION_STATUS_WAIT_UNPLUG, new_state); } else { migrate_set_state(&s->state, old_state, new_state); } }

+/* stop the VM before starting the migration but after device unplug */ +static void pause_vm_after_unplug(MigrationState *s) +{ + if (s->parameters.pause_vm) { + qemu_mutex_lock_iothread(); + qemu_system_wakeup_request(QEMU_WAKEUP_REASON_OTHER, NULL); + s->vm_was_running = runstate_is_running(); + if (vm_stop_force_state(RUN_STATE_PAUSED)) { + migrate_set_state(&s->state, MIGRATION_STATUS_ACTIVE, + MIGRATION_STATUS_FAILED); + } + qemu_mutex_unlock_iothread(); + } +} + /* * Master migration thread on the source VM. * It drives the migration and pumps the data down the outgoing channel. @@ -3790,6 +3822,8 @@ static void *migration_thread(void *opaque) qemu_savevm_wait_unplug(s, MIGRATION_STATUS_SETUP, MIGRATION_STATUS_ACTIVE);

+ pause_vm_after_unplug(s); + s->setup_time = qemu_clock_get_ms(QEMU_CLOCK_HOST) - setup_start;

trace_migration_thread_setup_complete(); @@ -4265,6 +4299,7 @@ static void migration_instance_init(Object *obj) params->has_announce_max = true; params->has_announce_rounds = true; params->has_announce_step = true; + params->has_pause_vm = true;

qemu_sem_init(&ms->postcopy_pause_sem, 0); qemu_sem_init(&ms->postcopy_pause_rp_sem, 0); diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c index b5e71d9e6f52..71bc8c15335b 100644 --- a/monitor/hmp-cmds.c +++ b/monitor/hmp-cmds.c @@ -513,6 +513,10 @@ void hmp_info_migrate_parameters(Monitor *mon, const QDict *qdict) } } } + + monitor_printf(mon, "%s: %s\n", + MigrationParameter_str(MIGRATION_PARAMETER_PAUSE_VM), + params->pause_vm ? "on" : "off"); }

qapi_free_MigrationParameters(params); @@ -1399,6 +1403,10 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict) error_setg(&err, "The block-bitmap-mapping parameter can only be set " "through QMP"); break; + case MIGRATION_PARAMETER_PAUSE_VM: + p->has_pause_vm = true; + visit_type_bool(v, param, &p->pause_vm, &err); + break; default: assert(0); }