Re: [Qemu-devel] [Spice-devel] [PATCH] spice: fix simple display on bigendian hosts
On Mi, 2015-04-15 at 15:55 +0300, Denis Kirjanov wrote: > On 4/15/15, Gerd Hoffmann wrote: > > On Di, 2015-04-14 at 17:47 +0300, Denis Kirjanov wrote: > >> On 4/14/15, Denis Kirjanov wrote: > >> > On 4/14/15, Denis Kirjanov wrote: > >> >> On 4/14/15, Gerd Hoffmann wrote: > >> >>> Denis Kirjanov is busy getting spice run on ppc64 and trapped into > >> >>> this > >> >>> one. Spice wire format is little endian, so we have to explicitly > >> >>> say > >> >>> we want little endian when letting pixman convert the data for us. > >> >>> > >> >>> Reported-by: Denis Kirjanov > >> >>> Signed-off-by: Gerd Hoffmann > >> >>> --- > >> >> Yeah, that fixes the issue. Thanks Gerd! > >> > > >> > Looks like that the patch fixes the half of the problem: the inverted > >> > colors appear on client reconnect to vm > >> > >> Program received signal SIGSEGV, Segmentation fault. > >> 0x74e10258 in ?? () from > >> /usr/lib/x86_64-linux-gnu/libpixman-1.so.0 > >> (gdb) bt > >> #0 0x74e10258 in ?? () from > >> /usr/lib/x86_64-linux-gnu/libpixman-1.so.0 > >> #1 0x74e10239 in pixman_image_unref () from > >> /usr/lib/x86_64-linux-gnu/libpixman-1.so.0 > >> #2 0x778e4117 in canvas_get_quic > >> (canvas=canvas@entry=0x7ceb80, image=image@entry=0xae2720, > >> want_original=want_original@entry=0) at > >> ../spice-common/common/canvas_base.c:390 > > > > spice client crash? > > is this spice client running on big endian or little endian machine? > > it's running on x86_64 as shown in the stack trace. Hmm. spice client should not crash no matter what. But beside that it is entirely possible that there is a byteswap missing somewhere in either qemu or spice-server where bigendian is sent instead of little endian, which in turn triggers the bug in the client. IIRC spice protocol support was added to wireshark a while back, so it should be possible to run tests and compare traces to le + be hosts to see whenever there is something wrong in the wire format. Which should help tracing down the bug in the source code. HTH, Gerd
Re: [Qemu-devel] [Spice-devel] [PATCH] spice: fix simple display on bigendian hosts
On 4/15/15, Gerd Hoffmann wrote: > On Di, 2015-04-14 at 17:47 +0300, Denis Kirjanov wrote: >> On 4/14/15, Denis Kirjanov wrote: >> > On 4/14/15, Denis Kirjanov wrote: >> >> On 4/14/15, Gerd Hoffmann wrote: >> >>> Denis Kirjanov is busy getting spice run on ppc64 and trapped into >> >>> this >> >>> one. Spice wire format is little endian, so we have to explicitly >> >>> say >> >>> we want little endian when letting pixman convert the data for us. >> >>> >> >>> Reported-by: Denis Kirjanov >> >>> Signed-off-by: Gerd Hoffmann >> >>> --- >> >> Yeah, that fixes the issue. Thanks Gerd! >> > >> > Looks like that the patch fixes the half of the problem: the inverted >> > colors appear on client reconnect to vm >> >> Program received signal SIGSEGV, Segmentation fault. >> 0x74e10258 in ?? () from >> /usr/lib/x86_64-linux-gnu/libpixman-1.so.0 >> (gdb) bt >> #0 0x74e10258 in ?? () from >> /usr/lib/x86_64-linux-gnu/libpixman-1.so.0 >> #1 0x74e10239 in pixman_image_unref () from >> /usr/lib/x86_64-linux-gnu/libpixman-1.so.0 >> #2 0x778e4117 in canvas_get_quic >> (canvas=canvas@entry=0x7ceb80, image=image@entry=0xae2720, >> want_original=want_original@entry=0) at >> ../spice-common/common/canvas_base.c:390 > > spice client crash? > is this spice client running on big endian or little endian machine? it's running on x86_64 as shown in the stack trace. > cheers, > Gerd > > > -- Regards, Denis
Re: [Qemu-devel] [Spice-devel] [PATCH] spice: fix simple display on bigendian hosts
On Di, 2015-04-14 at 17:47 +0300, Denis Kirjanov wrote: > On 4/14/15, Denis Kirjanov wrote: > > On 4/14/15, Denis Kirjanov wrote: > >> On 4/14/15, Gerd Hoffmann wrote: > >>> Denis Kirjanov is busy getting spice run on ppc64 and trapped into this > >>> one. Spice wire format is little endian, so we have to explicitly say > >>> we want little endian when letting pixman convert the data for us. > >>> > >>> Reported-by: Denis Kirjanov > >>> Signed-off-by: Gerd Hoffmann > >>> --- > >> Yeah, that fixes the issue. Thanks Gerd! > > > > Looks like that the patch fixes the half of the problem: the inverted > > colors appear on client reconnect to vm > > Program received signal SIGSEGV, Segmentation fault. > 0x74e10258 in ?? () from /usr/lib/x86_64-linux-gnu/libpixman-1.so.0 > (gdb) bt > #0 0x74e10258 in ?? () from > /usr/lib/x86_64-linux-gnu/libpixman-1.so.0 > #1 0x74e10239 in pixman_image_unref () from > /usr/lib/x86_64-linux-gnu/libpixman-1.so.0 > #2 0x778e4117 in canvas_get_quic > (canvas=canvas@entry=0x7ceb80, image=image@entry=0xae2720, > want_original=want_original@entry=0) at > ../spice-common/common/canvas_base.c:390 spice client crash? is this spice client running on big endian or little endian machine? cheers, Gerd
Re: [Qemu-devel] [Spice-devel] [PATCH] spice: fix simple display on bigendian hosts
On 4/14/15, Denis Kirjanov wrote: > On 4/14/15, Denis Kirjanov wrote: >> On 4/14/15, Gerd Hoffmann wrote: >>> Denis Kirjanov is busy getting spice run on ppc64 and trapped into this >>> one. Spice wire format is little endian, so we have to explicitly say >>> we want little endian when letting pixman convert the data for us. >>> >>> Reported-by: Denis Kirjanov >>> Signed-off-by: Gerd Hoffmann >>> --- >> Yeah, that fixes the issue. Thanks Gerd! > > Looks like that the patch fixes the half of the problem: the inverted > colors appear on client reconnect to vm Program received signal SIGSEGV, Segmentation fault. 0x74e10258 in ?? () from /usr/lib/x86_64-linux-gnu/libpixman-1.so.0 (gdb) bt #0 0x74e10258 in ?? () from /usr/lib/x86_64-linux-gnu/libpixman-1.so.0 #1 0x74e10239 in pixman_image_unref () from /usr/lib/x86_64-linux-gnu/libpixman-1.so.0 #2 0x778e4117 in canvas_get_quic (canvas=canvas@entry=0x7ceb80, image=image@entry=0xae2720, want_original=want_original@entry=0) at ../spice-common/common/canvas_base.c:390 #3 0x778e686d in canvas_get_image_internal (canvas=canvas@entry=0x7ceb80, image=0xae2720, want_original=want_original@entry=0, real_get=real_get@entry=1) at ../spice-common/common/canvas_base.c:1146 #4 0x778e83fd in canvas_get_image (want_original=0, image=, canvas=0x7ceb80) at ../spice-common/common/canvas_base.c:1309 #5 canvas_draw_copy (spice_canvas=0x7ceb80, bbox=0xae26c4, clip=, copy=0xae26e8) at ../spice-common/common/canvas_base.c:2281 #6 0x778c98db in display_handle_draw_copy (channel=0x7c59b0, in=0x841f00) at channel-display.c:1563 #7 0x778bfe7c in spice_channel_handle_msg (channel=0x7c59b0, msg=0x841f00) at spice-channel.c:2858 #8 0x778bce6c in spice_channel_recv_msg (channel=0x7c59b0, msg_handler=0x778bfd9f , data=0x0) at spice-channel.c:1869 #9 0x778bd4f3 in spice_channel_iterate_read (channel=0x7c59b0) at spice-channel.c:2106 #10 0x778bd6fd in spice_channel_iterate (channel=0x7c59b0) at spice-channel.c:2144 #11 0x778be4ad in spice_channel_coroutine (data=0x7c59b0) at spice-channel.c:2430 #12 0x778ea6ff in coroutine_trampoline (cc=0x7c5058) at coroutine_ucontext.c:63 #13 0x778ea4c9 in continuation_trampoline (i0=, i1=) at continuation.c:55 #14 0x763278b0 in ?? () from /lib/x86_64-linux-gnu/libc.so.6 #15 0x007c5420 in ?? () #16 0x in ?? () >>> include/ui/qemu-pixman.h | 2 ++ >>> ui/spice-display.c | 2 +- >>> 2 files changed, 3 insertions(+), 1 deletion(-) >>> >>> diff --git a/include/ui/qemu-pixman.h b/include/ui/qemu-pixman.h >>> index 5d7a9ac..e34c4ef 100644 >>> --- a/include/ui/qemu-pixman.h >>> +++ b/include/ui/qemu-pixman.h >>> @@ -35,6 +35,7 @@ >>> # define PIXMAN_BE_r8g8b8a8 PIXMAN_r8g8b8a8 >>> # define PIXMAN_BE_x8b8g8r8 PIXMAN_x8b8g8r8 >>> # define PIXMAN_BE_a8b8g8r8 PIXMAN_a8b8g8r8 >>> +# define PIXMAN_LE_x8r8g8b8 PIXMAN_b8g8r8x8 >>> #else >>> # define PIXMAN_BE_r8g8b8 PIXMAN_b8g8r8 >>> # define PIXMAN_BE_x8r8g8b8 PIXMAN_b8g8r8x8 >>> @@ -45,6 +46,7 @@ >>> # define PIXMAN_BE_r8g8b8a8 PIXMAN_a8b8g8r8 >>> # define PIXMAN_BE_x8b8g8r8 PIXMAN_r8g8b8x8 >>> # define PIXMAN_BE_a8b8g8r8 PIXMAN_r8g8b8a8 >>> +# define PIXMAN_LE_x8r8g8b8 PIXMAN_x8r8g8b8 >>> #endif >>> >>> /* >>> */ >>> diff --git a/ui/spice-display.c b/ui/spice-display.c >>> index 1644185..1a64e07 100644 >>> --- a/ui/spice-display.c >>> +++ b/ui/spice-display.c >>> @@ -178,7 +178,7 @@ static void >>> qemu_spice_create_one_update(SimpleSpiceDisplay *ssd, >>> image->bitmap.palette = 0; >>> image->bitmap.format = SPICE_BITMAP_FMT_32BIT; >>> >>> -dest = pixman_image_create_bits(PIXMAN_x8r8g8b8, bw, bh, >>> +dest = pixman_image_create_bits(PIXMAN_LE_x8r8g8b8, bw, bh, >>> (void *)update->bitmap, bw * 4); >>> pixman_image_composite(PIXMAN_OP_SRC, ssd->surface, NULL, >>> ssd->mirror, >>> rect->left, rect->top, 0, 0, >>> -- >>> 1.8.3.1 >>> >>> ___ >>> Spice-devel mailing list >>> spice-de...@lists.freedesktop.org >>> http://lists.freedesktop.org/mailman/listinfo/spice-devel >>> >> >> >> -- >> Regards, >> Denis >> > > > -- > Regards, > Denis > -- Regards, Denis
Re: [Qemu-devel] [Spice-devel] [PATCH] spice: fix simple display on bigendian hosts
On 4/14/15, Gerd Hoffmann wrote: > Denis Kirjanov is busy getting spice run on ppc64 and trapped into this > one. Spice wire format is little endian, so we have to explicitly say > we want little endian when letting pixman convert the data for us. > > Reported-by: Denis Kirjanov > Signed-off-by: Gerd Hoffmann > --- Yeah, that fixes the issue. Thanks Gerd! > include/ui/qemu-pixman.h | 2 ++ > ui/spice-display.c | 2 +- > 2 files changed, 3 insertions(+), 1 deletion(-) > > diff --git a/include/ui/qemu-pixman.h b/include/ui/qemu-pixman.h > index 5d7a9ac..e34c4ef 100644 > --- a/include/ui/qemu-pixman.h > +++ b/include/ui/qemu-pixman.h > @@ -35,6 +35,7 @@ > # define PIXMAN_BE_r8g8b8a8 PIXMAN_r8g8b8a8 > # define PIXMAN_BE_x8b8g8r8 PIXMAN_x8b8g8r8 > # define PIXMAN_BE_a8b8g8r8 PIXMAN_a8b8g8r8 > +# define PIXMAN_LE_x8r8g8b8 PIXMAN_b8g8r8x8 > #else > # define PIXMAN_BE_r8g8b8 PIXMAN_b8g8r8 > # define PIXMAN_BE_x8r8g8b8 PIXMAN_b8g8r8x8 > @@ -45,6 +46,7 @@ > # define PIXMAN_BE_r8g8b8a8 PIXMAN_a8b8g8r8 > # define PIXMAN_BE_x8b8g8r8 PIXMAN_r8g8b8x8 > # define PIXMAN_BE_a8b8g8r8 PIXMAN_r8g8b8a8 > +# define PIXMAN_LE_x8r8g8b8 PIXMAN_x8r8g8b8 > #endif > > /* */ > diff --git a/ui/spice-display.c b/ui/spice-display.c > index 1644185..1a64e07 100644 > --- a/ui/spice-display.c > +++ b/ui/spice-display.c > @@ -178,7 +178,7 @@ static void > qemu_spice_create_one_update(SimpleSpiceDisplay *ssd, > image->bitmap.palette = 0; > image->bitmap.format = SPICE_BITMAP_FMT_32BIT; > > -dest = pixman_image_create_bits(PIXMAN_x8r8g8b8, bw, bh, > +dest = pixman_image_create_bits(PIXMAN_LE_x8r8g8b8, bw, bh, > (void *)update->bitmap, bw * 4); > pixman_image_composite(PIXMAN_OP_SRC, ssd->surface, NULL, ssd->mirror, > rect->left, rect->top, 0, 0, > -- > 1.8.3.1 > > ___ > Spice-devel mailing list > spice-de...@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/spice-devel > -- Regards, Denis
Re: [Qemu-devel] [Spice-devel] [PATCH] spice: fix simple display on bigendian hosts
On 4/14/15, Denis Kirjanov wrote: > On 4/14/15, Gerd Hoffmann wrote: >> Denis Kirjanov is busy getting spice run on ppc64 and trapped into this >> one. Spice wire format is little endian, so we have to explicitly say >> we want little endian when letting pixman convert the data for us. >> >> Reported-by: Denis Kirjanov >> Signed-off-by: Gerd Hoffmann >> --- > Yeah, that fixes the issue. Thanks Gerd! Looks like that the patch fixes the half of the problem: the inverted colors appear on client reconnect to vm > >> include/ui/qemu-pixman.h | 2 ++ >> ui/spice-display.c | 2 +- >> 2 files changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/include/ui/qemu-pixman.h b/include/ui/qemu-pixman.h >> index 5d7a9ac..e34c4ef 100644 >> --- a/include/ui/qemu-pixman.h >> +++ b/include/ui/qemu-pixman.h >> @@ -35,6 +35,7 @@ >> # define PIXMAN_BE_r8g8b8a8 PIXMAN_r8g8b8a8 >> # define PIXMAN_BE_x8b8g8r8 PIXMAN_x8b8g8r8 >> # define PIXMAN_BE_a8b8g8r8 PIXMAN_a8b8g8r8 >> +# define PIXMAN_LE_x8r8g8b8 PIXMAN_b8g8r8x8 >> #else >> # define PIXMAN_BE_r8g8b8 PIXMAN_b8g8r8 >> # define PIXMAN_BE_x8r8g8b8 PIXMAN_b8g8r8x8 >> @@ -45,6 +46,7 @@ >> # define PIXMAN_BE_r8g8b8a8 PIXMAN_a8b8g8r8 >> # define PIXMAN_BE_x8b8g8r8 PIXMAN_r8g8b8x8 >> # define PIXMAN_BE_a8b8g8r8 PIXMAN_r8g8b8a8 >> +# define PIXMAN_LE_x8r8g8b8 PIXMAN_x8r8g8b8 >> #endif >> >> /* >> */ >> diff --git a/ui/spice-display.c b/ui/spice-display.c >> index 1644185..1a64e07 100644 >> --- a/ui/spice-display.c >> +++ b/ui/spice-display.c >> @@ -178,7 +178,7 @@ static void >> qemu_spice_create_one_update(SimpleSpiceDisplay *ssd, >> image->bitmap.palette = 0; >> image->bitmap.format = SPICE_BITMAP_FMT_32BIT; >> >> -dest = pixman_image_create_bits(PIXMAN_x8r8g8b8, bw, bh, >> +dest = pixman_image_create_bits(PIXMAN_LE_x8r8g8b8, bw, bh, >> (void *)update->bitmap, bw * 4); >> pixman_image_composite(PIXMAN_OP_SRC, ssd->surface, NULL, >> ssd->mirror, >> rect->left, rect->top, 0, 0, >> -- >> 1.8.3.1 >> >> ___ >> Spice-devel mailing list >> spice-de...@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/spice-devel >> > > > -- > Regards, > Denis > -- Regards, Denis
Re: [Qemu-devel] [Spice-devel] [PATCH] spice: fix simple display on bigendian hosts
Hey, On Tue, Apr 14, 2015 at 09:14:53AM +0200, Gerd Hoffmann wrote: > Denis Kirjanov is busy getting spice run on ppc64 and trapped into this > one. Spice wire format is little endian, so we have to explicitly say > we want little endian when letting pixman convert the data for us. This patch looks good to me, would be nice to get some testing from Denis first though. Christophe pgp7nToNGssqB.pgp Description: PGP signature
