Re: [Qemu-devel] qemu_chr_fe_add_watch interaction with VM start/stop/migrate

[Dr. David Alan Gilbert]

* Peter Maydell (address@hidden) wrote:
> On 22 June 2017 at 11:31, Paolo Bonzini <address@hidden> wrote:
> >
> >
> > On 22/06/2017 12:09, Peter Maydell wrote:
> >> For instance, suppose my UART tries to write to the chardev but
> >> can't, so it registers a callback with qemu_chr_fe_add_watch().
> >> Then the user stops the VM. Does the chardev UI guarantee that
> >> my callback won't get called until the user restarts the VM,
> >> or does my callback function have to do something to say "don't
> >> actually update the emulation state if we're stopped"?
> >> If the user then restarts the VM, do I need to do something to
> >> retry the chardev write?
> >
> > Currently, chardev code runs normally between stop and start.
> 
> I think that's rather counterintuitive -- as a user, if I've
> stopped the VM I don't expect any of its device state to change.
> Stopped should mean "simulation state doesn't change".

Yes and stuff breaks unless you're very careful (e.g. a23a6d18)
- make sure you don't change RAM state when the guest is stopped
and you should at least survive migration.

> >> For migration, if we're migrating in the state where I have a
> >> pending character I'd like to write to the chardev, what do I need
> >> to do on the destination side? Do I need to call qemu_chr_fe_add_watch
> >> in the post-load function, or is this too early and I need to do
> >> something to call it when the destination VM is actually restarted?
> >
> > Post-load is fine.  There could be interesting interactions with
> > interrupts, but these are not specific to character device backends
> > (real-time clocks are another example).  So boards should create
> > interrupt controllers and distributors as soon as possible.
> 
> This sounds wrong to me -- nobody should be asserting an
> interrupt line during post-load, in the same way that nobody
> should assert an interrupt line during reset. We don't have
> any guarantees that the device on the other end of the line
> has finished migration yet.

I remember someone added something to impose an order to get interrupt
controllers loaded before PCI devices etc - I thought it was ARM interrupt
controller related but I can't find it.

But yes, we've had bugs associated with this type of misordering
in the past;  they're normally terrible to debug because
they can depend on timing, especially if the thing that's
loaded in the post-load starts a timer and the issue is whether
the timer fires before or after the other devices or loaded
and or before the VM starts running. (Replace 'timer' with
'chardev' and 'fires' with 'a character is received)

> I think we really need the
> chardev backends to not start doing things until load has
> finished and the VM has been restarted. (If we make "VM
> stopped" mean "no chardev callbacks" then this would fall
> out in the wash, though, since the callback registered in
> post-load would only trigger on the following VM start.)

That depends on what the chardev is doing;
if it's a chardev to a monitor or something not directly
updating guest state then you'd probably want it to start.
What about chardev's for networking stuff?

Dave

> thanks
> -- PMM
> 
--
Dr. David Alan Gilbert / address@hidden / Manchester, UK