RE: [PATCH 3/4] net/colo-compare.c: Add secondary old packet detection

[Zhang, Chen]

> -----Original Message-----
> From: Li Zhijian <lizhijian@cn.fujitsu.com>
> Sent: Tuesday, September 22, 2020 2:47 PM
> To: Zhang, Chen <chen.zhang@intel.com>; Jason Wang
> <jasowang@redhat.com>; qemu-dev <qemu-devel@nongnu.org>
> Cc: Zhang Chen <zhangckid@gmail.com>
> Subject: Re: [PATCH 3/4] net/colo-compare.c: Add secondary old packet
> detection
> 
> 
> 
> On 9/18/20 5:22 PM, Zhang Chen wrote:
> > From: Zhang Chen <chen.zhang@intel.com>
> >
> > Detect queued secondary packet to sync VM state in time.
> >
> > Signed-off-by: Zhang Chen <chen.zhang@intel.com>
> > ---
> >   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
> > 3b72309d08..f7271b976f 100644
> > --- a/net/colo-compare.c
> > +++ b/net/colo-compare.c
> > @@ -641,19 +641,26 @@ void colo_compare_unregister_notifier(Notifier
> *notify)
> >   static int colo_old_packet_check_one_conn(Connection *conn,
> >                                             CompareState *s)
> >   {
> > -    GList *result = NULL;
> > -
> > -    result = g_queue_find_custom(&conn->primary_list,
> > -                                 &s->compare_timeout,
> > -                                 (GCompareFunc)colo_old_packet_check_one);
> > +    if (!g_queue_is_empty(&conn->primary_list)) {
> Looks we don't need to check is_empty

Re-checked glib code, it just checked the queue rather than inside content.
Maybe check empty before that will benefit performance.

Thanks
Zhang Chen

> 
> > +        if (g_queue_find_custom(&conn->primary_list,
> > +                                &s->compare_timeout,
> > +                                (GCompareFunc)colo_old_packet_check_one))
> > +            goto out;
> > +    }
> >
> > -    if (result) {
> > -        /* Do checkpoint will flush old packet */
> > -        colo_compare_inconsistency_notify(s);
> > -        return 0;
> > +    if (!g_queue_is_empty(&conn->secondary_list)) {
> Ditto
> 
> Thanks
> > +        if (g_queue_find_custom(&conn->secondary_list,
> > +                                &s->compare_timeout,
> > +                                (GCompareFunc)colo_old_packet_check_one))
> > +            goto out;
> >       }
> >
> >       return 1;
> > +
> > +out:
> > +    /* Do checkpoint will flush old packet */
> > +    colo_compare_inconsistency_notify(s);
> > +    return 0;
> >   }
> >
> >   /*
> 
>