From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39392) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cVxWQ-0002je-Gi for qemu-devel@nongnu.org; Tue, 24 Jan 2017 04:36:15 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1cVxWP-00088l-H4 for qemu-devel@nongnu.org; Tue, 24 Jan 2017 04:36:14 -0500 References: <1479837270-79005-1-git-send-email-vsementsov@virtuozzo.com> <1479837270-79005-4-git-send-email-vsementsov@virtuozzo.com> <87mvegajzv.fsf@emacs.mitica> From: Vladimir Sementsov-Ogievskiy Message-ID: <7890fa87-faa1-fd99-e8c8-76b51f039ad6@virtuozzo.com> Date: Tue, 24 Jan 2017 12:35:57 +0300 MIME-Version: 1.0 In-Reply-To: <87mvegajzv.fsf@emacs.mitica> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit 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: quintela@redhat.com 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 24.01.2017 12:24, Juan Quintela wrote: > 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. These parameters are unrelated if ram postcopy is disabled. So, I should be better to have a possibility of skipping them, then to send unneeded numbers and write separate code to read them from the stream (rewriting loadvm_postcopy_handle_advise to just read these two numbers and do nothing about ram postcopy for this case). -- Best regards, Vladimir