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

[Amit Shah <amit.shah@redhat.com>]

On (Wed) 27 Jul 2011 [10:07:56], Alon Levy wrote:
> On Wed, Jul 27, 2011 at 07:45:25AM +0200, Markus Armbruster wrote:
> > Alon Levy <alevy@redhat.com> writes:
> > 
> > > Signed-off-by: Alon Levy <alevy@redhat.com>
> > > ---
> > >  hw/virtio-serial-bus.c |    8 +++++++-
> > >  1 files changed, 7 insertions(+), 1 deletions(-)
> > >
> > > diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
> > > index c5eb931..7a652ff 100644
> > > --- a/hw/virtio-serial-bus.c
> > > +++ b/hw/virtio-serial-bus.c
> > > @@ -618,14 +618,20 @@ static int virtio_serial_load(QEMUFile *f, void *opaque, int version_id)
> > >      for (i = 0; i < nr_active_ports; i++) {
> > >          uint32_t id;
> > >          bool host_connected;
> > > +        VirtIOSerialPortInfo *info;
> > >  
> > >          id = qemu_get_be32(f);
> > >          port = find_port_by_id(s, id);
> > >          if (!port) {
> > >              return -EINVAL;
> > >          }
> > > -
> > >          port->guest_connected = qemu_get_byte(f);
> > > +        info = DO_UPCAST(VirtIOSerialPortInfo, qdev, port->dev.info);
> > > +        if (port->guest_connected && info->guest_open) {
> > > +            /* replay guest open */
> > > +            info->guest_open(port);
> > > +
> > > +        }
> > >          host_connected = qemu_get_byte(f);
> > >          if (host_connected != port->host_connected) {
> > >              /*
> > 
> > The patch makes enough sense to me, but the commit message is
> > insufficient.  Why do you have to replay?  And what's being fixed?
> 
> When migrating a host with with a spice agent running the mouse becomes
> non operational after the migration. This is rhbz #718463, currently on
> spice-server but it seems this is a qemu-kvm issue. The problem is that
> after migration spice doesn't know the guest agent is open. Spice is just
> a char dev here. And a chardev cannot query it's device, the device has
> to let the chardev know when it is open. Right now after migration the
> chardev which is recreated is in it's default state, which assumes the
> guest is disconnected. Char devices carry no information across migration,
> but the virtio-serial does already carry the guest_connected state. This
> patch passes that bit to the chardev.

It's not guaranteed all ports will be chardevs.

My thinking was this can be handled by qemu-char-spice.c since it can
add a new migration section and if an image is being restored and the
guest agent channel is open after migration finishes, it can continue
its work from there.  What's the benefit of all virtio-serial ports
receiving a guest_open() event in this case?

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.

So I'm not completely sure that this is the right place for such a
notification.  However, if others feel this is fine, I'll accept the
patch.

(Also, when resending, make sure the whitespace changes don't go
through.)

Thanks,
		Amit