Re: [libvirt] [PATCH] fix segfault during virsh save in pv guest

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.

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.

Signed-off-by: Bamvor Jian Zhang <bjzhang(a)suse.com&gt; --- 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;