Re: [Qemu-devel] [PATCH v12 13/16] tests: Disable image lock in test-replication

[Max Reitz <mreitz@redhat.com>]

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

On 23.01.2017 13:30, Fam Zheng wrote:
> The COLO block replication architecture requires one disk to be shared
> between primary and secondary, in the test both processes use posix file
> protocol (instead of over NBD) so it is affected by image locking.
> Disable the lock.
> 
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>  tests/test-replication.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)

Running these tests (even when fixed) results in a failure before the
next patch because disable-lock is not yet implemented. So just putting
this test fix after the next patch wouldn't be any worse, the test would
fail for a single revision.

I'm more or less fine with this, but you could also just squash this
patch into the next one; it's small enough.

Or you find a way to split up the next patch so this can go in between.

> diff --git a/tests/test-replication.c b/tests/test-replication.c
> index fac2da3..5bede49 100644
> --- a/tests/test-replication.c
> +++ b/tests/test-replication.c
> @@ -179,7 +179,8 @@ static BlockBackend *start_primary(void)
>      char *cmdline;
>  
>      cmdline = g_strdup_printf("driver=replication,mode=primary,node-name=xxx,"
> -                              "file.driver=qcow2,file.file.filename=%s"
> +                              "file.driver=qcow2,file.file.filename=%s,"
> +                              "file.file.disable-lock=on"
>                                , p_local_disk);

That is interesting formatting (albeit pre-existing).

>      opts = qemu_opts_parse_noisily(&qemu_drive_opts, cmdline, false);
>      g_free(cmdline);
> @@ -310,7 +311,9 @@ static BlockBackend *start_secondary(void)
>      Error *local_err = NULL;
>  
>      /* add s_local_disk and forge S_LOCAL_DISK_ID */
> -    cmdline = g_strdup_printf("file.filename=%s,driver=qcow2", s_local_disk);
> +    cmdline = g_strdup_printf("file.filename=%s,driver=qcow2,"
> +                              "file.file.disable-lock=on",
> +                              s_local_disk);
>      opts = qemu_opts_parse_noisily(&qemu_drive_opts, cmdline, false);
>      g_free(cmdline);
>  
> @@ -331,8 +334,10 @@ static BlockBackend *start_secondary(void)
>      /* add S_(ACTIVE/HIDDEN)_DISK and forge S_ID */
>      cmdline = g_strdup_printf("driver=replication,mode=secondary,top-id=%s,"
>                                "file.driver=qcow2,file.file.filename=%s,"
> +                              "file.file.disable-lock=on",

Comma should be before the " (gcc complains about a format string error
when compiling).

>                                "file.backing.driver=qcow2,"
>                                "file.backing.file.filename=%s,"
> +                              "file.backing.file.disable-lock=on",

Same here.

Max

>                                "file.backing.backing=%s"
>                                , S_ID, s_active_disk, s_hidden_disk
>                                , S_LOCAL_DISK_ID);
> 



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