[PATCH-for-5.1 v3 4/7] migration/colo: Add missing error-propagation code

[PATCH-for-5.1 v3 4/7] migration/colo: Add missing error-propagation code

[Philippe Mathieu-Daudé]

Running the coccinelle script produced:

  $ spatch \
    --macro-file scripts/cocci-macro-file.h --include-headers \
    --sp-file scripts/coccinelle/find-missing-error_propagate.cocci \
    --keep-comments --smpl-spacing --dir .
  HANDLING: ./migration/colo.c
  [[manual check required: error_propagate() might be missing in migrate_set_block_enabled() ./migration/colo.c:439:4]]

Add the missing error_propagate() after review.

Reviewed-by: Juan Quintela <quintela@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 migration/colo.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/migration/colo.c b/migration/colo.c
index a54ac84f41..57b2adb0cc 100644
--- a/migration/colo.c
+++ b/migration/colo.c
@@ -437,6 +437,9 @@ static int colo_do_checkpoint_transaction(MigrationState *s,
 
     /* Disable block migration */
     migrate_set_block_enabled(false, &local_err);
+    if (local_err) {
+        goto out;
+    }
     qemu_mutex_lock_iothread();
 
 #ifdef CONFIG_REPLICATION
-- 
2.21.1

[PATCH-for-5.1 v3 5/7] hw/mips/boston: Add missing error-propagation code

[Philippe Mathieu-Daudé]

Running the coccinelle script produced:

  $ spatch \
    --macro-file scripts/cocci-macro-file.h --include-headers \
    --sp-file scripts/coccinelle/find-missing-error_propagate.cocci \
    --keep-comments --smpl-spacing --dir .
  HANDLING: ./hw/mips/boston.c
  [[manual check required: error_propagate() might be missing in object_property_set_int() ./hw/mips/boston.c:462:4]]
  [[manual check required: error_propagate() might be missing in object_property_set_str() ./hw/mips/boston.c:460:4]]

Since the uses are inside a MachineClass::init() function,
directly use &error_fatal instead of error_propagate().

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Aleksandar Markovic <aleksandar.qemu.devel@gmail.com>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/mips/boston.c | 17 ++++++-----------
 1 file changed, 6 insertions(+), 11 deletions(-)

diff --git a/hw/mips/boston.c b/hw/mips/boston.c
index 98ecd25e8e..2e821ca7d6 100644
--- a/hw/mips/boston.c
+++ b/hw/mips/boston.c
@@ -426,7 +426,6 @@ static void boston_mach_init(MachineState *machine)
 {
     DeviceState *dev;
     BostonState *s;
-    Error *err = NULL;
     MemoryRegion *flash, *ddr_low_alias, *lcd, *platreg;
     MemoryRegion *sys_mem = get_system_memory();
     XilinxPCIEHost *pcie2;
@@ -458,19 +457,15 @@ static void boston_mach_init(MachineState *machine)
     sysbus_init_child_obj(OBJECT(machine), "cps", OBJECT(&s->cps),
                           sizeof(s->cps), TYPE_MIPS_CPS);
     object_property_set_str(OBJECT(&s->cps), machine->cpu_type, "cpu-type",
-                            &err);
-    object_property_set_int(OBJECT(&s->cps), machine->smp.cpus, "num-vp", &err);
-    object_property_set_bool(OBJECT(&s->cps), true, "realized", &err);
-
-    if (err != NULL) {
-        error_report("%s", error_get_pretty(err));
-        exit(1);
-    }
-
+                            &error_fatal);
+    object_property_set_int(OBJECT(&s->cps), machine->smp.cpus, "num-vp",
+                            &error_fatal);
+    object_property_set_bool(OBJECT(&s->cps), true, "realized", &error_fatal);
     sysbus_mmio_map_overlap(SYS_BUS_DEVICE(&s->cps), 0, 0, 1);
 
     flash =  g_new(MemoryRegion, 1);
-    memory_region_init_rom(flash, NULL, "boston.flash", 128 * MiB, &err);
+    memory_region_init_rom(flash, NULL, "boston.flash", 128 * MiB,
+                           &error_fatal);
     memory_region_add_subregion_overlap(sys_mem, 0x18000000, flash, 0);
 
     memory_region_add_subregion_overlap(sys_mem, 0x80000000, machine->ram, 0);
-- 
2.21.1

Re: [PATCH-for-5.1 v3 5/7] hw/mips/boston: Add missing error-propagation code

[Markus Armbruster]

Philippe Mathieu-Daudé <f4bug@amsat.org> writes:

> Running the coccinelle script produced:
>
>   $ spatch \
>     --macro-file scripts/cocci-macro-file.h --include-headers \
>     --sp-file scripts/coccinelle/find-missing-error_propagate.cocci \
>     --keep-comments --smpl-spacing --dir .
>   HANDLING: ./hw/mips/boston.c
>   [[manual check required: error_propagate() might be missing in object_property_set_int() ./hw/mips/boston.c:462:4]]
>   [[manual check required: error_propagate() might be missing in object_property_set_str() ./hw/mips/boston.c:460:4]]
>
> Since the uses are inside a MachineClass::init() function,
> directly use &error_fatal instead of error_propagate().
>
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> Reviewed-by: Aleksandar Markovic <aleksandar.qemu.devel@gmail.com>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

I duplicated this patch in

    [PATCH 08/11] mips/boston: Fix boston_mach_init() error handling
    [PATCH 09/11] mips/boston: Plug memory leak in boston_mach_init()

Sorry!

I'd replace my patches by yours to give you proper credit, but your
commit message mentions "the coccinelle script", presumably the one from
PATCH 3/7, and that patch isn't quite ready in my opinion.  Also, only
my version documents the memory leak.

Re: [PATCH-for-5.1 v3 4/7] migration/colo: Add missing error-propagation code

[Dr. David Alan Gilbert]

* Philippe Mathieu-Daudé (f4bug@amsat.org) wrote:
> Running the coccinelle script produced:
> 
>   $ spatch \
>     --macro-file scripts/cocci-macro-file.h --include-headers \
>     --sp-file scripts/coccinelle/find-missing-error_propagate.cocci \
>     --keep-comments --smpl-spacing --dir .
>   HANDLING: ./migration/colo.c
>   [[manual check required: error_propagate() might be missing in migrate_set_block_enabled() ./migration/colo.c:439:4]]
> 
> Add the missing error_propagate() after review.
> 
> Reviewed-by: Juan Quintela <quintela@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Queued this entry for migration

> ---
>  migration/colo.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/migration/colo.c b/migration/colo.c
> index a54ac84f41..57b2adb0cc 100644
> --- a/migration/colo.c
> +++ b/migration/colo.c
> @@ -437,6 +437,9 @@ static int colo_do_checkpoint_transaction(MigrationState *s,
>  
>      /* Disable block migration */
>      migrate_set_block_enabled(false, &local_err);
> +    if (local_err) {
> +        goto out;
> +    }
>      qemu_mutex_lock_iothread();
>  
>  #ifdef CONFIG_REPLICATION
> -- 
> 2.21.1
> 
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK