Re: [Qemu-devel] [RFC PATCH V3 1/4] colo-compare: introduce colo compare initlization

[Fam Zheng]

On Fri, 05/20 10:46, Jason Wang wrote:
> 
> 
> On 2016年05月13日 11:48, Jason Wang wrote:
> > 
> > 
> > On 2016年05月12日 16:16, Zhang Chen wrote:
> > > 
> > > 
> > > On 05/12/2016 04:01 PM, Jason Wang wrote:
> > > > 
> > > > 
> > > > On 2016年05月12日 14:49, Zhang Chen wrote:
> > > > > 
> > > > > 
> > > > > On 05/09/2016 06:49 PM, Zhang Chen wrote:
> > > > > > 
> > > > > > > +
> > > > > > > +    s->chr_sec_in = qemu_chr_find(s->sec_indev);
> > > > > > > +    if (s->chr_sec_in == NULL) {
> > > > > > > +        error_setg(errp, "Secondary IN Device '%s' not found",
> > > > > > > +                   s->sec_indev);
> > > > > > > +        return;
> > > > > > > +    }
> > > > > > > +
> > > > > > > +    s->chr_out = qemu_chr_find(s->outdev);
> > > > > > > +    if (s->chr_out == NULL) {
> > > > > > > +        error_setg(errp, "OUT Device '%s' not found", s->outdev);
> > > > > > > +        return;
> > > > > > > +    }
> > > > > > > +
> > > > > > > +    qemu_chr_fe_claim_no_fail(s->chr_pri_in);
> > > > > > > +    qemu_chr_add_handlers(s->chr_pri_in, compare_chr_can_read,
> > > > > > > +                          compare_pri_chr_in, NULL, s);
> > > > > > > +
> > > > > > > +    qemu_chr_fe_claim_no_fail(s->chr_sec_in);
> > > > > > > +    qemu_chr_add_handlers(s->chr_sec_in, compare_chr_can_read,
> > > > > > > +                          compare_sec_chr_in, NULL, s);
> > > > > > > +
> > > > > > > > > > > > > Btw, what's the reason of
> > > > > > > > > > > > > handling this in main loop?
> > > > > > > > > > > > > I thought it
> > > > > > > > > > > > > would
> > > > > > > > > > > > > be better to do this in colo
> > > > > > > > > > > > > thread? Otherwise, you need
> > > > > > > > > > > > > lots of
> > > > > > > > > > > > > extra
> > > > > > > > > > > > > synchronizations?
> > > > > > > > > > > > Do you mean we should
> > > > > > > > > > > > start/stop/do checkpoint it by
> > > > > > > > > > > > colo-frame?
> > > > > > > > > > > I mean we probably want to handle
> > > > > > > > > > > pri_in and sec_in in colo compare
> > > > > > > > > > > thread. Through this way, there's no
> > > > > > > > > > > need for extra synchronization
> > > > > > > > > > > with
> > > > > > > > > > > main loop.
> > > > > > > > > > I get your point, but how to do this.
> > > > > > > > > > Now, we use qemu_chr_add_handlers to do this job.
> > > > > > > > > You probably want to start a new main loop
> > > > > > > > > in colo comparing thread.
> > > > > > > > 
> > > > > > > > IIUC, do you mean
> > > > > > > > - remove char device read_handler
> > > > > > > > 
> > > > > > > >   ↓at colo comparing thread↓
> > > > > > > > while (true) {
> > > > > > > > - blocking read packet from char device with 
> > > > > > > > select(2)/poll(2)...
> > > > > > > > - compare packet
> > > > > > > > }
> > > > > > > Yes, something like this.
> > > > > > > 
> > > > > > 
> > > > > > But remove qemu_chr_add_handlers I can't get fd to select/poll.
> > > > > > 
> > > > > > How to get fd from all kinds of chardev?
> > > > > > 
> > > > > 
> > > > > Hi~ jason.
> > > > > 
> > > > > If we use chardev socket the fd save in QIOChannelSocket.
> > > > > 
> > > > > and if we use chardev file the fd save in QIOChannelFile.
> > > > > 
> > > > > Have any common method to get fd?
> > > > 
> > > > I'm not sure I get the question. But you probably can call
> > > > qemu_chr_add_handlers() in colo comparing thread to solve this I
> > > > think?
> > > > 
> > > 
> > > I have tested call qemu_chr_add_handlers() in colo comparing thread,
> > > but when data come,
> > > the handler always running in main loop.
> > > 
> > > Thanks
> > > Zhang Chen
> > 
> > Cc Amit for the help.
> > 
> > Amit, we want to poll and handle chardev in another thread other than
> > main loop. But looks like qemu_chr_add_handlers() can only work for
> > default context other than thread default context. Any other solution
> > for this?
> > 
> > Thanks
> > 
> 
> Cc Fam for more thought.
> 

Unfortunately QIOChannel in chardev uses GSource, so there is no easy way to
move that to another thread, at least I don't think any code in QEMU has ever
tried.

One possibility is in the colo compare thread, call
g_main_context_push_thread_default() before setting up the chr handler, but I'm
not sure how well that would work.

Fam