From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35422) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YMK6t-0003gW-Nu for qemu-devel@nongnu.org; Fri, 13 Feb 2015 12:33:00 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YMK6m-00031Y-Ip for qemu-devel@nongnu.org; Fri, 13 Feb 2015 12:32:59 -0500 Received: from mx1.redhat.com ([209.132.183.28]:45398) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YMK6m-00031E-5L for qemu-devel@nongnu.org; Fri, 13 Feb 2015 12:32:52 -0500 Message-ID: <54DE3540.3060104@redhat.com> Date: Fri, 13 Feb 2015 12:32:48 -0500 From: John Snow 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> In-Reply-To: <54DDBEA5.3020506@parallels.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: Vladimir Sementsov-Ogievskiy , 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 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.