From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([209.51.188.92]:41476) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ghtiH-0005eB-9J for qemu-devel@nongnu.org; Fri, 11 Jan 2019 05:06:54 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ghtiG-0007Bb-B8 for qemu-devel@nongnu.org; Fri, 11 Jan 2019 05:06:53 -0500 Received: from mx1.redhat.com ([209.132.183.28]:49696) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1ghtiG-0007AK-2o for qemu-devel@nongnu.org; Fri, 11 Jan 2019 05:06:52 -0500 Date: Fri, 11 Jan 2019 11:06:44 +0100 From: Igor Mammedov Message-ID: <20190111110644.19896d4d@redhat.com> In-Reply-To: <20190110201418.GK2589@work-vm> References: <20190110120120.9943-1-yury-kotov@yandex-team.ru> <20190110120120.9943-2-yury-kotov@yandex-team.ru> <20190110201418.GK2589@work-vm> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 1/4] migration: add RAMBlock's offset validation List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Dr. David Alan Gilbert" Cc: Yury Kotov , qemu-devel@nongnu.org, Eduardo Habkost , Paolo Bonzini , Peter Crosthwaite , Richard Henderson , Juan Quintela , Eric Blake , Markus Armbruster , Thomas Huth , Laurent Vivier , wrfsh@yandex-team.ru On Thu, 10 Jan 2019 20:14:19 +0000 "Dr. David Alan Gilbert" wrote: > * Yury Kotov (yury-kotov@yandex-team.ru) wrote: > > RAM migration has a RAMBlock validation stage (flag RAM_SAVE_FLAG_MEM_SIZE). > > In this stage QEMU checks further information about RAMBlock: > > 1. Presence (by idstr), > > 2. Length (trying to resize, when differs), > > 3. Optional page size. > > > > This patch adds a check for RAMBlock's offset. Currently we check it during > > RAM pages loading - every RAM page has an offset in its header. But there is a > > case when we don't send RAM pages (see below). > > > > The following commits introduce a capability (ignore-external) to skip some > > RAM blocks from migration. In such case the migration stream contains only > > meta information about RAM blocks to validate them. So, the only way to check > > block's offset is to send it explicitly. > > > > Signed-off-by: Yury Kotov > > But why check that offsets match? THey aren't supposed to! > Offset's are entirely private to each qemu and they're allowed to be > different; the only requirement is that the length and name of each > RAMBlock matches, then all the operations we do over the migration > stream are relative to the start of the block. > > One example where they are validly different is where you hotplug some > RAM, so for example: > > > source qemu > -M 4G > hotplug PCI card > hotplug 2G > > destination qemu > -M 4G > PCI card declared on the command line > extra 2G declared on the command line > > The offsets are different but we can migrate that case fine. PCI mappings are updated after migration is done, to make DST match SRC (get_pci_config_device) that mapping is irrelevant here, but I was under impression that offset in terms of MemoryRegion (GPA) was passed in migration stream, even if we do not update memory mappings for RAM (mapping depends on hotplug/CLI order or explicit addr on CLI for memory devices). > > Dave > > > --- > > migration/ram.c | 15 +++++++++++++-- > > 1 file changed, 13 insertions(+), 2 deletions(-) > > > > diff --git a/migration/ram.c b/migration/ram.c > > index 7e7deec4d8..39629254e1 100644 > > --- a/migration/ram.c > > +++ b/migration/ram.c > > @@ -3171,6 +3171,7 @@ static int ram_save_setup(QEMUFile *f, void *opaque) > > if (migrate_postcopy_ram() && block->page_size != qemu_host_page_size) { > > qemu_put_be64(f, block->page_size); > > } > > + qemu_put_be64(f, block->offset); > > } > > > > rcu_read_unlock(); > > @@ -4031,7 +4032,7 @@ static int ram_load(QEMUFile *f, void *opaque, int version_id) > > > > seq_iter++; > > > > - if (version_id != 4) { > > + if (version_id < 4) { > > ret = -EINVAL; > > } > > > > @@ -4132,6 +4133,16 @@ static int ram_load(QEMUFile *f, void *opaque, int version_id) > > ret = -EINVAL; > > } > > } > > + if (version_id >= 5) { > > + ram_addr_t offset; > > + offset = qemu_get_be64(f); > > + if (block->offset != offset) { > > + error_report("Mismatched RAM block offset %s " > > + "%" PRId64 "!= %" PRId64, > > + id, offset, (uint64_t)block->offset); > > + ret = -EINVAL; > > + } > > + } > > ram_control_load_hook(f, RAM_CONTROL_BLOCK_REG, > > block->idstr); > > } else { > > @@ -4363,5 +4374,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(NULL, "ram", 0, 5, &savevm_ram_handlers, &ram_state); > > } > > -- > > 2.20.1 > > > -- > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK