Re: [Qemu-devel] [PATCH 5/5] migration: Route more error paths

[Fam Zheng <famz@redhat.com>]

On Tue, 09/19 19:00, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> 
> vmstate_save is called in a few places, and vmstate_save_state is
> called in lots of places.
> 
> Route error returns from the easier cases back up;  there are lots
> of more complex cases where there own error paths need fixing.

Did you mean s/there/their/ ?

<snip>

> diff --git a/tests/test-vmstate.c b/tests/test-vmstate.c
> index e643ac662b..ab3e430c2c 100644
> --- a/tests/test-vmstate.c
> +++ b/tests/test-vmstate.c
> @@ -70,7 +70,7 @@ static void save_vmstate(const VMStateDescription *desc, void *obj)
>      QEMUFile *f = open_test_file(true);
>  
>      /* Save file with vmstate */
> -    vmstate_save_state(f, desc, obj, NULL);
> +    g_assert(!vmstate_save_state(f, desc, obj, NULL));

Though this is test code, isn't putting anything with a side effect into an
assert expression a very bad pattern in general?

>      qemu_put_byte(f, QEMU_VM_EOF);
>      g_assert(!qemu_file_get_error(f));
>      qemu_fclose(f);
> @@ -381,7 +381,7 @@ 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, NULL);
> +    g_assert(!vmstate_save_state(fsave, &vmstate_skipping, &obj, NULL));
>      g_assert(!qemu_file_get_error(fsave));
>  
>      uint8_t expected[] = {
> @@ -402,7 +402,7 @@ static void test_save_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, NULL);
> +    g_assert(!vmstate_save_state(fsave, &vmstate_skipping, &obj, NULL));
>      g_assert(!qemu_file_get_error(fsave));
>  
>      uint8_t expected[] = {
> -- 
> 2.13.5
> 
> 

Fam