Re: [Qemu-devel] [RFC PATCH V4 0/4] Introduce COLO-compare

[Jason Wang <jasowang@redhat.com>]

On 2016年05月31日 16:28, Zhang Chen wrote:
>
>
> On 05/31/2016 02:16 PM, Jason Wang wrote:
>>
>>
>> On 2016年05月31日 11:54, Zhang Chen wrote:
>>>
>>>
>>> On 05/30/2016 11:19 AM, Jason Wang wrote:
>>>>
>>>>
>>>> On 2016年05月25日 20:50, Zhang Chen wrote:
>>>>> COLO-compare is a part of COLO project. It is used
>>>>> to compare the network package to help COLO decide
>>>>> whether to do checkpoint.
>>>>>
>>>>> the full version in this github:
>>>>> https://github.com/zhangckid/qemu/tree/colo-v2.7-proxy-mode-compare-with-colo-base-may25 
>>>>>
>>>>>
>>>>>
>>>>> v4:
>>>>>   p4:
>>>>>      - add some comments
>>>>>      - fix some trace-events
>>>>>      - fix tcp compare error
>>>>>   p3:
>>>>>      - add rcu_read_lock().
>>>>>      - fix trace name
>>>>>      - fix jason's other comments
>>>>>      - rebase some Dave's branch function
>>>>>   p2:
>>>>>      - colo_compare_connection() change g_queue_push_head() to
>>>>>      - g_queue_push_tail() match to sorted order.
>>>>>      - remove QemuMutex conn_list_lock
>>>>
>>>> Looks like conn_list lock is still there. I still prefer to do all 
>>>> thing in the comparing thread. Have you tried Fam's suggestion to 
>>>> use g_main_context_push_thread_default()? If it does not work, does 
>>>> it work simply by replacing all:
>>>>
>>>> g_source_attach(x, NULL);
>>>>
>>>> with
>>>>
>>>> g_souce_attach(x, g_main_context_get_thread_default());
>>>>
>>>> after call g_main_context_push_thread_default()?
>>>>
>>>> Thanks
>>>
>>> I have tried fam's suggestion it does not work.
>>>
>>> so I tried what you suggestion like that:
>>>
>>> static void *colo_compare_thread(void *opaque)
>>> {
>>>     CompareState *s = opaque;
>>>     GSource *source;
>>>     GMainContext *worker_context;
>>>
>>>     source = g_source_new(&source_funcs, sizeof(DemoSource));
>>>     worker_context = g_main_context_new ();
>>>
>>>     g_source_attach(source, g_main_context_get_thread_default());
>>>     g_source_set_callback(source, NULL, NULL, NULL);
>>>     g_main_context_push_thread_default (worker_context);
> +    g_source_unref(source);
>>> qemu_chr_add_handlers(s->chr_pri_in, compare_chr_can_read,
>>>                           compare_pri_chr_in, NULL, s);
>>>     qemu_chr_add_handlers(s->chr_sec_in, compare_chr_can_read,
>>>                           compare_sec_chr_in, NULL, s);
>>>     g_main_context_pop_thread_default (worker_context);
>>
>> g_source_attach() should be done after 
>> g_main_context_push_thread_default()
>
> Like that? (+    g_source_unref(source);)
> But compare_pri_chr_in always run in main loop.
>
> Thanks
> Zhang Chen

Not very sure just from the above codes, since I don't see any 
connection between source and compare_pri_chr_in. The point is to attach 
the source of compare_pri_chr_in to thread default context instead of 
global default context I think.

>
>>
>>>
>>>
>>> but it does not work too. Do you mean like this?
>>
>> Not exactly the same, but should be worth to try also. My suggestion 
>> is to replace all g_source_attach(src, NULL) in qemu source 
>> (especially the ones in qemu-char.c) with g_source_attach(src, 
>> g_main_context_get_thread_default()). (Maybe not that easy, but 
>> better to have a try).
>>
>> You may refer 
>> https://developer.gnome.org/glib/unstable/glib-The-Main-Event-Loop.html 
>> for more docs on this. I believe there should have some solutions on 
>> this.
>>
>>>
>>> Thanks
>>> Zhang Chen
>>>
>>>>>      - remove pkt->s
>>>>>      - move data structure to colo-base.h
>>>>>      - add colo-base.c reuse codes for filter-rewriter
>>>>>      - add some filter-rewriter needs struct
>>>>>      - depends on previous SocketReadState patch
>>>>>   p1:
>>>>>      - except move qemu_chr_add_handlers()
>>>>>        to colo thread
>>>>>      - remove class_finalize
>>>>>      - remove secondary arp codes
>>>>>      - depends on previous SocketReadState patch
>>>>>
>>>>> v3:
>>>>>    - rebase colo-compare to colo-frame v2.7
>>>>>    - fix most of Dave's comments
>>>>>      (except RCU)
>>>>>    - add TCP,UDP,ICMP and other packet comparison
>>>>>    - add trace-event
>>>>>    - add some comments
>>>>>    - other bug fix
>>>>>    - add RFC index
>>>>>    - add usage in patch 1/4
>>>>>
>>>>> v2:
>>>>>    - add jhash.h
>>>>>
>>>>> v1:
>>>>>    - initial patch
>>>>>
>>>>>
>>>>> Zhang Chen (4):
>>>>>    colo-compare: introduce colo compare initialization
>>>>>    colo-compare: track connection and enqueue packet
>>>>>    colo-compare: introduce packet comparison thread
>>>>>    colo-compare: add TCP,UDP,ICMP packet comparison
>>>>>
>>>>>   include/qemu/jhash.h |  61 +++++
>>>>>   net/Makefile.objs    |   2 +
>>>>>   net/colo-base.c      | 183 +++++++++++++
>>>>>   net/colo-base.h      |  92 +++++++
>>>>>   net/colo-compare.c   | 745 
>>>>> +++++++++++++++++++++++++++++++++++++++++++++++++++
>>>>>   qemu-options.hx      |  34 +++
>>>>>   trace-events         |  11 +
>>>>>   vl.c                 |   3 +-
>>>>>   8 files changed, 1130 insertions(+), 1 deletion(-)
>>>>>   create mode 100644 include/qemu/jhash.h
>>>>>   create mode 100644 net/colo-base.c
>>>>>   create mode 100644 net/colo-base.h
>>>>>   create mode 100644 net/colo-compare.c
>>>>>
>>>>
>>>>
>>>>
>>>> .
>>>>
>>>
>>
>>
>>
>> .
>>
>