On 14.11.2016 15:24, Nikolay Shirokovskiy wrote: > qemuDomainObjExitAgent is unsafe. > > First it accesses domain object without domain lock. > Second it uses outdated logic that goes back to commit 79533da1 of > year 2009 when code was quite different. (unref function > instead of unreferencing only unlocked and disposed object > in case of last reference and leaved unlocking to the caller otherwise). > Nowadays this logic may lead to disposing locked object > i guess.
Well, I agree that the order of those two steps should be reversed, so ACK to that. > > Another problem is that the callers of qemuDomainObjEnterAgent > use domain object again (namely priv->agent) without domain lock. But why is this a problem? > > This patch address these two problems. > > qemuDomainGetAgent is dropped as unused. > --- > > I wonder should we make qemuDomainObj(Enter|Exit)Agent be macros. So > we don't need to declare a variable to hold agent reference and zero > it out at the end for safety. > > Also the qemu monitor code has probably same problems as the agent monitor > code > seems to be copied from there. > Well, qemu monitor can't come and go. I mean, if it does, the domain is killed. That's not the case with agent monitor. > src/qemu/qemu_domain.c | 41 ++++--------- > src/qemu/qemu_domain.h | 7 +-- > src/qemu/qemu_driver.c | 153 > ++++++++++++++++++++++++------------------------- > 3 files changed, 91 insertions(+), 110 deletions(-) Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
