From nobody Sat Oct 25 08:50:06 2025 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) client-ip=208.118.235.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; dmarc=fail(p=none dis=none) header.from=redhat.com Return-Path: Received: from lists.gnu.org (208.118.235.17 [208.118.235.17]) by mx.zohomail.com with SMTPS id 1518623124414910.0557741481282; Wed, 14 Feb 2018 07:45:24 -0800 (PST) Received: from localhost ([::1]:33307 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1elzFD-0005OM-BX for importer@patchew.org; Wed, 14 Feb 2018 10:45:15 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53921) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1elzA3-0008IW-BZ for qemu-devel@nongnu.org; Wed, 14 Feb 2018 10:39:59 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1elzA0-00076E-UG for qemu-devel@nongnu.org; Wed, 14 Feb 2018 10:39:55 -0500 Received: from mx3-rdu2.redhat.com ([66.187.233.73]:40572 helo=mx1.redhat.com) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1elzA0-000763-L0 for qemu-devel@nongnu.org; Wed, 14 Feb 2018 10:39:52 -0500 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.rdu2.redhat.com [10.11.54.5]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 26BEB407244E; Wed, 14 Feb 2018 15:39:52 +0000 (UTC) Received: from dgilbert-t530.redhat.com (ovpn-116-251.ams2.redhat.com [10.36.116.251]) by smtp.corp.redhat.com (Postfix) with ESMTP id 42092F41A2; Wed, 14 Feb 2018 15:39:51 +0000 (UTC) From: "Dr. David Alan Gilbert (git)" To: qemu-devel@nongnu.org, quintela@redhat.com, peterx@redhat.com, groug@kaod.org, ross.lagerwall@citrix.com Date: Wed, 14 Feb 2018 15:39:34 +0000 Message-Id: <20180214153938.5410-7-dgilbert@redhat.com> In-Reply-To: <20180214153938.5410-1-dgilbert@redhat.com> References: <20180214153938.5410-1-dgilbert@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.11.54.5 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.5]); Wed, 14 Feb 2018 15:39:52 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.5]); Wed, 14 Feb 2018 15:39:52 +0000 (UTC) for IP:'10.11.54.5' DOMAIN:'int-mx05.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'dgilbert@redhat.com' RCPT:'' X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 66.187.233.73 Subject: [Qemu-devel] [PULL 06/10] migration: better error handling with QEMUFile X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" From: Peter Xu If the postcopy down due to some reason, we can always see this on dst: qemu-system-x86_64: RP: Received invalid message 0x0000 length 0x0000 However in most cases that's not the real issue. The problem is that qemu_get_be16() has no way to show whether the returned data is valid or not, and we are _always_ assuming it is valid. That's possibly not wise. The best approach to solve this would be: refactoring QEMUFile interface to allow the APIs to return error if there is. However it needs quite a bit of work and testing. For now, let's explicitly check the validity first before using the data in all places for qemu_get_*(). This patch tries to fix most of the cases I can see. Only if we are with this, can we make sure we are processing the valid data, and also can we make sure we can capture the channel down events correctly. Reviewed-by: Dr. David Alan Gilbert Signed-off-by: Peter Xu Message-Id: <20180208103132.28452-2-peterx@redhat.com> Signed-off-by: Dr. David Alan Gilbert --- migration/migration.c | 5 +++++ migration/ram.c | 21 +++++++++++++++++---- migration/savevm.c | 40 ++++++++++++++++++++++++++++++++++++++-- 3 files changed, 60 insertions(+), 6 deletions(-) diff --git a/migration/migration.c b/migration/migration.c index 86d69120a6..e2a5a832c6 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -1709,6 +1709,11 @@ static void *source_return_path_thread(void *opaque) header_type =3D qemu_get_be16(rp); header_len =3D qemu_get_be16(rp); =20 + if (qemu_file_get_error(rp)) { + mark_source_rp_bad(ms); + goto out; + } + if (header_type >=3D MIG_RP_MSG_MAX || header_type =3D=3D MIG_RP_MSG_INVALID) { error_report("RP: Received invalid message 0x%04x length 0x%04= x", diff --git a/migration/ram.c b/migration/ram.c index 7095c1040e..5e33e5cc79 100644 --- a/migration/ram.c +++ b/migration/ram.c @@ -2700,6 +2700,16 @@ static int ram_load_postcopy(QEMUFile *f) uint8_t ch; =20 addr =3D qemu_get_be64(f); + + /* + * If qemu file error, we should stop here, and then "addr" + * may be invalid + */ + ret =3D qemu_file_get_error(f); + if (ret) { + break; + } + flags =3D addr & ~TARGET_PAGE_MASK; addr &=3D TARGET_PAGE_MASK; =20 @@ -2780,9 +2790,15 @@ static int ram_load_postcopy(QEMUFile *f) error_report("Unknown combination of migration flags: %#x" " (postcopy mode)", flags); ret =3D -EINVAL; + break; + } + + /* Detect for any possible file errors */ + if (!ret && qemu_file_get_error(f)) { + ret =3D qemu_file_get_error(f); } =20 - if (place_needed) { + if (!ret && place_needed) { /* This gets called at the last target page in the host page */ void *place_dest =3D host + TARGET_PAGE_SIZE - block->page_siz= e; =20 @@ -2794,9 +2810,6 @@ static int ram_load_postcopy(QEMUFile *f) place_source, block); } } - if (!ret) { - ret =3D qemu_file_get_error(f); - } } =20 return ret; diff --git a/migration/savevm.c b/migration/savevm.c index 68b652ff76..967c3bca0d 100644 --- a/migration/savevm.c +++ b/migration/savevm.c @@ -1781,6 +1781,11 @@ static int loadvm_process_command(QEMUFile *f) cmd =3D qemu_get_be16(f); len =3D qemu_get_be16(f); =20 + /* Check validity before continue processing of cmds */ + if (qemu_file_get_error(f)) { + return qemu_file_get_error(f); + } + trace_loadvm_process_command(cmd, len); if (cmd >=3D MIG_CMD_MAX || cmd =3D=3D MIG_CMD_INVALID) { error_report("MIG_CMD 0x%x unknown (len 0x%x)", cmd, len); @@ -1846,6 +1851,7 @@ static int loadvm_process_command(QEMUFile *f) */ static bool check_section_footer(QEMUFile *f, SaveStateEntry *se) { + int ret; uint8_t read_mark; uint32_t read_section_id; =20 @@ -1856,6 +1862,13 @@ static bool check_section_footer(QEMUFile *f, SaveSt= ateEntry *se) =20 read_mark =3D qemu_get_byte(f); =20 + ret =3D qemu_file_get_error(f); + if (ret) { + error_report("%s: Read section footer failed: %d", + __func__, ret); + return false; + } + if (read_mark !=3D QEMU_VM_SECTION_FOOTER) { error_report("Missing section footer for %s", se->idstr); return false; @@ -1891,6 +1904,13 @@ qemu_loadvm_section_start_full(QEMUFile *f, Migratio= nIncomingState *mis) instance_id =3D qemu_get_be32(f); version_id =3D qemu_get_be32(f); =20 + ret =3D qemu_file_get_error(f); + if (ret) { + error_report("%s: Failed to read instance/version ID: %d", + __func__, ret); + return ret; + } + trace_qemu_loadvm_state_section_startfull(section_id, idstr, instance_id, version_id); /* Find savevm section */ @@ -1938,6 +1958,13 @@ qemu_loadvm_section_part_end(QEMUFile *f, MigrationI= ncomingState *mis) =20 section_id =3D qemu_get_be32(f); =20 + ret =3D qemu_file_get_error(f); + if (ret) { + error_report("%s: Failed to read section ID: %d", + __func__, ret); + return ret; + } + trace_qemu_loadvm_state_section_partend(section_id); QTAILQ_FOREACH(se, &savevm_state.handlers, entry) { if (se->load_section_id =3D=3D section_id) { @@ -2005,8 +2032,14 @@ static int qemu_loadvm_state_main(QEMUFile *f, Migra= tionIncomingState *mis) uint8_t section_type; int ret =3D 0; =20 - while ((section_type =3D qemu_get_byte(f)) !=3D QEMU_VM_EOF) { - ret =3D 0; + while (true) { + section_type =3D qemu_get_byte(f); + + if (qemu_file_get_error(f)) { + ret =3D qemu_file_get_error(f); + break; + } + trace_qemu_loadvm_state_section(section_type); switch (section_type) { case QEMU_VM_SECTION_START: @@ -2030,6 +2063,9 @@ static int qemu_loadvm_state_main(QEMUFile *f, Migrat= ionIncomingState *mis) goto out; } break; + case QEMU_VM_EOF: + /* This is the end of migration */ + goto out; default: error_report("Unknown savevm section type %d", section_type); ret =3D -EINVAL; --=20 2.14.3