Re: [Qemu-devel] [PATCH v3] vnc: added initial websocket protocol support

[Tim Hardeck <thardeck@suse.de>]

[-- Attachment #1: Type: text/plain, Size: 7872 bytes --]

Hi Stefan,

On 12/03/2012 05:22 PM, Stefan Hajnoczi wrote:
> Thanks for the patch, Tim.  Some general code review comments below.
Thanks for the code review. I am going to incorporate them in my new patch.

> I hope someone has time to review the VNC and WebSocket specific stuff. 
> I didn't check the details of buffers, whether the WebSocket spec is
> correctly implemented, etc.
I have mainly tested my websockets implementation with the guest OS
openSUSE 12.2 which worked fine during all my tests on several browsers.
I recently found out though that when I run Firefox in openSUSE 12.1,
noVNC complains about an unsupported VNC encoding and QEMU crashes. I
have attached the back trace at the end of this mail.

This issue could be fixed by not encoding Websocket frames directly in
vnc_write but in vnc_client_write_locked. This should also decrease the
overhead through websocket frame headers.
Nevertheless it looks like QEMU did crash because of the sudden
disconnect which shouldn't happen.

I have created a vnc_client_write_ws function which is used instead of
vnc_client_write_plain. I have also moved the decoding part to
vnc_client_read_ws to keep consistency?
Is this Ok or should I add the websocket en/decoding to the existing vnc
plain functions?

Regards
Tim



#0  0x00007ffff3f92d25 in __GI_raise (sig=sig@entry=6) at
../nptl/sysdeps/unix/sysv/linux/raise.c:64
        resultvar = 0
        pid = 24308
        selftid = 24312
#1  0x00007ffff3f941a8 in __GI_abort () at abort.c:91
        save_stage = 2
        act = {__sigaction_handler = {sa_handler = 0x5555559153c0
<__func__.4908>, sa_sigaction =
    0x5555559153c0 <__func__.4908>}, sa_mask = {__val =
{140737287809753, 0, 18374686479671623680,
    0, 140737286898450, 131072, 93825010082032, 2064448, 93825010157472,
1989008, 22,
    140737218422160, 1, 140737488346032, 0, 140737218426624}}, sa_flags
= -201457138, sa_restorer =
    0x6d940}
        sigs = {__val = {32, 0 <repeats 15 times>}}
#2  0x000055555577b5c2 in error_exit (err=22, msg=0x5555559153c0
<__func__.4908> "qemu_mutex_lock")
    at qemu-thread-posix.c:28
No locals.
#3  0x000055555577b6e1 in qemu_mutex_lock (mutex=0x555556666328) at
qemu-thread-posix.c:59
        err = 22
        __func__ = "qemu_mutex_lock"
#4  0x00005555557bb075 in vnc_lock_output (vs=0x55555665a100) at
ui/vnc-jobs.h:63
No locals.
#5  0x00005555557bb5eb in vnc_jobs_consume_buffer (vs=0x55555665a100) at
ui/vnc-jobs.c:166
        flush = false
#6  0x00005555557bb5ae in vnc_jobs_join (vs=0x55555665a100) at
ui/vnc-jobs.c:159
No locals.
#7  0x00005555557bf9d9 in vnc_update_client_sync (vs=0x55555665a100,
has_dirty=1) at ui/vnc.c:876
        ret = 0
---Type <return> to continue, or q <return> to quit---
#8  0x00005555557bf308 in vnc_dpy_copy (ds=0x555556583240, src_x=99,
src_y=143, dst_x=99, dst_y=
    146, w=8, h=1) at ui/vnc.c:752
        vd = 0x7fffeea48010
        vs = 0x55555665a100
        vn = 0x0
        src_row = 0x1800000000 <Address 0x1800000000 out of bounds>
        dst_row = 0xc0000000c00 <Address 0xc0000000c00 out of bounds>
        i = 768
        x = -1
        y = 24
        pitch = 1024
        inc = 0
        w_lim = 0
        s = 8
        cmp_bytes = 2359296
#9  0x0000555555623e78 in dpy_gfx_copy (s=0x555556583240, src_x=99,
src_y=143, dst_x=99, dst_y=146,
    w=8, h=1) at console.h:275
        dcl = 0x5555565f38e0
#10 0x00005555556281f4 in qemu_console_copy (ds=0x555556583240,
src_x=99, src_y=143, dst_x=99,
    dst_y=146, w=8, h=1) at console.c:1598
No locals.
#11 0x000055555566a79a in cirrus_do_copy (s=0x5555565afc08, dst=448832,
src=439616, w=8, h=1)
    at hw/cirrus_vga.c:732
        sx = 99
        sy = 143
        dx = 99
---Type <return> to continue, or q <return> to quit---
        dy = 146
        depth = 3
        notify = 1
#12 0x000055555566a8f7 in cirrus_bitblt_videotovideo_copy
(s=0x5555565afc08) at hw/cirrus_vga.c:750
No locals.
#13 0x000055555566ae8b in cirrus_bitblt_videotovideo (s=0x5555565afc08)
at hw/cirrus_vga.c:872
        ret = 1
#14 0x000055555566b61c in cirrus_bitblt_start (s=0x5555565afc08) at
hw/cirrus_vga.c:1013
        blt_rop = 13 '\r'
#15 0x000055555566b6c8 in cirrus_write_bitblt (s=0x5555565afc08,
reg_value=2)
    at hw/cirrus_vga.c:1034
        old_value = 0
#16 0x000055555566c589 in cirrus_vga_write_gr (s=0x5555565afc08,
reg_index=49, reg_value=2)
    at hw/cirrus_vga.c:1529
No locals.
#17 0x000055555566cebb in cirrus_mmio_blt_write (s=0x5555565afc08,
address=64, value=2 '\002')
    at hw/cirrus_vga.c:1883
No locals.
#18 0x000055555566ed4c in cirrus_mmio_write (opaque=0x5555565afc08,
addr=320, val=2, size=1)
    at hw/cirrus_vga.c:2659
        s = 0x5555565afc08
#19 0x000055555584b7ca in memory_region_write_accessor
(opaque=0x5555565c0538, addr=320, value=
    0x7fffefe92a98, size=1, shift=0, mask=255) at
/suse/thardeck/Development/qemu/memory.c:334
        mr = 0x5555565c0538
        tmp = 2
#20 0x000055555584b8ac in access_with_adjusted_size (addr=320,
value=0x7fffefe92a98, size=4,
---Type <return> to continue, or q <return> to quit---
    access_size_min=1, access_size_max=1, access=0x55555584b745
<memory_region_write_accessor>,
    opaque=0x5555565c0538) at /suse/thardeck/Development/qemu/memory.c:364
        access_mask = 255
        access_size = 1
        i = 0
#21 0x000055555584e51a in memory_region_dispatch_write
(mr=0x5555565c0538, addr=320, data=
    4294967042, size=4) at /suse/thardeck/Development/qemu/memory.c:916
No locals.
#22 0x000055555585154a in io_mem_write (mr=0x5555565c0538, addr=320,
val=4294967042, size=4)
    at /suse/thardeck/Development/qemu/memory.c:1581
No locals.
#23 0x00005555557ea58f in address_space_rw (as=0x5555564ec440
<address_space_memory>, addr=
    4273930560, buf=0x7ffff7ff2028 "\002\377\377\377", len=4, is_write=true)
    at /suse/thardeck/Development/qemu/exec.c:3397
        addr1 = 320
        d = 0x555556561860
        l = 4
        ptr = 0x555555808a39 <cpu_set_apic_base+128> "H\213E\370dH3\004%("
        val = 4294967042
        page = 4273930240
        section = 0x55555664f1a0
#24 0x00005555557ea953 in cpu_physical_memory_rw (addr=4273930560, buf=
    0x7ffff7ff2028 "\002\377\377\377", len=4, is_write=1)
    at /suse/thardeck/Development/qemu/exec.c:3479
No locals.
#25 0x000055555584886e in kvm_cpu_exec (env=0x555556569300)
---Type <return> to continue, or q <return> to quit---
    at /suse/thardeck/Development/qemu/kvm-all.c:1580
        run = 0x7ffff7ff2000
        ret = 0
        run_ret = 0
#26 0x00005555557dc6a0 in qemu_kvm_cpu_thread_fn (arg=0x555556569300)
    at /suse/thardeck/Development/qemu/cpus.c:757
        env = 0x555556569300
        cpu = 0x5555565692a0
        r = 65536
#27 0x00007ffff4fdce0e in start_thread (arg=0x7fffefe93700) at
pthread_create.c:305
        __res = <optimized out>
        pd = 0x7fffefe93700
        now = <optimized out>
        unwind_buf = {cancel_jmp_buf = {{jmp_buf = {140737218426624,
-6613775898926182534, 1,
    140737488346032, 0, 140737218426624, 6613810991376830330,
6613760524492017530},
              mask_was_saved = 0}}, priv = {pad = {0x0, 0x0, 0x0, 0x0},
data = {prev = 0x0,
              cleanup = 0x0, canceltype = 0}}}
        not_first_call = 0
        pagesize_m1 = <optimized out>
        sp = <optimized out>
        freesize = <optimized out>
        __PRETTY_FUNCTION__ = "start_thread"
#28 0x00007ffff40422bd in clone () at
../sysdeps/unix/sysv/linux/x86_64/clone.S:115
No locals.

-- 
SUSE LINUX Products GmbH, GF: Jeff Hawn, Jennifer Guild, Felix
Imendörffer, HRB 16746 (AG Nürnberg)
Maxfeldstr. 5, 90409 Nürnberg, Germany
T: +49 (0) 911 74053-0  F: +49 (0) 911 74053-483
http://www.suse.de/


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 490 bytes --]