qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 1/3] virtio-console: Check if chardev backends available before calling into them
@ 2011-12-21  6:58 Amit Shah
  2011-12-21  6:58 ` [Qemu-devel] [PATCH 2/3] virtio-console: Properly initialise class methods Amit Shah
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Amit Shah @ 2011-12-21  6:58 UTC (permalink / raw)
  To: Anthony Liguori; +Cc: Amit Shah, Paolo Bonzini, qemu list, Markus Armbruster

For the callback functions invoked by the virtio-serial-bus code, check
if we have chardev backends registered before we call into the chardev
functions.

Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 hw/virtio-console.c |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/hw/virtio-console.c b/hw/virtio-console.c
index d3351c8..dbbea76 100644
--- a/hw/virtio-console.c
+++ b/hw/virtio-console.c
@@ -27,6 +27,11 @@ static ssize_t flush_buf(VirtIOSerialPort *port, const uint8_t *buf, size_t len)
     VirtConsole *vcon = DO_UPCAST(VirtConsole, port, port);
     ssize_t ret;
 
+    if (!vcon->chr) {
+        /* If there's no backend, we can just say we consumed all data. */
+        return len;
+    }
+
     ret = qemu_chr_fe_write(vcon->chr, buf, len);
     trace_virtio_console_flush_buf(port->id, len, ret);
 
@@ -52,6 +57,9 @@ static void guest_open(VirtIOSerialPort *port)
 {
     VirtConsole *vcon = DO_UPCAST(VirtConsole, port, port);
 
+    if (!vcon->chr) {
+        return;
+    }
     qemu_chr_fe_open(vcon->chr);
 }
 
@@ -60,6 +68,9 @@ static void guest_close(VirtIOSerialPort *port)
 {
     VirtConsole *vcon = DO_UPCAST(VirtConsole, port, port);
 
+    if (!vcon->chr) {
+        return;
+    }
     qemu_chr_fe_close(vcon->chr);
 }
 
-- 
1.7.7.4

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [Qemu-devel] [PATCH 2/3] virtio-console: Properly initialise class methods
  2011-12-21  6:58 [Qemu-devel] [PATCH 1/3] virtio-console: Check if chardev backends available before calling into them Amit Shah
@ 2011-12-21  6:58 ` Amit Shah
  2011-12-21  6:58 ` [Qemu-devel] [PATCH 3/3] virtio-serial-bus: Ports are expected to implement 'have_data' callback Amit Shah
  2011-12-21 22:05 ` [Qemu-devel] [PATCH 1/3] virtio-console: Check if chardev backends available before calling into them Anthony Liguori
  2 siblings, 0 replies; 4+ messages in thread
From: Amit Shah @ 2011-12-21  6:58 UTC (permalink / raw)
  To: Anthony Liguori; +Cc: Amit Shah, Paolo Bonzini, qemu list, Markus Armbruster

The earlier code really was a hack: initialising class methods in an
object init function as noted by Anthony.

The motivation for that was to not have the virtio-serial-bus call into
the callback functions if there was no chardev backend registered.
However, that really wasn't a worthwhile optimisation, and definitely
not one that was well-implemented.  Get rid of it.

Reported-by: Anthony Liguori <aliguori@us.ibm.com>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 hw/virtio-console.c |    9 ++++++---
 1 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/hw/virtio-console.c b/hw/virtio-console.c
index dbbea76..73d866a 100644
--- a/hw/virtio-console.c
+++ b/hw/virtio-console.c
@@ -120,9 +120,6 @@ static int virtconsole_initfn(VirtIOSerialPort *port)
     if (vcon->chr) {
         qemu_chr_add_handlers(vcon->chr, chr_can_read, chr_read, chr_event,
                               vcon);
-        info->have_data = flush_buf;
-        info->guest_open = guest_open;
-        info->guest_close = guest_close;
     }
 
     return 0;
@@ -149,6 +146,9 @@ static VirtIOSerialPortInfo virtconsole_info = {
     .is_console    = true,
     .init          = virtconsole_initfn,
     .exit          = virtconsole_exitfn,
+    .have_data     = flush_buf,
+    .guest_open    = guest_open,
+    .guest_close   = guest_close,
     .qdev.props = (Property[]) {
         DEFINE_PROP_CHR("chardev", VirtConsole, chr),
         DEFINE_PROP_END_OF_LIST(),
@@ -166,6 +166,9 @@ static VirtIOSerialPortInfo virtserialport_info = {
     .qdev.size     = sizeof(VirtConsole),
     .init          = virtconsole_initfn,
     .exit          = virtconsole_exitfn,
+    .have_data     = flush_buf,
+    .guest_open    = guest_open,
+    .guest_close   = guest_close,
     .qdev.props = (Property[]) {
         DEFINE_PROP_CHR("chardev", VirtConsole, chr),
         DEFINE_PROP_END_OF_LIST(),
-- 
1.7.7.4

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [Qemu-devel] [PATCH 3/3] virtio-serial-bus: Ports are expected to implement 'have_data' callback
  2011-12-21  6:58 [Qemu-devel] [PATCH 1/3] virtio-console: Check if chardev backends available before calling into them Amit Shah
  2011-12-21  6:58 ` [Qemu-devel] [PATCH 2/3] virtio-console: Properly initialise class methods Amit Shah
@ 2011-12-21  6:58 ` Amit Shah
  2011-12-21 22:05 ` [Qemu-devel] [PATCH 1/3] virtio-console: Check if chardev backends available before calling into them Anthony Liguori
  2 siblings, 0 replies; 4+ messages in thread
From: Amit Shah @ 2011-12-21  6:58 UTC (permalink / raw)
  To: Anthony Liguori; +Cc: Amit Shah, Paolo Bonzini, qemu list, Markus Armbruster

There's no need to check if ports can accept any incoming data from the
guest each time the guest sends data.  Check if the port implements such
functionality during port initialisation.

Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 hw/virtio-serial-bus.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index a4825b9..fe0233f 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -466,13 +466,11 @@ static void handle_output(VirtIODevice *vdev, VirtQueue *vq)
 {
     VirtIOSerial *vser;
     VirtIOSerialPort *port;
-    VirtIOSerialPortInfo *info;
 
     vser = DO_UPCAST(VirtIOSerial, vdev, vdev);
     port = find_port_by_vq(vser, vq);
-    info = port ? DO_UPCAST(VirtIOSerialPortInfo, qdev, port->dev.info) : NULL;
 
-    if (!port || !port->host_connected || !info->have_data) {
+    if (!port || !port->host_connected) {
         discard_vq_data(vq, vdev);
         return;
     }
@@ -746,6 +744,8 @@ static int virtser_port_qdev_init(DeviceState *qdev, DeviceInfo *base)
     port->vser = bus->vser;
     port->bh = qemu_bh_new(flush_queued_data_bh, port);
 
+    assert(info->have_data);
+
     /*
      * Is the first console port we're seeing? If so, put it up at
      * location 0. This is done for backward compatibility (old
-- 
1.7.7.4

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [Qemu-devel] [PATCH 1/3] virtio-console: Check if chardev backends available before calling into them
  2011-12-21  6:58 [Qemu-devel] [PATCH 1/3] virtio-console: Check if chardev backends available before calling into them Amit Shah
  2011-12-21  6:58 ` [Qemu-devel] [PATCH 2/3] virtio-console: Properly initialise class methods Amit Shah
  2011-12-21  6:58 ` [Qemu-devel] [PATCH 3/3] virtio-serial-bus: Ports are expected to implement 'have_data' callback Amit Shah
@ 2011-12-21 22:05 ` Anthony Liguori
  2 siblings, 0 replies; 4+ messages in thread
From: Anthony Liguori @ 2011-12-21 22:05 UTC (permalink / raw)
  To: Amit Shah; +Cc: Paolo Bonzini, qemu list, Markus Armbruster

On 12/21/2011 12:58 AM, Amit Shah wrote:
> For the callback functions invoked by the virtio-serial-bus code, check
> if we have chardev backends registered before we call into the chardev
> functions.
>
> Signed-off-by: Amit Shah<amit.shah@redhat.com>

Applied all.  Thanks.

Regards,

Anthony Liguori

> ---
>   hw/virtio-console.c |   11 +++++++++++
>   1 files changed, 11 insertions(+), 0 deletions(-)
>
> diff --git a/hw/virtio-console.c b/hw/virtio-console.c
> index d3351c8..dbbea76 100644
> --- a/hw/virtio-console.c
> +++ b/hw/virtio-console.c
> @@ -27,6 +27,11 @@ static ssize_t flush_buf(VirtIOSerialPort *port, const uint8_t *buf, size_t len)
>       VirtConsole *vcon = DO_UPCAST(VirtConsole, port, port);
>       ssize_t ret;
>
> +    if (!vcon->chr) {
> +        /* If there's no backend, we can just say we consumed all data. */
> +        return len;
> +    }
> +
>       ret = qemu_chr_fe_write(vcon->chr, buf, len);
>       trace_virtio_console_flush_buf(port->id, len, ret);
>
> @@ -52,6 +57,9 @@ static void guest_open(VirtIOSerialPort *port)
>   {
>       VirtConsole *vcon = DO_UPCAST(VirtConsole, port, port);
>
> +    if (!vcon->chr) {
> +        return;
> +    }
>       qemu_chr_fe_open(vcon->chr);
>   }
>
> @@ -60,6 +68,9 @@ static void guest_close(VirtIOSerialPort *port)
>   {
>       VirtConsole *vcon = DO_UPCAST(VirtConsole, port, port);
>
> +    if (!vcon->chr) {
> +        return;
> +    }
>       qemu_chr_fe_close(vcon->chr);
>   }
>

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2011-12-21 22:05 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-12-21  6:58 [Qemu-devel] [PATCH 1/3] virtio-console: Check if chardev backends available before calling into them Amit Shah
2011-12-21  6:58 ` [Qemu-devel] [PATCH 2/3] virtio-console: Properly initialise class methods Amit Shah
2011-12-21  6:58 ` [Qemu-devel] [PATCH 3/3] virtio-serial-bus: Ports are expected to implement 'have_data' callback Amit Shah
2011-12-21 22:05 ` [Qemu-devel] [PATCH 1/3] virtio-console: Check if chardev backends available before calling into them Anthony Liguori

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).