[Qemu-devel] [PULL] virtio-serial: crash fix, guest_writable()

[Qemu-devel] [PULL] virtio-serial: crash fix, guest_writable()

[Amit Shah]

The following changes since commit 6e76d125f244e10676b917208f2a074729820246:

  Update version for v2.2.0-rc0 release (2014-11-05 15:21:04 +0000)

are available in the git repository at:

  git://git.kernel.org/pub/scm/virt/qemu/amit/virtio-serial.git tags/vser-2.2.0-queue

for you to fetch changes up to 745d32d12f10badaafd26088c616025ebfe223fe:

  virtio-serial: avoid crash when port has no name (2014-11-06 16:43:35 +0530)

----------------------------------------------------------------
Couple of patches for 2.2.0: one fixes a crash, and the other adds the
guest_writable() API.

----------------------------------------------------------------
Amit Shah (1):
      virtio: serial: expose a 'guest_writable' callback for users

Marc-André Lureau (1):
      virtio-serial: avoid crash when port has no name

 hw/char/virtio-serial-bus.c       | 33 ++++++++++++++++++++++++++++++++-
 include/hw/virtio/virtio-serial.h | 11 +++++++++++
 2 files changed, 43 insertions(+), 1 deletion(-)

[Qemu-devel] [PATCH 1/2] virtio: serial: expose a 'guest_writable' callback for users

[Amit Shah]

Users of virtio-serial may want to know when a port becomes writable.  A
port can stop accepting writes if the guest port is open but not being
read from.  In this case, data gets queued up in the virtqueue, and
after the vq is full, writes to the port do not succeed.

When the guest reads off a vq element, and adds a new one for the host
to put data in, we can tell users the port is available for more writes,
via the new ->guest_writable() callback.

Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 hw/char/virtio-serial-bus.c       | 31 +++++++++++++++++++++++++++++++
 include/hw/virtio/virtio-serial.h | 11 +++++++++++
 2 files changed, 42 insertions(+)

diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c
index c6870f1..bea7a17 100644
--- a/hw/char/virtio-serial-bus.c
+++ b/hw/char/virtio-serial-bus.c
@@ -465,6 +465,37 @@ static void handle_output(VirtIODevice *vdev, VirtQueue *vq)
 
 static void handle_input(VirtIODevice *vdev, VirtQueue *vq)
 {
+    /*
+     * Users of virtio-serial would like to know when guest becomes
+     * writable again -- i.e. if a vq had stuff queued up and the
+     * guest wasn't reading at all, the host would not be able to
+     * write to the vq anymore.  Once the guest reads off something,
+     * we can start queueing things up again.  However, this call is
+     * made for each buffer addition by the guest -- even though free
+     * buffers existed prior to the current buffer addition.  This is
+     * done so as not to maintain previous state, which will need
+     * additional live-migration-related changes.
+     */
+    VirtIOSerial *vser;
+    VirtIOSerialPort *port;
+    VirtIOSerialPortClass *vsc;
+
+    vser = VIRTIO_SERIAL(vdev);
+    port = find_port_by_vq(vser, vq);
+
+    if (!port) {
+        return;
+    }
+    vsc = VIRTIO_SERIAL_PORT_GET_CLASS(port);
+
+    /*
+     * If guest_connected is false, this call is being made by the
+     * early-boot queueing up of descriptors, which is just noise for
+     * the host apps -- don't disturb them in that case.
+     */
+    if (port->guest_connected && port->host_connected && vsc->guest_writable) {
+        vsc->guest_writable(port);
+    }
 }
 
 static uint32_t get_features(VirtIODevice *vdev, uint32_t features)
