Re: [Qemu-devel] [PATCH 3/3] colo-compare: use notifier to notify inconsistent packets comparing

[Hailiang Zhang <zhang.zhanghailiang@huawei.com>]

On 2017/2/3 12:50, Jason Wang wrote:
>
>
> On 2017年01月24日 22:05, zhanghailiang wrote:
>> It's a good idea to use notifier to notify COLO frame of
>> inconsistent packets comparing.
>>
>> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
>> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
>> ---
>>    net/colo-compare.c | 24 ++++++++++++++++++++++--
>>    net/colo-compare.h |  2 ++
>>    2 files changed, 24 insertions(+), 2 deletions(-)
>>
>> diff --git a/net/colo-compare.c b/net/colo-compare.c
>> index 2ad577b..39c394d 100644
>> --- a/net/colo-compare.c
>> +++ b/net/colo-compare.c
>> @@ -30,6 +30,7 @@
>>    #include "qapi-visit.h"
>>    #include "net/colo.h"
>>    #include "net/colo-compare.h"
>> +#include "migration/migration.h"
>>
>>    #define TYPE_COLO_COMPARE "colo-compare"
>>    #define COLO_COMPARE(obj) \
>> @@ -38,6 +39,9 @@
>>    static QTAILQ_HEAD(, CompareState) net_compares =
>>           QTAILQ_HEAD_INITIALIZER(net_compares);
>>
>> +static NotifierList colo_compare_notifiers =
>> +    NOTIFIER_LIST_INITIALIZER(colo_compare_notifiers);
>> +
>>    #define COMPARE_READ_LEN_MAX NET_BUFSIZE
>>    #define MAX_QUEUE_SIZE 1024
>>
>> @@ -378,6 +382,22 @@ static int colo_old_packet_check_one(Packet *pkt, int64_t *check_time)
>>        }
>>    }
>>
>> +static void colo_compare_inconsistent_notify(void)
>> +{
>> +    notifier_list_notify(&colo_compare_notifiers,
>> +                migrate_get_current());
>> +}
>> +
>> +void colo_compare_register_notifier(Notifier *notify)
>> +{
>> +    notifier_list_add(&colo_compare_notifiers, notify);
>> +}
>> +
>> +void colo_compare_unregister_notifier(Notifier *notify)
>> +{
>> +    notifier_remove(notify);
>> +}
>> +
>>    static void colo_old_packet_check_one_conn(void *opaque,
>>                                               void *user_data)
>>    {
>> @@ -392,7 +412,7 @@ static void colo_old_packet_check_one_conn(void *opaque,
>>        qemu_mutex_unlock(&conn->conn_lock);
>>        if (result) {
>>            /* do checkpoint will flush old packet */
>> -        /* TODO: colo_notify_checkpoint();*/
>> +        colo_compare_inconsistent_notify();
>>        }
>>    }
>>
>> @@ -466,7 +486,7 @@ static void colo_compare_connection(void *opaque, void *user_data)
>>                 */
>>                trace_colo_compare_main("packet different");
>>                g_queue_push_tail(&conn->primary_list, pkt);
>> -            /* TODO: colo_notify_checkpoint();*/
>> +            colo_compare_inconsistent_notify();
>>                break;
>>            }
>>        }
>
> I don't see any users for
> colo_compare_register_notifier/colo_compare_unregister_notifier, is any
> patch missed in this series?
>

No, we will use these functions in later series which integrate COLO compare
with COLO frame.
So, should i move this patch to the series where it is been called ?
patch 2 has the same problem.

> And is it safe to do notification in colo thread?
>

Yes, the notification callback will be quite simple.

Thanks,
hailiang


> Thanks
>
>> diff --git a/net/colo-compare.h b/net/colo-compare.h
>> index 44f9014..769f55a 100644
>> --- a/net/colo-compare.h
>> +++ b/net/colo-compare.h
>> @@ -16,5 +16,7 @@
>>    #define QEMU_COLO_COMPARE_H
>>
>>    void colo_compare_do_checkpoint(void);
>> +void colo_compare_register_notifier(Notifier *notify);
>> +void colo_compare_unregister_notifier(Notifier *notify);
>>
>>    #endif /* QEMU_COLO_COMPARE_H */
>
>
> .
>