Re: [Xen-devel] [PATCH 2/2] libxl: fix stale timeout event callback race

[Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>]

Jim Fehlig writes ("Re: [Xen-devel] [PATCH 2/2] libxl: fix stale timeout event 
callback race"):
> Ian Jackson wrote:
> > Specifically, this code has an integer arithmetic overflow.
> 
> Well, this patch is removing that buggy code :).

I think you need to have some code somewhere which does the
complicated arithmetic dance to avoid the integer overflow.  Does your
timer registration function have the same bug ?

>  After again reading
> libxl_event.h, I'm considering the below patch in the libvirt libxl
> driver.  The change is primarily inspired by this comment for
> libxl_osevent_occurred_timeout:
...
> /* Implicitly, on entry to this function the timeout has been
>  * deregistered.  If _occurred_timeout is called, libxl will not
>  * call timeout_deregister; if it wants to requeue the timeout it
>  * will call timeout_register again.
>  */

Well your patch is only correct when used with the new libxl, with my
patches.

> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> index 302f81c..d4055be 100644
> --- a/src/libxl/libxl_driver.c
> +++ b/src/libxl/libxl_driver.c
> @@ -184,6 +184,8 @@ static void libxlTimerCallback(int timer
> ATTRIBUTE_UNUSED, void *timer_v)
>  {
>      struct libxlOSEventHookTimerInfo *timer_info = timer_v;
>  
> +    virEventRemoveTimeout(timer_info->id);
> +    timer_info->id = -1;

I don't understand why you need this.  Doesn't libvirt remove timers
when they fire ?  If it doesn't, do they otherwise not keep
reoccurring ?

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel