[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v7 23/42] MIGRATION_STATUS_POSTCOPY_ACTIVE: Add
From: |
Juan Quintela |
Subject: |
Re: [Qemu-devel] [PATCH v7 23/42] MIGRATION_STATUS_POSTCOPY_ACTIVE: Add new migration state |
Date: |
Mon, 13 Jul 2015 20:05:17 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) |
"Dr. David Alan Gilbert" <address@hidden> wrote:
> * Juan Quintela (address@hidden) wrote:
>> "Dr. David Alan Gilbert" <address@hidden> wrote:
>> > * Juan Quintela (address@hidden) wrote:
>> >> "Dr. David Alan Gilbert (git)" <address@hidden> wrote:
>> >> > From: "Dr. David Alan Gilbert" <address@hidden>
>> >> >
>> >> > 'MIGRATION_STATUS_POSTCOPY_ACTIVE' is entered after
>> >> > migrate_start_postcopy
>> >> >
>> >> > 'migration_postcopy_phase' is provided for other sections to know if
>> >> > they're in postcopy.
>> >> >
>> >> > Signed-off-by: Dr. David Alan Gilbert <address@hidden>
>> >> > Reviewed-by: David Gibson <address@hidden>
>> >> > Reviewed-by: Eric Blake <address@hidden>
>> >>
>> >> Reviewed-by: Juan Quintela <address@hidden>
>> >>
>> >>
>> >> But (there is always a but....)
>> >>
>> >>
>> >> > @@ -358,6 +359,39 @@ MigrationInfo *qmp_query_migrate(Error **errp)
>> >> >
>> >> > get_xbzrle_cache_stats(info);
>> >> > break;
>> >> > + case MIGRATION_STATUS_POSTCOPY_ACTIVE:
>> >> > + /* Mostly the same as active; TODO add some postcopy stats */
>> >> > + info->has_status = true;
>> >> > + info->has_total_time = true;
>> >> > + info->total_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME)
>> >> > + - s->total_time;
>> >> > + info->has_expected_downtime = true;
>> >> > + info->expected_downtime = s->expected_downtime;
>> >> > + info->has_setup_time = true;
>> >> > + info->setup_time = s->setup_time;
>> >> > +
>> >> > + info->has_ram = true;
>> >> > + info->ram = g_malloc0(sizeof(*info->ram));
>> >> > + info->ram->transferred = ram_bytes_transferred();
>> >> > + info->ram->remaining = ram_bytes_remaining();
>> >> > + info->ram->total = ram_bytes_total();
>> >> > + info->ram->duplicate = dup_mig_pages_transferred();
>> >> > + info->ram->skipped = skipped_mig_pages_transferred();
>> >> > + info->ram->normal = norm_mig_pages_transferred();
>> >> > + info->ram->normal_bytes = norm_mig_bytes_transferred();
>> >> > + info->ram->dirty_pages_rate = s->dirty_pages_rate;
>> >> > + info->ram->mbps = s->mbps;
>> >> > +
>> >> > + if (blk_mig_active()) {
>> >> > + info->has_disk = true;
>> >> > + info->disk = g_malloc0(sizeof(*info->disk));
>> >> > + info->disk->transferred = blk_mig_bytes_transferred();
>> >> > + info->disk->remaining = blk_mig_bytes_remaining();
>> >> > + info->disk->total = blk_mig_bytes_total();
>> >> > + }
>> >>
>> >> Can we have block migration active with postcopy? I would assume that
>> >> this would get disk corruption, no? Or if you preffer the other
>> >> question, what protects us from disk corruption?
>> >
>> > I think you can, I've not tried it; however I also think it should
>> > be safe.
>> >
>> > migration/block.c's block_save_pending always puts a value in the
>> > non_postcopiable_pending return value (and 0 in the postcopiable_pending);
>> > the migrate thread checks the non_postcopiable_pending size to
>> > decide when it can switch to postcopy, and performs a call to the complete
>> > method on each device before it does. Thus the block migration should
>> > be finished before we start doing the actual postcopy stage, and thus
>> > before the destination CPU starts running.
>>
>> I mean that as it is right now, the info under blk_mig_active() check
>> would be zero/the same than before entering postcopy.
>
> Ah, yes; would blk_mig_bytes_total/transferred still have valid values you
> would want to display, even at the end of the block migration phase?
>
>> >
>> > A possibly harder question is what happens if block.c did implement
>> > postcopy and you had both block postcopy and ram postcopy active at
>> > the same time; again I think it should work but I'm not sure if one
>> > would starve the other.
>> >
>> >> Once here, I guess we can get the migrate_already_active() bit without
>> >> problem?
>> >
>> > I'm not sure of the question here; but the idea of
>> > migration_already_active()
>> > is just to avoid all of the open-coded checks for each possible state;
>> > now we've added anothe state they were getting messy.
>>
>> Sorry. I mean that the migrate_already_active() bits can get in without
>> further ado. Don't need to wait for postcopy to be integrated.
>
> Yes; do you want it split out?
Whatever is easier for you. I mean that it can be integrated
independently of postcopy.
so, it is up to you.
Juan.
>
> Dave
>
>>
>> >
>> > Dave
>> >
>> >>
>> >> Later, Juan.
>> > --
>> > Dr. David Alan Gilbert / address@hidden / Manchester, UK
> --
> Dr. David Alan Gilbert / address@hidden / Manchester, UK