From 33ee3d96c7c21febbfb90b005ee9e646cf1f3794 Mon Sep 17 00:00:00 2001 From: Pan Nengyuan Date: Wed, 18 Mar 2020 15:16:20 +0800 Subject: [PATCH 1/9] hmp-cmd: fix a missing_break warning MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This fix coverity issues 94417686: 1260 break; CID 94417686: (MISSING_BREAK) 1261. unterminated_case: The case for value "MIGRATION_PARAMETER_THROTTLE_TRIGGER_THRESHOLD" is not terminated by a 'break' statement. 1261 case MIGRATION_PARAMETER_THROTTLE_TRIGGER_THRESHOLD: 1262 p->has_throttle_trigger_threshold = true; 1263 visit_type_int(v, param, &p->throttle_trigger_threshold, &err); 1264 case MIGRATION_PARAMETER_CPU_THROTTLE_INITIAL: Fixes: dc14a470763c96fd9d360e1028ce38e8c3613a77 Fixes: Coverity (CID 1421950) Reported-by: Euler Robot Signed-off-by: Pan Nengyuan Message-Id: <20200318071620.59748-1-pannengyuan@huawei.com> Reviewed-by: Keqian Zhu Reviewed-by: Dr. David Alan Gilbert Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Dr. David Alan Gilbert --- monitor/hmp-cmds.c | 1 + 1 file changed, 1 insertion(+) diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c index 58724031ea..c882c9f3cc 100644 --- a/monitor/hmp-cmds.c +++ b/monitor/hmp-cmds.c @@ -1261,6 +1261,7 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict) case MIGRATION_PARAMETER_THROTTLE_TRIGGER_THRESHOLD: p->has_throttle_trigger_threshold = true; visit_type_int(v, param, &p->throttle_trigger_threshold, &err); + break; case MIGRATION_PARAMETER_CPU_THROTTLE_INITIAL: p->has_cpu_throttle_initial = true; visit_type_int(v, param, &p->cpu_throttle_initial, &err); From 06b1c6f8b7efeaab5da5e092727cd1391a1d9f2c Mon Sep 17 00:00:00 2001 From: Mao Zhongyi Date: Fri, 20 Mar 2020 22:32:16 +0800 Subject: [PATCH 2/9] xbzrle: update xbzrle doc Add new parameter description, also: 1. Remove unsociable space. 2. Nit picking: s/two/2 in report Signed-off-by: Mao Zhongyi Message-Id: <20200320143216.423374-1-maozhongyi@cmss.chinamobile.com> Reviewed-by: Dr. David Alan Gilbert Signed-off-by: Dr. David Alan Gilbert --- docs/xbzrle.txt | 7 ++++++- migration/migration.c | 2 +- monitor/hmp-cmds.c | 2 +- 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/docs/xbzrle.txt b/docs/xbzrle.txt index c0a7dfd44c..b431bdaf0f 100644 --- a/docs/xbzrle.txt +++ b/docs/xbzrle.txt @@ -92,6 +92,11 @@ Usage power of 2. The cache default value is 64MBytes. (on source only) {qemu} migrate_set_cache_size 256m +Commit 73af8dd8d7 "migration: Make xbzrle_cache_size a migration parameter" +(v2.11.0) deprecated migrate-set-cache-size, therefore, the new parameter +is recommended. + {qemu} migrate_set_parameter xbzrle-cache-size 256m + 4. Start outgoing migration {qemu} migrate -d tcp:destination.host:4444 {qemu} info migrate @@ -108,7 +113,7 @@ power of 2. The cache default value is 64MBytes. (on source only) xbzrle transferred: I kbytes xbzrle pages: J pages xbzrle cache miss: K - xbzrle overflow : L + xbzrle overflow: L xbzrle cache-miss: the number of cache misses to date - high cache-miss rate indicates that the cache size is set too low. diff --git a/migration/migration.c b/migration/migration.c index c1d88ace7f..4b26110d57 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -1243,7 +1243,7 @@ static bool migrate_params_check(MigrationParameters *params, Error **errp) error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "xbzrle_cache_size", "is invalid, it should be bigger than target page size" - " and a power of two"); + " and a power of 2"); return false; } diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c index c882c9f3cc..76725c2ace 100644 --- a/monitor/hmp-cmds.c +++ b/monitor/hmp-cmds.c @@ -303,7 +303,7 @@ void hmp_info_migrate(Monitor *mon, const QDict *qdict) info->xbzrle_cache->cache_miss); monitor_printf(mon, "xbzrle cache miss rate: %0.2f\n", info->xbzrle_cache->cache_miss_rate); - monitor_printf(mon, "xbzrle overflow : %" PRIu64 "\n", + monitor_printf(mon, "xbzrle overflow: %" PRIu64 "\n", info->xbzrle_cache->overflow); } From 6d1da867e65f72b890e736a36cbaa9146a824b64 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Mon, 23 Mar 2020 19:40:15 +0100 Subject: [PATCH 3/9] tests/migration: Reduce autoconverge initial bandwidth MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When using max-bandwidth=~100Mb/s, this test fails on Travis-CI s390x when configured with --disable-tcg: $ make check-qtest TEST check-qtest-s390x: tests/qtest/boot-serial-test qemu-system-s390x: -accel tcg: invalid accelerator tcg qemu-system-s390x: falling back to KVM TEST check-qtest-s390x: tests/qtest/pxe-test TEST check-qtest-s390x: tests/qtest/test-netfilter TEST check-qtest-s390x: tests/qtest/test-filter-mirror TEST check-qtest-s390x: tests/qtest/test-filter-redirector TEST check-qtest-s390x: tests/qtest/drive_del-test TEST check-qtest-s390x: tests/qtest/device-plug-test TEST check-qtest-s390x: tests/qtest/virtio-ccw-test TEST check-qtest-s390x: tests/qtest/cpu-plug-test TEST check-qtest-s390x: tests/qtest/migration-test ** ERROR:tests/qtest/migration-test.c:1229:test_migrate_auto_converge: 'got_stop' should be FALSE ERROR - Bail out! ERROR:tests/qtest/migration-test.c:1229:test_migrate_auto_converge: 'got_stop' should be FALSE make: *** [tests/Makefile.include:633: check-qtest-s390x] Error 1 Per David Gilbert, "it could just be the writing is slow on s390 and the migration thread fast; in which case the autocomplete wouldn't be needed. Perhaps we just need to reduce the bandwidth limit." Tuning the threshold by reducing the initial bandwidth makes the autoconverge test pass. Suggested-by: Dr. David Alan Gilbert Signed-off-by: Philippe Mathieu-Daudé Message-Id: <20200323184015.11565-1-philmd@redhat.com> Reviewed-by: Dr. David Alan Gilbert Tested-by: Alex Bennée Signed-off-by: Dr. David Alan Gilbert --- tests/qtest/migration-test.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c index 3d6cc83b88..2568c9529c 100644 --- a/tests/qtest/migration-test.c +++ b/tests/qtest/migration-test.c @@ -1211,7 +1211,7 @@ static void test_migrate_auto_converge(void) * without throttling. */ migrate_set_parameter_int(from, "downtime-limit", 1); - migrate_set_parameter_int(from, "max-bandwidth", 100000000); /* ~100Mb/s */ + migrate_set_parameter_int(from, "max-bandwidth", 1000000); /* ~1Mb/s */ /* To check remaining size after precopy */ migrate_set_capability(from, "pause-before-switchover", true); From d4ff109373ce871928c7e9ef648973eba642b484 Mon Sep 17 00:00:00 2001 From: "Dr. David Alan Gilbert" Date: Mon, 23 Mar 2020 12:08:22 +0000 Subject: [PATCH 4/9] hmp/vnc: Fix info vnc list leak We're iterating the list, and then freeing the iteration pointer rather than the list head. Fixes: 0a9667ecdb6d ("hmp: Update info vnc") Reported-by: Coverity (CID 1421932) Signed-off-by: Dr. David Alan Gilbert Message-Id: <20200323120822.51266-1-dgilbert@redhat.com> Reviewed-by: Peter Maydell Signed-off-by: Dr. David Alan Gilbert --- monitor/hmp-cmds.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c index 76725c2ace..04ca342c51 100644 --- a/monitor/hmp-cmds.c +++ b/monitor/hmp-cmds.c @@ -527,10 +527,11 @@ static void hmp_info_vnc_servers(Monitor *mon, VncServerInfo2List *server) void hmp_info_vnc(Monitor *mon, const QDict *qdict) { - VncInfo2List *info2l; + VncInfo2List *info2l, *info2l_head; Error *err = NULL; info2l = qmp_query_vnc_servers(&err); + info2l_head = info2l; if (err) { hmp_handle_error(mon, err); return; @@ -559,7 +560,7 @@ void hmp_info_vnc(Monitor *mon, const QDict *qdict) info2l = info2l->next; } - qapi_free_VncInfo2List(info2l); + qapi_free_VncInfo2List(info2l_head); } #endif From e1cd92d95cd4f97b3464c4e08cd5b22bf5ca05cb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Sat, 21 Mar 2020 13:06:54 +0100 Subject: [PATCH 5/9] tools/virtiofsd/passthrough_ll: Fix double close() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On success, the fdopendir() call closes fd. Later on the error path we try to close an already-closed fd. This can lead to use-after-free. Fix by only closing the fd if the fdopendir() call failed. Cc: qemu-stable@nongnu.org Fixes: b39bce121b (add dirp_map to hide lo_dirp pointers) Reported-by: Coverity (CID 1421933 USE_AFTER_FREE) Suggested-by: Peter Maydell Signed-off-by: Philippe Mathieu-Daudé Message-Id: <20200321120654.7985-1-philmd@redhat.com> Reviewed-by: Stefan Hajnoczi Signed-off-by: Dr. David Alan Gilbert --- tools/virtiofsd/passthrough_ll.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c index 4f259aac70..4c35c95b25 100644 --- a/tools/virtiofsd/passthrough_ll.c +++ b/tools/virtiofsd/passthrough_ll.c @@ -1520,8 +1520,7 @@ out_err: if (d) { if (d->dp) { closedir(d->dp); - } - if (fd != -1) { + } else if (fd != -1) { close(fd); } free(d); From d96c4d5f193e0e45beec80a6277728b32875bddb Mon Sep 17 00:00:00 2001 From: Igor Mammedov Date: Wed, 4 Mar 2020 12:27:48 -0500 Subject: [PATCH 6/9] vl.c: fix migration failure for 3.1 and older machine types MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Migration from QEMU(v4.0) fails when using 3.1 or older machine type. For example if one attempts to migrate QEMU-2.12 started as qemu-system-ppc64 -nodefaults -M pseries-2.12 -m 4096 -mem-path /tmp/ to current master, it will fail with qemu-system-ppc64: Unknown ramblock "ppc_spapr.ram", cannot accept migration qemu-system-ppc64: error while loading state for instance 0x0 of device 'ram' qemu-system-ppc64: load of migration failed: Invalid argument Caused by 900c0ba373 commit which switches main RAM allocation to memory backends and the fact in 3.1 and older QEMU, backends used full[***] QOM path as memory region name instead of backend's name. That was changed after 3.1 to use prefix-less names by default (fa0cb34d22) for new machine types. *** effectively makes main RAM memory region names defined by MachineClass::default_ram_id being altered with '/objects/' prefix and therefore migration fails as old QEMU sends prefix-less name while new QEMU expects name with prefix when using 3.1 and older machine types. Fix it by forcing implicit[1] memory backend to always use prefix-less names for its memory region by setting 'x-use-canonical-path-for-ramblock-id' property to false. 1) i.e. memory backend created by compat glue which maps -m/-mem-path/-mem-prealloc/default RAM size into appropriate backend type/options to match old CLI format. Fixes: 900c0ba373 Signed-off-by: Igor Mammedov Reported-by: Lukáš Doktor Message-Id: <20200304172748.15338-1-imammedo@redhat.com> Tested-by: Lukáš Doktor Reviewed-by: Marc-André Lureau Signed-off-by: Dr. David Alan Gilbert --- softmmu/vl.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/softmmu/vl.c b/softmmu/vl.c index 1d33a28340..814537bb42 100644 --- a/softmmu/vl.c +++ b/softmmu/vl.c @@ -2801,6 +2801,9 @@ static void create_default_memdev(MachineState *ms, const char *path) object_property_set_int(obj, ms->ram_size, "size", &error_fatal); object_property_add_child(object_get_objects_root(), mc->default_ram_id, obj, &error_fatal); + /* Ensure backend's memory region name is equal to mc->default_ram_id */ + object_property_set_bool(obj, false, "x-use-canonical-path-for-ramblock-id", + &error_fatal); user_creatable_complete(USER_CREATABLE(obj), &error_fatal); object_unref(obj); object_property_set_str(OBJECT(ms), mc->default_ram_id, "memory-backend", From 27d07fcfa70c3afa0664288cbce5334ed9595a3a Mon Sep 17 00:00:00 2001 From: Vladimir Sementsov-Ogievskiy Date: Tue, 24 Mar 2020 18:36:28 +0300 Subject: [PATCH 7/9] migration/colo: fix use after free of local_err local_err is used again in secondary_vm_do_failover() after replication_stop_all(), so we must zero it. Otherwise try to set non-NULL local_err will crash. Signed-off-by: Vladimir Sementsov-Ogievskiy Message-Id: <20200324153630.11882-5-vsementsov@virtuozzo.com> Reviewed-by: Dr. David Alan Gilbert Signed-off-by: Dr. David Alan Gilbert --- migration/colo.c | 1 + 1 file changed, 1 insertion(+) diff --git a/migration/colo.c b/migration/colo.c index 44942c4e23..a54ac84f41 100644 --- a/migration/colo.c +++ b/migration/colo.c @@ -93,6 +93,7 @@ static void secondary_vm_do_failover(void) replication_stop_all(true, &local_err); if (local_err) { error_report_err(local_err); + local_err = NULL; } /* Notify all filters of all NIC to do checkpoint */ From b4a1733c5e6827c72b0dcfa295e07ef7b1ebccff Mon Sep 17 00:00:00 2001 From: Vladimir Sementsov-Ogievskiy Date: Tue, 24 Mar 2020 18:36:29 +0300 Subject: [PATCH 8/9] migration/ram: fix use after free of local_err local_err is used again in migration_bitmap_sync_precopy() after precopy_notify(), so we must zero it. Otherwise try to set non-NULL local_err will crash. Signed-off-by: Vladimir Sementsov-Ogievskiy Message-Id: <20200324153630.11882-6-vsementsov@virtuozzo.com> Reviewed-by: Dr. David Alan Gilbert Signed-off-by: Dr. David Alan Gilbert --- migration/ram.c | 1 + 1 file changed, 1 insertion(+) diff --git a/migration/ram.c b/migration/ram.c index c12cfdbe26..04f13feb2e 100644 --- a/migration/ram.c +++ b/migration/ram.c @@ -980,6 +980,7 @@ static void migration_bitmap_sync_precopy(RAMState *rs) */ if (precopy_notify(PRECOPY_NOTIFY_BEFORE_BITMAP_SYNC, &local_err)) { error_report_err(local_err); + local_err = NULL; } migration_bitmap_sync(rs); From 7cd75cbdb8a45d9e2d5912f774d8194cbafdfa97 Mon Sep 17 00:00:00 2001 From: Mao Zhongyi Date: Wed, 25 Mar 2020 09:49:30 +0800 Subject: [PATCH 9/9] migration: use "" instead of (null) for tls-authz run: (qemu) info migrate_parameters announce-initial: 50 ms ... announce-max: 550 ms multifd-compression: none xbzrle-cache-size: 4194304 max-postcopy-bandwidth: 0 tls-authz: '(null)' Migration parameter 'tls-authz' is used to provide the QOM ID of a QAuthZ subclass instance that provides the access control check, default is NULL. But the empty string is not a valid object ID, so use "" instead of the default. Although it will fail when lookup an object with ID "", it is harmless, just consistent with tls_creds. As a bonus, this patch also fixed the bad indentation on the last line and removed 'has_tls_authz' redundant check in 'hmp_info_migrate_parameters'. Signed-off-by: Mao Zhongyi Message-Id: <119f539a9f4d198bc3bcced46b8280520d60bc51.1585100802.git.maozhongyi@cmss.chinamobile.com> Reviewed-by: Dr. David Alan Gilbert Signed-off-by: Dr. David Alan Gilbert --- migration/migration.c | 3 ++- monitor/hmp-cmds.c | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/migration/migration.c b/migration/migration.c index 4b26110d57..c4c9aee15e 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -790,7 +790,8 @@ MigrationParameters *qmp_query_migrate_parameters(Error **errp) params->has_tls_hostname = true; params->tls_hostname = g_strdup(s->parameters.tls_hostname); params->has_tls_authz = true; - params->tls_authz = g_strdup(s->parameters.tls_authz); + params->tls_authz = g_strdup(s->parameters.tls_authz ? + s->parameters.tls_authz : ""); params->has_max_bandwidth = true; params->max_bandwidth = s->parameters.max_bandwidth; params->has_downtime_limit = true; diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c index 04ca342c51..9b94e67879 100644 --- a/monitor/hmp-cmds.c +++ b/monitor/hmp-cmds.c @@ -459,9 +459,9 @@ void hmp_info_migrate_parameters(Monitor *mon, const QDict *qdict) monitor_printf(mon, "%s: %" PRIu64 "\n", MigrationParameter_str(MIGRATION_PARAMETER_MAX_POSTCOPY_BANDWIDTH), params->max_postcopy_bandwidth); - monitor_printf(mon, " %s: '%s'\n", + monitor_printf(mon, "%s: '%s'\n", MigrationParameter_str(MIGRATION_PARAMETER_TLS_AUTHZ), - params->has_tls_authz ? params->tls_authz : ""); + params->tls_authz); } qapi_free_MigrationParameters(params);