Re: [PATCH v1 7/9] colo-compare: safe finalization

[Vladimir Sementsov-Ogievskiy]

29.03.2022 00:15, Maxim Davydov wrote:
> Fixes some possible issues with finalization. For example, finalization
> immediately after instance_init fails on the assert.
>
> Signed-off-by: Maxim Davydov <maxim.davydov@openvz.org>
> ---
>   net/colo-compare.c | 25 ++++++++++++++++---------
>   1 file changed, 16 insertions(+), 9 deletions(-)
>
> diff --git a/net/colo-compare.c b/net/colo-compare.c
> index 62554b5b3c..81d8de0aaa 100644
> --- a/net/colo-compare.c
> +++ b/net/colo-compare.c
> @@ -1426,7 +1426,7 @@ static void colo_compare_finalize(Object *obj)
>               break;
>           }
>       }
> -    if (QTAILQ_EMPTY(&net_compares)) {
> +    if (QTAILQ_EMPTY(&net_compares) && colo_compare_active) {
>           colo_compare_active = false;
>           qemu_mutex_destroy(&event_mtx);
>           qemu_cond_destroy(&event_complete_cond);
> @@ -1442,19 +1442,26 @@ static void colo_compare_finalize(Object *obj)
>   
>       colo_compare_timer_del(s);
>   
> -    qemu_bh_delete(s->event_bh);
> +    if (s->event_bh) {
> +        qemu_bh_delete(s->event_bh);
> +    }
>   
> -    AioContext *ctx = iothread_get_aio_context(s->iothread);
> -    aio_context_acquire(ctx);
> -    AIO_WAIT_WHILE(ctx, !s->out_sendco.done);
> -    if (s->notify_dev) {
> -        AIO_WAIT_WHILE(ctx, !s->notify_sendco.done);
> +    if (s->iothread) {
> +        AioContext *ctx = iothread_get_aio_context(s->iothread);
> +        aio_context_acquire(ctx);
> +        AIO_WAIT_WHILE(ctx, !s->out_sendco.done);
> +        if (s->notify_dev) {
> +            AIO_WAIT_WHILE(ctx, !s->notify_sendco.done);
> +        }
> +        aio_context_release(ctx);
>       }
> -    aio_context_release(ctx);
>   
>       /* Release all unhandled packets after compare thead exited */
>       g_queue_foreach(&s->conn_list, colo_flush_packets, s);
> -    AIO_WAIT_WHILE(NULL, !s->out_sendco.done);
> +    /* Without colo_compare_complete done == false without packets */
> +    if (!g_queue_is_empty(&s->out_sendco.send_list)) {
> +        AIO_WAIT_WHILE(NULL, !s->out_sendco.done);
> +    }

I think, would be good to add more description for this last change. It's not 
as obvious as previous two changes.

>   
>       g_queue_clear(&s->conn_list);
>       g_queue_clear(&s->out_sendco.send_list);


--
Best regards,
Vladimir