23-Nov-16 11:00, Nikolay Shirokovskiy пишет:
On 23.11.2016 10:50, Michal Privoznik wrote:
On 23.11.2016 08:08, Nikolay Shirokovskiy wrote:
On 22.11.2016 17:49, Michal Privoznik wrote:
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?
qemuProcessHandleAgentEOF for example can zero out priv->agent after
we call qemuDomainObjEnterAgent and before we call
qemuAgentGetTime(priv->agent, seconds, nseconds).
Because we drop domain lock in qemuDomainObjEnterAgent and
qemuProcessHandleAgentEOF does not acquire job condition, only domain lock.
At the same time qemuAgentGetTime is not ready for NULL agent and will crash.
It could get even worse as priv->agent is not atomic and instead of
NULL we can get garbage there. Long story short we just should
not access domain state without lock. Thats why I change all the places
so we get copy of priv->agent before we drop domain lock.
Ah. Sounds reasonable. Mind adding it to the commit message?
Of course not.
ACK then.
Thanx! I have no push rights so can you push this and another agent series
you ACKed recently instead of me? (Sorry this will take changing commit
message too:).
I went ahead and pushed this and another series you mentioned.
Maxim
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
