[Spice-devel] [PATCH spice-server 14/16] tests: Make test-two-servers work reliably

[Christophe Fergeau]

On Mon, Sep 04, 2017 at 11:57:22AM +0100, Frediano Ziglio wrote:
> This test run 2 spice server in one program.

"runs"

> Use two different tcp port to be able to connect to both servers.
> Also fix a race condition getting the command to execute from
> the worker threads.

I would have put the race condition commit close to the commit
adding pthread usage.

> 
> Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> ---
>  server/tests/test-display-base.c | 29 +++++++++++++++--------------
>  server/tests/test-display-base.h |  2 ++
>  server/tests/test-two-servers.c  |  1 +
>  3 files changed, 18 insertions(+), 14 deletions(-)
> 
> diff --git a/server/tests/test-display-base.c b/server/tests/test-display-base.c
> index 101df44d..82dcdf35 100644
> --- a/server/tests/test-display-base.c
> +++ b/server/tests/test-display-base.c
> @@ -70,6 +70,8 @@ static void test_spice_destroy_update(SimpleSpiceUpdate *update)
>      free(update);
>  }
>  
> +int test_port = 5912;
> +
>  #define DEFAULT_WIDTH 640
>  #define DEFAULT_HEIGHT 320
>  
> @@ -485,20 +487,19 @@ static void push_command(QXLCommandExt *ext)
>      pthread_mutex_unlock(&command_mutex);
>  }
>  
> -static struct QXLCommandExt *get_simple_command(void)
> +static int get_num_commands(void)
>  {
> -    pthread_mutex_lock(&command_mutex);
> -    struct QXLCommandExt *ret = commands[commands_start % COMMANDS_SIZE];
> -    spice_assert(commands_start < commands_end);
> -    commands_start++;
> -    pthread_mutex_unlock(&command_mutex);
> -    return ret;
> +    return commands_end - commands_start;
>  }
>  
> -static int get_num_commands(void)
> +static struct QXLCommandExt *get_simple_command(void)
>  {
>      pthread_mutex_lock(&command_mutex);
> -    int ret = commands_end - commands_start;
> +    struct QXLCommandExt *ret = NULL;
> +    if (get_num_commands() > 0) {
> +        ret = commands[commands_start % COMMANDS_SIZE];
> +        commands_start++;
> +    }
>      pthread_mutex_unlock(&command_mutex);
>      return ret;
>  }
> @@ -507,10 +508,11 @@ static int get_num_commands(void)
>  static int get_command(SPICE_GNUC_UNUSED QXLInstance *qin,
>                         struct QXLCommandExt *ext)
>  {
> -    if (get_num_commands() == 0) {
> +    struct QXLCommandExt *cmd = get_simple_command();
> +    if (!cmd) {
>          return FALSE;
>      }
> -    *ext = *get_simple_command();
> +    *ext = *cmd;
>      return TRUE;
>  }
>  
> @@ -902,7 +904,6 @@ void test_set_command_list(Test *test, Command *commands, int num_commands)
>  
>  Test *test_new(SpiceCoreInterface *core)
>  {
> -    int port = 5912;
>      Test *test = spice_new0(Test, 1);
>      SpiceServer* server = spice_server_new();
>  
> @@ -914,8 +915,8 @@ Test *test_new(SpiceCoreInterface *core)
>      test->wakeup_ms = 1;
>      test->cursor_notify = NOTIFY_CURSOR_BATCH;
>      // some common initialization for all display tests
> -    printf("TESTER: listening on port %d (unsecure)\n", port);
> -    spice_server_set_port(server, port);
> +    printf("TESTER: listening on port %d (unsecure)\n", test_port);
> +    spice_server_set_port(server, test_port);
>      spice_server_set_noauth(server);
>      spice_server_init(server, core);
>  
> diff --git a/server/tests/test-display-base.h b/server/tests/test-display-base.h
> index 1a4f20c5..23c2ce18 100644
> --- a/server/tests/test-display-base.h
> +++ b/server/tests/test-display-base.h
> @@ -130,6 +130,8 @@ struct Test {
>      void (*on_client_disconnected)(Test *test);
>  };
>  
> +extern int test_port;
> +

I think I would add a test_new_with_port() rather than an exported
global.

Christophe

>  void test_set_simple_command_list(Test *test, const int *command, int num_commands);
>  void test_set_command_list(Test *test, Command *command, int num_commands);
>  void test_add_display_interface(Test *test);
> diff --git a/server/tests/test-two-servers.c b/server/tests/test-two-servers.c
> index 40a0e571..2372e730 100644
> --- a/server/tests/test-two-servers.c
> +++ b/server/tests/test-two-servers.c
> @@ -42,6 +42,7 @@ int main(void)
>  
>      core = basic_event_loop_init();
>      t1 = test_new(core);
> +    ++test_port;
>      t2 = test_new(core);
>      //spice_server_set_image_compression(server, SPICE_IMAGE_COMPRESSION_OFF);
>      test_add_display_interface(t1);
> -- 
> 2.13.5
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel