From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([209.51.188.92]:52039) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ghuWH-0004yu-LW for qemu-devel@nongnu.org; Fri, 11 Jan 2019 05:58:34 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ghuWE-0006cu-7D for qemu-devel@nongnu.org; Fri, 11 Jan 2019 05:58:32 -0500 Received: from mx1.redhat.com ([209.132.183.28]:41462) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1ghuW8-0006aa-Tr for qemu-devel@nongnu.org; Fri, 11 Jan 2019 05:58:27 -0500 Date: Fri, 11 Jan 2019 10:58:15 +0000 From: "Dr. David Alan Gilbert" Message-ID: <20190111105815.GC2738@work-vm> References: <20190110120120.9943-1-yury-kotov@yandex-team.ru> <20190110120120.9943-2-yury-kotov@yandex-team.ru> <20190110201418.GK2589@work-vm> <20190111110644.19896d4d@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20190111110644.19896d4d@redhat.com> 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: Igor Mammedov 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 * Igor Mammedov (imammedo@redhat.com) wrote: > 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). The GPA must match, but the offset within ram_addr_t space is abstract and isn't visible to the guest. Dave > > > > 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 > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK