Re: [Qemu-devel] [PATCH] runstate: do not discard runstate changes when paused

[Luiz Capitulino]

On Wed, 05 Oct 2011 20:02:04 +0200
Avi Kivity <address@hidden> wrote:

> On 10/05/2011 07:39 PM, Luiz Capitulino wrote:
> > On Wed, 05 Oct 2011 19:23:21 +0200
> > Avi Kivity<address@hidden>  wrote:
> >
> > >  On 10/05/2011 07:02 PM, Luiz Capitulino wrote:
> > >  >  On Wed, 05 Oct 2011 18:37:51 +0200
> > >  >  Avi Kivity<address@hidden>   wrote:
> > >  >
> > >  >  >   On 10/05/2011 06:31 PM, Jan Kiszka wrote:
> > >  >  >   >   >>
> > >  >  >   >   >
> > >  >  >   >   >    vm_start() should be symmetric with vm_stop().  That is, 
> > > if a piece of
> > >  >  >   >   >    code wants to execute with vcpus stopped, it should just 
> > > run inside a
> > >  >  >   >   >    stop/start pair.
> > >  >  >   >   >
> > >  >  >   >   >    The only confusion can come from the user, if he sees 
> > > multiple stop
> > >  >  >   >   >    events and expects that just one cont will continue the 
> > > vm.  For the
> > >  >  >   >   >    machine monitor, we should just document that the you 
> > > have to issue one
> > >  >  >   >   >    cont for every stop event you see (plus any stops you 
> > > issue).  It's not
> > >  >  >   >   >    unnatural - the code that handles a stop_due_to_enospace 
> > > can work to fix
> > >  >  >   >   >    the error and issue a cont, disregarding any other stops 
> > > in progress
> > >  >  >   >   >    (due to a user pressing the stop button, or migration, 
> > > or cpu hotplug,
> > >  >  >   >   >    or whatever).  For the human monitor, it's not so 
> > > intuitive, but the
> > >  >  >   >   >    situation is so rare we can just rely on the user to 
> > > issue cont again.
> > >  >  >   >
> > >  >  >   >   Making this kind of user-visible change would be a bad idea.
> > >  >  >
> > >  >  >   The current situation is a bad idea.
> > >  >
> > >  >  Let's take the migration use-case as an example (ie. the user stops 
> > > the VM
> > >  >  before performing the migration). Today, if migration fails,
> > >  >  migrate_fd_put_ready() will call vm_start() which will put the VM to
> > >  >  run again.
> > >  >
> > >  >  But if we implement the ref count idea, then vm_start() will just 
> > > "unlock"
> > >  >  migrate_fd_put_ready()'s own call to vm_stop(), that's, the user stop 
> > > will
> > >  >  remain and the user is required to do a 'cont'.
> > >  >
> > >  >  I'd probably agree that that's the ideal semantics, but chances are 
> > > we're
> > >  >  going to break qmp clients here.
> > >
> > >  There are two questions here.  Is this autostart desirable?  (IMO no,
> > >  but haven't given it much thought).
> >
> > It should be configurable at migration time I think.
> 
> Why? autostart can easily be emulated by non-autostart, but not vice versa.

What I meant is to give the user the choice to use.

> > >  If yes, we should provide it
> > >  somehow.  If not, we should default to providing it, but switch to
> > >  non-autostart if a newer client indicates it understands the new 
> > > semantics.
> >
> > That's only one example. You mention another one above: if qemu stops
> > itself twice, will the user be required to run 'cont' twice?
> 
> Yes.  That's how the user tells qemu it saw the two events and handled 
> both of them.  Otherwise there is a race condition and an event goes 
> unhandled (or rather, it is handled while the guest is running instead 
> of stopped).

Breaks compatibility.

> > I'm not exactly against the semantics you're proposing, but they don't
> > seem to fit today's qemu.
> 
> Today's qemu is broken here.

For me it's broken because it will abort() if you migrate a paused vm, for
you it seems to be broken at the semantic level.

We can fix the semantics without breaking compatibility.