CI: https://gitlab.com/peterx/qemu/-/pipelines/1577280033

(note: it's a pipeline of two patchsets, to save CI credits and time)

v1: https://lore.kernel.org/r/20241204005138.702289-1-peterx@redhat.com

This is v2 of the series, removing RFC tag, because my goal is to have them

(or some newer version) merged.

The major change is I merged last three patches, and did quite some changes

here and there, to make sure the global disk activation status is always

consistent. The whole idea is still the same. I say changelog won't help.

I also temporarily dropped Fabiano's ping-pong test cases to avoid

different versions floating on the list (as I know a new version is coming

at some point. Fabiano: you're taking over the 10.0 pulls, so I assume

you're aware so there's no concern on order of merges). I'll review the

test cases separately when they're ready, but this series is still tested

with that pingpong test and it keeps working.

qmp/cont: Only activate disks if migration completed

This patch proposes a flag to maintain disk activation status globally. It

mostly rewrites disk activation mgmt for QEMU, including COLO and QMP

command xen_save_devices_state.

Backgrounds

===========

We have two problems on disk activations, one resolved, one not.

Problem 1: disk activation recover (for switchover interruptions)

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

+ trace_migration_block_activation("inactive");

+

@@ -XXX,XX +XXX,XX @@ void qmp_migrate_incoming(const char *uri, bool has_channels,

return;

}

+ /*

+ * Newly setup incoming QEMU. Mark the block active state to reflect

+ * that the src currently owns the disks.

+ */

+ migration_block_active_setup(false);

+

once = false;

}

@@ -XXX,XX +XXX,XX @@ static int postcopy_start(MigrationState *ms, Error **errp)

QIOChannelBuffer *bioc;

QEMUFile *fb;

uint64_t bandwidth = migrate_max_postcopy_bandwidth();

- bool restart_block = false;

int cur_state = MIGRATION_STATUS_ACTIVE;

if (migrate_postcopy_preempt()) {

@@ -XXX,XX +XXX,XX @@ static int postcopy_start(MigrationState *ms, Error **errp)

goto fail;

}

- ret = bdrv_inactivate_all();

- if (ret < 0) {

- error_setg_errno(errp, -ret, "%s: Failed in bdrv_inactivate_all()",

- __func__);

+ if (!migration_block_inactivate()) {

+ error_setg(errp, "%s: Failed in bdrv_inactivate_all()", __func__);

goto fail;

}

- restart_block = true;

/*

* Cause any non-postcopiable, but iterative devices to

@@ -XXX,XX +XXX,XX @@ static int postcopy_start(MigrationState *ms, Error **errp)

goto fail_closefb;

}

- restart_block = false;

-

/* Now send that blob */

if (qemu_savevm_send_packaged(ms->to_dst_file, bioc->data, bioc->usage)) {

error_setg(errp, "%s: Failed to send packaged data", __func__);

@@ -XXX,XX +XXX,XX @@ fail_closefb:

fail:

migrate_set_state(&ms->state, MIGRATION_STATUS_POSTCOPY_ACTIVE,

MIGRATION_STATUS_FAILED);

- if (restart_block) {

- /* A failure happened early enough that we know the destination hasn't

- * accessed block devices, so we're safe to recover.

- */

- Error *local_err = NULL;

-

- bdrv_activate_all(&local_err);

- if (local_err) {

- error_report_err(local_err);

- }

- }

+ migration_block_activate(NULL);

migration_call_notifiers(ms, MIG_EVENT_PRECOPY_FAILED, NULL);

bql_unlock();

return -1;

}

@@ -XXX,XX +XXX,XX @@ static int loadvm_postcopy_handle_listen(MigrationIncomingState *mis)

static void loadvm_postcopy_handle_run_bh(void *opaque)

{

- Error *local_err = NULL;

MigrationIncomingState *mis = opaque;

trace_vmstate_downtime_checkpoint("dst-postcopy-bh-enter");

@@ -XXX,XX +XXX,XX @@ static void loadvm_postcopy_handle_run_bh(void *opaque)

* Make sure all file formats throw away their mutable metadata.

* If we get an error here, just don't restart the VM yet.

*/

- bdrv_activate_all(&local_err);

+ bool success = migration_block_activate(NULL);

+

trace_vmstate_downtime_checkpoint("dst-postcopy-bh-cache-invalidated");

- if (local_err) {

- error_report_err(local_err);

- local_err = NULL;

- } else {

+

+ if (success) {

vm_start();

}

} else {

@@ -XXX,XX +XXX,XX @@ void qmp_xen_save_devices_state(const char *filename, bool has_live, bool live,

* side of the migration take control of the images.

*/

if (live && !saved_vm_running) {

- ret = bdrv_inactivate_all();

- if (ret) {

- error_setg(errp, "%s: bdrv_inactivate_all() failed (%d)",

- __func__, ret);

- }

+ migration_block_inactivate();

}

}

diff --git a/monitor/qmp-cmds.c b/monitor/qmp-cmds.c

index XXXXXXX..XXXXXXX 100644

--- a/monitor/qmp-cmds.c

+++ b/monitor/qmp-cmds.c

@@ -XXX,XX +XXX,XX @@

#include "qapi/type-helpers.h"

#include "hw/mem/memory-device.h"

#include "hw/intc/intc.h"

+#include "migration/misc.h"

NameInfo *qmp_query_name(Error **errp)

{

@@ -XXX,XX +XXX,XX @@ void qmp_cont(Error **errp)

* Continuing after completed migration. Images have been

* inactivated to allow the destination to take control. Need to

* get control back now.

- *

- * If there are no inactive block nodes (e.g. because the VM was

- * just paused rather than completing a migration),

- * bdrv_inactivate_all() simply doesn't do anything.

*/

- bdrv_activate_all(&local_err);

- if (local_err) {

+ if (!migration_block_activate(&local_err)) {

error_propagate(errp, local_err);

return;

}

diff --git a/migration/meson.build b/migration/meson.build

index XXXXXXX..XXXXXXX 100644

--- a/migration/meson.build

+++ b/migration/meson.build

@@ -XXX,XX +XXX,XX @@ migration_files = files(

system_ss.add(files(

'block-dirty-bitmap.c',

+ 'block-active.c',

'channel.c',

'channel-block.c',

'cpu-throttle.c',

diff --git a/migration/trace-events b/migration/trace-events

index XXXXXXX..XXXXXXX 100644

--- a/migration/trace-events

+++ b/migration/trace-events

@@ -XXX,XX +XXX,XX @@ migration_pagecache_insert(void) "Error allocating page"

# cpu-throttle.c

cpu_throttle_set(int new_throttle_pct) "set guest CPU throttled by %d%%"

cpu_throttle_dirty_sync(void) ""

+

+# block-active.c

+migration_block_activation(const char *name) "%s"