Skip to contentSkip to conversation

migration: Guard ram_bytes_remaining against early call

PATCH In review Dr. David Alan Gilbert (git) 3 participants 3 messages Dec 15, 2017 – Dec 27, 2017

lore
Share & tools
lore

Dr. David Alan Gilbert (git)

author
#0 lore raw

PATCH 1/1 migration: Guard ram_bytes_remaining against early call

From: "Dr. David Alan Gilbert" <[email protected]>

Calling ram_bytes_remaining during the early part of setup is unsafe because the ram_state isn't yet initialised.

This can happen in the sequence:
migrate
migrate_cancel
info migrate

if the migrate sticks trying to connect (e.g. to an unresponsive destination due to the connect timeout). Here 'info migrate' sees a state of CANCELLING and so assumes the migrate has partially happened.

partial fix for:
RH bz: https://bugzilla.redhat.com/show_bug.cgi?id=1525899

Reported-by Xianxian Wang <[email protected]>
Signed-off-by Dr. David Alan Gilbert <[email protected]>
---
 migration/ram.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
migration/ram.c +2 -1
index 021d583b9b..cb1950f3eb 100644
@@ -237,7 +237,8 @@ static RAMState *ram_state;
 
 uint64_t ram_bytes_remaining(void)
 {
-    return ram_state->migration_dirty_pages * TARGET_PAGE_SIZE;
+    return ram_state ? (ram_state->migration_dirty_pages * TARGET_PAGE_SIZE) :
+                       0;
 }
 
 MigrationStats ram_counters;
Signature · 1 line
2.14.3
Reviewed-by on v1 1/1
#1 lore raw
Quoted 10 lines · Dr. David Alan Gilbert (git)

From: "Dr. David Alan Gilbert" <[email protected]>

Calling ram_bytes_remaining during the early part of setup is unsafe because the ram_state isn't yet initialised.

This can happen in the sequence:
migrate
migrate_cancel
info migrate

Quoted 8 lines

if the migrate sticks trying to connect (e.g. to an unresponsive destination due to the connect timeout). Here 'info migrate' sees a state of CANCELLING and so assumes the migrate has partially happened.

partial fix for:
RH bz: https://bugzilla.redhat.com/show_bug.cgi?id=1525899

Reported-by Xianxian Wang <[email protected]>
Signed-off-by Dr. David Alan Gilbert <[email protected]>
Reviewed-by Peter Xu <[email protected]>

Now we fetch this as long as !COMPLETE:

    if (s->state != MIGRATION_STATUS_COMPLETED) {
        info->ram->remaining = ram_bytes_remaining();
        info->ram->dirty_pages_rate = ram_counters.dirty_pages_rate;
    }

Maybe we should also narrow this down some day. Thanks,

Signature · 1 line
Peter Xu
Reviewed-by on v1 1/1
#2 lore raw
In reply to Dr. David Alan Gilbert (git)
Quoted 10 lines · Dr. David Alan Gilbert (git)

From: "Dr. David Alan Gilbert" <[email protected]>

Calling ram_bytes_remaining during the early part of setup is unsafe because the ram_state isn't yet initialised.

This can happen in the sequence:
migrate
migrate_cancel
info migrate

Quoted 8 lines

if the migrate sticks trying to connect (e.g. to an unresponsive destination due to the connect timeout). Here 'info migrate' sees a state of CANCELLING and so assumes the migrate has partially happened.

partial fix for:
RH bz: https://bugzilla.redhat.com/show_bug.cgi?id=1525899

Reported-by Xianxian Wang <[email protected]>
Signed-off-by Dr. David Alan Gilbert <[email protected]>
Reviewed-by Juan Quintela <[email protected]>