diff --git a/include/hw/virtio/virtio-serial.h b/include/hw/virtio/virtio-serial.h
index a679e54..6c7b3b8 100644
--- a/include/hw/virtio/virtio-serial.h
+++ b/include/hw/virtio/virtio-serial.h
@@ -98,6 +98,17 @@ typedef struct VirtIOSerialPortClass {
         /* Guest is now ready to accept data (virtqueues set up). */
     void (*guest_ready)(VirtIOSerialPort *port);
 
+        /*
+         * Guest has enqueued a buffer for the host to write into.
+         * Called each time a buffer is enqueued by the guest;
+         * irrespective of whether there already were free buffers the
+         * host could have consumed.
+         *
+         * This is dependent on both, the guest and host ends being
+         * connected.
+         */
+    void (*guest_writable)(VirtIOSerialPort *port);
+
     /*
      * Guest wrote some data to the port. This data is handed over to
      * the app via this callback.  The app can return a size less than
-- 
1.9.3

[Qemu-devel] [PATCH 2/2] virtio-serial: avoid crash when port has no name

[Amit Shah]

From: Marc-André Lureau <marcandre.lureau@gmail.com>

It seems "name" is not mandatory, and the following command line (based
on one generated by current libvirt) will crash qemu at start:

qemu-system-x86_64 \
    -device virtio-serial-pci \
    -device virtserialport,name=foo \
    -device virtconsole

Program received signal SIGSEGV, Segmentation fault.
__strcmp_ssse3 () at ../sysdeps/x86_64/strcmp.S:210
210        movlpd    (%rsi), %xmm2
Missing separate debuginfos, use: debuginfo-install
python-libs-2.7.5-13.fc20.x86_64
(gdb) bt
 #0  __strcmp_ssse3 () at ../sysdeps/x86_64/strcmp.S:210
 #1  0x000055555566bdc6 in find_port_by_name (name=0x0) at /home/elmarco/src/qemu/hw/char/virtio-serial-bus.c:67

Signed-off-by: Marc-André Lureau <marcandre.lureau@gmail.com>
Reviewed-by: Amos Kong <akong@redhat.com>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 hw/char/virtio-serial-bus.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c
index bea7a17..c191283 100644
--- a/hw/char/virtio-serial-bus.c
+++ b/hw/char/virtio-serial-bus.c
@@ -902,7 +902,7 @@ static void virtser_port_device_realize(DeviceState *dev, Error **errp)
         return;
     }
 
-    if (find_port_by_name(port->name)) {
+    if (port->name != NULL && find_port_by_name(port->name)) {
         error_setg(errp, "virtio-serial-bus: A port already exists by name %s",
                    port->name);
         return;
-- 
1.9.3

Re: [Qemu-devel] [PULL] virtio-serial: crash fix, guest_writable()

[Peter Maydell]

On 6 November 2014 11:20, Amit Shah <amit.shah@redhat.com> wrote:
> The following changes since commit 6e76d125f244e10676b917208f2a074729820246:
>
>   Update version for v2.2.0-rc0 release (2014-11-05 15:21:04 +0000)
>
> are available in the git repository at:
>
>   git://git.kernel.org/pub/scm/virt/qemu/amit/virtio-serial.git tags/vser-2.2.0-queue
>
> for you to fetch changes up to 745d32d12f10badaafd26088c616025ebfe223fe:
>
>   virtio-serial: avoid crash when port has no name (2014-11-06 16:43:35 +0530)
>
> ----------------------------------------------------------------
> Couple of patches for 2.2.0: one fixes a crash, and the other adds the
> guest_writable() API.
>
> ----------------------------------------------------------------
> Amit Shah (1):
>       virtio: serial: expose a 'guest_writable' callback for users

...this looks suspiciously like a new feature, not a bugfix?

thanks
-- PMM

Re: [Qemu-devel] [PULL] virtio-serial: crash fix, guest_writable()

[Amit Shah]

On (Thu) 06 Nov 2014 [12:49:17], Peter Maydell wrote:
> On 6 November 2014 11:20, Amit Shah <amit.shah@redhat.com> wrote:
> > The following changes since commit 6e76d125f244e10676b917208f2a074729820246:
> >
> >   Update version for v2.2.0-rc0 release (2014-11-05 15:21:04 +0000)
> >
> > are available in the git repository at:
> >
> >   git://git.kernel.org/pub/scm/virt/qemu/amit/virtio-serial.git tags/vser-2.2.0-queue
> >
> > for you to fetch changes up to 745d32d12f10badaafd26088c616025ebfe223fe:
> >
> >   virtio-serial: avoid crash when port has no name (2014-11-06 16:43:35 +0530)
> >
> > ----------------------------------------------------------------
> > Couple of patches for 2.2.0: one fixes a crash, and the other adds the
> > guest_writable() API.
> >
> > ----------------------------------------------------------------
> > Amit Shah (1):
> >       virtio: serial: expose a 'guest_writable' callback for users
> 
> ...this looks suspiciously like a new feature, not a bugfix?

Removed that and re-sent the pull req.

		Amit