On 09/28/2018 11:36 AM, Wang Yechao wrote:
After calling qemuAgentClose(), it is still possible for the QEMU Agent I/O event callback to get invoked. This will trigger an agent error because mon->fd has been set to -1 at this point. Then vm->privateData->agentError is always 'true' except that restart libvirtd or restart qemu-guest-agent process in guest.
Silently ignore the case where mon->fd is -1, likewise for mon->watch being zero.
Signed-off-by: Wang Yechao <wang.yechao255@zte.com.cn> --- v1 patch: https://www.redhat.com/archives/libvir-list/2018-September/msg01382.html
Changes in v2: - do not set agentError, let agent state as disconnected instead of error. --- src/qemu/qemu_agent.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index 97ad0e7..d842b0e 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -530,6 +530,9 @@ static void qemuAgentUpdateWatch(qemuAgentPtr mon) VIR_EVENT_HANDLE_HANGUP | VIR_EVENT_HANDLE_ERROR;
+ if (!mon->watch) + return; + if (mon->lastError.code == VIR_ERR_OK) { events |= VIR_EVENT_HANDLE_READABLE;
@@ -555,6 +558,12 @@ qemuAgentIO(int watch, int fd, int events, void *opaque) VIR_DEBUG("Agent %p I/O on watch %d fd %d events %d", mon, watch, fd, events); #endif
+ if (mon->fd == -1 || mon->watch == 0) { + virObjectUnlock(mon); + virObjectUnref(mon); + return; + } +
Is this safe to do? What if there's a thread waiting for a message from the agent? Shouldn't @eof variable be set in this case so that appropriate code is run?
It is safe to do. The waiting thread is waked up when qemuAgentClose invoked, and it cannot get any message from the agent at this time. There is no need to set the @eof variable, because the EOF callback's job has been done when closing the agent.
if (mon->fd != fd || mon->watch != watch) { if (events & (VIR_EVENT_HANDLE_HANGUP | VIR_EVENT_HANDLE_ERROR)) eof = true; @@ -788,8 +797,10 @@ void qemuAgentClose(qemuAgentPtr mon) virObjectLock(mon);
if (mon->fd >= 0) { - if (mon->watch) + if (mon->watch) { virEventRemoveHandle(mon->watch); + mon->watch = 0; + } VIR_FORCE_CLOSE(mon->fd); }
Michal
--- Best wishes Wang Yechao