Re: [Qemu-block] [PATCH v12 16/16] tests: Add test-image-lock

[Max Reitz]

On 23.01.2017 13:30, Fam Zheng wrote:
> Signed-off-by: Fam Zheng <address@hidden>
> ---
>  tests/Makefile.include   |   2 +
>  tests/test-image-lock.c  | 206 
> +++++++++++++++++++++++++++++++++++++++++++++++
>  tests/test-replication.c |   6 +-
>  3 files changed, 211 insertions(+), 3 deletions(-)
>  create mode 100644 tests/test-image-lock.c

Looks good overall, just some minor issues below.

[...]

> diff --git a/tests/test-image-lock.c b/tests/test-image-lock.c
> new file mode 100644
> index 0000000..9c929ca
> --- /dev/null
> +++ b/tests/test-image-lock.c
> @@ -0,0 +1,206 @@

[...]

> +int main(int argc, char **argv)
> +{
> +#ifndef F_OFD_SETLK
> +    return 0;
> +#endif
> +    int r;
> +    test_image_fd = mkstemp(test_image);
> +
> +    qemu_init_main_loop(&error_fatal);
> +    bdrv_init();
> +
> +    g_assert(test_image_fd >= 0);
> +    r = ftruncate(test_image_fd, TEST_IMAGE_SIZE);
> +    g_assert_cmpint(r, ==, 0);
> +    g_test_init(&argc, &argv, NULL);
> +    g_test_add_func("/image-lock/compat", test_compat);
> +    g_test_add_func("/image-lock/compat_after_reopen", 
> test_compat_after_reopen);
> +    g_test_add_func("/image-lock/compat_0bytefile", test_0bytefile);
> +    g_test_add_func("/image-lock/disable", test_disable);

Maybe test_0bytefile should be the last test since it truncates the test
image to 0 bytes.

> +    aio_context_acquire(qemu_get_aio_context());
> +    r = g_test_run();
> +    aio_context_release(qemu_get_aio_context());

Should test_image be unlinked here?

> +    return r;
> +}


> diff --git a/tests/test-replication.c b/tests/test-replication.c
> index 5bede49..5fb69d2 100644
> --- a/tests/test-replication.c
> +++ b/tests/test-replication.c
> @@ -312,7 +312,7 @@ static BlockBackend *start_secondary(void)
>  
>      /* add s_local_disk and forge S_LOCAL_DISK_ID */
>      cmdline = g_strdup_printf("file.filename=%s,driver=qcow2,"
> -                              "file.file.disable-lock=on",
> +                              "file.disable-lock=on",
>                                s_local_disk);
>      opts = qemu_opts_parse_noisily(&qemu_drive_opts, cmdline, false);
>      g_free(cmdline);
> @@ -334,10 +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",
> +                              "file.file.disable-lock=on,"
>                                "file.backing.driver=qcow2,"
>                                "file.backing.file.filename=%s,"
> -                              "file.backing.file.disable-lock=on",
> +                              "file.backing.file.disable-lock=on,"
>                                "file.backing.backing=%s"
>                                , S_ID, s_active_disk, s_hidden_disk
>                                , S_LOCAL_DISK_ID);
> 

These two hunks should be in patch 13, I presume.

Max

signature.asc
Description: OpenPGP digital signature