[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 04/27] vmstate: Port versioned tests to new form
From: |
Dr. David Alan Gilbert |
Subject: |
Re: [Qemu-devel] [PATCH 04/27] vmstate: Port versioned tests to new format |
Date: |
Tue, 17 Jun 2014 12:56:40 +0100 |
User-agent: |
Mutt/1.5.23 (2014-03-12) |
* Juan Quintela (address@hidden) wrote:
> Signed-off-by: Juan Quintela <address@hidden>
> ---
> tests/test-vmstate.c | 363
> ++++++++++++++++++++++++++-------------------------
> 1 file changed, 185 insertions(+), 178 deletions(-)
>
> diff --git a/tests/test-vmstate.c b/tests/test-vmstate.c
> index a462335..d0839c3 100644
> --- a/tests/test-vmstate.c
> +++ b/tests/test-vmstate.c
> @@ -272,217 +272,226 @@ static void test_simple_primitive(void)
> }
> #undef FIELD_EQUAL
>
> -typedef struct TestStruct {
> +typedef struct TestVersioned {
> uint32_t a, b, c, e;
> uint64_t d, f;
> bool skip_c_e;
> -} TestStruct;
> +} TestVersioned;
> +
> +TestVersioned obj_versioned = {
> + .a = 10,
> + .b = 200,
> + .c = 30,
> + .d = 40,
> + .e = 500,
> + .f = 600,
> + .skip_c_e = true,
> +};
>
> -static const VMStateDescription vmstate_versioned = {
> +static const VMStateDescription vmstate_simple_versioned = {
> .name = "test/versioned",
> .version_id = 2,
> .minimum_version_id = 1,
> .fields = (VMStateField[]) {
> - VMSTATE_UINT32(a, TestStruct),
> - VMSTATE_UINT32_V(b, TestStruct, 2), /* Versioned field in the
> middle, so
> - * we catch bugs more easily.
> - */
> - VMSTATE_UINT32(c, TestStruct),
> - VMSTATE_UINT64(d, TestStruct),
> - VMSTATE_UINT32_V(e, TestStruct, 2),
> - VMSTATE_UINT64_V(f, TestStruct, 2),
> + VMSTATE_UINT32(a, TestVersioned),
> + /* Versioned field in the middle, so we catch bugs more
> + * easily. */
> + VMSTATE_UINT32_V(b, TestVersioned, 2),
> + VMSTATE_UINT32(c, TestVersioned),
> + VMSTATE_UINT64(d, TestVersioned),
> + VMSTATE_UINT32_V(e, TestVersioned, 2),
> + VMSTATE_UINT64_V(f, TestVersioned, 2),
> VMSTATE_END_OF_LIST()
> }
> };
>
> -static void test_load_v1(void)
> +uint8_t wire_simple_v1[] = {
> + /* a */ 0x00, 0x00, 0x00, 0x0a,
> + /* c */ 0x00, 0x00, 0x00, 0x1e,
> + /* d */ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x28,
> + QEMU_VM_EOF, /* just to ensure we won't get EOF reported prematurely */
> +};
> +
> +uint8_t wire_simple_v2[] = {
> + /* a */ 0x00, 0x00, 0x00, 0x0a,
> + /* b */ 0x00, 0x00, 0x00, 0xc8,
> + /* c */ 0x00, 0x00, 0x00, 0x1e,
> + /* d */ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x28,
> + /* e */ 0x00, 0x00, 0x01, 0xf4,
> + /* f */ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0x58,
> + QEMU_VM_EOF, /* just to ensure we won't get EOF reported prematurely */
> +};
> +
> +static void obj_versioned_copy(void *arg1, void *arg2)
> {
> - QEMUFile *fsave = open_test_file(true);
> - uint8_t buf[] = {
> - 0, 0, 0, 10, /* a */
> - 0, 0, 0, 30, /* c */
> - 0, 0, 0, 0, 0, 0, 0, 40, /* d */
> - QEMU_VM_EOF, /* just to ensure we won't get EOF reported prematurely
> */
> - };
> - qemu_put_buffer(fsave, buf, sizeof(buf));
> - qemu_fclose(fsave);
> -
> - QEMUFile *loading = open_test_file(false);
> - TestStruct obj = { .b = 200, .e = 500, .f = 600 };
> - vmstate_load_state(loading, &vmstate_versioned, &obj, 1);
> - g_assert(!qemu_file_get_error(loading));
> - g_assert_cmpint(obj.a, ==, 10);
> - g_assert_cmpint(obj.b, ==, 200);
> - g_assert_cmpint(obj.c, ==, 30);
> - g_assert_cmpint(obj.d, ==, 40);
> - g_assert_cmpint(obj.e, ==, 500);
> - g_assert_cmpint(obj.f, ==, 600);
> - qemu_fclose(loading);
> + TestVersioned *target = arg1;
> + TestVersioned *source = arg2;
> +
> + target->a = source->a;
> + target->b = source->b;
> + target->c = source->c;
> + target->d = source->d;
> + target->e = source->e;
> + target->f = source->f;
> + target->skip_c_e = source->skip_c_e;
Why's that not simply *target = *source?
> }
>
> -static void test_load_v2(void)
> +static void test_simple_v2(void)
> {
> - QEMUFile *fsave = open_test_file(true);
> - uint8_t buf[] = {
> - 0, 0, 0, 10, /* a */
> - 0, 0, 0, 20, /* b */
> - 0, 0, 0, 30, /* c */
> - 0, 0, 0, 0, 0, 0, 0, 40, /* d */
> - 0, 0, 0, 50, /* e */
> - 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);
> -
> - QEMUFile *loading = open_test_file(false);
> - TestStruct obj;
> - vmstate_load_state(loading, &vmstate_versioned, &obj, 2);
> - g_assert_cmpint(obj.a, ==, 10);
> - g_assert_cmpint(obj.b, ==, 20);
> - g_assert_cmpint(obj.c, ==, 30);
> - g_assert_cmpint(obj.d, ==, 40);
> - g_assert_cmpint(obj.e, ==, 50);
> - g_assert_cmpint(obj.f, ==, 60);
> - qemu_fclose(loading);
> + TestVersioned obj, obj_clone;
> +
> + memset(&obj, 0, sizeof(obj));
> + save_vmstate(&vmstate_simple_versioned, &obj_versioned);
> +
> + compare_vmstate(wire_simple_v2, sizeof(wire_simple_v2));
> +
> + SUCCESS(load_vmstate(&vmstate_simple_versioned, &obj, &obj_clone,
> + obj_versioned_copy, 2, wire_simple_v2,
> + sizeof(wire_simple_v2)));
> +
> +#define FIELD_EQUAL(name) g_assert_cmpint(obj.name, ==, obj_versioned.name)
> +#define FIELD_NOT_EQUAL(name) \
> + g_assert_cmpint(obj.name, !=, obj_versioned.name)
Given that macro is shared with the next few functions it would be seem
to declare it outside of the function.
> + FIELD_EQUAL(a);
> + FIELD_EQUAL(b);
> + FIELD_EQUAL(c);
> + FIELD_EQUAL(d);
> + FIELD_EQUAL(e);
> + FIELD_EQUAL(f);
> +}
> +
> +static void test_simple_v1(void)
> +{
> + TestVersioned obj, obj_clone;
> +
> + memset(&obj, 0, sizeof(obj));
> + SUCCESS(load_vmstate(&vmstate_simple_versioned, &obj, &obj_clone,
> + obj_versioned_copy, 1, wire_simple_v1,
> + sizeof(wire_simple_v1)));
> +
> + FIELD_EQUAL(a);
> + FIELD_NOT_EQUAL(b);
> + FIELD_EQUAL(c);
> + FIELD_EQUAL(d);
> + FIELD_NOT_EQUAL(e);
> + FIELD_NOT_EQUAL(f);
> }
>
> static bool test_skip(void *opaque, int version_id)
> {
> - TestStruct *t = (TestStruct *)opaque;
> + TestVersioned *t = opaque;
> return !t->skip_c_e;
> }
>
> -static const VMStateDescription vmstate_skipping = {
> - .name = "test/skip",
> +static const VMStateDescription vmstate_simple_skipping = {
> + .name = "simple/skip",
> .version_id = 2,
> .minimum_version_id = 1,
> .fields = (VMStateField[]) {
> - VMSTATE_UINT32(a, TestStruct),
> - VMSTATE_UINT32(b, TestStruct),
> - VMSTATE_UINT32_TEST(c, TestStruct, test_skip),
> - VMSTATE_UINT64(d, TestStruct),
> - VMSTATE_UINT32_TEST(e, TestStruct, test_skip),
> - VMSTATE_UINT64_V(f, TestStruct, 2),
> + VMSTATE_BOOL(skip_c_e, TestVersioned),
> + VMSTATE_UINT32(a, TestVersioned),
> + VMSTATE_UINT32(b, TestVersioned),
> + VMSTATE_UINT32_TEST(c, TestVersioned, test_skip),
> + VMSTATE_UINT64(d, TestVersioned),
> + VMSTATE_UINT32_TEST(e, TestVersioned, test_skip),
> + VMSTATE_UINT64_V(f, TestVersioned, 2),
> VMSTATE_END_OF_LIST()
> }
> };
>
> +uint8_t wire_simple_no_skip[] = {
> + /* s */ 0x00,
> + /* a */ 0x00, 0x00, 0x00, 0x0a,
> + /* b */ 0x00, 0x00, 0x00, 0xc8,
> + /* c */ 0x00, 0x00, 0x00, 0x1e,
> + /* d */ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x28,
> + /* e */ 0x00, 0x00, 0x01, 0xf4,
> + /* f */ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0x58,
> + QEMU_VM_EOF, /* just to ensure we won't get EOF reported prematurely */
> +};
>
> -static void test_save_noskip(void)
> -{
> - 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);
> - g_assert(!qemu_file_get_error(fsave));
> - qemu_fclose(fsave);
> -
> - QEMUFile *loading = open_test_file(false);
> - uint8_t expected[] = {
> - 0, 0, 0, 1, /* a */
> - 0, 0, 0, 2, /* b */
> - 0, 0, 0, 3, /* c */
> - 0, 0, 0, 0, 0, 0, 0, 4, /* d */
> - 0, 0, 0, 5, /* e */
> - 0, 0, 0, 0, 0, 0, 0, 6, /* f */
> - };
> - uint8_t result[sizeof(expected)];
> - g_assert_cmpint(qemu_get_buffer(loading, result, sizeof(result)), ==,
> - sizeof(result));
> - g_assert(!qemu_file_get_error(loading));
> - g_assert_cmpint(memcmp(result, expected, sizeof(result)), ==, 0);
> -
> - /* Must reach EOF */
> - qemu_get_byte(loading);
> - g_assert_cmpint(qemu_file_get_error(loading), ==, -EIO);
> -
> - qemu_fclose(loading);
> -}
> +uint8_t wire_simple_skip[] = {
> + /* s */ 0x01,
> + /* a */ 0x00, 0x00, 0x00, 0x0a,
> + /* b */ 0x00, 0x00, 0x00, 0xc8,
> + /* d */ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x28,
> + /* f */ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0x58,
> + QEMU_VM_EOF, /* just to ensure we won't get EOF reported prematurely */
> +};
>
> -static void test_save_skip(void)
> +static void test_simple_no_skip(void)
> {
> - 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);
> - g_assert(!qemu_file_get_error(fsave));
> - qemu_fclose(fsave);
> -
> - QEMUFile *loading = open_test_file(false);
> - uint8_t expected[] = {
> - 0, 0, 0, 1, /* a */
> - 0, 0, 0, 2, /* b */
> - 0, 0, 0, 0, 0, 0, 0, 4, /* d */
> - 0, 0, 0, 0, 0, 0, 0, 6, /* f */
> - };
> - uint8_t result[sizeof(expected)];
> - g_assert_cmpint(qemu_get_buffer(loading, result, sizeof(result)), ==,
> - sizeof(result));
> - g_assert(!qemu_file_get_error(loading));
> - g_assert_cmpint(memcmp(result, expected, sizeof(result)), ==, 0);
> + TestVersioned obj, obj_clone;
>
> -
> - /* Must reach EOF */
> - qemu_get_byte(loading);
> - g_assert_cmpint(qemu_file_get_error(loading), ==, -EIO);
> -
> - qemu_fclose(loading);
> + memset(&obj, 0, sizeof(obj));
> + obj_versioned.skip_c_e = false;
> + save_vmstate(&vmstate_simple_skipping, &obj_versioned);
> +
> + compare_vmstate(wire_simple_no_skip, sizeof(wire_simple_no_skip));
> +
> + /* We abuse the fact that f has a 0x00 value in the right position */
A bit nasty.
> + SUCCESS(load_vmstate(&vmstate_simple_skipping, &obj, &obj_clone,
> + obj_versioned_copy, 1, wire_simple_no_skip,
> + sizeof(wire_simple_no_skip) - 8));
> +
> + FIELD_EQUAL(skip_c_e);
> + FIELD_EQUAL(a);
> + FIELD_EQUAL(b);
> + FIELD_EQUAL(c);
> + FIELD_EQUAL(d);
> + FIELD_EQUAL(e);
> + FIELD_NOT_EQUAL(f);
> +
> + SUCCESS(load_vmstate(&vmstate_simple_skipping, &obj, &obj_clone,
> + obj_versioned_copy, 2, wire_simple_no_skip,
> + sizeof(wire_simple_no_skip)));
> +
> + FIELD_EQUAL(skip_c_e);
> + FIELD_EQUAL(a);
> + FIELD_EQUAL(b);
> + FIELD_EQUAL(c);
> + FIELD_EQUAL(d);
> + FIELD_EQUAL(e);
> + FIELD_EQUAL(f);
> }
>
> -static void test_load_noskip(void)
> +static void test_simple_skip(void)
> {
> - QEMUFile *fsave = open_test_file(true);
> - uint8_t buf[] = {
> - 0, 0, 0, 10, /* a */
> - 0, 0, 0, 20, /* b */
> - 0, 0, 0, 30, /* c */
> - 0, 0, 0, 0, 0, 0, 0, 40, /* d */
> - 0, 0, 0, 50, /* e */
> - 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);
> -
> - 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));
> - g_assert_cmpint(obj.a, ==, 10);
> - g_assert_cmpint(obj.b, ==, 20);
> - g_assert_cmpint(obj.c, ==, 30);
> - g_assert_cmpint(obj.d, ==, 40);
> - g_assert_cmpint(obj.e, ==, 50);
> - g_assert_cmpint(obj.f, ==, 60);
> - qemu_fclose(loading);
> -}
> + TestVersioned obj, obj_clone;
>
> -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 */
> - 0, 0, 0, 0, 0, 0, 0, 40, /* d */
> - 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);
> -
> - 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));
> - g_assert_cmpint(obj.a, ==, 10);
> - g_assert_cmpint(obj.b, ==, 20);
> - g_assert_cmpint(obj.c, ==, 300);
> - g_assert_cmpint(obj.d, ==, 40);
> - g_assert_cmpint(obj.e, ==, 500);
> - g_assert_cmpint(obj.f, ==, 60);
> - qemu_fclose(loading);
> + memset(&obj, 0, sizeof(obj));
> + obj_versioned.skip_c_e = true;
> + save_vmstate(&vmstate_simple_skipping, &obj_versioned);
> +
> + compare_vmstate(wire_simple_skip, sizeof(wire_simple_skip));
> +
> + /* We abuse the fact that f has a 0x00 value in the right position */
> + SUCCESS(load_vmstate(&vmstate_simple_skipping, &obj, &obj_clone,
> + obj_versioned_copy, 1, wire_simple_skip,
> + sizeof(wire_simple_skip) - 8));
> +
> + FIELD_EQUAL(skip_c_e);
> + FIELD_EQUAL(a);
> + FIELD_EQUAL(b);
> + FIELD_NOT_EQUAL(c);
> + FIELD_EQUAL(d);
> + FIELD_NOT_EQUAL(e);
> + FIELD_NOT_EQUAL(f);
> +
> + SUCCESS(load_vmstate(&vmstate_simple_skipping, &obj, &obj_clone,
> + obj_versioned_copy, 2, wire_simple_skip,
> + sizeof(wire_simple_skip)));
> +
> + FIELD_EQUAL(skip_c_e);
> + FIELD_EQUAL(a);
> + FIELD_EQUAL(b);
> + FIELD_NOT_EQUAL(c);
> + FIELD_EQUAL(d);
> + FIELD_NOT_EQUAL(e);
> + FIELD_EQUAL(f);
> }
Couldn't those functions just be merged and take a flag?
> +#undef FIELD_EQUAL
> +#undef FIELD_NOT_EQUAL
>
> int main(int argc, char **argv)
> {
> @@ -490,12 +499,10 @@ int main(int argc, char **argv)
>
> g_test_init(&argc, &argv, NULL);
> g_test_add_func("/vmstate/simple/primitive", test_simple_primitive);
> - g_test_add_func("/vmstate/versioned/load/v1", test_load_v1);
> - g_test_add_func("/vmstate/versioned/load/v2", test_load_v2);
> - g_test_add_func("/vmstate/field_exists/load/noskip", test_load_noskip);
> - g_test_add_func("/vmstate/field_exists/load/skip", test_load_skip);
> - g_test_add_func("/vmstate/field_exists/save/noskip", test_save_noskip);
> - g_test_add_func("/vmstate/field_exists/save/skip", test_save_skip);
> + g_test_add_func("/vmstate/simple/v1", test_simple_v1);
> + g_test_add_func("/vmstate/simple/v2", test_simple_v2);
> + g_test_add_func("/vmstate/simple/skip", test_simple_skip);
> + g_test_add_func("/vmstate/simple/no_skip", test_simple_no_skip);
> g_test_run();
>
> close(temp_fd);
> --
> 1.9.3
>
>
--
Dr. David Alan Gilbert / address@hidden / Manchester, UK
[Qemu-devel] [PATCH 06/27] vmstate: Remove VMSTATE_UINTL_EQUAL_V, Juan Quintela, 2014/06/16
[Qemu-devel] [PATCH 05/27] vmstate: Create test functions for versions until 15, Juan Quintela, 2014/06/16
[Qemu-devel] [PATCH 07/27] vmstate: Change VMSTATE_INTTL_V to VMSTATE_INTTL_TEST, Juan Quintela, 2014/06/16
[Qemu-devel] [PATCH 08/27] vmstate: Remove unused VMSTATE_UINTTL_ARRAY_V, Juan Quintela, 2014/06/16
[Qemu-devel] [PATCH 10/27] vmstate: Test for VMSTATE_INT8_TEST, Juan Quintela, 2014/06/16
[Qemu-devel] [PATCH 11/27] vmstate: Test for VMSTATE_INT16_TEST, Juan Quintela, 2014/06/16
[Qemu-devel] [PATCH 09/27] vmstate: Test for VMSTATE_BOOL_TEST, Juan Quintela, 2014/06/16