|
From: | Vladimir Sementsov-Ogievskiy |
Subject: | Re: [Qemu-block] [PATCH v10 05/12] migration: introduce postcopy-only pending |
Date: | Tue, 13 Mar 2018 16:11:44 +0300 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0 |
13.03.2018 13:30, Dr. David Alan
Gilbert wrote:
* Vladimir Sementsov-Ogievskiy (address@hidden) wrote:12.03.2018 18:30, Dr. David Alan Gilbert wrote:* Vladimir Sementsov-Ogievskiy (address@hidden) wrote:There would be savevm states (dirty-bitmap) which can migrate only in postcopy stage. The corresponding pending is introduced here. Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden> ---[...]static MigIterateState migration_iteration_run(MigrationState *s) { - uint64_t pending_size, pend_post, pend_nonpost; + uint64_t pending_size, pend_pre, pend_compat, pend_post; bool in_postcopy = s->state == MIGRATION_STATUS_POSTCOPY_ACTIVE; - qemu_savevm_state_pending(s->to_dst_file, s->threshold_size, - &pend_nonpost, &pend_post); - pending_size = pend_nonpost + pend_post; + qemu_savevm_state_pending(s->to_dst_file, s->threshold_size, &pend_pre, + &pend_compat, &pend_post); + pending_size = pend_pre + pend_compat + pend_post; trace_migrate_pending(pending_size, s->threshold_size, - pend_post, pend_nonpost); + pend_pre, pend_compat, pend_post); if (pending_size && pending_size >= s->threshold_size) { /* Still a significant amount to transfer */ if (migrate_postcopy() && !in_postcopy && - pend_nonpost <= s->threshold_size && - atomic_read(&s->start_postcopy)) { + pend_pre <= s->threshold_size && + (atomic_read(&s->start_postcopy) || + (pend_pre + pend_compat <= s->threshold_size)))This change does something different from the description; it causes a postcopy_start even if the user never ran the postcopy-start command; so sorry, we can't do that; because postcopy for RAM is something that users can enable but only switch into when they've given up on it completing normally. However, I guess that leaves you with a problem; which is what happens to the system when you've run out of pend_pre+pend_compat but can't complete because pend_post is non-0; so I don't know the answer to that.Hmm. Here, we go to postcopy only if "pend_pre + pend_compat <= s->threshold_size". Pre-patch, in this case we will go to migration_completion(). So, precopy stage is finishing anyway.Right.So, we want in this case to finish ram migration like it was finished by migration_completion(), and then, run postcopy, which will handle only dirty bitmaps, yes?It's a bit tricky; the first important thing is that we can't change the semantics of the migration without the 'dirty bitmaps'. So then there's the question of how a migration with both postcopy-ram+dirty bitmaps should work; again I don't think we should enter the postcopy-ram phase until start-postcopy is issued. Then there's the 3rd case; dirty-bitmaps but no postcopy-ram; in that case I worry less about the semantics of how you want to do it. I have an idea: in postcopy_start(), in ram_has_postcopy() (and may be some other places?), check atomic_read(&s->start_postcopy) instead of migrate_postcopy_ram() then: 1. behavior without dirty-bitmaps is not changed, as currently we cant go into postcopy_start and ram_has_postcopy without s->start_postcopy 2. dirty-bitmaps+ram: if user don't set s->start_postcopy, postcopy_start() will operate as if migration capability was not enabled, so ram should complete its migration 3. only dirty-bitmaps: again, postcopy_start() will operate as if migration capability was not enabled, so ram should complete its migration Hmm2. Looked through migration_completion(), I don't understand, how it finishes ram migration without postcopy. It calls qemu_savevm_state_complete_precopy(), which skips states with has_postcopy=true, which is ram...Because savevm_state_complete_precopy only skips has_postcopy=true in the in_postcopy case: (in_postcopy && se->ops->has_postcopy && se->ops->has_postcopy(se->opaque)) || so when we call it in migration_completion(), if we've not entered postcopy yet, then that test doesn't trigger. (Apologies for not spotting this earlier; but I thought this patch was a nice easy one just adding the postcopy_only_pending - I didn't realise it changed existing semantics until I spotted that) oh, yes, I was inattentive :( Dave-- Best regards, Vladimir-- Dr. David Alan Gilbert / address@hidden / Manchester, UK -- Best regards, Vladimir |
[Prev in Thread] | Current Thread | [Next in Thread] |