[Qemu-devel] [PATCH 2/2] virtio-console: Notify resize to the guest.

[Qemu-devel] [PATCH 2/2] virtio-console: Notify resize to the guest.

[Kusanagi Kouichi]

Signed-off-by: Kusanagi Kouichi <slash@ac.auone-net.jp>
---
 hw/virtio-console.c    |    3 +++
 hw/virtio-serial-bus.c |    8 ++++++++
 hw/virtio-serial.h     |    9 +++++++--
 3 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/hw/virtio-console.c b/hw/virtio-console.c
index bd44ec6..92dd03e 100644
--- a/hw/virtio-console.c
+++ b/hw/virtio-console.c
@@ -58,6 +58,9 @@ static void chr_event(void *opaque, int event)
     case CHR_EVENT_CLOSED:
         virtio_serial_close(&vcon->port);
         break;
+    case CHR_EVENT_RESIZE:
+        virtio_serial_resize(&vcon->port, vcon->chr->rows, vcon->chr->cols);
+        break;
     }
 }
 
diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index 17c1ec1..66891ee 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -163,6 +163,13 @@ int virtio_serial_close(VirtIOSerialPort *port)
     return 0;
 }
 
+void virtio_serial_resize(VirtIOSerialPort *port, int rows, int cols)
+{
+    port->vser->config.rows = rows;
+    port->vser->config.cols = cols;
+    send_control_event(port, VIRTIO_CONSOLE_RESIZE, 0);
+}
+
 /* Individual ports/apps call this function to write to the guest. */
 ssize_t virtio_serial_write(VirtIOSerialPort *port, const uint8_t *buf,
                             size_t size)
@@ -336,6 +343,7 @@ static void handle_input(VirtIODevice *vdev, VirtQueue *vq)
 static uint32_t get_features(VirtIODevice *vdev, uint32_t features)
 {
     VirtIOSerial *vser = DO_UPCAST(VirtIOSerial, vdev, vdev);
+    features |= (1 << VIRTIO_CONSOLE_F_SIZE);
     if (vser->bus->max_nr_ports > 1) {
         features |= (1 << VIRTIO_CONSOLE_F_MULTIPORT);
     }
diff --git a/hw/virtio-serial.h b/hw/virtio-serial.h
index f297b00..8af83b8 100644
--- a/hw/virtio-serial.h
+++ b/hw/virtio-serial.h
@@ -25,12 +25,12 @@
 #define VIRTIO_ID_CONSOLE		3
 
 /* Features supported */
+#define VIRTIO_CONSOLE_F_SIZE		0
 #define VIRTIO_CONSOLE_F_MULTIPORT	1
 
 struct virtio_console_config {
     /*
-     * These two fields are used by VIRTIO_CONSOLE_F_SIZE which
-     * isn't implemented here yet
+     * These two fields are used by VIRTIO_CONSOLE_F_SIZE
      */
     uint16_t cols;
     uint16_t rows;
@@ -160,6 +160,11 @@ int virtio_serial_open(VirtIOSerialPort *port);
 int virtio_serial_close(VirtIOSerialPort *port);
 
 /*
+ * Notify resize to the guest
+ */
+void virtio_serial_resize(VirtIOSerialPort *port, int rows, int cols);
+
+/*
  * Send data to Guest
  */
 ssize_t virtio_serial_write(VirtIOSerialPort *port, const uint8_t *buf,
-- 
1.7.0.4

Re: [Qemu-devel] [PATCH 2/2] virtio-console: Notify resize to the guest.

[Amit Shah]

On (Fri) Apr 16 2010 [20:42:47], Kusanagi Kouichi wrote:
> 
> Signed-off-by: Kusanagi Kouichi <slash@ac.auone-net.jp>
> ---
>  hw/virtio-console.c    |    3 +++
>  hw/virtio-serial-bus.c |    8 ++++++++
>  hw/virtio-serial.h     |    9 +++++++--
>  3 files changed, 18 insertions(+), 2 deletions(-)

Hello,

The patches look good. How did you test them?

Just one comment here:

> diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
> index 17c1ec1..66891ee 100644
> --- a/hw/virtio-serial-bus.c
> +++ b/hw/virtio-serial-bus.c
> @@ -163,6 +163,13 @@ int virtio_serial_close(VirtIOSerialPort *port)
>      return 0;
>  }
>  
> +void virtio_serial_resize(VirtIOSerialPort *port, int rows, int cols)

Would virtio_serial_resize_console() be better, since this is only to be
used for console ports?

		Amit

Re: [Qemu-devel] [PATCH 2/2] virtio-console: Notify resize to the guest.

[Kusanagi Kouichi]

On 2010-04-16 18:32:19 +0530, Amit Shah wrote:
> On (Fri) Apr 16 2010 [20:42:47], Kusanagi Kouichi wrote:
> > 
> > Signed-off-by: Kusanagi Kouichi <slash@ac.auone-net.jp>
> > ---
> >  hw/virtio-console.c    |    3 +++
> >  hw/virtio-serial-bus.c |    8 ++++++++
> >  hw/virtio-serial.h     |    9 +++++++--
> >  3 files changed, 18 insertions(+), 2 deletions(-)
> 
> Hello,
> 
> The patches look good. How did you test them?
> 
> Just one comment here:
> 
> > diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
> > index 17c1ec1..66891ee 100644
> > --- a/hw/virtio-serial-bus.c
> > +++ b/hw/virtio-serial-bus.c
> > @@ -163,6 +163,13 @@ int virtio_serial_close(VirtIOSerialPort *port)
> >      return 0;
> >  }
> >  
> > +void virtio_serial_resize(VirtIOSerialPort *port, int rows, int cols)
> 
> Would virtio_serial_resize_console() be better, since this is only to be
> used for console ports?
> 

It is better, I think. I will update the patch. Thanks.


> 		Amit

Re: [Qemu-devel] [PATCH 2/2] virtio-console: Notify resize to the guest.

[Amit Shah]

On (Thu) Apr 29 2010 [16:12:38], Kusanagi Kouichi wrote:
> On 2010-04-16 18:32:19 +0530, Amit Shah wrote:
> > On (Fri) Apr 16 2010 [20:42:47], Kusanagi Kouichi wrote:
> > > 
> > > Signed-off-by: Kusanagi Kouichi <slash@ac.auone-net.jp>
> > > ---
> > >  hw/virtio-console.c    |    3 +++
> > >  hw/virtio-serial-bus.c |    8 ++++++++
> > >  hw/virtio-serial.h     |    9 +++++++--
> > >  3 files changed, 18 insertions(+), 2 deletions(-)
> > 
> > Hello,
> > 
> > The patches look good. How did you test them?
> > 
> > Just one comment here:
> > 
> > > diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
> > > index 17c1ec1..66891ee 100644
> > > --- a/hw/virtio-serial-bus.c
> > > +++ b/hw/virtio-serial-bus.c
> > > @@ -163,6 +163,13 @@ int virtio_serial_close(VirtIOSerialPort *port)
> > >      return 0;
> > >  }
> > >  
> > > +void virtio_serial_resize(VirtIOSerialPort *port, int rows, int cols)
> > 
> > Would virtio_serial_resize_console() be better, since this is only to be
> > used for console ports?
> > 
> 
> It is better, I think. I will update the patch. Thanks.

Thanks.

Please include me in the CC when you send it. Also please mention how
you test this feature.

		Amit

Re: [Qemu-devel] [PATCH 2/2] virtio-console: Notify resize to the guest.

[Kusanagi Kouichi]

