[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 2/3] Add migrate -u option for -incoming pause
From: |
Dr. David Alan Gilbert |
Subject: |
Re: [Qemu-devel] [PATCH 2/3] Add migrate -u option for -incoming pause |
Date: |
Wed, 11 Feb 2015 16:53:33 +0000 |
User-agent: |
Mutt/1.5.23 (2014-03-12) |
* Juan Quintela (address@hidden) wrote:
> "Dr. David Alan Gilbert (git)" <address@hidden> wrote:
> > From: "Dr. David Alan Gilbert" <address@hidden>
> >
> > Once a qemu has been started with -incoming pause the
> > migration can be started by issuing:
> >
> > migrate -u uri
> >
> > Signed-off-by: Dr. David Alan Gilbert <address@hidden>
>
> > - "(base image shared between src and destination)",
> > + "(base image shared between src and destination)"
> > + "\n\t\t\t -u unpauses an incoming migration started
> > with "
> > + "-incoming pause using the given uri.",
>
> Spaces vs tabs.
>
> > + -u to unpause an incoming migration started with -incoming pause
>
> more spaces
All this is reworked anyway since I split it out.
> > - qmp_migrate(uri, !!blk, blk, !!inc, inc, false, false, &err);
> > + qmp_migrate(uri, !!blk, blk, !!inc, inc, false, false, !!unpause,
> > unpause,
> > + &err);
>
> I don't claim to understand QMP, but this whole bussines of !!foo, foo
> is getting confusing, no?
Yes, and it's very very easy to screw up and get them in the wrong order.
> No, this is not relaced to this patch.
>
> > {
> > Error *local_err = NULL;
> > MigrationState *s = migrate_get_current();
> > @@ -450,6 +450,25 @@ void qmp_migrate(const char *uri, bool has_blk, bool
> > blk,
> > return;
> > }
> >
>
> I would preffer something like:
>
> if (runstate_check(RUN_STATE_INMIGRATE)) {
> if (unpause) {
> ... unpause code
> }
> } else {
> error_setg(errp, "Guest is waiting for an incoming migration");
> return;
> }
>
> if (unpause) {
> error_setg(errp, "Guest is waiting for an incoming migration");
> return;
> }
>
> if (s->state == MIG_STATE_ACTIVE || s->state == MIG_STATE_SETUP ||
> s->state == MIG_STATE_CANCELLING) {
> error_set(errp, QERR_MIGRATION_ACTIVE);
> return;
> }
>
> if (qemu_savevm_state_blocked(errp)) {
> return;
> }
>
> .... and now continue with the rest ...
Again, all gone in the new version.
> Thinking more about this problem, I am not sure this is the "cleanest
> approach". What do you think of:
>
> - create RUN_STATE_INMIGRATE_PAUSED
> bonus: no need of paused_incoming variable
That we could do separately; doing that means carefully looking at
the existing users of RUN_STATE_INMIGRATE, and since it's a
visible state that includes anything that uses the interface.
> - create a new migrate_incoming command
>
> And then we have cleaner separation of what we are doing?
Done.
Dave
>
> Later, Juan.
>
--
Dr. David Alan Gilbert / address@hidden / Manchester, UK