[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v4] monitor: let cur_mon be per-thread
From: |
Peter Xu |
Subject: |
Re: [Qemu-devel] [PATCH v4] monitor: let cur_mon be per-thread |
Date: |
Thu, 19 Jul 2018 13:01:45 +0800 |
User-agent: |
Mutt/1.10.0 (2018-05-17) |
On Wed, Jul 18, 2018 at 05:38:11PM +0200, Markus Armbruster wrote:
> Peter Xu <address@hidden> writes:
>
> > After the Out-Of-Band work, the monitor iothread may be accessing the
> > cur_mon as well (via monitor_qmp_dispatch_one()). Let's convert the
> > cur_mon variable to be a per-thread variable to make sure there won't be
> > a race between threads when accessing the variable.
>
> Hmm... why hasn't the OOB work created such a race already?
>
> A monitor reads, parses, dispatches and executes commands, formats and
> sends replies.
>
> Before OOB, all of that ran in the main thread. Any access of cur_mon
> should therefore be from the main thread. No races.
>
> OOB moves read, parse, format and send to an I/O thread. Dispatch and
> execute remain in the main thread. *Except* for commands executed OOB,
> dispatch and execute move to the I/O thread, too.
>
> Why is this not racy? I guess it relies on careful non-use of cur_mon
> in any part that may now execute in the I/O thread. Scary...
I think it's because cur_mon is not really used in out-of-band command
executions - now we only have a few out-of-band enabled commands, and
IIUC none of them is using cur_mon (for example, in
qmp_migrate_recover() we don't even call error_report, and the code
path is quite straight forward to make sure of that). So IIUC cur_mon
variable is still only touched by main thread for now hence we should
be safe. However that condition might change in the future when we
add more out-of-band capable commands.
(not to mention that I don't even know whether there are real users of
out-of-band if we haven't yet started to support that for libvirt...)
>
> Should this go into 3.0 to reduce the risk of bugs?
Yes I think it would be good to have that even for 3.0, since it still
can be seen as a bug fix of existing code.
Regards,
--
Peter Xu
- Re: [Qemu-devel] [PATCH v4] monitor: let cur_mon be per-thread, Markus Armbruster, 2018/07/18
- Re: [Qemu-devel] [PATCH v4] monitor: let cur_mon be per-thread,
Peter Xu <=
- Re: [Qemu-devel] [PATCH v4] monitor: let cur_mon be per-thread, Markus Armbruster, 2018/07/19
- Re: [Qemu-devel] [PATCH v4] monitor: let cur_mon be per-thread, Peter Xu, 2018/07/19
- Re: [Qemu-devel] [PATCH v4] monitor: let cur_mon be per-thread, Markus Armbruster, 2018/07/19
- Re: [Qemu-devel] [PATCH v4] monitor: let cur_mon be per-thread, Peter Xu, 2018/07/19
- Re: [Qemu-devel] [PATCH v4] monitor: let cur_mon be per-thread, Markus Armbruster, 2018/07/19
- Re: [Qemu-devel] [PATCH v4] monitor: let cur_mon be per-thread, Peter Xu, 2018/07/19
- Re: [Qemu-devel] [PATCH v4] monitor: let cur_mon be per-thread, Markus Armbruster, 2018/07/19