[Qemu-devel] [PULL 2.7 0/1] virtio-console: fix receiving data from guest

[Qemu-devel] [PULL 2.7 0/1] virtio-console: fix receiving data from guest

[Amit Shah]

The following changes since commit d08306dc42ea599ffcf8aad056fa9c23acfbe230:

  Merge remote-tracking branch 'remotes/mst/tags/for_upstream' into staging (2016-08-10 17:14:35 +0100)

are available in the git repository at:

  http://git.kernel.org/pub/scm/virt/qemu/amit/virtio-serial.git tags/vser-for-2.7-1

for you to fetch changes up to bce6261eb2d879625126485d4ddd28cacb93152e:

  virtio-console: set frontend open permanently for console devs (2016-08-11 16:38:58 +0530)

----------------------------------------------------------------
virtio-console: fix receiving data from guest

----------------------------------------------------------------


Daniel P. Berrange (1):
  virtio-console: set frontend open permanently for console devs

 hw/char/virtio-console.c | 25 +++++++++++++++++++++----
 1 file changed, 21 insertions(+), 4 deletions(-)

-- 
2.7.4

[Qemu-devel] [PULL 2.7 1/1] virtio-console: set frontend open permanently for console devs

[Amit Shah]

From: "Daniel P. Berrange" <berrange@redhat.com>

The virtio-console.c file handles both serial consoles
and interactive consoles, since they're backed by the
same device model.

Since serial devices are expected to be reliable and
need to notify the guest when the backend is opened
or closed, the virtio-console.c file wires up support
for chardev events. This affects both serial consoles
and interactive consoles, using a network connection
based chardev backend such as 'socket', but not when
using a PTY based backend or plain 'file' backends.

When the host side is not connected the handle_output()
method in virtio-serial-bus.c will drop any data sent
by the guest, before it even reaches the virtio-console.c
code. This means that if the chardev has a logfile
configured, the data will never get logged.

Consider for example, configuring a x86_64 guest with a
plain UART serial port

  -chardev socket,id=charserial1,host=127.0.0.1,port=9001,server,nowait,logfile=console1.log,logappend=on
  -device isa-serial,chardev=charserial1,id=serial1

vs a s390 guest which has to use the virtio-console port

  -chardev socket,id=charconsole1,host=127.0.0.1,port=9000,server,nowait,logfile=console2.log,logappend=on
  -device virtconsole,chardev=charconsole1,id=console1

The isa-serial one gets data written to the log regardless
of whether a client is connected, while the virtioconsole
one only gets data written to the log when a client is
connected.

There is no need for virtio-serial-bus.c to aggressively
drop the data for console devices, as the chardev code is
prefectly capable of discarding the data itself.

So this patch changes virtconsole devices so that they
are always marked as having the host side open. This
ensures that the guest OS will always send any data it
has (Linux virtio-console hvc driver actually ignores
the host open state and sends data regardless, but we
should not rely on that), and also prevents the
virtio-serial-bus code prematurely discarding data.

The behaviour of virtserialport devices is *not* changed,
only virtconsole, because for the former, it is important
that the guest OSknow exactly when the host side is opened
/ closed so it can do any protocol re-negotiation that may
be required.

Fixes bug: https://bugs.launchpad.net/qemu/+bug/1599214

Acked-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Message-Id: <1470241360-3574-2-git-send-email-berrange@redhat.com>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 hw/char/virtio-console.c | 25 +++++++++++++++++++++----
 1 file changed, 21 insertions(+), 4 deletions(-)

diff --git a/hw/char/virtio-console.c b/hw/char/virtio-console.c
index 2e36481..4f0e03d 100644
--- a/hw/char/virtio-console.c
+++ b/hw/char/virtio-console.c
@@ -85,8 +85,9 @@ static void set_guest_connected(VirtIOSerialPort *port, int guest_connected)
 {
     VirtConsole *vcon = VIRTIO_CONSOLE(port);
     DeviceState *dev = DEVICE(port);
+    VirtIOSerialPortClass *k = VIRTIO_SERIAL_PORT_GET_CLASS(port);
 
-    if (vcon->chr) {
+    if (vcon->chr && !k->is_console) {
         qemu_chr_fe_set_open(vcon->chr, guest_connected);
     }
 
@@ -156,9 +157,25 @@ static void virtconsole_realize(DeviceState *dev, Error **errp)
     }
 
     if (vcon->chr) {
-        vcon->chr->explicit_fe_open = 1;
-        qemu_chr_add_handlers(vcon->chr, chr_can_read, chr_read, chr_event,
-                              vcon);
+        /*
+         * For consoles we don't block guest data transfer just
+         * because nothing is connected - we'll just let it go
+         * whetherever the chardev wants - /dev/null probably.
+         *
+         * For serial ports we need 100% reliable data transfer
+         * so we use the opened/closed signals from chardev to
+         * trigger open/close of the device
+         */
+        if (k->is_console) {
+            vcon->chr->explicit_fe_open = 0;
+            qemu_chr_add_handlers(vcon->chr, chr_can_read, chr_read,
+                                  NULL, vcon);
+            virtio_serial_open(port);
+        } else {
+            vcon->chr->explicit_fe_open = 1;
+            qemu_chr_add_handlers(vcon->chr, chr_can_read, chr_read,
+                                  chr_event, vcon);
+        }
     }
 }
 
-- 
2.7.4

Re: [Qemu-devel] [PULL 2.7 0/1] virtio-console: fix receiving data from guest

[Peter Maydell]

On 11 August 2016 at 12:23, Amit Shah <amit.shah@redhat.com> wrote:
> The following changes since commit d08306dc42ea599ffcf8aad056fa9c23acfbe230:
>
>   Merge remote-tracking branch 'remotes/mst/tags/for_upstream' into staging (2016-08-10 17:14:35 +0100)
>
> are available in the git repository at:
>
>   http://git.kernel.org/pub/scm/virt/qemu/amit/virtio-serial.git tags/vser-for-2.7-1
>
> for you to fetch changes up to bce6261eb2d879625126485d4ddd28cacb93152e:
>
>   virtio-console: set frontend open permanently for console devs (2016-08-11 16:38:58 +0530)
>
> ----------------------------------------------------------------
> virtio-console: fix receiving data from guest
>
> ----------------------------------------------------------------
>
>
> Daniel P. Berrange (1):
>   virtio-console: set frontend open permanently for console devs
>
>  hw/char/virtio-console.c | 25 +++++++++++++++++++++----
>  1 file changed, 21 insertions(+), 4 deletions(-)

This hung in make check (clang-on-x86-64-linux build):

Offending process:

i386-softmmu/qemu-system-i386 -qtest unix:/tmp/qtest-5861.sock,nowait
-qtest-log /dev/null -qmp unix:/tmp/qtest-5861.qmp,nowait -machine
accel=qtest -display none -machine accel=tcg -m 512 -object
memory-backend-file,id=mem,size=512M,mem-path=/tmp/vhost-test-wWBX5W,share=on
-numa node,memdev=mem -chardev
socket,id=chr-test,path=/tmp/vhost-test-wWBX5W/test.sock -netdev
vhost-user,id=net0,chardev=chr-test,vhostforce -device
virtio-net-pci,netdev=net0,romfile=./pc-bios/pxe-virtio.rom

Backtraces below, but the fact that thread 3 is busy-looping in
qemu_chr_fe_read_all() looks suspicious.


(gdb) thread apply all bt

Thread 3 (Thread 0x7fd2b3135700 (LWP 5865)):
#0  0x00007fd2ea53bc5d in nanosleep () at ../sysdeps/unix/syscall-template.S:84
#1  0x00007fd2ed74b238 in g_usleep (microseconds=<optimised out>)
    at /build/glib2.0-7IO_Yw/glib2.0-2.48.1/./glib/gtimer.c:259
#2  0x0000563a5c14d08a in qemu_chr_fe_read_all (s=<optimised out>,
buf=<optimised out>, len=<optimised out>) at
/home/petmay01/linaro/qemu-for-merges/qemu-char.c:337
#3  0x0000563a5bfc1a79 in vhost_user_read (dev=<optimised out>,
msg=0x7fd2b3131500)
    at /home/petmay01/linaro/qemu-for-merges/hw/virtio/vhost-user.c:123
#4  0x0000563a5bfc1799 in vhost_user_get_u64 (dev=0x563a606a5b60,
request=1, u64=0x7fd2b31316a0) at
/home/petmay01/linaro/qemu-for-merges/hw/virtio/vhost-user.c:455
#5  0x0000563a5bfc0a6d in vhost_user_set_mem_table
(dev=0x563a606a5b60, features=0x0)
    at /home/petmay01/linaro/qemu-for-merges/hw/virtio/vhost-user.c:477
#6  0x0000563a5bfc0a6d in vhost_user_set_mem_table (dev=<optimised
out>, mem=<optimised out>) at
/home/petmay01/linaro/qemu-for-merges/hw/virtio/vhost-user.c:541
#7  0x0000563a5bfb578f in vhost_dev_start (hdev=<optimised out>,
vdev=0x563a61ec3d50)
    at /home/petmay01/linaro/qemu-for-merges/hw/virtio/vhost.c:1304
#8  0x0000563a5bf4686a in vhost_net_start (net=0x563a606a5b60,
dev=<optimised out>)
    at /home/petmay01/linaro/qemu-for-merges/hw/net/vhost_net.c:232
#9  0x0000563a5bf4686a in vhost_net_start (dev=<optimised out>,
ncs=<optimised out>, total_queues=<optimised out>)
    at /home/petmay01/linaro/qemu-for-merges/hw/net/vhost_net.c:324
#10 0x0000563a5bf39b53 in virtio_net_set_status (status=<optimised
out>, n=<optimised out>) at
/home/petmay01/linaro/qemu-for-merges/hw/net/virtio-net.c:151
#11 0x0000563a5bf39b53 in virtio_net_set_status (vdev=<optimised out>,
status=<optimised out>) at
/home/petmay01/linaro/qemu-for-merges/hw/net/virtio-net.c:224
#12 0x0000563a5bfa273e in virtio_set_status (vdev=0x563a61ec3d50,
val=<optimised out>) at
/home/petmay01/linaro/qemu-for-merges/hw/virtio/virtio.c:759
#13 0x0000563a5c7126ea in virtio_pci_config_write (opaque=<optimised
out>, addr=18, val=6) at
/home/petmay01/linaro/qemu-for-merges/hw/virtio/virtio-pci.c:400
#14 0x0000563a5c7126ea in virtio_pci_config_write (opaque=<optimised
out>, addr=18, val=<optimised out>, size=1)
    at /home/petmay01/linaro/qemu-for-merges/hw/virtio/virtio-pci.c:525
#15 0x0000563a5be748cc in memory_region_write_accessor (mr=<optimised
out>, addr=<optimised out>, value=<optimised out>, size=<optimised
out>, shift=<optimised out>, mask=<optimised out>, attrs=...) at
/home/petmay01/linaro/qemu-for-merges/memory.c:525
#16 0x0000563a5be7448d in access_with_adjusted_size (addr=<optimised
out>, value=<optimised out>, size=<optimised out>, access_size_min=1,
access_size_max=4, access=<optimised out>, mr=<optimised out>,
attrs=...)
    at /home/petmay01/linaro/qemu-for-merges/memory.c:591
#17 0x0000563a5be7401a in memory_region_dispatch_write (mr=<optimised
out>, addr=<optimised out>, data=<optimised out>, size=<optimised
out>, attrs=...)
    at /home/petmay01/linaro/qemu-for-merges/memory.c:1275
#18 0x0000563a5bdd1344 in address_space_write (as=<optimised out>,
addr=<optimised out>, attrs=..., len=<optimised out>,
addr1=140542924231664, l=<optimised out>, mr=0x563a61ebc2a0,
buf=<optimised out>) at
/home/petmay01/linaro/qemu-for-merges/exec.c:2538
#19 0x0000563a5bdd1344 in address_space_write (as=<optimised out>,
addr=<optimised out>, attrs=..., buf=<optimised out>, len=<optimised
out>)
    at /home/petmay01/linaro/qemu-for-merges/exec.c:2601
#20 0x0000563a5bdd4250 in address_space_stb (as=0x7fd2b31313e0,
addr=140542924231664, attrs=..., buf=0xfffffffffffffdfc <error: Cannot
access memory at address 0xfffffffffffffdfc>, len=1, is_write=255) at
/home/petmay01/linaro/qemu-for-merges/exec.c:2703
#21 0x0000563a5bdd4250 in address_space_stb (as=0x7fd2b31313e0,
addr=140542924231664, val=<optimised out>, attrs=..., result=0x0)
    at /home/petmay01/linaro/qemu-for-merges/exec.c:3443
#22 0x00007fd2b5710bb1 in code_gen_buffer ()
#23 0x0000563a5bde1e7d in cpu_tb_exec (cpu=0x563a606ad370, itb=<optimised out>)
    at /home/petmay01/linaro/qemu-for-merges/cpu-exec.c:166
#24 0x0000563a5bddf375 in cpu_exec (tb=0x7fd2b3288c90,
last_tb=<optimised out>, sc=<optimised out>, cpu=<optimised out>,
tb_exit=<optimised out>)
    at /home/petmay01/linaro/qemu-for-merges/cpu-exec.c:530
#25 0x0000563a5bddf375 in cpu_exec (cpu=<optimised out>)
---Type <return> to continue, or q <return> to quit---
    at /home/petmay01/linaro/qemu-for-merges/cpu-exec.c:625
#26 0x0000563a5be35e66 in qemu_tcg_cpu_thread_fn (cpu=0x563a606ad370)
    at /home/petmay01/linaro/qemu-for-merges/cpus.c:1541
#27 0x0000563a5be35e66 in qemu_tcg_cpu_thread_fn ()
    at /home/petmay01/linaro/qemu-for-merges/cpus.c:1574
#28 0x0000563a5be35e66 in qemu_tcg_cpu_thread_fn (arg=<optimised out>)
    at /home/petmay01/linaro/qemu-for-merges/cpus.c:1171
#29 0x00007fd2ea5326fa in start_thread (arg=0x7fd2b3135700) at
pthread_create.c:333
#30 0x00007fd2e9c46b5d in clone () at
../sysdeps/unix/sysv/linux/x86_64/clone.S:109

Thread 2 (Thread 0x7fd2ddf95700 (LWP 5864)):
#0  0x00007fd2e9c40c19 in syscall ()
    at ../sysdeps/unix/sysv/linux/x86_64/syscall.S:38
#1  0x0000563a5caedae6 in qemu_event_wait (ev=<optimised out>,
val=<optimised out>)
    at /home/petmay01/linaro/qemu-for-merges/util/qemu-thread-posix.c:292
#2  0x0000563a5caedae6 in qemu_event_wait (ev=<optimised out>)
    at /home/petmay01/linaro/qemu-for-merges/util/qemu-thread-posix.c:399
#3  0x0000563a5cb2b32e in synchronize_rcu ()
    at /home/petmay01/linaro/qemu-for-merges/util/rcu.c:132
#4  0x0000563a5cb2b32e in synchronize_rcu ()
    at /home/petmay01/linaro/qemu-for-merges/util/rcu.c:163
#5  0x0000563a5cb2c1cd in call_rcu_thread (opaque=<optimised out>)
    at /home/petmay01/linaro/qemu-for-merges/util/rcu.c:257
