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: Paolo Bonzini
Subject: Re: [Qemu-devel] [PATCH 1/3] iothread: provide helpers for internal use
Date: Fri, 22 Sep 2017 12:14:04 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0

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.

Paolo

Paolo



reply via email to

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