From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38316) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z8QA1-0005wy-2I for qemu-devel@nongnu.org; Fri, 26 Jun 2015 05:43:02 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Z8Q9r-0006f9-4e for qemu-devel@nongnu.org; Fri, 26 Jun 2015 05:43:01 -0400 Received: from [59.151.112.132] (port=47288 helo=heian.cn.fujitsu.com) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z8Q9o-0006eQ-73 for qemu-devel@nongnu.org; Fri, 26 Jun 2015 05:42:51 -0400 Message-ID: <558D1E8F.6000204@cn.fujitsu.com> Date: Fri, 26 Jun 2015 17:42:39 +0800 From: Li Zhijian MIME-Version: 1.0 References: <1435303314-10021-1-git-send-email-lizhijian@cn.fujitsu.com> <1435303314-10021-3-git-send-email-lizhijian@cn.fujitsu.com> <87d20i7sbw.fsf@neno.neno> In-Reply-To: <87d20i7sbw.fsf@neno.neno> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 2/2] migration: extend migration_bitmap List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: quintela@redhat.com Cc: amit.shah@redhat.com, pbonzini@redhat.com, qemu-devel@nongnu.org On 06/26/2015 05:05 PM, Juan Quintela wrote: > Li Zhijian wrote: >> Prevously, if we hotplug a device(e.g. device_add e1000) during >> migration is processing in source side, qemu will add a new ram >> block but migration_bitmap is not extended. >> In this case, migration_bitmap will overflow and lead qemu abort >> unexpectedly. >> >> Signed-off-by: Li Zhijian >> Signed-off-by: Wen Congyang > Just curious, how are you testing this? > because you need a way of doing the hot-plug "kind of" atomically on > both source and destination, no? > > >> --- >> exec.c | 7 ++++++- >> include/exec/exec-all.h | 1 + >> migration/ram.c | 16 ++++++++++++++++ >> 3 files changed, 23 insertions(+), 1 deletion(-) >> >> diff --git a/exec.c b/exec.c >> index f7883d2..04d5c05 100644 >> --- a/exec.c >> +++ b/exec.c >> @@ -1401,6 +1401,11 @@ static ram_addr_t ram_block_add(RAMBlock *new_block, Error **errp) >> } >> } >> >> + new_ram_size = MAX(old_ram_size, >> + (new_block->offset + new_block->max_length) >> TARGET_PAGE_BITS); >> + if (new_ram_size > old_ram_size) { >> + migration_bitmap_extend(old_ram_size, new_ram_size); >> + } >> /* Keep the list sorted from biggest to smallest block. Unlike QTAILQ, >> * QLIST (which has an RCU-friendly variant) does not have insertion at >> * tail, so save the last element in last_block. >> @@ -1435,7 +1440,7 @@ static ram_addr_t ram_block_add(RAMBlock *new_block, Error **errp) >> ram_list.dirty_memory[i] = >> bitmap_zero_extend(ram_list.dirty_memory[i], >> old_ram_size, new_ram_size); >> - } >> + } > Whitespace noise > >> } >> cpu_physical_memory_set_dirty_range(new_block->offset, >> new_block->used_length, >> diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h >> index 2573e8c..dd9be44 100644 >> --- a/include/exec/exec-all.h >> +++ b/include/exec/exec-all.h >> @@ -385,4 +385,5 @@ static inline bool cpu_can_do_io(CPUState *cpu) >> return cpu->can_do_io != 0; >> } >> >> +void migration_bitmap_extend(ram_addr_t old, ram_addr_t new); >> #endif >> diff --git a/migration/ram.c b/migration/ram.c >> index 4754aa9..70dd8da 100644 >> --- a/migration/ram.c >> +++ b/migration/ram.c >> @@ -1063,6 +1063,22 @@ static void reset_ram_globals(void) >> >> #define MAX_WAIT 50 /* ms, half buffered_file limit */ >> >> +void migration_bitmap_extend(ram_addr_t old, ram_addr_t new) >> +{ >> + qemu_mutex_lock(&migration_bitmap_mutex); >> + if (migration_bitmap) { >> + unsigned long *old_bitmap = migration_bitmap, *bitmap; >> + bitmap = bitmap_new(new); >> + bitmap_set(bitmap, old, new - old); >> + memcpy(bitmap, old_bitmap, >> + BITS_TO_LONGS(old) * sizeof(unsigned long)); > Shouldn't the last two sentences be reversed? memcpy could "potentially" > overwrote part of the bits setted on bitmap_set. (notice the > potentially part, my guess is that we never get a bitmap that is not > word aligned, but well ....) reverse the last two sentences would be better. i will fix it in next version. Thanks Li Zhijian > My understanding of the rest look right. > > Later, Juan. > . >