Re: [Qemu-devel] [PATCH v4 11/13] virtio-console: chardev hotswap support

[Anton Nefedov]

On 06/29/2017 01:02 PM, Marc-André Lureau wrote:
> Hi
>
> Looks good, but please write something in the commit message about what needs 
> to be done for be-change (what this patch does).
>
> thanks

Hi,

thank you! I guess the description should look like

  virtio-console: chardev hotswap support

  In case of a backend change, the handler functions and the watch have
  to be reset.
  Also, avoid unsafe qemu_chr_fe_get_driver() usage even though the pointer
  is not really stored.



Amit, have you had a chance to kindly look at this?


/Anton

> ----- Original Message -----
> > Signed-off-by: Anton Nefedov <address@hidden>
> > Reviewed-by: Vladimir Sementsov-Ogievskiy <address@hidden>
> > CC: Amit Shah <address@hidden>
> > ---
> >   hw/char/virtio-console.c | 35 ++++++++++++++++++++++++++++++-----
> >   1 file changed, 30 insertions(+), 5 deletions(-)
> >
> > diff --git a/hw/char/virtio-console.c b/hw/char/virtio-console.c
> > index afb4949..198b2a8 100644
> > --- a/hw/char/virtio-console.c
> > +++ b/hw/char/virtio-console.c
> > @@ -49,7 +49,7 @@ static ssize_t flush_buf(VirtIOSerialPort *port,
> >       VirtConsole *vcon = VIRTIO_CONSOLE(port);
> >       ssize_t ret;
> >   
> > -    if (!qemu_chr_fe_get_driver(&vcon->chr)) {
> > +    if (!qemu_chr_fe_backend_connected(&vcon->chr)) {
> >           /* If there's no backend, we can just say we consumed all data. */
> >           return len;
> >       }
> > @@ -163,12 +163,35 @@ static void chr_event(void *opaque, int event)
> >       }
> >   }
> >   
> > +static int chr_be_change(void *opaque)
> > +{
> > +    VirtConsole *vcon = opaque;
> > +    VirtIOSerialPort *port = VIRTIO_SERIAL_PORT(vcon);
> > +    VirtIOSerialPortClass *k = VIRTIO_SERIAL_PORT_GET_CLASS(port);
> > +
> > +    if (k->is_console) {
> > +        qemu_chr_fe_set_handlers(&vcon->chr, chr_can_read, chr_read,
> > +                                 NULL, chr_be_change, vcon, NULL, true);
> > +    } else {
> > +        qemu_chr_fe_set_handlers(&vcon->chr, chr_can_read, chr_read,
> > +                                 chr_event, chr_be_change, vcon, NULL,
> > false);
> > +    }
> > +
> > +    if (vcon->watch) {
> > +        g_source_remove(vcon->watch);
> > +        vcon->watch = qemu_chr_fe_add_watch(&vcon->chr,
> > +                                            G_IO_OUT | G_IO_HUP,
> > +                                            chr_write_unblocked, vcon);
> > +    }
> > +
> > +    return 0;
> > +}
> > +
> >   static void virtconsole_realize(DeviceState *dev, Error **errp)
> >   {
> >       VirtIOSerialPort *port = VIRTIO_SERIAL_PORT(dev);
> >       VirtConsole *vcon = VIRTIO_CONSOLE(dev);
> >       VirtIOSerialPortClass *k = VIRTIO_SERIAL_PORT_GET_CLASS(dev);
> > -    Chardev *chr = qemu_chr_fe_get_driver(&vcon->chr);
> >   
> >       if (port->id == 0 && !k->is_console) {
> >           error_setg(errp, "Port number 0 on virtio-serial devices reserved "
> > @@ -176,7 +199,7 @@ static void virtconsole_realize(DeviceState *dev, Error
> > **errp)
> >           return;
> >       }
> >   
> > -    if (chr) {
> > +    if (qemu_chr_fe_backend_connected(&vcon->chr)) {
> >           /*
> >            * For consoles we don't block guest data transfer just
> >            * because nothing is connected - we'll just let it go
> > @@ -188,11 +211,13 @@ static void virtconsole_realize(DeviceState *dev, Error
> > **errp)
> >            */
> >           if (k->is_console) {
> >               qemu_chr_fe_set_handlers(&vcon->chr, chr_can_read, chr_read,
> > -                                     NULL, NULL, vcon, NULL, true);
> > +                                     NULL, chr_be_change,
> > +                                     vcon, NULL, true);
> >               virtio_serial_open(port);
> >           } else {
> >               qemu_chr_fe_set_handlers(&vcon->chr, chr_can_read, chr_read,
> > -                                     chr_event, NULL, vcon, NULL, false);
> > +                                     chr_event, chr_be_change,
> > +                                     vcon, NULL, false);
> >           }
> >       }
> >   }
> > --
> > 2.7.4

--
/Anton