From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:36814) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YMKFh-0000aR-GT for qemu-devel@nongnu.org; Fri, 13 Feb 2015 12:42:06 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YMKFe-0006B6-3h for qemu-devel@nongnu.org; Fri, 13 Feb 2015 12:42:05 -0500 Received: from mx2.parallels.com ([199.115.105.18]:34917) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YMKFd-0005hU-Uk for qemu-devel@nongnu.org; Fri, 13 Feb 2015 12:42:02 -0500 Message-ID: <54DE372E.7090905@parallels.com> Date: Fri, 13 Feb 2015 20:41:02 +0300 From: Vladimir Sementsov-Ogievskiy MIME-Version: 1.0 References: <1422356197-5285-1-git-send-email-vsementsov@parallels.com> <1422356197-5285-9-git-send-email-vsementsov@parallels.com> <54DA793C.9020707@redhat.com> <54DDB398.3010907@parallels.com> <54DDBEA5.3020506@parallels.com> <54DE3540.3060104@redhat.com> In-Reply-To: <54DE3540.3060104@redhat.com> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH RFC v2 8/8] migration: add migration/dirty-bitmap.c List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: John Snow , qemu-devel@nongnu.org Cc: kwolf@redhat.com, Peter Maydell , "Juan quin >> Juan Jose Quintela Carreira" , "Dr. David Alan Gilbert" , stefanha@redhat.com, pbonzini@redhat.com, amit Shah , den@openvz.org On 13.02.2015 20:32, John Snow wrote: > > > On 02/13/2015 04:06 AM, Vladimir Sementsov-Ogievskiy wrote: >>>> >>>>> + >>>>> + blk_mig_reset_dirty_cursor(); >>>>> + dirty_phase(f, false); >>>>> + >>>>> + QSIMPLEQ_FOREACH(dbms, &dirty_bitmap_mig_state.dbms_list, >>>>> entry) { >>>>> + uint8_t flags = DIRTY_BITMAP_MIG_FLAG_DEVICE_NAME | >>>>> + DIRTY_BITMAP_MIG_FLAG_BITMAP_NAME | >>>>> + DIRTY_BITMAP_MIG_FLAG_ENABLED; >>>>> + >>>>> + qemu_put_byte(f, flags); >>>>> + qemu_put_name(f, bdrv_get_device_name(dbms->bs)); >>>>> + qemu_put_name(f, bdrv_dirty_bitmap_name(dbms->bitmap)); >>>>> + qemu_put_byte(f, bdrv_dirty_bitmap_enabled(dbms->bitmap)); >>>>> + } >>>>> + >>>>> + qemu_put_byte(f, DIRTY_BITMAP_MIG_FLAG_EOS); >>>>> + >>>>> + DPRINTF("Dirty bitmaps migration completed\n"); >>>>> + >>>>> + dirty_bitmap_mig_cleanup(); >>>>> + return 0; >>>>> +} >>>>> + >>>> >>>> I suppose we don't need a flag that distinctly SAYS this is the end >>>> section, since we can tell by omission of >>>> DIRTY_BITMAP_MIG_FLAG_NORMAL_CHUNK or ZERO_CHUNK. >>> Hmm. I think it simplifies the logic (to use EOS after each section). >>> And the same approach is in migration/block.c.. It's a question about >>> which format is better: "Each section for dirty_bitmap_load ends with >>> EOS" or "Each section for dirty_bitmap_load ends with EOS except the >>> last one. The last one may be recognized by absent NORMAL_CHUNK and >>> ZERO_CHUNK" >> >> Oh, sorry, no, it's important EOS. There are several blocks with no >> *_CHUNK! Several bitmaps. And loop in dirty_bitmap_load will read them >> iteratively, and it will finish when find EOS. >> >> > > Sorry, I worded that poorly. I was wondering why you didn't have an > explicit "end of bitmap" flag, and I realized that you are doing this > check essentially by the absence of the NORMAL_CHUNK/ZERO_CHUNK flags. > > This is really just a comment on my part; I was expecting a more > distinct "It is now safe to rebuild the bitmap" flag and was just > commenting on why we didn't necessarily need one. > > I think in another comment I point out that an "end of bitmap" flag > might make the loading function simpler, and I still think that might > be nice. Ok. Today I've refactored these things, there would be separate start and complete frames of bitmap, the first with additional granularity field (without any dirty bitmap data) and the second with additional enabled field (also without data). -- Best regards, Vladimir