On 2010-04-29 16:17:18 +0530, Amit Shah wrote:
> On (Thu) Apr 29 2010 [16:12:38], Kusanagi Kouichi wrote:
> > On 2010-04-16 18:32:19 +0530, Amit Shah wrote:
> > > On (Fri) Apr 16 2010 [20:42:47], Kusanagi Kouichi wrote:
> > > > 
> > > > Signed-off-by: Kusanagi Kouichi <slash@ac.auone-net.jp>
> > > > ---
> > > >  hw/virtio-console.c    |    3 +++
> > > >  hw/virtio-serial-bus.c |    8 ++++++++
> > > >  hw/virtio-serial.h     |    9 +++++++--
> > > >  3 files changed, 18 insertions(+), 2 deletions(-)
> > > 
> > > Hello,
> > > 
> > > The patches look good. How did you test them?
> > > 
> > > Just one comment here:
> > > 
> > > > diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
> > > > index 17c1ec1..66891ee 100644
> > > > --- a/hw/virtio-serial-bus.c
> > > > +++ b/hw/virtio-serial-bus.c
> > > > @@ -163,6 +163,13 @@ int virtio_serial_close(VirtIOSerialPort *port)
> > > >      return 0;
> > > >  }
> > > >  
> > > > +void virtio_serial_resize(VirtIOSerialPort *port, int rows, int cols)
> > > 
> > > Would virtio_serial_resize_console() be better, since this is only to be
> > > used for console ports?
> > > 
> > 
> > It is better, I think. I will update the patch. Thanks.
> 
> Thanks.
> 
> Please include me in the CC when you send it. Also please mention how
> you test this feature.
> 
> 		Amit

I put printf()s into involved functions. Then ran qemu on a terminal
emulator, and resized it.
That's how I did. Is this what you need?

Re: [Qemu-devel] [PATCH 2/2] virtio-console: Notify resize to the guest.

[Amit Shah]

On (Thu) Apr 29 2010 [21:36:30], Kusanagi Kouichi wrote:
> > Please include me in the CC when you send it. Also please mention how
> > you test this feature.
> > 
> > 		Amit
> 
> I put printf()s into involved functions. Then ran qemu on a terminal
> emulator, and resized it.
> That's how I did. Is this what you need?

Yes, I'm also wondering if it works ok: as in the guest kernel sees the
updates and resizes the screen accordingly.

		Amit

[Qemu-devel] [PATCH 1/2] char: Handle resize.

[Amit Shah]

From: Kusanagi Kouichi <slash@ac.auone-net.jp>

Signed-off-by: Kusanagi Kouichi <slash@ac.auone-net.jp>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 qemu-char.c |   34 ++++++++++++++++++++++++++++++++++
 qemu-char.h |    3 +++
 2 files changed, 37 insertions(+), 0 deletions(-)

diff --git a/qemu-char.c b/qemu-char.c
index ac65a1c..ce24483 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -437,6 +437,11 @@ static void mux_chr_event(void *opaque, int event)
     MuxDriver *d = chr->opaque;
     int i;
 
+    if (event == CHR_EVENT_RESIZE) {
+        chr->rows = d->drv->rows;
+        chr->cols = d->drv->cols;
+    }
+
     /* Send the event to all registered listeners */
     for (i = 0; i < d->mux_cnt; i++)
         mux_chr_send_event(d, i, event);
@@ -465,6 +470,10 @@ static void mux_chr_update_read_handler(CharDriverState *chr)
     d->focus = d->mux_cnt;
     d->mux_cnt++;
     mux_chr_send_event(d, d->focus, CHR_EVENT_MUX_IN);
+
+    if (chr->rows > 0 && chr->cols > 0) {
+        mux_chr_send_event(d, d->focus, CHR_EVENT_RESIZE);
+    }
 }
 
 static CharDriverState *qemu_chr_open_mux(CharDriverState *drv)
@@ -545,6 +554,7 @@ int send_all(int fd, const void *buf, int len1)
 typedef struct {
     int fd_in, fd_out;
     int max_size;
+    QEMUTimer *timer;
 } FDCharDriver;
 
 #define STDIO_MAX_CLIENTS 1
@@ -600,6 +610,8 @@ static void fd_chr_update_read_handler(CharDriverState *chr)
                                  fd_chr_read, NULL, chr);
         }
     }
+
+    qemu_mod_timer(s->timer, qemu_get_clock(vm_clock));
 }
 
 static void fd_chr_close(struct CharDriverState *chr)
@@ -613,10 +625,31 @@ static void fd_chr_close(struct CharDriverState *chr)
         }
     }
 
+    qemu_del_timer(s->timer);
+    qemu_free_timer(s->timer);
     qemu_free(s);
     qemu_chr_event(chr, CHR_EVENT_CLOSED);
 }
 
