diff mbox

[v4,11/13] virtio-console: chardev hotswap support

Message ID 1498495550-72357-12-git-send-email-anton.nefedov@virtuozzo.com (mailing list archive)
State New, archived
Headers show

Commit Message

Anton Nefedov June 26, 2017, 4:45 p.m. UTC 
Signed-off-by: Anton Nefedov <anton.nefedov@virtuozzo.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
CC: Amit Shah <amit@kernel.org>
---
 hw/char/virtio-console.c | 35 ++++++++++++++++++++++++++++++-----
 1 file changed, 30 insertions(+), 5 deletions(-)

Comments

Marc-André Lureau June 29, 2017, 10:02 a.m. UTC | #1 
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

----- Original Message -----
> Signed-off-by: Anton Nefedov <anton.nefedov@virtuozzo.com>
> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> CC: Amit Shah <amit@kernel.org>
> ---
>  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 Nefedov July 3, 2017, 11:50 a.m. UTC | #2 
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 <anton.nefedov@virtuozzo.com>
>> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>> CC: Amit Shah <amit@kernel.org>
>> ---
>>   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
>>
>>
Amit Shah July 4, 2017, 6:25 a.m. UTC | #3 
On (Mon) 03 Jul 2017 [14:50:07], Anton Nefedov wrote:
> 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?

If it gets a reviewed-by by someone who's looked at the series,
Michael could pull it all in.

I could look at it if you CC'ed me on the series (hint ;)

		Amit
diff mbox

Patch

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);
         }
     }
 }