From e2ef4fc7aeb7e5856594f3172e08e6650011084a Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Tue, 20 Jul 2021 14:53:54 +0200 Subject: spapr: Plug memory leak when we can't add a migration blocker MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes: 2500fb423adb17995485de0b4d507cf2f09e3a7f Cc: Aravinda Prasad Cc: Ganesh Goudar Cc: David Gibson Signed-off-by: Markus Armbruster Message-Id: <20210720125408.387910-3-armbru@redhat.com> Acked-by: Michael S. Tsirkin Reviewed-by: Philippe Mathieu-Daudé --- hw/ppc/spapr_events.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'hw/ppc') diff --git a/hw/ppc/spapr_events.c b/hw/ppc/spapr_events.c index 23e2e2fff1..690533cbdc 100644 --- a/hw/ppc/spapr_events.c +++ b/hw/ppc/spapr_events.c @@ -872,7 +872,6 @@ void spapr_mce_req_event(PowerPCCPU *cpu, bool recovered) SpaprMachineState *spapr = SPAPR_MACHINE(qdev_get_machine()); CPUState *cs = CPU(cpu); int ret; - Error *local_err = NULL; if (spapr->fwnmi_machine_check_addr == -1) { /* Non-FWNMI case, deliver it like an architected CPU interrupt. */ @@ -912,7 +911,7 @@ void spapr_mce_req_event(PowerPCCPU *cpu, bool recovered) } } - ret = migrate_add_blocker(spapr->fwnmi_migration_blocker, &local_err); + ret = migrate_add_blocker(spapr->fwnmi_migration_blocker, NULL); if (ret == -EBUSY) { /* * We don't want to abort so we let the migration to continue. -- cgit v1.2.3 From d7f5013e122e14c6f5ac8d973e6567413cfa3790 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Tue, 20 Jul 2021 14:53:55 +0200 Subject: spapr: Explain purpose of ->fwnmi_migration_blocker more clearly spapr_mce_req_event() makes an effort to prevent migration from degrading the reporting of FWNMIs. It adds a migration blocker when it receives one, and deletes it when it's done handling it. This is a best effort. Commit 2500fb423a "migration: Include migration support for machine check handling" tried to explain this in a comment. Rewrite the comment for clarity, and reposition it to make it clear it applies to all failure modes, not just "migration already in progress". Cc: David Gibson Cc: Aravinda Prasad Cc: Ganesh Goudar Cc: Dr. David Alan Gilbert Signed-off-by: Markus Armbruster Message-Id: <20210720125408.387910-4-armbru@redhat.com> Acked-by: Michael S. Tsirkin --- hw/ppc/spapr_events.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) (limited to 'hw/ppc') diff --git a/hw/ppc/spapr_events.c b/hw/ppc/spapr_events.c index 690533cbdc..630e86282c 100644 --- a/hw/ppc/spapr_events.c +++ b/hw/ppc/spapr_events.c @@ -911,16 +911,17 @@ void spapr_mce_req_event(PowerPCCPU *cpu, bool recovered) } } + /* + * Try to block migration while FWNMI is being handled, so the + * machine check handler runs where the information passed to it + * actually makes sense. This shouldn't actually block migration, + * only delay it slightly, assuming migration is retried. If the + * attempt to block fails, carry on. Unfortunately, it always + * fails when running with -only-migrate. A proper interface to + * delay migration completion for a bit could avoid that. + */ ret = migrate_add_blocker(spapr->fwnmi_migration_blocker, NULL); if (ret == -EBUSY) { - /* - * We don't want to abort so we let the migration to continue. - * In a rare case, the machine check handler will run on the target. - * Though this is not preferable, it is better than aborting - * the migration or killing the VM. It is okay to call - * migrate_del_blocker on a blocker that was not added (which the - * nmi-interlock handler would do when it's called after this). - */ warn_report("Received a fwnmi while migration was in progress"); } -- cgit v1.2.3