Bamvor Jian Zhang wrote: > this patch fix the wrong sequence for fd and timeout register. the sequence > was right in dfa1e1dd for fd register, but it changed in e0622ca2. > in this patch, set priv, xl_priv in info and increase info->priv ref count > before virEventAddHandle. if do this after virEventAddHandle, the fd > callback or fd deregister maybe got the empty priv, xl_priv or wrong ref > count. > Bamvor and I discussed this patch off-list while investigating reports of segfaults in the libxl driver, e.g. https://www.redhat.com/archives/libvir-list/2013-March/msg00502.html > after apply this patch, test more than 100 rounds passed compare to fail > within 3 rounds without this patch. each round includes define -> start -> > destroy -> create -> suspend -> resume -> reboot -> shutdown -> save -> > resotre -> dump -> destroy -> create -> setmem -> setvcpus -> destroy. > ACK. I'll push this since it is a bug fix. Regards, Jim > Signed-off-by: Bamvor Jian Zhang <bjzhang@xxxxxxxx> > --- > src/libxl/libxl_driver.c | 39 +++++++++++++++++++++------------------ > 1 files changed, 21 insertions(+), 18 deletions(-) > > diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c > index b4f1889..212d0fc 100644 > --- a/src/libxl/libxl_driver.c > +++ b/src/libxl/libxl_driver.c > @@ -181,26 +181,28 @@ libxlFDRegisterEventHook(void *priv, int fd, void **hndp, > return -1; > } > > + info->priv = priv; > + /* > + * Take a reference on the domain object. Reference is dropped in > + * libxlEventHookInfoFree, ensuring the domain object outlives the fd > + * event objects. > + */ > + virObjectRef(info->priv); > + info->xl_priv = xl_priv; > + > if (events & POLLIN) > vir_events |= VIR_EVENT_HANDLE_READABLE; > if (events & POLLOUT) > vir_events |= VIR_EVENT_HANDLE_WRITABLE; > + > info->id = virEventAddHandle(fd, vir_events, libxlFDEventCallback, > info, libxlEventHookInfoFree); > if (info->id < 0) { > + virObjectUnref(info->priv); > VIR_FREE(info); > return -1; > } > > - info->priv = priv; > - /* > - * Take a reference on the domain object. Reference is dropped in > - * libxlEventHookInfoFree, ensuring the domain object outlives the fd > - * event objects. > - */ > - virObjectRef(info->priv); > - > - info->xl_priv = xl_priv; > *hndp = info; > > return 0; > @@ -283,6 +285,15 @@ libxlTimeoutRegisterEventHook(void *priv, > return -1; > } > > + info->priv = priv; > + /* > + * Also take a reference on the domain object. Reference is dropped in > + * libxlEventHookInfoFree, ensuring the domain object outlives the timeout > + * event objects. > + */ > + virObjectRef(info->priv); > + info->xl_priv = xl_priv; > + > gettimeofday(&now, NULL); > timersub(&abs_t, &now, &res); > /* Ensure timeout is not overflowed */ > @@ -296,22 +307,14 @@ libxlTimeoutRegisterEventHook(void *priv, > info->id = virEventAddTimeout(timeout, libxlTimerCallback, > info, libxlEventHookInfoFree); > if (info->id < 0) { > + virObjectUnref(info->priv); > VIR_FREE(info); > return -1; > } > > - info->priv = priv; > - /* > - * Also take a reference on the domain object. Reference is dropped in > - * libxlEventHookInfoFree, ensuring the domain object outlives the timeout > - * event objects. > - */ > - virObjectRef(info->priv); > - > virObjectLock(info->priv); > LIBXL_EV_REG_APPEND(info->priv->timerRegistrations, info); > virObjectUnlock(info->priv); > - info->xl_priv = xl_priv; > *hndp = info; > > return 0; > -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list