Rao Lei <lei....@intel.com> wrote: > When doing failover and checkpoint, some returns are missed in error > handling. Let's add it. > > Signed-off-by: Lei Rao <lei....@intel.com> > --- > migration/colo.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/migration/colo.c b/migration/colo.c > index 5f7071b3cd..014d3cba01 100644 > --- a/migration/colo.c > +++ b/migration/colo.c > @@ -94,12 +94,15 @@ static void secondary_vm_do_failover(void) > if (local_err) { > error_report_err(local_err); > local_err = NULL; > + return;
Assign a local variable before a return is a NOP, so remove the assignmenent? > } > > /* Notify all filters of all NIC to do checkpoint */ > colo_notify_filters_event(COLO_EVENT_FAILOVER, &local_err); > if (local_err) { > error_report_err(local_err); > + local_err = NULL; > + return; Same here. > } > > if (!autostart) { > @@ -178,6 +181,7 @@ static void primary_vm_do_failover(void) > if (local_err) { > error_report_err(local_err); > local_err = NULL; > + return; And here. > } > > /* Notify COLO thread that failover work is finished */ > @@ -507,12 +511,11 @@ static int > colo_do_checkpoint_transaction(MigrationState *s, > goto out; > } > > - ret = 0; > - > qemu_mutex_lock_iothread(); > vm_start(); > qemu_mutex_unlock_iothread(); > trace_colo_vm_state_change("stop", "run"); > + return 0; > > out: > if (local_err) { This is really a NOP, but it is one line less, so I will not complain. But ther, it is better to just rename the label from "out" to "error" or something like that. Later, Juan.