* Laurent Vivier (lviv...@redhat.com) 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
>
> 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 <lviv...@redhat.com>
A mix of comments:
a) As a boolean, this should be a MigrationCapability rather than a
parameter
b) We already have a pause-before-switchover capability for a pause
that happens later in the flow; so this would be something like
pause-after-unplug
c) Is this really the right answer? Could this be done a different
way by doing the unplugs using (a possibly new) qmp command - so
that you can explicitly trigger the unplug prior to the migration?
Dave
> ---
> 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);
> }
> --
> 2.31.1
>
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
