[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 v2 2/2] virtio-console: Notify resize to the guest.

[Kusanagi Kouichi]

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. However, it doesn't update the size.
It seems to need to enable multiport for resize.

v2:
 Rename virtio_serial_resize to virtio_serial_resize_console.

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 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..72fbf3e 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -205,6 +205,13 @@ int virtio_serial_close(VirtIOSerialPort *port)
     return 0;
 }
 
+void virtio_serial_resize_console(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)
@@ -425,6 +432,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..6a00d0c 100644
--- a/hw/virtio-serial.h
+++ b/hw/virtio-serial.h
@@ -25,14 +25,14 @@
 #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 are used by VIRTIO_CONSOLE_F_SIZE
      */
     uint16_t cols;
     uint16_t rows;
@@ -165,6 +165,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.7.1

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

[Amit Shah]

On (Fri) Apr 30 2010 [17:12:19], Kusanagi Kouichi wrote:
> 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. However, it doesn't update the size.
> It seems to need to enable multiport for resize.

If the guest kernel supports multiport, you should send the control
event.

If the guest kernel doesn't support multiport, then you should send a
config space update so that the console size is updated for the only
port that exists.

Sorry for not catching this earlier.

Can you test both the possibilities and let me know? This patch will
have to be updated, of course, to support resize over the config space
in the following function:

> +void virtio_serial_resize_console(VirtIOSerialPort *port, int rows, int cols)
> +{
> +    port->vser->config.rows = rows;
> +    port->vser->config.cols = cols;
> +    send_control_event(port, VIRTIO_CONSOLE_RESIZE, 0);
> +}
> +

...

>  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

This comment could be expanded to explain when we use the config space
for size updates (!multiport) and when we use control messages
(multiport).

Thanks!
		Amit

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

[Kusanagi Kouichi]

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.

v3:
 If multiport is enabled, send the control message. If not, send a config space update.

v2:
 Rename virtio_serial_resize to virtio_serial_resize_console.

Signed-off-by: Kusanagi Kouichi <slash@ac.auone-net.jp>
---
 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..b51a040 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 the 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.7.1

[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 v3 2/2] virtio-console: Notify resize to the guest.

[Amit Shah]

On (Mon) May 03 2010 [13:58:13], Kusanagi Kouichi wrote:
> 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.
> 
> v3:
>  If multiport is enabled, send the control message. If not, send a config space update.

Thanks!

I've just sent out both the patches, and have modified this patch a bit
in virtio-serial.h comment and the patch description.

The 1/2 patch uses a timer. I'm not sure that's the best way to do it,
but I'll leave that upto the maintainers.

		Amit