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

[Li Zhijian]

On 9/23/20 2:47 PM, Zhang, Chen wrote:
> > -----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.
Yeah,  you are right

Reviewed-by: Li Zhijian <lizhijian@cn.fujitsu.com>

Thank


> 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;
> > >    }
> > >
> > >    /*