All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
To: Juan Quintela <quintela@redhat.com>
Cc: Markus Armbruster <armbru@redhat.com>,
	Eric Blake <eblake@redhat.com>,
	lvivier@redhat.com, qemu-devel@nongnu.org,
	Peter Xu <peterx@redhat.com>
Subject: Re: [Qemu-devel] [PATCH 2/3] migration: Remove use of old MigrationParams
Date: Mon, 15 May 2017 17:38:41 +0100	[thread overview]
Message-ID: <20170515163841.GB2324@work-vm> (raw)
In-Reply-To: <87efvqdqqj.fsf@secure.mitica>

* Juan Quintela (quintela@redhat.com) wrote:
> Markus Armbruster <armbru@redhat.com> wrote:
> > Juan Quintela <quintela@redhat.com> writes:
> >
> >> Eric Blake <eblake@redhat.com> wrote:
> 
> 
> >>> Or is the proposal that we are also going to simplify the QMP 'migrate'
> >>> command to get rid of crufty parameters?
> >>
> >> I didn't read it that way, but I would not oppose O:-)
> >>
> >> Later, Juan.
> >
> > I'm not too familiar with this stuff, so please correct my
> > misunderstandings.
> >
> > "Normal" migration configuration is global state, i.e. it applies to all
> > future migrations.
> >
> > Except the "migrate" command's flags apply to just the migration kicked
> > off by that command.
> >
> > QMP command "migrate" has two flags "blk" (HMP: -b) and "inc" (HMP: -i).
> > !blk && inc makes no sense and is silently treated like !blk && !inc.
> >
> > There's a third flag "detach" (HMP: -d), but it does nothing in QMP.
> 
> As qmp command is asynchronous, you can think that -d is *always* on in
> QMP O:-)
> 
> > You'd like to deprecate these flags in favour of "normal" configuration.
> > However, we need to maintain QMP backward compatibility at least for a
> > while.  HMP backward compatibility is nice to have, but not required.
> >
> > First step is to design the new interface you want.  Second step is to
> > figure out backward compatibility.
> >
> > The new interface adds a block migration tri-state (off,
> > non-incremental, incremental) to global state, default off.  Whether
> > it's done as two bools or an enum of three values doesn't matter here.
> 
> Tristates will complicate it.  I still think that:
> 
> - capability: block_migration
> - parameter: block_shared
> 
> Makes more sense, no?

I don't understand what making block_shared a parameter gives you as
opposed to simply having two capabilities.

(And how did we get 'shared'? We started off with block & incremental)

Dave

> If block_migration is not enabled, we ignore the shared parameter.  We
> already do that for other parameters.
> 
> > If the new interface isn't used, the old one still needs to work.  If it
> > is used, the old one either has to do "the right thing", or fail
> > cleanly.
> >
> > We approximate "new interface isn't used" by "block migration is off in
> > global state".  When it is off, the migration command needs to honor its
> > two flags for compatibility.  It must leave block migration off in
> > global state.  Yes, this will complicate the implementation until we
> > actually remove the deprecated flags.  Par for the backward compatility
> > course.
> >
> > When block migration isn't off in global state, we can either
> >
> > * let the flags take precedence over the global state (one
> >   interpretation of "do the right thing"), or
> >
> > * reject flags that conflict with global state (another interpretation),
> >   or
> >
> > * reject *all* flags (fail cleanly).
> >
> > The last one looks perfectly servicable to me.
> 
> Yeap,  I think that makes sense.  If you use capabilities, parameters,
> old interface don't work at all.
> 
> We still have a problem that is what happens if the user does:
> 
> migrate -b <foo>
> migrate_cancel (or error)
> migrate <bar> (without -b)
> 
> With current patches, it will still use -b.  Fixing it requires still
> anding more code.  But I think that this use case is so weird what we
> should not even care about it.
> 
> Later, Juan.
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

  reply	other threads:[~2017-05-15 16:38 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-05-11 16:32 [Qemu-devel] [PATCH v2 0/3] Remove old MigrationParams Juan Quintela
2017-05-11 16:32 ` [Qemu-devel] [PATCH 1/3] migration: Create block capabilities for shared and enable Juan Quintela
2017-05-12 19:52   ` Eric Blake
2017-05-15  9:41     ` Juan Quintela
2017-05-15  9:46       ` Dr. David Alan Gilbert
2017-05-15 14:24         ` Eric Blake
2017-05-15 15:38           ` Markus Armbruster
2017-05-15 16:06             ` Juan Quintela
2017-05-16  6:49               ` Markus Armbruster
2017-05-15 15:56           ` Juan Quintela
2017-05-11 16:32 ` [Qemu-devel] [PATCH 2/3] migration: Remove use of old MigrationParams Juan Quintela
2017-05-12  3:40   ` Peter Xu
2017-05-12 10:55     ` Juan Quintela
2017-05-12 19:59       ` Eric Blake
2017-05-15  9:48         ` Juan Quintela
2017-05-15 10:43           ` Dr. David Alan Gilbert
2017-05-15 14:28           ` Eric Blake
2017-05-15 15:59             ` Juan Quintela
2017-05-15 16:06           ` Markus Armbruster
2017-05-15 16:33             ` Juan Quintela
2017-05-15 16:38               ` Dr. David Alan Gilbert [this message]
2017-05-15 16:56                 ` Juan Quintela
2017-05-15 17:27                   ` Dr. David Alan Gilbert
2017-05-15 17:35                     ` Juan Quintela
2017-05-15 17:38                       ` Dr. David Alan Gilbert
2017-05-15 17:45                         ` Juan Quintela
2017-05-15 18:32                           ` Dr. David Alan Gilbert
2017-05-16  7:25               ` Markus Armbruster
2017-05-16  8:00                 ` Juan Quintela
2017-05-15 10:05       ` Peter Xu
2017-05-11 16:32 ` [Qemu-devel] [PATCH 3/3] migration: Remove " Juan Quintela
2017-05-12  2:01 ` [Qemu-devel] [PATCH v2 0/3] " Hailiang Zhang
  -- strict thread matches above, loose matches on Subject: below --
2017-04-25 10:30 [Qemu-devel] [PATCH " Juan Quintela
2017-04-25 10:30 ` [Qemu-devel] [PATCH 2/3] migration: Remove use of " Juan Quintela
2017-04-28 16:55   ` Dr. David Alan Gilbert
2017-05-04  8:51     ` Juan Quintela
2017-05-04  9:14       ` Hailiang Zhang
2017-05-11 16:33         ` Juan Quintela
2017-05-12  2:02           ` Hailiang Zhang
2017-04-28 18:49   ` Eric Blake

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170515163841.GB2324@work-vm \
    --to=dgilbert@redhat.com \
    --cc=armbru@redhat.com \
    --cc=eblake@redhat.com \
    --cc=lvivier@redhat.com \
    --cc=peterx@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=quintela@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.