Re: [Qemu-devel] [PATCH] virtio-serial-bus: replay guest_open on migration

[Markus Armbruster]

Alon Levy <address@hidden> writes:

> On Wed, Jul 27, 2011 at 05:35:28PM +0530, Amit Shah wrote:
>> On (Wed) 27 Jul 2011 [14:09:45], Alon Levy wrote:
>> 
>> > > Also, we'll be lying that a guest opened, since a guest was opened
>> > > much earlier, before migration.  Nothing has changed as far as the
>> > > guest is concerned, this is just some host-side tracking that has to
>> > > be done post-migrate, which belongs in individual devices / ports.
>> > 
>> > The callback is there on purpose, some qemu side users exist surely. While
>> > I understand the lying part about the time, it is worst to lie completely
>> > by not mentioning the guest has opened the port.
>> 
>> Guest has not re-opened the port.  When the guest opened the port,
>> spice did get the callback called.  After migration, guest state has
>> not changed.  Why should you get the callback again?
> Again, my point is that from the chardev pov this *is* the first callback.
> You say the timing is wrong - correct, but I don't see any part in the api
> that talks about timing, i.e. no gurantee to be broken.
>
> Actually searching for current users it appears it is just spicevmc 
> (spice-qemu-char.c).
>
>> If you depend on
>> guest connectedness, after migration, just ensure you do whatever is
>> necessary.  I think there's no need to involve any other code in this.
>
> There is no migration mechanism for char devices. There is no migration
> mechanism for the spice server. All data that is passed is done via device 
> migration -
> qxl or in this case I suggest virtio-serial.
>
> Do you think chardevices should be migratable? that would also work.

No.

 We migrate device models (frontends).  The backends get recreated from
scratch.

> Do you think a separate callback would be better worded? or add a "migrated" 
> boolean?
> both are ugly, I agree.

I don't see the need for two separate callbacks.  From the backend's
point of view, there's no difference between normal startup and
migration startup.

If the name "guest_open" bothers you, find another one.  guest_connect?
The guest (or rather the device model), connects to a newly created
backend chrdev.  The just guest doesn't know it (the device model does).

> The chardev does receive parameters, it's possible to have the remote vm 
> start with
> a spicevmc chardev that has "migrated=1" as part of the command line. That 
> looks much
> uglier then this patch to me.

There's no precedence for "migrated=1".

For what it's worth, my recent block layer series also has device models
run backend callbacks after migration.  Check out "[PATCH 37/55]
ide/atapi: Preserve tray state on migration" and "[PATCH 38/55]
scsi-disk: Preserve tray state on migration".