[libvirt] [Xen-devel] [PATCH 00/12] libxl: fork: SIGCHLD flexibility
Jim Fehlig
jfehlig at suse.com
Tue Jan 28 01:39:36 UTC 2014
[Adding libvirt list...]
Ian Jackson wrote:
> Jim Fehlig writes ("Re: [Xen-devel] [PATCH 00/12] libxl: fork: SIGCHLD flexibility"):
>
>> BTW, I only see the crash when the save/restore script is running. I
>> stopped the other scripts and domains, running only save/restore on a
>> single domain, and see the crash rather quickly (within 10 iterations).
>>
>
> I'll look at the libvirt code, but:
>
> With a recurring timeout, how can you ever know it's cancelled ?
> There might be threads out there, which don't hold any locks, which
> are in the process of executing a callback for a timeout. That might
> be arbitrarily delayed from the pov of the rest of the program.
>
> E.g.:
>
> Thread A Thread B
>
> invoke some libxl operation
> X do some libxl stuff
> X register timeout (libxl)
> XV record timeout info
> X do some more libxl stuff
> ...
> X do some more libxl stuff
> X deregister timeout (libxl internal)
> X converted to request immediate timeout
> XV record new timeout info
> X release libvirt event loop lock
> entering libvirt event loop
> V observe timeout is immediate
> V need to do callback
> call libxl driver
>
> entering libvirt event loop
> V observe timeout is immediate
> V need to do callback
> call libxl driver
> call libxl
> X libxl sees timeout is live
> X libxl does libxl stuff
> libxl driver deregisters
> V record lack of timeout
> free driver's timeout struct
> call libxl
> X libxl sees timeout is dead
> X libxl does nothing
> libxl driver deregisters
> V CRASH due to deregistering
> V already-deregistered timeout
>
> If this is how things are, then I think there is no sane way to use
> libvirt's timeouts (!)
>
Looking at the libvirt code again, it seems a single thread services the
event loop. See virNetServerRun() in src/util/virnetserver.c. Indeed, I
see the same thread ID in all the timer and fd callbacks. One of the
libvirt core devs can correct me if I'm wrong.
Regards,
Jim
> In principle I guess the driver could keep its per-timeout structs
> around forever and remember whether they've been deregistered or not.
> Each one would have to have a lock in it.
>
> But if you think about it, if you have 10 threads all running the
> event loop and you set a timeout to zero, doesn't that mean that every
> thread's event loop should do the timeout callback as fast as it can ?
> That could be a lot of wasted effort.
>
> The best solution would appear to be to provide a non-recurring
> callback.
>
>
>> I'm not so thrilled with the timeout handling code in the libvirt libxl
>> driver. The driver maintains a list of active timeouts because IIRC,
>> there were cases when the driver received timeout deregistrations when
>> calling libxl_ctx_free, at which point some of the associated structures
>> were freed. The idea was to call libxl_osevent_occurred_timeout on any
>> active timeouts before freeing libxlDomainObjPrivate and its contents.
>>
>
> libxl does deregister fd callbacks in libxl_ctx_free.
>
> But libxl doesn't currently "deregister" any timeouts in
> libxl_ctx_free; indeed it would be a bit daft for it to do so as at
> libxl_ctx_free there are no aos running so there would be nothing to
> time out.
>
> But there is a difficulty with timeouts which libxl has set to occur
> immediately but which have not yet actually had the callback. The the
> application cannot call libxl_ctx_free with such timeouts outstanding,
> because that would imply later calling back into libxl with a stale
> ctx.
>
> (Looking at the code I see that the "nexi" are never actually freed.
> Bah.)
>
> Thanks,
> Ian.
>
>
More information about the libvir-list
mailing list