"Shribman, Aidan" <aidan.shrib...@sap.com> wrote:
> From: Aidan Shribman <aidan.shrib...@sap.com>
>
> [PATCH] Add warmup phase for live migration of large memory apps
>
> By invoking "migrate -w <url>" we initiate a background live-migration
> transferring of dirty pages continuously until invocation of "migrate_end"
> which attempts to complete the live migration operation.
>
> Qemu host: Ubuntu 10.10
Codding style comments already commented by Stefan. Please use checkpatch.
> +static int is_migrate_warmup = 0;
once here, this can be "bool" O:-)
And this should be part of FdMigrationState, not a global variable.
> +> DPRINTF("iterate\n");
> - if (qemu_savevm_state_iterate(s->mon, s->file) == 1) {
> + if (is_migrate_warmup) {
> + qemu_savevm_state_warmup(s->mon, s->file);
> + } else if (qemu_savevm_state_iterate(s->mon, s->file) == 1) {
Refactor this out? qemu_savevm_state_warmup is just calling
qemu_savevm_state_iterate() and you are just ignoring error return (what
is wrong).
Why not just do something like.
ret = qemu_savevm_state_iterate(s->mon, s->file);
if (ret < 0) {
s->state = state;
notifier_list_notify(&migration_state_notifiers);
return;
}
if (s->is_migrate_warmup) {
return;
}
if (ret == 1) {
int state;
int old_vm_running = vm_running;
/* and rest of old normal code */
}
> +void do_migrate_end(Monitor *mon, const QDict *qdict)
> +{
> + if (!vm_running) {
> + return;
> + }
> + if (!is_migrate_warmup) {
> + return;
> + }
> + is_migrate_warmup = 0;
> +}
If we add this, we should generalize it to always work. i.e. just
convert the current migration in non-live, or something like that.
> diff --git a/savevm.c b/savevm.c
> index 4e49765..521589c 100644
> --- a/savevm.c
> +++ b/savevm.c
> @@ -1471,6 +1471,18 @@ int qemu_savevm_state_begin(Monitor *mon, QEMUFile *f,
> int blk_enable,
> return 0;
> }
>
> +int qemu_savevm_state_warmup(Monitor *mon, QEMUFile *f) {
> + int ret = 1;
> +
> + ret = qemu_savevm_state_iterate(mon, f);
> +
> + if (ret == -EIO) {
> + return ret;
> + }
> +
> + return 0;
> +}
You can see here why I don't like this function, it is just a wrapper to
qemu_savevm_state_iterate() that does nothing, just "detects" the return
value equal to 1.
As stated by Anthony, I think that we can "simulate" this functionality
playing with max_downtime value. Why is that solution not enough for you?
Later, Juan.
