On Sun, Dec 02, 2018 at 23:10:16 -0600, Chris Venteicher wrote:
> Locking the monitor object immediately after call to qemuMonitorOpen
> doesn't make sense now that we have expanded the QEMU process code to
> cover more than the original capabilities usecase.
The expanded use case has nothing to do with locking. You just call QMP
commands and the use case only changes what commands are called.
> Removing the monitor lock makes the qemuProcessQmpConnectMonitor code
> consistent with the qemuConnectMonitor code used to establish the
> monitor when QEMU process is started for domains.
qemuConnectMonitor does not lock the monitor code because we have
EnterMonitor and ExitMonitor calls which lock/unlock the monitor around
every QMP command we call.
> Signed-off-by: Chris Venteicher <cventeic@xxxxxxxxxx>
> ---
> src/qemu/qemu_process.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index 26ba59143d..b491f9f91a 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -8331,8 +8331,6 @@ qemuProcessQmpConnectMonitor(qemuProcessQmpPtr proc)
>
> VIR_STEAL_PTR(proc->mon, mon);
>
> - virObjectLock(proc->mon);
> -
> /* Exit capabilities negotiation mode and enter QEMU command mode
> * by issuing qmp_capabilities command to QEMU */
> if (qemuMonitorSetCapabilities(proc->mon) < 0) {
All monitor code expects the monitor to be locked. Not to mention that
there is a corresponding unlock in qemuProcessQmpStop().
In other words, NACK to this patch.
Did you hit any issues which inspired you to make this patch? Or was it
just a way to make the code seemingly consistent with
qemuConnectMonitor?
Jirka
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
