> -----Original Message-----
> From: Markus Armbruster <arm...@redhat.com>
> Sent: Monday, April 20, 2020 4:33 PM
> To: qemu-devel@nongnu.org
> Cc: Zhang, Chen <chen.zh...@intel.com>; zhanghailiang
> <zhang.zhanghaili...@huawei.com>
> Subject: [PATCH 11/11] migration/colo: Fix qmp_xen_colo_do_checkpoint()
> error handling
>
> The Error ** argument must be NULL, &error_abort, &error_fatal, or a
> pointer to a variable containing NULL. Passing an argument of the latter kind
> twice without clearing it in between is wrong: if the first call sets an
> error, it
> no longer points to NULL for the second call.
>
> qmp_xen_colo_do_checkpoint() passes @errp first to
> replication_do_checkpoint_all(), and then to colo_notify_filters_event(). If
> both fail, this will trip the assertion in error_setv().
>
> Similar code in secondary_vm_do_failover() calls
> colo_notify_filters_event() only after replication_do_checkpoint_all()
> succeeded. Do the same here.
>
> Fixes: 0e8818f023616677416840d6ddc880db8de3c967
> Cc: Zhang Chen <chen.zh...@intel.com>
> Cc: zhanghailiang <zhang.zhanghaili...@huawei.com>
> Signed-off-by: Markus Armbruster <arm...@redhat.com>
Looks good for me.
Reviewed-by: Zhang Chen <chen.zh...@intel.com>
Thanks
Zhang Chen
> ---
> migration/colo.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/migration/colo.c b/migration/colo.c index a54ac84f41..1b3493729b
> 100644
> --- a/migration/colo.c
> +++ b/migration/colo.c
> @@ -263,7 +263,13 @@ ReplicationStatus
> *qmp_query_xen_replication_status(Error **errp)
>
> void qmp_xen_colo_do_checkpoint(Error **errp) {
> - replication_do_checkpoint_all(errp);
> + Error *err = NULL;
> +
> + replication_do_checkpoint_all(&err);
> + if (err) {
> + error_propagate(errp, err);
> + return;
> + }
> /* Notify all filters of all NIC to do checkpoint */
> colo_notify_filters_event(COLO_EVENT_CHECKPOINT, errp); }
> --
> 2.21.1