+static void fd_chr_timer(void *opaque)
+{
+    struct CharDriverState *chr = opaque;
+    FDCharDriver *s = chr->opaque;
+    struct winsize size;
+
+    if (ioctl(s->fd_out, TIOCGWINSZ, &size) == -1) {
+        return;
+    }
+
+    if (size.ws_row != chr->rows || size.ws_col != chr->cols) {
+        chr->rows = size.ws_row;
+        chr->cols = size.ws_col;
+        qemu_chr_event(chr, CHR_EVENT_RESIZE);
+    }
+
+    qemu_mod_timer(s->timer, qemu_get_clock(vm_clock) + get_ticks_per_sec());
+}
+
 /* open a character device to a unix fd */
 static CharDriverState *qemu_chr_open_fd(int fd_in, int fd_out)
 {
@@ -627,6 +660,7 @@ static CharDriverState *qemu_chr_open_fd(int fd_in, int fd_out)
     s = qemu_mallocz(sizeof(FDCharDriver));
     s->fd_in = fd_in;
     s->fd_out = fd_out;
+    s->timer = qemu_new_timer(vm_clock, fd_chr_timer, chr);
     chr->opaque = s;
     chr->chr_write = fd_chr_write;
     chr->chr_update_read_handler = fd_chr_update_read_handler;
diff --git a/qemu-char.h b/qemu-char.h
index e3a0783..4fa2812 100644
--- a/qemu-char.h
+++ b/qemu-char.h
@@ -15,6 +15,7 @@
 #define CHR_EVENT_MUX_IN  3 /* mux-focus was set to this terminal */
 #define CHR_EVENT_MUX_OUT 4 /* mux-focus will move on */
 #define CHR_EVENT_CLOSED  5 /* connection closed */
+#define CHR_EVENT_RESIZE  6 /* terminal resize */
 
 
 #define CHR_IOCTL_SERIAL_SET_PARAMS   1
@@ -68,6 +69,8 @@ struct CharDriverState {
     char *label;
     char *filename;
     int opened;
+    int rows;
+    int cols;
     QTAILQ_ENTRY(CharDriverState) next;
 };
 
-- 
1.6.2.5

[Qemu-devel] [PATCH 2/2] virtio-console: Notify resize to the guest.

[Amit Shah]

From: Kusanagi Kouichi <slash@ac.auone-net.jp>

I tested this patch as follows: I put printf()s into involved
functions. Then ran qemu on a terminal emulator, and resized it. The
guest kernel gets initial size, and follows host resize. Both singleport
and multiport work.

Signed-off-by: Kusanagi Kouichi <slash@ac.auone-net.jp>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 hw/virtio-console.c    |    3 +++
 hw/virtio-serial-bus.c |   15 +++++++++++++++
 hw/virtio-serial.h     |   11 +++++++++--
 3 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/hw/virtio-console.c b/hw/virtio-console.c
index caea11f..58246d1 100644
--- a/hw/virtio-console.c
+++ b/hw/virtio-console.c
@@ -55,6 +55,9 @@ static void chr_event(void *opaque, int event)
     case CHR_EVENT_CLOSED:
         virtio_serial_close(&vcon->port);
         break;
+    case CHR_EVENT_RESIZE:
+        virtio_serial_resize_console(&vcon->port, vcon->chr->rows, vcon->chr->cols);
+        break;
     }
 }
 
diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index 97694d5..8766f1d 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -205,6 +205,20 @@ int virtio_serial_close(VirtIOSerialPort *port)
     return 0;
 }
 
+void virtio_serial_resize_console(VirtIOSerialPort *port, int rows, int cols)
+{
+    VirtIOSerial *vser = port->vser;
+
+    vser->config.rows = rows;
+    vser->config.cols = cols;
+
+    if (use_multiport(vser)) {
+        send_control_event(port, VIRTIO_CONSOLE_RESIZE, 0);
+    } else {
+        virtio_notify_config(&vser->vdev);
+    }
+}
+
 /* Individual ports/apps call this function to write to the guest. */
 ssize_t virtio_serial_write(VirtIOSerialPort *port, const uint8_t *buf,
                             size_t size)
@@ -425,6 +439,7 @@ static uint32_t get_features(VirtIODevice *vdev, uint32_t features)
 
     vser = DO_UPCAST(VirtIOSerial, vdev, vdev);
 
+    features |= (1 << VIRTIO_CONSOLE_F_SIZE);
     if (vser->bus->max_nr_ports > 1) {
         features |= (1 << VIRTIO_CONSOLE_F_MULTIPORT);
     }
diff --git a/hw/virtio-serial.h b/hw/virtio-serial.h
index a93b545..f685ee4 100644
--- a/hw/virtio-serial.h
+++ b/hw/virtio-serial.h
@@ -25,14 +25,16 @@
 #define VIRTIO_ID_CONSOLE		3
 
 /* Features supported */
+#define VIRTIO_CONSOLE_F_SIZE		0
 #define VIRTIO_CONSOLE_F_MULTIPORT	1
 
 #define VIRTIO_CONSOLE_BAD_ID           (~(uint32_t)0)
 
 struct virtio_console_config {
     /*
-     * These two fields are used by VIRTIO_CONSOLE_F_SIZE which
-     * isn't implemented here yet
+     * These two fields hold the size of the underlying chardev.
+     * When it resized, the guest is notified by a control message
+     * if multiport is enabled, or by a config space update if not.
      */
     uint16_t cols;
     uint16_t rows;
@@ -165,6 +167,11 @@ int virtio_serial_open(VirtIOSerialPort *port);
 int virtio_serial_close(VirtIOSerialPort *port);
 
 /*
+ * Notify resize to the guest
+ */
+void virtio_serial_resize_console(VirtIOSerialPort *port, int rows, int cols);
+
+/*
  * Send data to Guest
  */
 ssize_t virtio_serial_write(VirtIOSerialPort *port, const uint8_t *buf,
-- 
1.6.2.5

[Qemu-devel] Re: [PATCH 2/2] virtio-console: Notify resize to the guest.

[Amit Shah]

On (Mon) May 03 2010 [12:06:33], Amit Shah wrote:
> From: Kusanagi Kouichi <slash@ac.auone-net.jp>
> 
> I tested this patch as follows: I put printf()s into involved
> functions. Then ran qemu on a terminal emulator, and resized it. The
> guest kernel gets initial size, and follows host resize. Both singleport
> and multiport work.
> 
> Signed-off-by: Kusanagi Kouichi <slash@ac.auone-net.jp>
> Signed-off-by: Amit Shah <amit.shah@redhat.com>
> ---
>  hw/virtio-console.c    |    3 +++
>  hw/virtio-serial-bus.c |   15 +++++++++++++++
>  hw/virtio-serial.h     |   11 +++++++++--
>  3 files changed, 27 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/virtio-console.c b/hw/virtio-console.c
> index caea11f..58246d1 100644
> --- a/hw/virtio-console.c
> +++ b/hw/virtio-console.c
> @@ -55,6 +55,9 @@ static void chr_event(void *opaque, int event)
>      case CHR_EVENT_CLOSED:
>          virtio_serial_close(&vcon->port);
>          break;
> +    case CHR_EVENT_RESIZE:
> +        virtio_serial_resize_console(&vcon->port, vcon->chr->rows, vcon->chr->cols);
> +        break;
>      }
>  }
>  
> diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
> index 97694d5..8766f1d 100644
> --- a/hw/virtio-serial-bus.c
> +++ b/hw/virtio-serial-bus.c
> @@ -205,6 +205,20 @@ int virtio_serial_close(VirtIOSerialPort *port)
>      return 0;
>  }
>  
> +void virtio_serial_resize_console(VirtIOSerialPort *port, int rows, int cols)
> +{
> +    VirtIOSerial *vser = port->vser;
> +
> +    vser->config.rows = rows;
> +    vser->config.cols = cols;
> +
> +    if (use_multiport(vser)) {
> +        send_control_event(port, VIRTIO_CONSOLE_RESIZE, 0);

Thinking about this, this isn't the right way of sending console resize
updates: we write in the config space and send an update event via
control messages. The control message could be processed after another
config-space update for another port, leading to races.

I think it's better to send the row and column sizes along with the
resize message. We do something similar for sending the 'name'; we could
also imbed a struct within the VIRTIO_CONSOLE_RESIZE message similarly.
This would need some guest kernel changes too.

Kusanagi, can you do it? For guest kernel changes, please work off
linux-next.git for your changes.

If not, I'll get to this in a couple of weeks' time.

Thanks,

		Amit