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

[Zhang Chen <zhangchen.fnst@cn.fujitsu.com>]

On 06/20/2016 01:24 PM, Jason Wang wrote:
>
>
> On 2016年06月20日 11:27, Zhang Chen wrote:
>>
>>
>> On 06/20/2016 11:03 AM, Jason Wang wrote:
>>>
>>>
>>> On 2016年06月17日 10:25, Zhang Chen wrote:
>>>> Hi~ jason.
>>>>
>>>> I tried a lot of ways to make it run in compare thread, but it not 
>>>> work.
>>>>
>>>> Because that:
>>>> void g_main_context_push_thread_default (GMainContext *context);
>>>> Acquires context and sets it as the thread-default context for the 
>>>> current thread.
>>>>
>>>> So, this function g_main_context_push_thread_default() just can set 
>>>> thread-default context,
>>>> not the global default context. and I can't find a function to set 
>>>> global default context in glib.
>>>> The qemu's QIOChannel uses g_source_attach(source, NULL); to attach 
>>>> GSource.
>>>> g_source_attach (GSource *source,
>>>>                  GMainContext *context);
>>>>
>>>> context
>>>> a GMainContext (if NULL, the default context will be used).
>>>>
>>>> But he not say "the default context" is global default context or 
>>>> thread-default context.
>>>> So I read glib codes find that:
>>>> guint
>>>> g_source_attach (GSource *source,
>>>>                  GMainContext *context)
>>>> {
>>>> .....
>>>> if (!context)
>>>>     context = g_main_context_default ();
>>>> .....
>>>> }
>>>>
>>>> g_main_context_default ()
>>>> Returns the global default main context.
>>>>
>>>> So...I think we can't make qemu_chr_add_handlers() run in colo 
>>>> thread in this way.
>>>> Do you have any comments about that? 
>>>
>>> How about (like I suggest previously) just use
>>>
>>> g_souce_attach(x, g_main_context_get_thread_default());
>>>
>>> in qemu's QIOChannel code?
>>>
>>>
>>>
>>
>> I feel it seems can work, and will try it.
>> but I don't know how this change in qemu's QIOChannel code
>> will affect the other code. needs other people confirm it?
>
> Yes, we need convince them. I believe we don't do this in the past is 
> because there's no users.
>
>> If no affect and works good,I will send a independent patch
>> for this.
>>
>
> Right, if it works, please send a RFC and explain why it is needed.
>

Hi~ Jason.
It work! I have send a patch for this and signed-off-by you:

[RFC PATCH] Change g_source_attach(xx, NULL) to g_souce_attach(xx, 
g_main_context_get_thread_default())

But I find a problem when fix colo-compare.
if we want poll and handle chardev in compare thread,
we must use g_main_loop_run(compare_loop). and this function
block here, we can not run other job in compare thread...
like that:

static void *colo_compare_thread(void *opaque)
{
.......

     worker_context = g_main_context_new();
     g_main_context_push_thread_default(worker_context);
     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);

     compare_loop = g_main_loop_new(worker_context, FALSE);

     g_main_loop_run(compare_loop);

     //will block here...

     while (s->thread_status == COMPARE_THREAD_RUNNING) {
         qemu_event_wait(&s->event);
         qemu_event_reset(&s->event);
         rcu_read_lock();
         g_queue_foreach(&s->conn_list, colo_compare_connection, s);
         rcu_read_unlock();
     }

.....


Have any comments?


Thanks
Zhang Chen


> Thanks
>
>>
>> Thanks
>> Zhang Chen
>
>
>
> .
>

-- 
Thanks
zhangchen