From 2f4aefd320a8c3aeb739d59724c5c4dd6a995919 Mon Sep 17 00:00:00 2001 From: Ivan Ren Date: Thu, 29 Aug 2019 10:16:36 +0800 Subject: [PATCH 01/12] migration: multifd_send_thread always post p->sem_sync when error happen When encounter error, multifd_send_thread should always notify who pay attention to it before exit. Otherwise it may block migration_thread at multifd_send_sync_main forever. Error as follow: ------------------------------------------------------------------------------- (gdb) bt #0 0x00007f4d669dfa0b in do_futex_wait.constprop.1 () from /lib64/libpthread.so.0 #1 0x00007f4d669dfa9f in __new_sem_wait_slow.constprop.0 () from /lib64/libpthread.so.0 #2 0x00007f4d669dfb3b in sem_wait@@GLIBC_2.2.5 () from /lib64/libpthread.so.0 #3 0x0000562ccf0a5614 in qemu_sem_wait (sem=sem@entry=0x562cd1b698e8) at util/qemu-thread-posix.c:319 #4 0x0000562ccecb4752 in multifd_send_sync_main (rs=) at /qemu/migration/ram.c:1099 #5 0x0000562ccecb95f4 in ram_save_iterate (f=0x562cd0ecc000, opaque=) at /qemu/migration/ram.c:3550 #6 0x0000562ccef43c23 in qemu_savevm_state_iterate (f=0x562cd0ecc000, postcopy=false) at migration/savevm.c:1189 #7 0x0000562ccef3dcf3 in migration_iteration_run (s=0x562cd09fabf0) at migration/migration.c:3131 #8 migration_thread (opaque=opaque@entry=0x562cd09fabf0) at migration/migration.c:3258 #9 0x0000562ccf0a4c26 in qemu_thread_start (args=) at util/qemu-thread-posix.c:502 #10 0x00007f4d669d9e25 in start_thread () from /lib64/libpthread.so.0 #11 0x00007f4d6670635d in clone () from /lib64/libc.so.6 (gdb) f 4 #4 0x0000562ccecb4752 in multifd_send_sync_main (rs=) at /qemu/migration/ram.c:1099 1099 qemu_sem_wait(&p->sem_sync); (gdb) list 1094 } 1095 for (i = 0; i < migrate_multifd_channels(); i++) { 1096 MultiFDSendParams *p = &multifd_send_state->params[i]; 1097 1098 trace_multifd_send_sync_main_wait(p->id); 1099 qemu_sem_wait(&p->sem_sync); 1100 } 1101 trace_multifd_send_sync_main(multifd_send_state->packet_num); 1102 } 1103 (gdb) p i $1 = 0 (gdb) p multifd_send_state->params[0].pending_job $2 = 2 //It means the job before MULTIFD_FLAG_SYNC has already fail (gdb) p multifd_send_state->params[0].quit $3 = true Signed-off-by: Ivan Ren Message-Id: <1567044996-2362-1-git-send-email-ivanren@tencent.com> Reviewed-by: Juan Quintela Signed-off-by: Dr. David Alan Gilbert --- migration/ram.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/migration/ram.c b/migration/ram.c index b01a37e7ca..0047286b7e 100644 --- a/migration/ram.c +++ b/migration/ram.c @@ -1112,6 +1112,7 @@ static void *multifd_send_thread(void *opaque) rcu_register_thread(); if (multifd_send_initial_packet(p, &local_err) < 0) { + ret = -1; goto out; } /* initial packet */ @@ -1179,9 +1180,7 @@ out: * who pay attention to me. */ if (ret != 0) { - if (flags & MULTIFD_FLAG_SYNC) { - qemu_sem_post(&p->sem_sync); - } + qemu_sem_post(&p->sem_sync); qemu_sem_post(&multifd_send_state->channels_ready); } From cea3b4c083d26d3114cd2881af46efb52cff6c81 Mon Sep 17 00:00:00 2001 From: Wei Yang Date: Mon, 19 Aug 2019 11:28:04 +0800 Subject: [PATCH 02/12] migration: cleanup check on ops in savevm.handlers iterations During migration, there are several places to iterate on savevm.handlers. And on each iteration, we need to check its ops and related callbacks before invoke it. Generally, ops is the first element to check, and it is only necessary to check it once. This patch clean all the related part in savevm.c to check ops only once in those iterations. Signed-off-by: Wei Yang Message-Id: <20190819032804.8579-1-richardw.yang@linux.intel.com> Reviewed-by: Juan Quintela Signed-off-by: Dr. David Alan Gilbert --- migration/savevm.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/migration/savevm.c b/migration/savevm.c index 4a86128ac4..35426d1db8 100644 --- a/migration/savevm.c +++ b/migration/savevm.c @@ -1100,7 +1100,7 @@ void qemu_savevm_state_setup(QEMUFile *f) if (!se->ops || !se->ops->save_setup) { continue; } - if (se->ops && se->ops->is_active) { + if (se->ops->is_active) { if (!se->ops->is_active(se->opaque)) { continue; } @@ -1131,7 +1131,7 @@ int qemu_savevm_state_resume_prepare(MigrationState *s) if (!se->ops || !se->ops->resume_prepare) { continue; } - if (se->ops && se->ops->is_active) { + if (se->ops->is_active) { if (!se->ops->is_active(se->opaque)) { continue; } @@ -1227,7 +1227,7 @@ void qemu_savevm_state_complete_postcopy(QEMUFile *f) if (!se->ops || !se->ops->save_live_complete_postcopy) { continue; } - if (se->ops && se->ops->is_active) { + if (se->ops->is_active) { if (!se->ops->is_active(se->opaque)) { continue; } @@ -1264,7 +1264,7 @@ int qemu_savevm_state_complete_precopy_iterable(QEMUFile *f, bool in_postcopy) continue; } - if (se->ops && se->ops->is_active) { + if (se->ops->is_active) { if (!se->ops->is_active(se->opaque)) { continue; } @@ -1413,7 +1413,7 @@ void qemu_savevm_state_pending(QEMUFile *f, uint64_t threshold_size, if (!se->ops || !se->ops->save_live_pending) { continue; } - if (se->ops && se->ops->is_active) { + if (se->ops->is_active) { if (!se->ops->is_active(se->opaque)) { continue; } @@ -2334,7 +2334,7 @@ static int qemu_loadvm_state_setup(QEMUFile *f) if (!se->ops || !se->ops->load_setup) { continue; } - if (se->ops && se->ops->is_active) { + if (se->ops->is_active) { if (!se->ops->is_active(se->opaque)) { continue; } From fd418e520eca16e3b148633ed23ec20b14db9b0c Mon Sep 17 00:00:00 2001 From: "Dr. David Alan Gilbert" Date: Thu, 22 Aug 2019 12:12:18 +0100 Subject: [PATCH 03/12] hw/net/vmxnet3: Fix leftover unregister_savevm Commit 78dd48df3 reworked vmxnet3's live migration but left a straggling unregister_savevm call. Remove it, although it doesn't seem to have any bad effect. Signed-off-by: Dr. David Alan Gilbert Message-Id: <20190822111218.12079-1-dgilbert@redhat.com> Reviewed-by: Dmitry Fleytman Signed-off-by: Dr. David Alan Gilbert --- hw/net/vmxnet3.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c index b07adeed9c..39ff6624c5 100644 --- a/hw/net/vmxnet3.c +++ b/hw/net/vmxnet3.c @@ -2242,13 +2242,10 @@ static void vmxnet3_instance_init(Object *obj) static void vmxnet3_pci_uninit(PCIDevice *pci_dev) { - DeviceState *dev = DEVICE(pci_dev); VMXNET3State *s = VMXNET3(pci_dev); VMW_CBPRN("Starting uninit..."); - unregister_savevm(dev, "vmxnet3-msix", s); - vmxnet3_net_uninit(s); vmxnet3_cleanup_msix(s); From ce62df5378bd66963b3e096b86b31f342f001cfe Mon Sep 17 00:00:00 2001 From: "Dr. David Alan Gilbert" Date: Thu, 22 Aug 2019 12:54:33 +0100 Subject: [PATCH 04/12] migration: register_savevm_live doesn't need dev Commit 78dd48df3 removed the last caller of register_savevm_live for an instantiable device (rather than a single system wide device); so trim out the parameter. Signed-off-by: Dr. David Alan Gilbert Message-Id: <20190822115433.12070-1-dgilbert@redhat.com> Reviewed-by: Stefan Hajnoczi Reviewed-by: Cornelia Huck Signed-off-by: Dr. David Alan Gilbert --- docs/devel/migration.rst | 3 +-- hw/ppc/spapr.c | 2 +- hw/s390x/s390-skeys.c | 2 +- hw/s390x/s390-stattrib.c | 2 +- hw/s390x/tod.c | 2 +- include/migration/register.h | 3 +-- migration/block-dirty-bitmap.c | 2 +- migration/block.c | 2 +- migration/ram.c | 2 +- migration/savevm.c | 23 +---------------------- net/slirp.c | 2 +- 11 files changed, 11 insertions(+), 34 deletions(-) diff --git a/docs/devel/migration.rst b/docs/devel/migration.rst index f7668ae389..e88918f763 100644 --- a/docs/devel/migration.rst +++ b/docs/devel/migration.rst @@ -183,8 +183,7 @@ another to load the state back. .. code:: c - int register_savevm_live(DeviceState *dev, - const char *idstr, + int register_savevm_live(const char *idstr, int instance_id, int version_id, SaveVMHandlers *ops, diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index 222a325056..08a2a5a770 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -3078,7 +3078,7 @@ static void spapr_machine_init(MachineState *machine) * interface, this is a legacy from the sPAPREnvironment structure * which predated MachineState but had a similar function */ vmstate_register(NULL, 0, &vmstate_spapr, spapr); - register_savevm_live(NULL, "spapr/htab", -1, 1, + register_savevm_live("spapr/htab", -1, 1, &savevm_htab_handlers, spapr); qbus_set_hotplug_handler(sysbus_get_default(), OBJECT(machine), diff --git a/hw/s390x/s390-skeys.c b/hw/s390x/s390-skeys.c index d4807f7777..bd37f39120 100644 --- a/hw/s390x/s390-skeys.c +++ b/hw/s390x/s390-skeys.c @@ -389,7 +389,7 @@ static inline void s390_skeys_set_migration_enabled(Object *obj, bool value, ss->migration_enabled = value; if (ss->migration_enabled) { - register_savevm_live(NULL, TYPE_S390_SKEYS, 0, 1, + register_savevm_live(TYPE_S390_SKEYS, 0, 1, &savevm_s390_storage_keys, ss); } else { unregister_savevm(DEVICE(ss), TYPE_S390_SKEYS, ss); diff --git a/hw/s390x/s390-stattrib.c b/hw/s390x/s390-stattrib.c index eda5ca3bb6..bf5ac014c4 100644 --- a/hw/s390x/s390-stattrib.c +++ b/hw/s390x/s390-stattrib.c @@ -381,7 +381,7 @@ static void s390_stattrib_instance_init(Object *obj) { S390StAttribState *sas = S390_STATTRIB(obj); - register_savevm_live(NULL, TYPE_S390_STATTRIB, 0, 0, + register_savevm_live(TYPE_S390_STATTRIB, 0, 0, &savevm_s390_stattrib_handlers, sas); object_property_add_bool(obj, "migration-enabled", diff --git a/hw/s390x/tod.c b/hw/s390x/tod.c index 1bf0875afa..2499d6f656 100644 --- a/hw/s390x/tod.c +++ b/hw/s390x/tod.c @@ -101,7 +101,7 @@ static void s390_tod_realize(DeviceState *dev, Error **errp) S390TODState *td = S390_TOD(dev); /* Legacy migration interface */ - register_savevm_live(NULL, "todclock", 0, 1, &savevm_tod, td); + register_savevm_live("todclock", 0, 1, &savevm_tod, td); } static void s390_tod_class_init(ObjectClass *oc, void *data) diff --git a/include/migration/register.h b/include/migration/register.h index 3d0b9833c6..a13359a08d 100644 --- a/include/migration/register.h +++ b/include/migration/register.h @@ -68,8 +68,7 @@ typedef struct SaveVMHandlers { int (*resume_prepare)(MigrationState *s, void *opaque); } SaveVMHandlers; -int register_savevm_live(DeviceState *dev, - const char *idstr, +int register_savevm_live(const char *idstr, int instance_id, int version_id, const SaveVMHandlers *ops, diff --git a/migration/block-dirty-bitmap.c b/migration/block-dirty-bitmap.c index dd40724b9e..5121f86d73 100644 --- a/migration/block-dirty-bitmap.c +++ b/migration/block-dirty-bitmap.c @@ -733,7 +733,7 @@ void dirty_bitmap_mig_init(void) { QSIMPLEQ_INIT(&dirty_bitmap_mig_state.dbms_list); - register_savevm_live(NULL, "dirty-bitmap", 0, 1, + register_savevm_live("dirty-bitmap", 0, 1, &savevm_dirty_bitmap_handlers, &dirty_bitmap_mig_state); } diff --git a/migration/block.c b/migration/block.c index aa747b55fa..0de9d84198 100644 --- a/migration/block.c +++ b/migration/block.c @@ -1030,6 +1030,6 @@ void blk_mig_init(void) QSIMPLEQ_INIT(&block_mig_state.blk_list); qemu_mutex_init(&block_mig_state.lock); - register_savevm_live(NULL, "block", 0, 1, &savevm_block_handlers, + register_savevm_live("block", 0, 1, &savevm_block_handlers, &block_mig_state); } diff --git a/migration/ram.c b/migration/ram.c index 0047286b7e..01df326767 100644 --- a/migration/ram.c +++ b/migration/ram.c @@ -4675,5 +4675,5 @@ static SaveVMHandlers savevm_ram_handlers = { void ram_mig_init(void) { qemu_mutex_init(&XBZRLE.lock); - register_savevm_live(NULL, "ram", 0, 4, &savevm_ram_handlers, &ram_state); + register_savevm_live("ram", 0, 4, &savevm_ram_handlers, &ram_state); } diff --git a/migration/savevm.c b/migration/savevm.c index 35426d1db8..3454bc937e 100644 --- a/migration/savevm.c +++ b/migration/savevm.c @@ -684,8 +684,7 @@ static void savevm_state_handler_insert(SaveStateEntry *nse) of the system, so instance_id should be removed/replaced. Meanwhile pass -1 as instance_id if you do not already have a clearly distinguishing id for all instances of your device class. */ -int register_savevm_live(DeviceState *dev, - const char *idstr, +int register_savevm_live(const char *idstr, int instance_id, int version_id, const SaveVMHandlers *ops, @@ -704,26 +703,6 @@ int register_savevm_live(DeviceState *dev, se->is_ram = 1; } - if (dev) { - char *id = qdev_get_dev_path(dev); - if (id) { - if (snprintf(se->idstr, sizeof(se->idstr), "%s/", id) >= - sizeof(se->idstr)) { - error_report("Path too long for VMState (%s)", id); - g_free(id); - g_free(se); - - return -1; - } - g_free(id); - - se->compat = g_new0(CompatEntry, 1); - pstrcpy(se->compat->idstr, sizeof(se->compat->idstr), idstr); - se->compat->instance_id = instance_id == -1 ? - calculate_compat_instance_id(idstr) : instance_id; - instance_id = -1; - } - } pstrcat(se->idstr, sizeof(se->idstr), idstr); if (instance_id == -1) { diff --git a/net/slirp.c b/net/slirp.c index b34cb29276..f42f496641 100644 --- a/net/slirp.c +++ b/net/slirp.c @@ -576,7 +576,7 @@ static int net_slirp_init(NetClientState *peer, const char *model, * specific version? */ g_assert(slirp_state_version() == 4); - register_savevm_live(NULL, "slirp", 0, slirp_state_version(), + register_savevm_live("slirp", 0, slirp_state_version(), &savevm_slirp_state, s->slirp); s->poll_notifier.notify = net_slirp_poll_notify; From 3b348706729c2154aae78f593b682d960bfb5930 Mon Sep 17 00:00:00 2001 From: "Dr. David Alan Gilbert" Date: Fri, 23 Aug 2019 11:39:46 +0100 Subject: [PATCH 05/12] qemu-file: Rework old qemu_fflush comment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Commit 11808bb removed the non-iovec based write support, the comment hung on. Signed-off-by: Dr. David Alan Gilbert Message-Id: <20190823103946.7388-1-dgilbert@redhat.com> Reviewed-by: Daniel P. Berrangé Reviewed-by: Stefano Garzarella Reviewed-by: Juan Quintela Signed-off-by: Dr. David Alan Gilbert --- migration/qemu-file.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/migration/qemu-file.c b/migration/qemu-file.c index e33c46764f..075faf03c3 100644 --- a/migration/qemu-file.c +++ b/migration/qemu-file.c @@ -201,9 +201,8 @@ static void qemu_iovec_release_ram(QEMUFile *f) /** * Flushes QEMUFile buffer * - * If there is writev_buffer QEMUFileOps it uses it otherwise uses - * put_buffer ops. This will flush all pending data. If data was - * only partially flushed, it will set an error state. + * This will flush all pending data. If data was only partially flushed, it + * will set an error state. */ void qemu_fflush(QEMUFile *f) { From b9d68df62a1340529f7eef251353d2040c66f403 Mon Sep 17 00:00:00 2001 From: Yury Kotov Date: Tue, 3 Sep 2019 19:22:44 +0300 Subject: [PATCH 06/12] migration: Add validate-uuid capability This capability realizes simple source validation by UUID. It's useful for live migration between hosts. Signed-off-by: Yury Kotov Message-Id: <20190903162246.18524-2-yury-kotov@yandex-team.ru> Reviewed-by: Dr. David Alan Gilbert Signed-off-by: Dr. David Alan Gilbert --- migration/migration.c | 9 +++++++++ migration/migration.h | 1 + migration/savevm.c | 45 +++++++++++++++++++++++++++++++++++++++++++ qapi/migration.json | 5 ++++- 4 files changed, 59 insertions(+), 1 deletion(-) diff --git a/migration/migration.c b/migration/migration.c index 8b9f2fe30a..2391a8d418 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -2140,6 +2140,15 @@ bool migrate_ignore_shared(void) return s->enabled_capabilities[MIGRATION_CAPABILITY_X_IGNORE_SHARED]; } +bool migrate_validate_uuid(void) +{ + MigrationState *s; + + s = migrate_get_current(); + + return s->enabled_capabilities[MIGRATION_CAPABILITY_VALIDATE_UUID]; +} + bool migrate_use_events(void) { MigrationState *s; diff --git a/migration/migration.h b/migration/migration.h index 3e1ea2b5dc..4f2fe193dc 100644 --- a/migration/migration.h +++ b/migration/migration.h @@ -290,6 +290,7 @@ bool migrate_postcopy_ram(void); bool migrate_zero_blocks(void); bool migrate_dirty_bitmaps(void); bool migrate_ignore_shared(void); +bool migrate_validate_uuid(void); bool migrate_auto_converge(void); bool migrate_use_multifd(void); diff --git a/migration/savevm.c b/migration/savevm.c index 3454bc937e..ee06f91d42 100644 --- a/migration/savevm.c +++ b/migration/savevm.c @@ -256,6 +256,7 @@ typedef struct SaveState { uint32_t target_page_bits; uint32_t caps_count; MigrationCapability *capabilities; + QemuUUID uuid; } SaveState; static SaveState savevm_state = { @@ -307,6 +308,7 @@ static int configuration_pre_save(void *opaque) state->capabilities[j++] = i; } } + state->uuid = qemu_uuid; return 0; } @@ -464,6 +466,48 @@ static const VMStateDescription vmstate_capabilites = { } }; +static bool vmstate_uuid_needed(void *opaque) +{ + return qemu_uuid_set && migrate_validate_uuid(); +} + +static int vmstate_uuid_post_load(void *opaque, int version_id) +{ + SaveState *state = opaque; + char uuid_src[UUID_FMT_LEN + 1]; + char uuid_dst[UUID_FMT_LEN + 1]; + + if (!qemu_uuid_set) { + /* + * It's warning because user might not know UUID in some cases, + * e.g. load an old snapshot + */ + qemu_uuid_unparse(&state->uuid, uuid_src); + warn_report("UUID is received %s, but local uuid isn't set", + uuid_src); + return 0; + } + if (!qemu_uuid_is_equal(&state->uuid, &qemu_uuid)) { + qemu_uuid_unparse(&state->uuid, uuid_src); + qemu_uuid_unparse(&qemu_uuid, uuid_dst); + error_report("UUID received is %s and local is %s", uuid_src, uuid_dst); + return -EINVAL; + } + return 0; +} + +static const VMStateDescription vmstate_uuid = { + .name = "configuration/uuid", + .version_id = 1, + .minimum_version_id = 1, + .needed = vmstate_uuid_needed, + .post_load = vmstate_uuid_post_load, + .fields = (VMStateField[]) { + VMSTATE_UINT8_ARRAY_V(uuid.data, SaveState, sizeof(QemuUUID), 1), + VMSTATE_END_OF_LIST() + } +}; + static const VMStateDescription vmstate_configuration = { .name = "configuration", .version_id = 1, @@ -478,6 +522,7 @@ static const VMStateDescription vmstate_configuration = { .subsections = (const VMStateDescription*[]) { &vmstate_target_page_bits, &vmstate_capabilites, + &vmstate_uuid, NULL } }; diff --git a/qapi/migration.json b/qapi/migration.json index 9cfbaf8c6c..82feb5bd39 100644 --- a/qapi/migration.json +++ b/qapi/migration.json @@ -415,6 +415,9 @@ # # @x-ignore-shared: If enabled, QEMU will not migrate shared memory (since 4.0) # +# @validate-uuid: Send the UUID of the source to allow the destination +# to ensure it is the same. (since 4.2) +# # Since: 1.2 ## { 'enum': 'MigrationCapability', @@ -422,7 +425,7 @@ 'compress', 'events', 'postcopy-ram', 'x-colo', 'release-ram', 'block', 'return-path', 'pause-before-switchover', 'multifd', 'dirty-bitmaps', 'postcopy-blocktime', 'late-block-activate', - 'x-ignore-shared' ] } + 'x-ignore-shared', 'validate-uuid' ] } ## # @MigrationCapabilityStatus: From d43e59e7ab883249c25cca513837407461216900 Mon Sep 17 00:00:00 2001 From: Yury Kotov Date: Tue, 3 Sep 2019 19:22:45 +0300 Subject: [PATCH 07/12] tests/libqtest: Allow setting expected exit status Add qtest_set_expected_status function to set expected exit status of child process. By default expected exit status is 0. Signed-off-by: Yury Kotov Message-Id: <20190903162246.18524-3-yury-kotov@yandex-team.ru> Acked-by: Thomas Huth Reviewed-by: Dr. David Alan Gilbert Signed-off-by: Dr. David Alan Gilbert --- tests/libqtest.c | 34 ++++++++++++++++++++-------------- tests/libqtest.h | 9 +++++++++ 2 files changed, 29 insertions(+), 14 deletions(-) diff --git a/tests/libqtest.c b/tests/libqtest.c index 0a6b91737e..4a7556462d 100644 --- a/tests/libqtest.c +++ b/tests/libqtest.c @@ -41,6 +41,7 @@ struct QTestState int qmp_fd; pid_t qemu_pid; /* our child QEMU process */ int wstatus; + int expected_status; bool big_endian; bool irq_level[MAX_IRQ]; GString *rx; @@ -111,6 +112,11 @@ bool qtest_probe_child(QTestState *s) return false; } +void qtest_set_expected_status(QTestState *s, int status) +{ + s->expected_status = status; +} + static void kill_qemu(QTestState *s) { pid_t pid = s->qemu_pid; @@ -124,24 +130,23 @@ static void kill_qemu(QTestState *s) } /* - * We expect qemu to exit with status 0; anything else is + * Check whether qemu exited with expected exit status; anything else is * fishy and should be logged with as much detail as possible. */ wstatus = s->wstatus; - if (wstatus) { - if (WIFEXITED(wstatus)) { - fprintf(stderr, "%s:%d: kill_qemu() tried to terminate QEMU " - "process but encountered exit status %d\n", - __FILE__, __LINE__, WEXITSTATUS(wstatus)); - } else if (WIFSIGNALED(wstatus)) { - int sig = WTERMSIG(wstatus); - const char *signame = strsignal(sig) ?: "unknown ???"; - const char *dump = WCOREDUMP(wstatus) ? " (core dumped)" : ""; + if (WIFEXITED(wstatus) && WEXITSTATUS(wstatus) != s->expected_status) { + fprintf(stderr, "%s:%d: kill_qemu() tried to terminate QEMU " + "process but encountered exit status %d (expected %d)\n", + __FILE__, __LINE__, WEXITSTATUS(wstatus), s->expected_status); + abort(); + } else if (WIFSIGNALED(wstatus)) { + int sig = WTERMSIG(wstatus); + const char *signame = strsignal(sig) ?: "unknown ???"; + const char *dump = WCOREDUMP(wstatus) ? " (core dumped)" : ""; - fprintf(stderr, "%s:%d: kill_qemu() detected QEMU death " - "from signal %d (%s)%s\n", - __FILE__, __LINE__, sig, signame, dump); - } + fprintf(stderr, "%s:%d: kill_qemu() detected QEMU death " + "from signal %d (%s)%s\n", + __FILE__, __LINE__, sig, signame, dump); abort(); } } @@ -246,6 +251,7 @@ QTestState *qtest_init_without_qmp_handshake(const char *extra_args) g_test_message("starting QEMU: %s", command); s->wstatus = 0; + s->expected_status = 0; s->qemu_pid = fork(); if (s->qemu_pid == 0) { setenv("QEMU_AUDIO_DRV", "none", true); diff --git a/tests/libqtest.h b/tests/libqtest.h index c8cffe5d68..a177e502d9 100644 --- a/tests/libqtest.h +++ b/tests/libqtest.h @@ -708,4 +708,13 @@ void qmp_assert_error_class(QDict *rsp, const char *class); */ bool qtest_probe_child(QTestState *s); +/** + * qtest_set_expected_status: + * @s: QTestState instance to operate on. + * @status: an expected exit status. + * + * Set expected exit status of the child. + */ +void qtest_set_expected_status(QTestState *s, int status); + #endif From 3af31a3469342a621949b721f3a020b452092bf9 Mon Sep 17 00:00:00 2001 From: Yury Kotov Date: Tue, 3 Sep 2019 19:22:46 +0300 Subject: [PATCH 08/12] tests/migration: Add a test for validate-uuid capability Signed-off-by: Yury Kotov Message-Id: <20190903162246.18524-4-yury-kotov@yandex-team.ru> Reviewed-by: Dr. David Alan Gilbert Signed-off-by: Dr. David Alan Gilbert --- tests/migration-test.c | 140 ++++++++++++++++++++++++++++++++--------- 1 file changed, 110 insertions(+), 30 deletions(-) diff --git a/tests/migration-test.c b/tests/migration-test.c index a9f81cc185..258aa064d4 100644 --- a/tests/migration-test.c +++ b/tests/migration-test.c @@ -512,7 +512,8 @@ static void migrate_postcopy_start(QTestState *from, QTestState *to) static int test_migrate_start(QTestState **from, QTestState **to, const char *uri, bool hide_stderr, - bool use_shmem) + bool use_shmem, const char *opts_src, + const char *opts_dst) { gchar *cmd_src, *cmd_dst; char *bootpath = NULL; @@ -521,6 +522,9 @@ static int test_migrate_start(QTestState **from, QTestState **to, const char *arch = qtest_get_arch(); const char *accel = "kvm:tcg"; + opts_src = opts_src ? opts_src : ""; + opts_dst = opts_dst ? opts_dst : ""; + if (use_shmem) { if (!g_file_test("/dev/shm", G_FILE_TEST_IS_DIR)) { g_test_skip("/dev/shm is not supported"); @@ -539,16 +543,16 @@ static int test_migrate_start(QTestState **from, QTestState **to, cmd_src = g_strdup_printf("-machine accel=%s -m 150M" " -name source,debug-threads=on" " -serial file:%s/src_serial" - " -drive file=%s,format=raw %s", + " -drive file=%s,format=raw %s %s", accel, tmpfs, bootpath, - extra_opts ? extra_opts : ""); + extra_opts ? extra_opts : "", opts_src); cmd_dst = g_strdup_printf("-machine accel=%s -m 150M" " -name target,debug-threads=on" " -serial file:%s/dest_serial" " -drive file=%s,format=raw" - " -incoming %s %s", + " -incoming %s %s %s", accel, tmpfs, bootpath, uri, - extra_opts ? extra_opts : ""); + extra_opts ? extra_opts : "", opts_dst); start_address = X86_TEST_MEM_START; end_address = X86_TEST_MEM_END; } else if (g_str_equal(arch, "s390x")) { @@ -556,15 +560,15 @@ static int test_migrate_start(QTestState **from, QTestState **to, extra_opts = use_shmem ? get_shmem_opts("128M", shmem_path) : NULL; cmd_src = g_strdup_printf("-machine accel=%s -m 128M" " -name source,debug-threads=on" - " -serial file:%s/src_serial -bios %s %s", + " -serial file:%s/src_serial -bios %s %s %s", accel, tmpfs, bootpath, - extra_opts ? extra_opts : ""); + extra_opts ? extra_opts : "", opts_src); cmd_dst = g_strdup_printf("-machine accel=%s -m 128M" " -name target,debug-threads=on" " -serial file:%s/dest_serial -bios %s" - " -incoming %s %s", + " -incoming %s %s %s", accel, tmpfs, bootpath, uri, - extra_opts ? extra_opts : ""); + extra_opts ? extra_opts : "", opts_dst); start_address = S390_TEST_MEM_START; end_address = S390_TEST_MEM_END; } else if (strcmp(arch, "ppc64") == 0) { @@ -575,14 +579,15 @@ static int test_migrate_start(QTestState **from, QTestState **to, " -prom-env 'use-nvramrc?=true' -prom-env " "'nvramrc=hex .\" _\" begin %x %x " "do i c@ 1 + i c! 1000 +loop .\" B\" 0 " - "until' %s", accel, tmpfs, end_address, - start_address, extra_opts ? extra_opts : ""); + "until' %s %s", accel, tmpfs, end_address, + start_address, extra_opts ? extra_opts : "", + opts_src); cmd_dst = g_strdup_printf("-machine accel=%s -m 256M" " -name target,debug-threads=on" " -serial file:%s/dest_serial" - " -incoming %s %s", + " -incoming %s %s %s", accel, tmpfs, uri, - extra_opts ? extra_opts : ""); + extra_opts ? extra_opts : "", opts_dst); start_address = PPC_TEST_MEM_START; end_address = PPC_TEST_MEM_END; @@ -592,16 +597,16 @@ static int test_migrate_start(QTestState **from, QTestState **to, cmd_src = g_strdup_printf("-machine virt,accel=%s,gic-version=max " "-name vmsource,debug-threads=on -cpu max " "-m 150M -serial file:%s/src_serial " - "-kernel %s %s", + "-kernel %s %s %s", accel, tmpfs, bootpath, - extra_opts ? extra_opts : ""); + extra_opts ? extra_opts : "", opts_src); cmd_dst = g_strdup_printf("-machine virt,accel=%s,gic-version=max " "-name vmdest,debug-threads=on -cpu max " "-m 150M -serial file:%s/dest_serial " "-kernel %s " - "-incoming %s %s", + "-incoming %s %s %s", accel, tmpfs, bootpath, uri, - extra_opts ? extra_opts : ""); + extra_opts ? extra_opts : "", opts_dst); start_address = ARM_TEST_MEM_START; end_address = ARM_TEST_MEM_END; @@ -731,7 +736,7 @@ static int migrate_postcopy_prepare(QTestState **from_ptr, char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs); QTestState *from, *to; - if (test_migrate_start(&from, &to, uri, hide_error, false)) { + if (test_migrate_start(&from, &to, uri, hide_error, false, NULL, NULL)) { return -1; } @@ -841,20 +846,16 @@ static void test_postcopy_recovery(void) migrate_postcopy_complete(from, to); } -static void test_baddest(void) +static void wait_for_migration_fail(QTestState *from, bool allow_active) { - QTestState *from, *to; QDict *rsp_return; char *status; bool failed; - if (test_migrate_start(&from, &to, "tcp:0:0", true, false)) { - return; - } - migrate(from, "tcp:0:0", "{}"); do { status = migrate_query_status(from); - g_assert(!strcmp(status, "setup") || !(strcmp(status, "failed"))); + g_assert(!strcmp(status, "setup") || !strcmp(status, "failed") || + (allow_active && !strcmp(status, "active"))); failed = !strcmp(status, "failed"); g_free(status); } while (!failed); @@ -864,7 +865,17 @@ static void test_baddest(void) g_assert(qdict_haskey(rsp_return, "running")); g_assert(qdict_get_bool(rsp_return, "running")); qobject_unref(rsp_return); +} +static void test_baddest(void) +{ + QTestState *from, *to; + + if (test_migrate_start(&from, &to, "tcp:0:0", true, false, NULL, NULL)) { + return; + } + migrate(from, "tcp:0:0", "{}"); + wait_for_migration_fail(from, false); test_migrate_end(from, to, false); } @@ -873,7 +884,7 @@ static void test_precopy_unix(void) char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs); QTestState *from, *to; - if (test_migrate_start(&from, &to, uri, false, false)) { + if (test_migrate_start(&from, &to, uri, false, false, NULL, NULL)) { return; } @@ -916,7 +927,7 @@ static void test_ignore_shared(void) char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs); QTestState *from, *to; - if (test_migrate_start(&from, &to, uri, false, true)) { + if (test_migrate_start(&from, &to, uri, false, true, NULL, NULL)) { return; } @@ -951,7 +962,7 @@ static void test_xbzrle(const char *uri) { QTestState *from, *to; - if (test_migrate_start(&from, &to, uri, false, false)) { + if (test_migrate_start(&from, &to, uri, false, false, NULL, NULL)) { return; } @@ -1003,7 +1014,8 @@ static void test_precopy_tcp(void) char *uri; QTestState *from, *to; - if (test_migrate_start(&from, &to, "tcp:127.0.0.1:0", false, false)) { + if (test_migrate_start(&from, &to, "tcp:127.0.0.1:0", false, false, + NULL, NULL)) { return; } @@ -1049,7 +1061,7 @@ static void test_migrate_fd_proto(void) QDict *rsp; const char *error_desc; - if (test_migrate_start(&from, &to, "defer", false, false)) { + if (test_migrate_start(&from, &to, "defer", false, false, NULL, NULL)) { return; } @@ -1125,6 +1137,68 @@ static void test_migrate_fd_proto(void) test_migrate_end(from, to, true); } +static void do_test_validate_uuid(const char *uuid_arg_src, + const char *uuid_arg_dst, + bool should_fail, bool hide_stderr) +{ + char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs); + QTestState *from, *to; + + if (test_migrate_start(&from, &to, uri, hide_stderr, false, + uuid_arg_src, uuid_arg_dst)) { + return; + } + + /* + * UUID validation is at the begin of migration. So, the main process of + * migration is not interesting for us here. Thus, set huge downtime for + * very fast migration. + */ + migrate_set_parameter_int(from, "downtime-limit", 1000000); + migrate_set_capability(from, "validate-uuid", true); + + /* Wait for the first serial output from the source */ + wait_for_serial("src_serial"); + + migrate(from, uri, "{}"); + + if (should_fail) { + qtest_set_expected_status(to, 1); + wait_for_migration_fail(from, true); + } else { + wait_for_migration_complete(from); + } + + test_migrate_end(from, to, false); + g_free(uri); +} + +static void test_validate_uuid(void) +{ + do_test_validate_uuid("-uuid 11111111-1111-1111-1111-111111111111", + "-uuid 11111111-1111-1111-1111-111111111111", + false, false); +} + +static void test_validate_uuid_error(void) +{ + do_test_validate_uuid("-uuid 11111111-1111-1111-1111-111111111111", + "-uuid 22222222-2222-2222-2222-222222222222", + true, true); +} + +static void test_validate_uuid_src_not_set(void) +{ + do_test_validate_uuid(NULL, "-uuid 11111111-1111-1111-1111-111111111111", + false, true); +} + +static void test_validate_uuid_dst_not_set(void) +{ + do_test_validate_uuid("-uuid 11111111-1111-1111-1111-111111111111", NULL, + false, true); +} + int main(int argc, char **argv) { char template[] = "/tmp/migration-test-XXXXXX"; @@ -1180,6 +1254,12 @@ int main(int argc, char **argv) /* qtest_add_func("/migration/ignore_shared", test_ignore_shared); */ qtest_add_func("/migration/xbzrle/unix", test_xbzrle_unix); qtest_add_func("/migration/fd_proto", test_migrate_fd_proto); + qtest_add_func("/migration/validate_uuid", test_validate_uuid); + qtest_add_func("/migration/validate_uuid_error", test_validate_uuid_error); + qtest_add_func("/migration/validate_uuid_src_not_set", + test_validate_uuid_src_not_set); + qtest_add_func("/migration/validate_uuid_dst_not_set", + test_validate_uuid_dst_not_set); ret = g_test_run(); From 8504ddeca0d1f592877dba8dc0db44b4d7fd8c52 Mon Sep 17 00:00:00 2001 From: Peter Xu Date: Fri, 6 Sep 2019 21:01:03 +0800 Subject: [PATCH 09/12] migration: Fix postcopy bw for recovery We've got max-postcopy-bandwidth parameter but it's not applied correctly after a postcopy recovery so the recovered migration stream will still eat the whole net bandwidth. Fix that up. Reported-by: Xiaohui Li Signed-off-by: Peter Xu Message-Id: <20190906130103.20961-1-peterx@redhat.com> Reviewed-by: Juan Quintela Signed-off-by: Dr. David Alan Gilbert --- migration/migration.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/migration/migration.c b/migration/migration.c index 2391a8d418..e45270c23b 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -3336,7 +3336,8 @@ void migrate_fd_connect(MigrationState *s, Error *error_in) if (resume) { /* This is a resumed migration */ - rate_limit = INT64_MAX; + rate_limit = s->parameters.max_postcopy_bandwidth / + XFER_LIMIT_RATIO; } else { /* This is a fresh new migration */ rate_limit = s->parameters.max_bandwidth / XFER_LIMIT_RATIO; From 89fe04b4585c97fda7c173bee97bbacb32f64e80 Mon Sep 17 00:00:00 2001 From: Wei Yang Date: Wed, 11 Sep 2019 13:28:38 +0000 Subject: [PATCH 10/12] migration/qemu-file: remove check on writev_buffer in qemu_put_compression_data The check of writev_buffer is in qemu_fflush, which means it is not harmful if it is NULL. And removing it will make the code consistent since all other add_to_iovec() is called without the check. Signed-off-by: Wei Yang Reviewed-by: Dr. David Alan Gilbert Message-Id: <20190911132839.23336-2-richard.weiyang@gmail.com> Signed-off-by: Dr. David Alan Gilbert --- migration/qemu-file.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/migration/qemu-file.c b/migration/qemu-file.c index 075faf03c3..66627088d4 100644 --- a/migration/qemu-file.c +++ b/migration/qemu-file.c @@ -760,9 +760,7 @@ ssize_t qemu_put_compression_data(QEMUFile *f, z_stream *stream, } qemu_put_be32(f, blen); - if (f->ops->writev_buffer) { - add_to_iovec(f, f->buf + f->buf_index, blen, false); - } + add_to_iovec(f, f->buf + f->buf_index, blen, false); f->buf_index += blen; if (f->buf_index == IO_BUF_SIZE) { qemu_fflush(f); From 1bf57fb3df9615b50219c50b381cc52e303f682c Mon Sep 17 00:00:00 2001 From: Wei Yang Date: Wed, 11 Sep 2019 13:28:39 +0000 Subject: [PATCH 11/12] migration/qemu-file: fix potential buf waste for extra buf_index adjustment In add_to_iovec(), qemu_fflush() will be called if iovec is full. If this happens, buf_index is reset. Currently, this is not checked and buf_index would always been adjust with buf size. This is not harmful, but will waste some space in file buffer. This patch make add_to_iovec() return 1 when it has flushed the file. Then the caller could check the return value to see whether it is necessary to adjust the buf_index any more. Signed-off-by: Wei Yang Reviewed-by: Dr. David Alan Gilbert Message-Id: <20190911132839.23336-3-richard.weiyang@gmail.com> Signed-off-by: Dr. David Alan Gilbert --- migration/qemu-file.c | 43 ++++++++++++++++++++++++++----------------- 1 file changed, 26 insertions(+), 17 deletions(-) diff --git a/migration/qemu-file.c b/migration/qemu-file.c index 66627088d4..26fb25ddc1 100644 --- a/migration/qemu-file.c +++ b/migration/qemu-file.c @@ -381,8 +381,16 @@ int qemu_fclose(QEMUFile *f) return ret; } -static void add_to_iovec(QEMUFile *f, const uint8_t *buf, size_t size, - bool may_free) +/* + * Add buf to iovec. Do flush if iovec is full. + * + * Return values: + * 1 iovec is full and flushed + * 0 iovec is not flushed + * + */ +static int add_to_iovec(QEMUFile *f, const uint8_t *buf, size_t size, + bool may_free) { /* check for adjacent buffer and coalesce them */ if (f->iovcnt > 0 && buf == f->iov[f->iovcnt - 1].iov_base + @@ -400,6 +408,19 @@ static void add_to_iovec(QEMUFile *f, const uint8_t *buf, size_t size, if (f->iovcnt >= MAX_IOV_SIZE) { qemu_fflush(f); + return 1; + } + + return 0; +} + +static void add_buf_to_iovec(QEMUFile *f, size_t len) +{ + if (!add_to_iovec(f, f->buf + f->buf_index, len, false)) { + f->buf_index += len; + if (f->buf_index == IO_BUF_SIZE) { + qemu_fflush(f); + } } } @@ -429,11 +450,7 @@ void qemu_put_buffer(QEMUFile *f, const uint8_t *buf, size_t size) } memcpy(f->buf + f->buf_index, buf, l); f->bytes_xfer += l; - add_to_iovec(f, f->buf + f->buf_index, l, false); - f->buf_index += l; - if (f->buf_index == IO_BUF_SIZE) { - qemu_fflush(f); - } + add_buf_to_iovec(f, l); if (qemu_file_get_error(f)) { break; } @@ -450,11 +467,7 @@ void qemu_put_byte(QEMUFile *f, int v) f->buf[f->buf_index] = v; f->bytes_xfer++; - add_to_iovec(f, f->buf + f->buf_index, 1, false); - f->buf_index++; - if (f->buf_index == IO_BUF_SIZE) { - qemu_fflush(f); - } + add_buf_to_iovec(f, 1); } void qemu_file_skip(QEMUFile *f, int size) @@ -760,11 +773,7 @@ ssize_t qemu_put_compression_data(QEMUFile *f, z_stream *stream, } qemu_put_be32(f, blen); - add_to_iovec(f, f->buf + f->buf_index, blen, false); - f->buf_index += blen; - if (f->buf_index == IO_BUF_SIZE) { - qemu_fflush(f); - } + add_buf_to_iovec(f, blen); return blen + sizeof(int32_t); } From 268dcd46ae6d608a4ce93b191b51a318504bf1fb Mon Sep 17 00:00:00 2001 From: Wei Yang Date: Thu, 12 Sep 2019 10:49:57 +0800 Subject: [PATCH 12/12] migration: fix one typo in comment of function migration_total_bytes() Signed-off-by: Wei Yang Message-Id: <20190912024957.11780-1-richardw.yang@linux.intel.com> Reviewed-by: Juan Quintela Signed-off-by: Dr. David Alan Gilbert --- migration/migration.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/migration/migration.c b/migration/migration.c index e45270c23b..01863a95f5 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -3025,7 +3025,7 @@ static MigThrError migration_detect_error(MigrationState *s) } } -/* How many bytes have we transferred since the beggining of the migration */ +/* How many bytes have we transferred since the beginning of the migration */ static uint64_t migration_total_bytes(MigrationState *s) { return qemu_ftell(s->to_dst_file) + ram_counters.multifd_bytes;