#6  0x00007fd2ea5326fa in start_thread (arg=0x7fd2ddf95700) at
pthread_create.c:333
#7  0x00007fd2e9c46b5d in clone () at
../sysdeps/unix/sysv/linux/x86_64/clone.S:109

Thread 1 (Thread 0x7fd305271f80 (LWP 5863)):
#0  0x00007fd2ea53b2ad in __lll_lock_wait ()
    at ../sysdeps/unix/sysv/linux/x86_64/lowlevellock.S:135
#1  0x00007fd2ea534dfd in __GI___pthread_mutex_lock
(mutex=0x563a5e756570 <qemu_global_mutex>) at
../nptl/pthread_mutex_lock.c:80
#2  0x0000563a5caed17b in qemu_mutex_lock (mutex=0x563a5e756570
<qemu_global_mutex>)
    at /home/petmay01/linaro/qemu-for-merges/util/qemu-thread-posix.c:64
#3  0x0000563a5be31b7e in qemu_mutex_lock_iothread ()
    at /home/petmay01/linaro/qemu-for-merges/cpus.c:1276
#4  0x0000563a5c8e6276 in main_loop_wait (timeout=50860067)
    at /home/petmay01/linaro/qemu-for-merges/main-loop.c:255
#5  0x0000563a5c8e6276 in main_loop_wait (nonblocking=<optimised out>)
    at /home/petmay01/linaro/qemu-for-merges/main-loop.c:506
#6  0x0000563a5c176cea in main () at
/home/petmay01/linaro/qemu-for-merges/vl.c:1908
#7  0x0000563a5c176cea in main (argc=<optimised out>, argv=<optimised
out>, envp=<optimised out>) at
/home/petmay01/linaro/qemu-for-merges/vl.c:4603

thanks
-- PMM

Re: [Qemu-devel] [PULL 2.7 0/1] virtio-console: fix receiving data from guest

[Daniel P. Berrange]

On Thu, Aug 11, 2016 at 03:37:27PM +0100, Peter Maydell wrote:
> On 11 August 2016 at 12:23, Amit Shah <amit.shah@redhat.com> wrote:
> > The following changes since commit d08306dc42ea599ffcf8aad056fa9c23acfbe230:
> >
> >   Merge remote-tracking branch 'remotes/mst/tags/for_upstream' into staging (2016-08-10 17:14:35 +0100)
> >
> > are available in the git repository at:
> >
> >   http://git.kernel.org/pub/scm/virt/qemu/amit/virtio-serial.git tags/vser-for-2.7-1
> >
> > for you to fetch changes up to bce6261eb2d879625126485d4ddd28cacb93152e:
> >
> >   virtio-console: set frontend open permanently for console devs (2016-08-11 16:38:58 +0530)
> >
> > ----------------------------------------------------------------
> > virtio-console: fix receiving data from guest
> >
> > ----------------------------------------------------------------
> >
> >
> > Daniel P. Berrange (1):
> >   virtio-console: set frontend open permanently for console devs
> >
> >  hw/char/virtio-console.c | 25 +++++++++++++++++++++----
> >  1 file changed, 21 insertions(+), 4 deletions(-)
> 
> This hung in make check (clang-on-x86-64-linux build):
> 
> Offending process:
> 
> i386-softmmu/qemu-system-i386 -qtest unix:/tmp/qtest-5861.sock,nowait
> -qtest-log /dev/null -qmp unix:/tmp/qtest-5861.qmp,nowait -machine
> accel=qtest -display none -machine accel=tcg -m 512 -object
> memory-backend-file,id=mem,size=512M,mem-path=/tmp/vhost-test-wWBX5W,share=on
> -numa node,memdev=mem -chardev
> socket,id=chr-test,path=/tmp/vhost-test-wWBX5W/test.sock -netdev
> vhost-user,id=net0,chardev=chr-test,vhostforce -device
> virtio-net-pci,netdev=net0,romfile=./pc-bios/pxe-virtio.rom
> 
> Backtraces below, but the fact that thread 3 is busy-looping in
> qemu_chr_fe_read_all() looks suspicious.

Strange, because that command line doesn't even add a virtio-console
or virtio-serial device and AFAICT it doesn't hotplug any such device
either. So I'm unclear how this patch could affect that test at all. 


Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PULL 2.7 0/1] virtio-console: fix receiving data from guest

[Peter Maydell]

On 11 August 2016 at 15:56, Daniel P. Berrange <berrange@redhat.com> wrote:
> On Thu, Aug 11, 2016 at 03:37:27PM +0100, Peter Maydell wrote:
>> On 11 August 2016 at 12:23, Amit Shah <amit.shah@redhat.com> wrote:
>> > The following changes since commit d08306dc42ea599ffcf8aad056fa9c23acfbe230:
>> >
>> >   Merge remote-tracking branch 'remotes/mst/tags/for_upstream' into staging (2016-08-10 17:14:35 +0100)
>> >
>> > are available in the git repository at:
>> >
>> >   http://git.kernel.org/pub/scm/virt/qemu/amit/virtio-serial.git tags/vser-for-2.7-1
>> >
>> > for you to fetch changes up to bce6261eb2d879625126485d4ddd28cacb93152e:
>> >
>> >   virtio-console: set frontend open permanently for console devs (2016-08-11 16:38:58 +0530)
>> >
>> > ----------------------------------------------------------------
>> > virtio-console: fix receiving data from guest
>> >
>> > ----------------------------------------------------------------
>> >
>> >
>> > Daniel P. Berrange (1):
>> >   virtio-console: set frontend open permanently for console devs
>> >
>> >  hw/char/virtio-console.c | 25 +++++++++++++++++++++----
>> >  1 file changed, 21 insertions(+), 4 deletions(-)
>>
>> This hung in make check (clang-on-x86-64-linux build):
>>
>> Offending process:
>>
>> i386-softmmu/qemu-system-i386 -qtest unix:/tmp/qtest-5861.sock,nowait
>> -qtest-log /dev/null -qmp unix:/tmp/qtest-5861.qmp,nowait -machine
>> accel=qtest -display none -machine accel=tcg -m 512 -object
>> memory-backend-file,id=mem,size=512M,mem-path=/tmp/vhost-test-wWBX5W,share=on
>> -numa node,memdev=mem -chardev
>> socket,id=chr-test,path=/tmp/vhost-test-wWBX5W/test.sock -netdev
>> vhost-user,id=net0,chardev=chr-test,vhostforce -device
>> virtio-net-pci,netdev=net0,romfile=./pc-bios/pxe-virtio.rom
>>
>> Backtraces below, but the fact that thread 3 is busy-looping in
>> qemu_chr_fe_read_all() looks suspicious.
>
> Strange, because that command line doesn't even add a virtio-console
> or virtio-serial device and AFAICT it doesn't hotplug any such device
> either. So I'm unclear how this patch could affect that test at all.

Yeah, I did a manual rerun of the test and it didn't reproduce,
so it might be an intermittent bug already in master :-(

thanks
-- PMM

Re: [Qemu-devel] [PULL 2.7 0/1] virtio-console: fix receiving data from guest

[Peter Maydell]

On 11 August 2016 at 16:01, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 11 August 2016 at 15:56, Daniel P. Berrange <berrange@redhat.com> wrote:
>> Strange, because that command line doesn't even add a virtio-console
>> or virtio-serial device and AFAICT it doesn't hotplug any such device
>> either. So I'm unclear how this patch could affect that test at all.
>
> Yeah, I did a manual rerun of the test and it didn't reproduce,
> so it might be an intermittent bug already in master :-(

Rerun of the test suite was ok, so I'm going to push this to master.

thanks
-- PMM