qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Jason Wang
Subject: Re: [Qemu-devel] [RFC PATCH V4 0/4] Introduce COLO-compare
Date: Wed, 22 Jun 2016 14:48:45 +0800
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.8.0



On 2016年06月22日 14:24, Zhang Chen wrote:


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?

Why not just trigger the comparing in compare_{pri|sec}_chr_in() ?



Thanks
Zhang Chen


Thanks


Thanks
Zhang Chen



.






reply via email to

[Prev in Thread] Current Thread [Next in Thread]