[Spice-devel] [PATCH v2] Palette cache: Use correct marshal function

[Frediano Ziglio]

> 
> In order to invalidate a single palette cache item, we were using
> spice_marshall_msg_cursor_inval_one(), which is the marshal function
> used to send an invalidation message for the Cursor channel's cache.
> This didn't cause any problems because SPICE_MSG_CURSOR_INVAL_ONE and
> SPICE_MSG_DISPLAY_INVAL_PALETTE have the same message ID and parameters,
> but it's better to use the correct marshalling function.
> ---
> 
> Changes since last version:
>  - leave the PIPE_ITEM_TYPE_INVAL_ONE case in
>    display_channel_client_release_item_before_push().
> 
> without that change, it introduces this regression:
> 
> (/usr/bin/qemu-system-x86_64:20414): SpiceWorker-ERROR **:
> ../../server/red_worker.c:8024:display_channel_send_item: invalid pipe item
> type
> Thread 5 (Thread 0x7ffa86403700 (LWP 20415)):
> #0  0x00007ffa93516eb9 in syscall () at /lib64/libc.so.6
> #1  0x0000558dcf94d631 in qemu_event_wait ()
> #2  0x0000558dcf95b666 in call_rcu_thread ()
> #3  0x00007ffaa083c555 in start_thread () at /lib64/libpthread.so.0
> #4  0x00007ffa9351cb9d in clone () at /lib64/libc.so.6
> Thread 4 (Thread 0x7ffa84702700 (LWP 20416)):
> #0  0x00007ffaa0843a41 in do_futex_wait () at /lib64/libpthread.so.0
> #1  0x00007ffaa0843b0f in __new_sem_wait_slow () at /lib64/libpthread.so.0
> #2  0x00007ffaa0843bc2 in sem_timedwait () at /lib64/libpthread.so.0
> #3  0x0000558dcf94d487 in qemu_sem_timedwait ()
> #4  0x0000558dcf8d297c in worker_thread ()
> #5  0x00007ffaa083c555 in start_thread () at /lib64/libpthread.so.0
> #6  0x00007ffa9351cb9d in clone () at /lib64/libc.so.6
> Thread 3 (Thread 0x7ffa83114700 (LWP 20417)):
> #0  0x00007ffa93512a07 in ioctl () at /lib64/libc.so.6
> #1  0x0000558dcf6c7315 in kvm_vcpu_ioctl ()
> #2  0x0000558dcf6c73cc in kvm_cpu_exec ()
> #3  0x0000558dcf6b47ac in qemu_kvm_cpu_thread_fn ()
> #4  0x00007ffaa083c555 in start_thread () at /lib64/libpthread.so.0
> #5  0x00007ffa9351cb9d in clone () at /lib64/libc.so.6
> Thread 2 (Thread 0x7ffa823ff700 (LWP 20418)):
> #0  0x00007ffaa084454d in read () at /lib64/libpthread.so.0
> #1  0x00007ffaa1ad2fcc in spice_backtrace_gstack () at
> ../../../spice-common/common/backtrace.c:99
> #2  0x00007ffaa1ad3089 in spice_backtrace () at
> ../../../spice-common/common/backtrace.c:130
> #3  0x00007ffaa1adc3c5 in spice_logv (log_domain=0x7ffaa1b839ec
> "SpiceWorker", log_level=SPICE_LOG_LEVEL_ERROR, strloc=0x7ffaa1b862b0
> "../../server/red_worker.c:8024", function=0x7ffaa1b88b10
> <__FUNCTION__.34269> "display_channel_send_item", format=0x7ffaa1b86297
> "invalid pipe item type", args=0x7ffa823fe7d8) at
> ../../../spice-common/common/log.c:108
> #4  0x00007ffaa1adc48e in spice_log (log_domain=0x7ffaa1b839ec "SpiceWorker",
> log_level=SPICE_LOG_LEVEL_ERROR, strloc=0x7ffaa1b862b0
> "../../server/red_worker.c:8024", function=0x7ffaa1b88b10
> <__FUNCTION__.34269> "display_channel_send_item", format=0x7ffaa1b86297
> "invalid pipe item type") at ../../../spice-common/common/log.c:123
> #5  0x00007ffaa1aa1717 in display_channel_send_item (rcc=0x7ffa78037850,
> pipe_item=0x7ffa7841dd80) at ../../server/red_worker.c:8024
> #6  0x00007ffaa1a7367d in red_channel_client_send_item (rcc=0x7ffa78037850,
> item=0x7ffa7841dd80) at ../../server/red_channel.c:597
> #7  0x00007ffaa1a75d9a in red_channel_client_push (rcc=0x7ffa78037850) at
> ../../server/red_channel.c:1356
> #8  0x00007ffaa1a75e30 in red_channel_push (channel=0x7ffa78000900) at
> ../../server/red_channel.c:1373
> #9  0x00007ffaa1aa17be in red_push (worker=0x7ffa8243a010) at
> ../../server/red_worker.c:8041
> #10 0x00007ffaa1aa94a1 in red_worker_main (arg=0x7ffa8243a010) at
> ../../server/red_worker.c:10679
> #11 0x00007ffaa083c555 in start_thread () at /lib64/libpthread.so.0
> #12 0x00007ffa9351cb9d in clone () at /lib64/libc.so.6
> Thread 1 (Thread 0x7ffaa1f36c00 (LWP 20414)):
> #0  0x00007ffa935113c1 in ppoll () at /lib64/libc.so.6
> #1  0x0000558dcf8e0269 in qemu_poll_ns ()
> #2  0x0000558dcf8dfb1c in main_loop_wait ()
> #3  0x0000558dcf6896db in main ()
> Aborted (core dumped)
> 
> 
> To test, boot a rhel 7.1 vm
> 
>  server/red_worker.c | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/server/red_worker.c b/server/red_worker.c
> index b6f38f8..bbd3ba9 100644
> --- a/server/red_worker.c
> +++ b/server/red_worker.c
> @@ -7493,15 +7493,17 @@ static inline void
> marshall_qxl_drawable(RedChannelClient *rcc,
>          red_lossy_marshall_qxl_drawable(display_channel->common.worker, rcc,
>          m, dpi);
>  }
>  
> -static inline void red_marshall_inval(RedChannelClient *rcc,
> -                                      SpiceMarshaller *base_marshaller,
> CacheItem *cach_item)
> +static inline void red_marshall_inval_palette(RedChannelClient *rcc,
> +                                              SpiceMarshaller
> *base_marshaller,
> +                                              CacheItem *cache_item)
>  {
>      SpiceMsgDisplayInvalOne inval_one;
>  
> -    red_channel_client_init_send_data(rcc, cach_item->inval_type, NULL);
> -    inval_one.id = *(uint64_t *)&cach_item->id;
> +    red_channel_client_init_send_data(rcc, cache_item->inval_type, NULL);
> +    inval_one.id = *(uint64_t *)&cache_item->id;
> +
> +    spice_marshall_msg_display_inval_palette(base_marshaller, &inval_one);
>  
> -    spice_marshall_msg_cursor_inval_one(base_marshaller, &inval_one);
>  }
>  
>  static void
>  display_channel_marshall_migrate_data_surfaces(DisplayChannelClient *dcc,
> @@ -7958,7 +7960,7 @@ static void display_channel_send_item(RedChannelClient
> *rcc, PipeItem *pipe_item
>          break;
>      }
>      case PIPE_ITEM_TYPE_INVAL_ONE:
> -        red_marshall_inval(rcc, m, (CacheItem *)pipe_item);
> +        red_marshall_inval_palette(rcc, m, (CacheItem *)pipe_item);
>          break;
>      case PIPE_ITEM_TYPE_STREAM_CREATE: {
>          StreamAgent *agent = SPICE_CONTAINEROF(pipe_item, StreamAgent,
>          create_item);
> --
> 2.4.3

Merged

Frediano