qemu-devel
[Top][All Lists]
Advanced

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

Re: [Qemu-devel] [PATCH 1/3] iothread: provide helpers for internal use


From: Peter Xu
Subject: Re: [Qemu-devel] [PATCH 1/3] iothread: provide helpers for internal use
Date: Fri, 22 Sep 2017 18:54:45 +0800
User-agent: Mutt/1.5.24 (2015-08-30)

On Fri, Sep 22, 2017 at 12:14:04PM +0200, Paolo Bonzini wrote:
> On 22/09/2017 11:43, Daniel P. Berrange wrote:
> > On Fri, Sep 22, 2017 at 11:38:31AM +0200, Paolo Bonzini wrote:
> >> On 22/09/2017 11:36, Daniel P. Berrange wrote:
> >>> On Fri, Sep 22, 2017 at 05:14:30PM +0800, Peter Xu wrote:
> >>>> On Fri, Sep 22, 2017 at 10:04:33AM +0100, Daniel P. Berrange wrote:
> >>>>> On Fri, Sep 22, 2017 at 04:56:10PM +0800, Peter Xu wrote:
> >>>>>> IOThread is a general framework that contains IO loop environment and a
> >>>>>> real thread behind.  It's also good to be used internally inside qemu.
> >>>>>> Provide some helpers for it to create iothreads to be used internally.
> >>>>>>
> >>>>>> Signed-off-by: Peter Xu <address@hidden>
> >>>>>> ---
> >>>>>>  include/sysemu/iothread.h |  8 ++++++++
> >>>>>>  iothread.c                | 21 +++++++++++++++++++++
> >>>>>>  2 files changed, 29 insertions(+)
> >>>>>>
> >>>>>> diff --git a/include/sysemu/iothread.h b/include/sysemu/iothread.h
> >>>>>> index d2985b3..b07663f 100644
> >>>>>> --- a/include/sysemu/iothread.h
> >>>>>> +++ b/include/sysemu/iothread.h
> >>>>>> @@ -46,4 +46,12 @@ AioContext *iothread_get_aio_context(IOThread 
> >>>>>> *iothread);
> >>>>>>  void iothread_stop_all(void);
> >>>>>>  GMainContext *iothread_get_g_main_context(IOThread *iothread);
> >>>>>>  
> >>>>>> +/*
> >>>>>> + * Helpers used to allocate iothreads for internal use.  These
> >>>>>> + * iothreads will not be seen by monitor clients when query using
> >>>>>> + * "query-iothreads".
> >>>>>> + */
> >>>>>> +IOThread *iothread_create(const char *id, Error **errp);
> >>>>>> +void iothread_destroy(IOThread *iothread);
> >>>>>> +
> >>>>>>  #endif /* IOTHREAD_H */
> >>>>>> diff --git a/iothread.c b/iothread.c
> >>>>>> index 44c8944..74e400c 100644
> >>>>>> --- a/iothread.c
> >>>>>> +++ b/iothread.c
> >>>>>> @@ -354,3 +354,24 @@ GMainContext 
> >>>>>> *iothread_get_g_main_context(IOThread *iothread)
> >>>>>>  
> >>>>>>      return iothread->worker_context;
> >>>>>>  }
> >>>>>> +
> >>>>>> +static Object *iothread_get_internal_parent(void)
> >>>>>> +{
> >>>>>> +    return container_get(object_get_root(), "/internal-iothreads");
> >>>>>> +}
> >>>>>
> >>>>> I tend to think we might benefit from having this generalized in the
> >>>>> QOM API instead. We have object_get_objects_root() for things that
> >>>>> are created by the mgmt app / user via CLI / QMP.  A parallel method
> >>>>> object_get_internal_root() could be useful for cases like this where
> >>>>> we want to create user-creatable objects, but not have them be
> >>>>> visible to the mgmt app / user, as that would confuse the mgmt app.
> >>>>>
> >>>>> Example for this scenario - libvirt calls query-iothreads to identify
> >>>>> IO thread PIDs, and would get very unhappy if the IOThread used by
> >>>>> the monitor would appear in that response, which is why Peter has
> >>>>> put it under /internal-iothreads. I think this scenario will apply
> >>>>> more broadly, so benefit from us having a general helper in QOM.
> >>>>
> >>>> Yeah, I can split the patch if we want it to be exposed to public.
> >>>>
> >>>> In that case, would the name "object_get_internal_root" be good?
> >>>
> >>> Yeah that's fine with me, but lets see if Paolo has any thoughts from
> >>> the QOM side.
> >>
> >> No, I don't.  However, I wonder if query-iothreads should have an
> >> argument to include internal iothreads.
> > 
> > I guess it depends whether we consider use of iothreads for migration
> > to be a private impl detail that may be ripped out & replaced at any
> > time, or a semi-public detail that apps may rely on.  Personally I
> > would suggest it should be a private impl detail of migration code
> > that is never exposed to anything outside QEMU, so we have flexibility
> > to change it at will later.
> 
> As far as I understood it's not migration, it's the whole monitor that
> moves to the iothread.  So it's a thing that lasts for the whole
> existence of the QEMU process.  But I may be wrong.

Yes it is about the monitor thread.

One thing to mention is that, it's not the whole monitor thread - my
plan is only to handle QMP IOs in that thread (and execute oob
commands only, while oob commands should be really rare).  Most of the
QMP command dispatching and execution will still be in main thread
(which I suppose should be most of the real workload).

IMHO I would prefer user know nothing of this thread, or tune it in
any way.  Maybe there will be a time when we would like user to tune
monitor threads, but IMHO current OOB threading is not that far yet.

Thanks,

-- 
Peter Xu



reply via email to

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