Re: [libvirt] [PATCH] qemu: monitor: fix unsafe monitor access
On 02/28/2018 06:09 PM, Peng Hao wrote:
From: root <root(a)localhost.localdomain>
Don't you want to take credit for the patch?
git config --global user.name "Mona Lisa"
git config --global user.email "email(a)example.com"
qemuDomainObjExitMonitor is unsafe
domain lock released when qemuDomainObjEnterMonitor finish,
So other thread (qemuProcessStop) has chance to modify priv->mon
to NULL. qemuDomainObjExitMonitor will never release the mon->lock,
that may cause problem:
thread get monitor ptr early, and then try to get mon->lock,
it will block forerver cause mon->lock not released by
qemuDomainObjExitMonitor.
I don't quite understand. There can be only one thread talking to
monitor, and actually only one thread calling
qemuDomainObjEnterMonitor(). Because in order to call EnterMonitor() the
caller needs to have domainObj lock. So there can be only one thread
holding the domainObj lock and another which holds the monitor lock. I'm
failing to see why not unlocking monitor would be a problem. Do you
perhaps have a reproducer?
Signed-off-by: Wang Yechao <wang.yechao255(a)zte.com.cn>
Signed-off-by: Peng Hao <peng.hao2(a)zte.com.cn>
---
src/qemu/THREADS.txt | 12 +-
src/qemu/qemu_block.c | 5 +-
src/qemu/qemu_domain.c | 64 +++++----
src/qemu/qemu_domain.h | 12 +-
src/qemu/qemu_driver.c | 258 ++++++++++++++++++++--------------
src/qemu/qemu_hotplug.c | 296 ++++++++++++++++++++++-----------------
src/qemu/qemu_migration.c | 104 ++++++++------
src/qemu/qemu_migration_cookie.c | 5 +-
src/qemu/qemu_process.c | 108 ++++++++------
9 files changed, 507 insertions(+), 357 deletions(-)
This fails to apply cleanly. Can you please rebase?
mode change 100644 => 100755 src/qemu/THREADS.txt
mode change 100644 => 100755 src/qemu/qemu_block.c
mode change 100644 => 100755 src/qemu/qemu_domain.c
mode change 100644 => 100755 src/qemu/qemu_domain.h
mode change 100644 => 100755 src/qemu/qemu_driver.c
mode change 100644 => 100755 src/qemu/qemu_hotplug.c
mode change 100644 => 100755 src/qemu/qemu_migration.c
mode change 100644 => 100755 src/qemu/qemu_migration_cookie.c
mode change 100644 => 100755 src/qemu/qemu_process.c
No. This is not what we want. We don't need *.c files executable.
Michal