Re: [Qemu-devel] [PATCH v1 02/22] migration: remove use of qemu_bufopen from vmstate tests

["Dr. David Alan Gilbert" <dgilbert@redhat.com>]

* Daniel P. Berrange (berrange@redhat.com) wrote:
> Some of the test-vmstate.c test cases use a temporary file
> while others use a memory buffer. To facilitate the future
> removal of the qemu_bufopen() function, convert all the tests
> to use a temporary file.
> 
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

> ---
>  tests/Makefile       |  2 +-
>  tests/test-vmstate.c | 44 +++++++++++++-------------------------------
>  2 files changed, 14 insertions(+), 32 deletions(-)
> 
> diff --git a/tests/Makefile b/tests/Makefile
> index b7352f1..5a1b25f 100644
> --- a/tests/Makefile
> +++ b/tests/Makefile
> @@ -422,7 +422,7 @@ tests/test-qdev-global-props$(EXESUF): tests/test-qdev-global-props.o \
>  	hw/core/fw-path-provider.o \
>  	$(test-qapi-obj-y)
>  tests/test-vmstate$(EXESUF): tests/test-vmstate.o \
> -	migration/vmstate.o migration/qemu-file.o migration/qemu-file-buf.o \
> +	migration/vmstate.o migration/qemu-file.o \
>          migration/qemu-file-unix.o qjson.o \
>  	$(test-qom-obj-y)
>  tests/test-timed-average$(EXESUF): tests/test-timed-average.o qemu-timer.o \
> diff --git a/tests/test-vmstate.c b/tests/test-vmstate.c
> index 4d13bd0..0f943d5 100644
> --- a/tests/test-vmstate.c
> +++ b/tests/test-vmstate.c
> @@ -43,11 +43,6 @@ void yield_until_fd_readable(int fd)
>      select(fd + 1, &fds, NULL, NULL, NULL);
>  }
>  
> -/*
> - * Some tests use 'open_test_file' to work on a real fd, some use
> - * an in memory file (QEMUSizedBuffer+qemu_bufopen); we could pick one
> - * but this way we test both.
> - */
>  
>  /* Duplicate temp_fd and seek to the beginning of the file */
>  static QEMUFile *open_test_file(bool write)
> @@ -60,20 +55,6 @@ static QEMUFile *open_test_file(bool write)
>      return qemu_fdopen(fd, write ? "wb" : "rb");
>  }
>  
> -/*
> - * Check that the contents of the memory-buffered file f match
> - * the given size/data.
> - */
> -static void check_mem_file(QEMUFile *f, void *data, size_t size)
> -{
> -    uint8_t *result = g_malloc(size);
> -    const QEMUSizedBuffer *qsb = qemu_buf_get(f);
> -    g_assert_cmpint(qsb_get_length(qsb), ==, size);
> -    g_assert_cmpint(qsb_get_buffer(qsb, 0, size, result), ==, size);
> -    g_assert_cmpint(memcmp(result, data, size), ==, 0);
> -    g_free(result);
> -}
> -
>  #define SUCCESS(val) \
>      g_assert_cmpint((val), ==, 0)
>  
> @@ -391,7 +372,7 @@ static const VMStateDescription vmstate_skipping = {
>  
>  static void test_save_noskip(void)
>  {
> -    QEMUFile *fsave = qemu_bufopen("w", NULL);
> +    QEMUFile *fsave = open_test_file(true);
>      TestStruct obj = { .a = 1, .b = 2, .c = 3, .d = 4, .e = 5, .f = 6,
>                         .skip_c_e = false };
>      vmstate_save_state(fsave, &vmstate_skipping, &obj, NULL);
> @@ -405,13 +386,14 @@ static void test_save_noskip(void)
>          0, 0, 0, 5,             /* e */
>          0, 0, 0, 0, 0, 0, 0, 6, /* f */
>      };
> -    check_mem_file(fsave, expected, sizeof(expected));
> +
>      qemu_fclose(fsave);
> +    compare_vmstate(expected, sizeof(expected));
>  }
>  
>  static void test_save_skip(void)
>  {
> -    QEMUFile *fsave = qemu_bufopen("w", NULL);
> +    QEMUFile *fsave = open_test_file(true);
>      TestStruct obj = { .a = 1, .b = 2, .c = 3, .d = 4, .e = 5, .f = 6,
>                         .skip_c_e = true };
>      vmstate_save_state(fsave, &vmstate_skipping, &obj, NULL);
> @@ -423,13 +405,14 @@ static void test_save_skip(void)
>          0, 0, 0, 0, 0, 0, 0, 4, /* d */
>          0, 0, 0, 0, 0, 0, 0, 6, /* f */
>      };
> -    check_mem_file(fsave, expected, sizeof(expected));
>  
>      qemu_fclose(fsave);
> +    compare_vmstate(expected, sizeof(expected));
>  }
>  
>  static void test_load_noskip(void)
>  {
> +    QEMUFile *fsave = open_test_file(true);
>      uint8_t buf[] = {
>          0, 0, 0, 10,             /* a */
>          0, 0, 0, 20,             /* b */
> @@ -439,10 +422,10 @@ static void test_load_noskip(void)
>          0, 0, 0, 0, 0, 0, 0, 60, /* f */
>          QEMU_VM_EOF, /* just to ensure we won't get EOF reported prematurely */
>      };
> +    qemu_put_buffer(fsave, buf, sizeof(buf));
> +    qemu_fclose(fsave);
>  
> -    QEMUSizedBuffer *qsb = qsb_create(buf, sizeof(buf));
> -    g_assert(qsb);
> -    QEMUFile *loading = qemu_bufopen("r", qsb);
> +    QEMUFile *loading = open_test_file(false);
>      TestStruct obj = { .skip_c_e = false };
>      vmstate_load_state(loading, &vmstate_skipping, &obj, 2);
>      g_assert(!qemu_file_get_error(loading));
> @@ -453,11 +436,11 @@ static void test_load_noskip(void)
>      g_assert_cmpint(obj.e, ==, 50);
>      g_assert_cmpint(obj.f, ==, 60);
>      qemu_fclose(loading);
> -    qsb_free(qsb);
>  }
>  
>  static void test_load_skip(void)
>  {
> +    QEMUFile *fsave = open_test_file(true);
>      uint8_t buf[] = {
>          0, 0, 0, 10,             /* a */
>          0, 0, 0, 20,             /* b */
> @@ -465,10 +448,10 @@ static void test_load_skip(void)
>          0, 0, 0, 0, 0, 0, 0, 60, /* f */
>          QEMU_VM_EOF, /* just to ensure we won't get EOF reported prematurely */
>      };
> +    qemu_put_buffer(fsave, buf, sizeof(buf));
> +    qemu_fclose(fsave);
>  
> -    QEMUSizedBuffer *qsb = qsb_create(buf, sizeof(buf));
> -    g_assert(qsb);
> -    QEMUFile *loading = qemu_bufopen("r", qsb);
> +    QEMUFile *loading = open_test_file(false);
>      TestStruct obj = { .skip_c_e = true, .c = 300, .e = 500 };
>      vmstate_load_state(loading, &vmstate_skipping, &obj, 2);
>      g_assert(!qemu_file_get_error(loading));
> @@ -479,7 +462,6 @@ static void test_load_skip(void)
>      g_assert_cmpint(obj.e, ==, 500);
>      g_assert_cmpint(obj.f, ==, 60);
>      qemu_fclose(loading);
> -    qsb_free(qsb);
>  }
>  
>  int main(int argc, char **argv)
> -- 
> 2.5.0
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK