Re: [Qemu-devel] [PATCH v8 19/23] qmp: isolate responses into io thread

[Marc-André Lureau]

Hi

On Fri, Mar 23, 2018 at 6:50 AM, Peter Xu <address@hidden> wrote:
> On Thu, Mar 22, 2018 at 01:00:19PM +0100, Marc-André Lureau wrote:
>> Hi
>>
>> On Fri, Mar 9, 2018 at 10:00 AM, Peter Xu <address@hidden> wrote:
>> > For those monitors who have enabled IO thread, we'll offload the
>> > responding procedure into IO thread.  The main reason is that chardev is
>> > not thread safe, and we need to do all the read/write IOs in the same
>> > thread.  For use_io_thr=true monitors, that thread is the IO thread.
>>
>> Actually, the chr write path is suppose to be somewhat thread safe
>> (chr_write_lock).
>>
>> Secondly, the function responsible to write monitor data has some
>> thread-safety, it's called monitor_flush_locked(), because you need
>> the mon->out_lock.
>>
>> I think that patch is making things more complicated than they need to
>> be. You should be able to call monitor_json_emitter/monitor_puts()
>> directly from any thread, it will queue the data, start writing, and
>> add a watch if necessary in the appropriate context.
>>
>> Am I missing something?
>
> Yes there are the locks either in monitor code and chardev code to
> protect write operations.  However I don't know enough to be sure of
> its safety.  Considering that, having the whole chardev operations
> separated into the other IOThread seems to be the only safe approach
> to me for now.

I have studied the code in more details, and I'll propose to revert
this patch early in 2.13. I'll send a 2.13 RFC qmp series shortly.



-- 
Marc-André Lureau