On 23.01.2017 13:30, Fam Zheng wrote: > Signed-off-by: Fam Zheng <f...@redhat.com> > --- > 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
