Re: [PATCH 11/11] migration/colo: Fix qmp_xen_colo_do_checkpoint() error handling

["Philippe Mathieu-Daudé" <philmd@redhat.com>]

On 4/20/20 10:32 AM, Markus Armbruster wrote:
> 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.zhang@intel.com>
> Cc: zhanghailiang <zhang.zhanghailiang@huawei.com>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>   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);
>   }
> 

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>