From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:36803) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cVxL5-0001XS-6B for qemu-devel@nongnu.org; Tue, 24 Jan 2017 04:24:32 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1cVxL4-00050b-A4 for qemu-devel@nongnu.org; Tue, 24 Jan 2017 04:24:31 -0500 From: Juan Quintela In-Reply-To: <1479837270-79005-4-git-send-email-vsementsov@virtuozzo.com> (Vladimir Sementsov-Ogievskiy's message of "Tue, 22 Nov 2016 20:54:16 +0300") References: <1479837270-79005-1-git-send-email-vsementsov@virtuozzo.com> <1479837270-79005-4-git-send-email-vsementsov@virtuozzo.com> Reply-To: quintela@redhat.com Date: Tue, 24 Jan 2017 10:24:20 +0100 Message-ID: <87mvegajzv.fsf@emacs.mitica> MIME-Version: 1.0 Content-Type: text/plain Subject: Re: [Qemu-devel] [PATCH 03/17] migration: split common postcopy out of ram postcopy List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Vladimir Sementsov-Ogievskiy Cc: qemu-block@nongnu.org, qemu-devel@nongnu.org, pbonzini@redhat.com, armbru@redhat.com, eblake@redhat.com, famz@redhat.com, stefanha@redhat.com, amit.shah@redhat.com, mreitz@redhat.com, kwolf@redhat.com, peter.maydell@linaro.org, dgilbert@redhat.com, den@openvz.org, jsnow@redhat.com, lirans@il.ibm.com Vladimir Sementsov-Ogievskiy wrote: > Split common postcopy staff from ram postcopy staff. > > Signed-off-by: Vladimir Sementsov-Ogievskiy > --- > include/migration/migration.h | 1 + > migration/migration.c | 39 +++++++++++++++++++++++++++------------ > migration/postcopy-ram.c | 4 +++- > migration/savevm.c | 31 ++++++++++++++++++++++--------- > 4 files changed, 53 insertions(+), 22 deletions(-) > Hi { > MigrationState *s; > @@ -1587,9 +1592,11 @@ static int postcopy_start(MigrationState *ms, bool *old_vm_running) > * need to tell the destination to throw any pages it's already received > * that are dirty > */ > - if (ram_postcopy_send_discard_bitmap(ms)) { > - error_report("postcopy send discard bitmap failed"); > - goto fail; > + if (migrate_postcopy_ram()) { > + if (ram_postcopy_send_discard_bitmap(ms)) { > + error_report("postcopy send discard bitmap failed"); > + goto fail; > + } I will have preffered that for the ram commands, to embed the migrate_postocpy_ram() check inside them, but that is taste, and everyone has its own O:-) > diff --git a/migration/savevm.c b/migration/savevm.c > index e436cb2..c8a71c8 100644 > --- a/migration/savevm.c > +++ b/migration/savevm.c > @@ -73,7 +73,7 @@ static struct mig_cmd_args { > [MIG_CMD_INVALID] = { .len = -1, .name = "INVALID" }, > [MIG_CMD_OPEN_RETURN_PATH] = { .len = 0, .name = "OPEN_RETURN_PATH" }, > [MIG_CMD_PING] = { .len = sizeof(uint32_t), .name = "PING" }, > - [MIG_CMD_POSTCOPY_ADVISE] = { .len = 16, .name = "POSTCOPY_ADVISE" }, > + [MIG_CMD_POSTCOPY_ADVISE] = { .len = -1, .name = "POSTCOPY_ADVISE" }, > [MIG_CMD_POSTCOPY_LISTEN] = { .len = 0, .name = "POSTCOPY_LISTEN" }, > [MIG_CMD_POSTCOPY_RUN] = { .len = 0, .name = "POSTCOPY_RUN" }, > [MIG_CMD_POSTCOPY_RAM_DISCARD] = { > @@ -827,12 +827,17 @@ int qemu_savevm_send_packaged(QEMUFile *f, const uint8_t *buf, size_t len) > /* Send prior to any postcopy transfer */ > void qemu_savevm_send_postcopy_advise(QEMUFile *f) > { > - uint64_t tmp[2]; > - tmp[0] = cpu_to_be64(getpagesize()); > - tmp[1] = cpu_to_be64(1ul << qemu_target_page_bits()); > + if (migrate_postcopy_ram()) { > + uint64_t tmp[2]; > + tmp[0] = cpu_to_be64(getpagesize()); > + tmp[1] = cpu_to_be64(1ul << qemu_target_page_bits()); > > - trace_qemu_savevm_send_postcopy_advise(); > - qemu_savevm_command_send(f, MIG_CMD_POSTCOPY_ADVISE, 16, (uint8_t *)tmp); > + trace_qemu_savevm_send_postcopy_advise(); > + qemu_savevm_command_send(f, MIG_CMD_POSTCOPY_ADVISE, > + 16, (uint8_t *)tmp); > + } else { > + qemu_savevm_command_send(f, MIG_CMD_POSTCOPY_ADVISE, 0, NULL); > + } > } > > /* Sent prior to starting the destination running in postcopy, discard pages I haven't yet figured out why you are reusing this command with a different number of parameters. For this to pass, I need that Dave comment on this. So, Reviewed-by: Juan Quintela conditioned that Dave agrees with this.