| Message ID | 20211228073517.88193-1-lei.rao@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | migration/colo.c: Add missed return in error handling | expand |
> -----Original Message----- > From: Rao, Lei <lei.rao@intel.com> > Sent: Tuesday, December 28, 2021 3:35 PM > To: Zhang, Chen <chen.zhang@intel.com>; zhanghailiang@xfusion.com; > quintela@redhat.com; dgilbert@redhat.com > Cc: qemu-devel@nongnu.org; Rao, Lei <lei.rao@intel.com> > Subject: [PATCH] migration/colo.c: Add missed return in error handling > > When doing failover and checkpoint, some returns are missed in error > handling. Let's add it. > > Signed-off-by: Lei Rao <lei.rao@intel.com> Reviewed-by: Zhang Chen <chen.zhang@intel.com> Thanks Chen > --- > 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; > } > > /* 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; > } > > 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; > } > > /* 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) { > -- > 2.32.0
Rao Lei <lei.rao@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.rao@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.
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; } /* 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; } 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; } /* 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) {
When doing failover and checkpoint, some returns are missed in error handling. Let's add it. Signed-off-by: Lei Rao <lei.rao@intel.com> --- migration/colo.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)