* Lai Jiangshan (jiangshan...@gmail.com) wrote: > On Fri, Apr 20, 2018 at 12:38 AM, Dr. David Alan Gilbert > <dgilb...@redhat.com> wrote: > > >> -static void ram_list_init_bitmaps(void) > >> +static void ram_list_init_bitmaps(RAMState *rs) > >> { > >> RAMBlock *block; > >> unsigned long pages; > >> @@ -2151,9 +2152,17 @@ static void ram_list_init_bitmaps(void) > >> /* Skip setting bitmap if there is no RAM */ > >> if (ram_bytes_total()) { > >> QLIST_FOREACH_RCU(block, &ram_list.blocks, next) { > >> + if (migrate_bypass_shared_memory() && > >> qemu_ram_is_shared(block)) { > >> + continue; > >> + } > >> pages = block->max_length >> TARGET_PAGE_BITS; > >> block->bmap = bitmap_new(pages); > >> bitmap_set(block->bmap, 0, pages); > >> + /* > >> + * Count the total number of pages used by ram blocks not > >> + * including any gaps due to alignment or unplugs. > >> + */ > >> + rs->migration_dirty_pages += pages; > >> if (migrate_postcopy_ram()) { > >> block->unsentmap = bitmap_new(pages); > >> bitmap_set(block->unsentmap, 0, pages); > > > > Can you please rework this to combine with Cédric Le Goater's > > 'discard non-migratable RAMBlocks' - it's quite similar to what you're > > trying to do but for a different reason; If you look at the v2 from > > April 13, I think you can just find somewhere to clear the > > RAM_MIGRATABLE flag. > > Hello Dave: > > It seems we need to add new qmp/hmp command to clear/add > RAM_MIGRATABLE flag which is overkill for such a simple feature. > Please point out if there is any simple way to do so.
I'm fine with you still using a capability to enable/disable it - I think that part of your patch is fine; but then I think you just need to check that capability somewhere in Cedric's code; perhaps in his qemu_ram_is_migratable? > And this kind of memory is not "un-MIGRATABLE", the user > just decided not to migrate it/them for one of the migrations. > But they are always MIGRATABLE regardless the user migrate > them or not. So clearing/setting the flag may > cause confusion in this case. What do you think? The 'RAM_MIGRATABLE' is just an internal name for the flag; it's not seen by the user; it's as good a name as any. > Bypassing is an option for every migration. For the > same vm instance, the user might migrate it out > multiple times. He wants to bypass shared memory > in some migrations and do the normal migrations in > other times. So it is better that Bypassing is an option > or capability of migration instead of ramblock. > > I don't insist on avoiding using RAM_MIGRATABLE. and so it might be best for you not to change the flag, just to add to qemu_ram_is_migratable. > Thanks, > Lai > > > > > One thing I noticed; in my world I've got some code that checks if we > > ever do a RAM iteration, don't find any dirty blocks but then still have > > migration_dirty_pages being none-0; and with this patch I'm seeing that > > check trigger: > > > > ram_find_and_save_block: no page found, yet dirty_pages=480 > > > > it doesn't seem to trigger without the patch. > > Does initializing the migration_dirty_pages as you suggested help? I've not had a chance to try yet; here is the debug patch I've got: @@ -1594,6 +1594,13 @@ static int ram_find_and_save_block(RAMState *rs, bool last_stage) } } while (!pages && again); + if (!pages && !again && pss.complete_round && rs->migration_dirty_pages) + { + /* Should make this fail migration ? */ + fprintf(stderr, "%s: no page found, yet dirty_pages=%"PRIu64"\n", + __func__, rs->migration_dirty_pages); + } + rs->last_seen_block = pss.block; rs->last_page = pss.page; Dave > > > > Dave > > > >> @@ -2169,7 +2178,7 @@ static void ram_init_bitmaps(RAMState *rs) > >> qemu_mutex_lock_ramlist(); > >> rcu_read_lock(); > >> > >> - ram_list_init_bitmaps(); > >> + ram_list_init_bitmaps(rs); > >> memory_global_dirty_log_start(); > >> migration_bitmap_sync(rs); > >> > >> diff --git a/qapi/migration.json b/qapi/migration.json > >> index 9d0bf82cf4..45326480bd 100644 > >> --- a/qapi/migration.json > >> +++ b/qapi/migration.json > >> @@ -357,13 +357,17 @@ > >> # @dirty-bitmaps: If enabled, QEMU will migrate named dirty bitmaps. > >> # (since 2.12) > >> # > >> +# @bypass-shared-memory: the shared memory region will be bypassed on > >> migration. > >> +# This feature allows the memory region to be reused by new > >> qemu(s) > >> +# or be migrated separately. (since 2.13) > >> +# > >> # Since: 1.2 > >> ## > >> { 'enum': 'MigrationCapability', > >> 'data': ['xbzrle', 'rdma-pin-all', 'auto-converge', 'zero-blocks', > >> 'compress', 'events', 'postcopy-ram', 'x-colo', 'release-ram', > >> 'block', 'return-path', 'pause-before-switchover', 'x-multifd', > >> - 'dirty-bitmaps' ] } > >> + 'dirty-bitmaps', 'bypass-shared-memory' ] } > >> > >> ## > >> # @MigrationCapabilityStatus: > >> -- > >> 2.15.1 (Apple Git-101) > >> > > -- > > Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK