Re: [PATCH v2] qemu: agent: Avoid agentError when closing the QEMU agent

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> 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@xxxxxxxxxx>
> > ---
> > 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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux