These functions should be used to log messages when the domain
id is known. libxl__log will now prepend the log message by
"Domain %PRIu32:" if the domain id is a valid one.
This aims at helping consumers filter logs on domain IDs.
Signed-off-by: Cédric Bosdonnat <cbosdonnat@suse.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Otherwise the name clashes with the libxc evtchn_compat use of the
name (which is compat, so can't really change), and triggers an error
with older gcc relating to redefining typedefs (which are not allowed
per the spec, but which are tolerated by newer gcc)
None of qemu-trad, qemu-xen or mini-os actually use this type, so this
should be fine. Note that the library API is not actually stable until
4.7 is releases.
Reported-by: Olaf Hering <olaf@aepfle.de>
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Tested-by: Olaf Hering <olaf@aepfle.de>
Acked-by: Wei Liu <wei.liu2@citrix.com>
libxenevtchn will provide a stable API and ABI for accessing the
evtchn device.
The functions are moved into the xenevtchn namespace to make a clean
break from libxc and avoid ambiguity regarding which interfaces are
stable.
All in-tree users are updated to use the new names.
Upon request (via #define XC_WANT_COMPAT_EVTCHN_API) libxenctrl will
provide a compat API for the old names. This is used by qemu-xen for
the time being. qemu-xen-traditional is updated in lockstep.
This leaves a few event channel related functions which go via privcmd
(EVTCHNOP) rather than ioctls on the /dev/xen/evtchn device in
libxenctrl. Specifically:
- xc_evtchn_alloc_unbound
- xc_evtchn_reset
- xc_evtchn_status
Note that xc_evtchn_alloc_unbound's functionality is also provided by
xenevtchn_bind_unbound_port() (née xc_evtchn_bind_unbound_port) and is
probably redundant.
These functions do not appear to be needed by qemu-dm, qemu-pv
(provision of device model to HVM guests and PV backends respectively)
or by libvchan suggesting they are not needed by non-toolstack uses of
event channels. QEMU does use these in hw/xenpv/xen_domainbuild.c but
that is a "toolstack use".
The new library uses a version script to ensure that only expected
symbols are exported and to version them such that ABI guarantees can
be kept in the future.
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
[ ijc -- updated MINIOS_UPSTREAM_REVISION and QEMU_TRADITIONAL_REVISION ]
The rune used is:
sed -i 's/libxl__sprintf(gc,\s*/GCSPRINTF(/g' libxl*.c
This rune is simple and better than trying to match every possible
patterns.
Two instances in libxl_dm.c need fixing up. They are in fact better to just
use libxl__strdup.
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
assert is not async-signal-safe.
In practice the effect of calling assert there is that if the
assertion fails we might get a secondary crash, or other undesirable
behaviour from stdio (which is how assert usually reports failures).
Mention in a comment in libxl__self_pipe_wakeup that it has to be
async-signal-safe.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
libxl__ao_inprogress_gc() is hidden behind various macros used to
construct local variables. Assert() that NULL is not passed, to make
such an error very obvious, rather than a plain segfault at 0.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
It can happen that an fd is deregistered, and closed, and then a new
fd opened, and reregistered, all while another thread is in poll().
If this happens poll might report POLLNVAL, but the event loop would
think that the fd was supposed to have been valid, and then fail an
assertion:
libxl_event.c:1183: afterpoll_check_fd: Assertion `poller->fds_changed || !(fds[slot].revents & 0x020)' failed.
We can't simply ignore POLLNVAL because if we have bugs which cause
messed-up fds, it is a serious problem which we really need to detect.
Instead, add extra tracking to spot when this possibility arises, and
abort on POLLNVAL if we are sure that it is unexpected.
Reported-by: Jim Fehlig <jfehlig@suse.com>
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Jim Fehlig <jfehlig@suse.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Tested-by: Jim Fehlig <jfehlig@suse.com>
This makes the code more regular. We are going to want to do some
more work in poller_get and poller_put, which work also wants to be
done for poller_app.
Two very minor functional changes:
* We call malloc an extra time since poller_app is now a pointer
* ERROR_FAIL on poller_get failing for poller_app is generated in
libxl_ctx_init rather than passed through by libxl_poller_init
from libxl__pipe_nonblock.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Jim Fehlig <jfehlig@suse.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Tested-by: Jim Fehlig <jfehlig@suse.com>
Make an active libxl__domaindeathcheck contain an active
libxl__ao_abortable.
Consequential changes are:
* domaindeath callbacks now take an rc value.
* libxl__domaindeathcheck_start takes an ao, not a gc.
* bootloader_domaindeath plumbs the rc through to its caller.
* libxl__domaindeathcheck_init and _stop are not quite trivial any
more so are moved from (inline functions) in libxl_internal.h, to
ordinary functions defined in libxl_event.c.
* libxl__domaindeathcheck_start is not trivial any more, and now has
the standard error-handling pattern.
The only current user of libxl__domaindeathcheck is the bootloader.
So the result is that now it is possible to effectively abort domain
creation while the bootloader is running.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
v2: New in this version of the series.
Make libxl__ev_time* register with the abort machinery, so that
libxl_ao_abort can stop any operation which has a timeout.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
---
v4: Do not immediately deregister the abortable (!)
(v3 was Acked-by: Ian Campbell <ian.campbell@citrix.com>)
Some places in libxl which can't handle abort via a
libxl__ao_abortable callback might nevertheless benefit from being
able to explicitly check whether abort has been requested.
Provide the (fairly trivial) internal API function to do this.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
v2: New in this version of the series.
Provide libxl_ao_abort.
There is machinery to allow an ao to register an interest in abort
requests, using a libxl__ao_abortable.
This API is not currently very functional: requesting abort will
never have any effect.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
---
v5: Clarify API doc comment re aborting synchronous operations.
v4: Rename from cancel to abort.
Actually record aos on aos_inprogress.
(Report from Koushik Chakravarty at Citrix.)
Do not mark libxl_ao_cancel hidden (!)
Abolish ERROR_NOTIMPLEMENTED from libxl_ao_cancel.
All operations are supposed to support cancellation.
v2: Minor comment improvements
Previously we used in_initiator to stop the ao being freed while we
were still in the initiator function (which would result in the
initiator's call to libxl__ao_inprogress accessing the ao after it had
been freed).
We are going to introduce a new libxl entrypoint which finds, and
operates on, ongoing aos. This function needs the same protection,
and might even end up running on the same ao multiple times
concurrently.
So do this with reference counting instead, with a new variable
ao->manip_refcnt.
We keep ao->in_initiator because that allows us to keep some useful
asserts about the sequencing of libxl__ao_inprogress, etc.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
v5: Fix typo in commit message.
v3: Add a missing space.
Mention locking in the comment.
The callback functions provided by users must take an rc value. This
rc value can be ERROR_TIMEDOUT or ERROR_ABORTED.
Users of xswait are now expected to deal correctly with
ERROR_ABORTED. If they experience this, it hasn't been logged.
And the caller won't log it either since it's not TIMEDOUT.
Luckily this is correct, so we can just change the doc comment.
Currently nothing generates ERROR_ABORTED; in particular the timeouts
cannot in fact signal abort requests.
There should be no publicly visible change except that some error
returns from libxl will change from ERROR_FAIL to ERROR_TIMEDOUT, and
some changes to debugging messages.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
v4: ABORTED not CANCELLED.
Change the timeout setup functions to take a libxl__ao, not a
libxl__gc. This is going to be needed for ao abort, because timeouts
are going to be a main hook for ao abort requests - so the timeouts
need to be associated with an ao.
This means that timeouts can only occur as part of a long-running
libxl function (but this is of course correct, as libxl shouldn't have
any global timeouts, and indeed all the call sites have an ao).
Also remove the gc parameter from libxl__async_exec_start. It can
just use the gc from the ao supplied in the aes.
All the callers follow the obvious patterns and therefore supply the
ao's gc to libxl__async_exec_start and the timeout setup functions.
There is therefore no functional change in this patch.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
CC: Yang Hongyang <yanghy@cn.fujitsu.com>
CC: Wen Congyang <wency@cn.fujitsu.com>
CC: Lai Jiangshan <laijs@cn.fujitsu.com>
Acked-by: Wen Congyang <wency@cn.fujitsu.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
v2: This patch split off from "Permit timeouts to signal cancellation".
Rebased; consequently, deal with libxl__async_exec_start.
CC'd authors of the libxl__async_exec_* functions.
We are going to introduce a new meaning for aborting an ao, so rename
this to avoid confusion.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Always recheck with poll() right before making the callback.
All sorts of things may have happened since poll() originally signaled
the fd. We would like the main functional libxl code not to have to
worry about spurious wakeups.
In particular, this fixes a bug in the save/restore callout: the save
helper message reader operates with the fd in blocking mode. In a
multithreaded program one thread might have eaten all the messages out
of the fd while another one is busy returning from poll and reacquiring
the libxl lock, possibly resulting in a deadlock.
(Also, we abolish the anomalous direct caller of efd->func.)
Reported-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Reported-by: Jim Fehlig <jfehlig@suse.com>
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Jim Fehlig <jfehlig@suse.com>
CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Tested-by: Jim Fehlig <jfehlig@suse.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
No functional change, only code motion.
Currently, contrary to this function's name, there are two sites where
efd->func() is called so one of them doesn't go through here just yet.
That will be dealt with in the next commit.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Jim Fehlig <jfehlig@suse.com>
CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Tested-by: Jim Fehlig <jfehlig@suse.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
libxl__ao_inprogress needs to check (like
libxl__ao_complete_check_progress_reports) that there are no
oustanding progress callbacks.
Otherwise it might happen that we would destroy the ao while another
thread has an outstanding callback its egc report queue. The other
thread would then, in its egc_run_callbacks, touch the destroyed ao.
Instead, when this happens in libxl__ao_inprogress, simply run round
the event loop again. The thread which eventually makes the callback
will spot our poller in the ao, and notify the poller, waking us up.
This fixes an assertion failure race seen with libvirt:
libvirtd: libxl_event.c:1792: libxl__ao_complete_check_progress_reports: Assertion `ao->in_initiator' failed.
or (after "Add an assert to egc_run_callbacks")
libvirtd: libxl_event.c:1338: egc_run_callbacks: Assertion `aop->ao->magic == 0xA0FACE00ul' failed.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Ian Campbell <ian.campbell@citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
CC: Jim Fehlig <jfehlig@suse.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Break out the test in libxl__ao_complete_check_progress_reports, into
ao_work_outstanding, which reports false if either (i) the ao is still
ongoing or (ii) there is a progress report (perhaps on a different
thread's callback queue) which has yet to be reported to the
application.
No functional change.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Ian Campbell <ian.campbell@citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
CC: Jim Fehlig <jfehlig@suse.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Check that the ao is still live when we are about to running some of
its callbacks.
This reveals an existing bug in libxl which is exercised by libvirt,
converting
libvirtd: libxl_event.c:1792: libxl__ao_complete_check_progress_reports: Assertion `ao->in_initiator' failed.
into
libvirtd: libxl_event.c:1338: egc_run_callbacks: Assertion `aop->ao->magic == 0xA0FACE00ul' failed.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Ian Campbell <ian.campbell@citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
CC: Jim Fehlig <jfehlig@suse.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
libxl_event_register_callbacks cannot reasonably be called while libxl
is busy (has outstanding operations and/or enabled events).
This is because the previous spec implied (although not entirely
clearly) that event hooks would not be called for existing fd and
timeout interests. There is thus no way to reliably ensure that libxl
would get told about fds and timeouts which it became interested in
beforehand.
So there have to be no such fds or timeouts, which means that the
callbacks must only be registered or changed when the ctx is idle.
Document this restriction, and enforce it with a pair of asserts.
(It would be nicer, perhaps, to say that the application may not call
libxl_osevent_register_hooks other than right after creating the ctx.
But there are existing callers, including libvirt, who do it later -
even after doing major operations such as domain creation.)
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
Tested-by: Ian Campbell <ian.campbell@citrix.com>
Release-Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
We want to have no fd events registered when we are idle.
In this patch, deal with the evtchn fd:
* Defer setup of the evtchn handle to the first use.
* Defer registration of the evtchn fd; register as needed on use.
* When cancelling an evtchn wait, or when wait setup fails, check
whether there are now no evtchn waits and if so deregister the fd.
* On libxl teardown, the evtchn fd should therefore be unregistered.
assert that this is the case.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
Tested-by: Ian Campbell <ian.campbell@citrix.com>
Release-Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
v2: Do not bother putting evtchn_fd in the ctx; instead, get it
from xc_evtchn_fd when we need it. (Cosmetic.)
Do not register the evtchn fd multiple times: check it's not
registered before we call libxl__ev_fd_register. (Bugfix.)
We want to have no fd events registered when we are idle.
In this patch, deal with the xenstore watch fd:
* Track the total number of active watches.
* When deregistering a watch, or when watch registration fails, check
whether there are now no watches and if so deregister the fd.
* On libxl teardown, the watch fd should therefore be unregistered.
assert that this is the case.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
Tested-by: Ian Campbell <ian.campbell@citrix.com>
Release-Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
v2: Fix commit message not to refer to libxl_ctx_alloc's gc, done earlier
Change type of port in evtchn_fd_callback to evtchn_port_or_error_t
Clarify comment about use of ctx->xce.
Fix typo in comment.
Change libxl__poller_init and libxl__poller__get to take a libxl__gc*
rather than a libxl_ctx*. The gc is not used for memory allocation
but simply to provide the standard local variable "gc" expected by the
convenience macros. Doing this makes the error logging more
convenient.
Hence, convert the logging calls to use the LOG* convenience macros.
And consequently, change the call sites, and the function bodies to
use CTX rather than ctx.
Also convert a call to malloc() (with error check) in
libxl__poller_get, to libxl__zalloc (no error check needed).
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
CC: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
When a timeout has occurred, it is deregistered. However, we failed
to record this fact by updating etime->func. As a result,
libxl__ev_time_isregistered would say `true' for a timeout which has
already happened.
It is necessary to clear etime->func before the callback, because the
callback might want to reinstate the timeout, or might free the etime
(or its containing struct) entirely.
The results are that we might try to have the timeout occur again
(causing problems for the call site), and/or corrupt the timeout list.
This fixes the timedereg event system unit test.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jim Fehlig <jfehlig@suse.com>
Cc: Ian Campbell <Ian.Campbell@citrix.com>
Acked-by: Ian Campbell <Ian.Campbell@citrix.com>
Bring together the two places where etime->func() is called into a new
function time_occurs. For one call site this is pure code motion.
For the other the only semantic change is the introduction of a new
debugging message.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jim Fehlig <jfehlig@suse.com>
Cc: Ian Campbell <Ian.Campbell@citrix.com>
Acked-by: Ian Campbell <Ian.Campbell@citrix.com>
Break out the pipe creation and destruction from the poller code
into two new functions libxl__pipe_nonblock and libxl__pipe_close.
Also change direct use of pipe() to libxl_pipe.
No overall functional difference other than minor differences in exact
log messages.
Also move libxl__self_pipe_wakeup and libxl__self_pipe_eatall into the
new pipe utilities section in libxl_event.c; this is pure code motion.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Cc: Jim Fehlig <jfehlig@suse.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
--
v3: Mention that we switched pipe() -> libxl_pipe()
libxl_event.c:eventloop_iteration would pass the allocated pollfds
array size, rather than the used size, to poll (and to
afterpoll_internal).
The effect is that if the number of fds to poll on reduces, libxl will
poll on stale entries. Because of the way the return value from poll
is processed these stale entries are often harmless because any events
coming back from poll ignored by libxl. However, it could cause
malfunctions:
It could result in unwanted SIGTTIN/SIGTTOU/SIGPIPE, for example, if
the fd has been reused to refer to an object which can generate those
signals. Alternatively, it could result in libxl spinning if the
stale entry refers to an fd which happens now to be ready for the
previously-requested operation.
I have tested this with a localhost migration and inspected the strace
output.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Cc: Jim Fehlig <jfehlig@suse.com>
Cc: Ian Campbell <Ian.Campbell@citrix.com>
Acked-by: Ian Campbell <Ian.Campbell@citrix.com>
An application which uses libxl_osevent_register_hooks, and doesn't
use libxl_sigchld_owner_mainloop, would never properly experience the
deaths of its (libxl) children.
This is because the SIGCHLD self pipe would be handled using ad-hoc
code in beforepoll_internal and afterpoll_internal. However, this is
no good if application is using the fd registration system instead; in
that case these functions would not be called and nothing would deal
with readability of the self pipe.
Fix this as follows:
The SIGCHLD self pipe now is now dealt with by a standard libxl__ev_fd
handler, which is registered and deregistered along with the SIGCHLD
handler itself. The handler function subsumes the ad-hoc response
code removed from beforepoll_internal, and the functionality of
libxl__fork_selfpipe_woken.
This is also tidier as the SIGCHLD self pipe is no longer touched
outside libxl_fork.c other than in ctx initialisation and teardown.
(The ad-hoc arrangements for poller->wakeup_pipe in
beforepoll_internal and afterpoll_internal are OK because the
libxl__poller mechanism exists to wake up threads which are sitting
inside libxl's poll loop, so is not applicable to the application's
event loop.)
Reported-by: Bamvor Jian Zhang <bjzhang@suse.com>
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Cc: Bamvor Jian Zhang <bjzhang@suse.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Jim Fehlig <jfehlig@suse.com>
Patch generated by the following semantic patch
(http://coccinelle.lip6.fr/):
@@
expression *P;
@@
- if(P) free(P);
+ free(P);
...and then by filtering through the following command:
filterdiff -p1 -x 'stubdom/*' -x 'tools/firmware/*' -x 'tools/qemu-*' -x 'tools/blktap*'
Signed-off-by: Matthew Daley <mattjd@gmail.com>
Acked-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
This is less fragile, and more in keeping with the usual style of
initialising everything to 0 and freeing things unconditionally.
Correspondingly, remove the tests at the call sites.
Apropos of c1f3f174. No overall functional change.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
Because there is not necessarily any lock held at the point the
application (eg, libvirt) calls libxl_osevent_occurred_timeout, in a
multithreaded program those calls may be arbitrarily delayed in
relation to other activities within the program.
Specifically this means when ->timeout_deregister returns, libxl does
not know whether it can safely dispose of the for_libxl value or
whether it needs to retain it in case of an in-progress call to
_occurred_timeout.
The interface could be fixed by requiring the application to make a
new call into libxl to say that the deregistration was complete.
However that new call would have to be threaded through the
application's event loop; this is complicated and some application
authors are likely not to implement it properly. Furthermore the
easiest way to implement this facility in most event loops is to queue
up a time event for "now".
Shortcut all of this by having libxl always call timeout_modify
setting abs={0,0} (ie, ASAP) instead of timeout_deregister. This will
cause the application to call _occurred_timeout. When processing this
calldown we see that we were no longer actually interested and simply
throw it away.
Additionally, there is a race between _occurred_timeout and
->timeout_modify. If libxl ever adjusts the deadline for a timeout
the application may already be in the process of calling _occurred, in
which case the situation with for_app's lifetime becomes very
complicated. Therefore abolish libxl__ev_time_modify_{abs,rel} (which
have no callers) and promise to the application only ever to call
->timeout_modify with abs=={0,0}. The application still needs to cope
with ->timeout_modify racing with its internal function which calls
_occurred_timeout. Document this.
This is a forwards-compatible change for applications using the libxl
API, and will hopefully eliminate these races in callback-supplying
applications (such as libvirt) without the need for corresponding
changes to the application. (It is possible that this might expose
bugs in applications, though, as previously libxl would never call
libxl_osevent_hooks->timeout_modify and now it never calls
->timeout_deregister).
For clarity, fold the body of time_register_finite into its one
remaining call site. This makes the semantics of ev->infinite
slightly clearer.
Cc: Bamvor Jian Zhang <bjzhang@suse.com>
Cc: Ian Campbell <Ian.Campbell@citrix.com>
Tested-by: Jim Fehlig <jfehlig@suse.com>
Acked-by: Jim Fehlig <jfehlig@suse.com>
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Committed-by: Ian Campbell <ian.campbell@citrix.com>
Because there is not necessarily any lock held at the point the
application (eg, libvirt) calls libxl_osevent_occurred_timeout and
..._fd, in a multithreaded program those calls may be arbitrarily
delayed in relation to other activities within the program.
libxl therefore needs to be prepared to receive very old event
callbacks. Arrange for this to be the case for fd callbacks.
This requires a new layer of indirection through a "hook nexus" struct
which can outlive the libxl__ev_foo. Allocation and deallocation of
these nexi is mostly handled in the OSEVENT macros which wrap up
the application's callbacks.
Document the problem and the solution in a comment in libxl_event.c
just before the definition of struct libxl__osevent_hook_nexus.
There is still a race relating to libxl__osevent_occurred_timeout;
this will be addressed in the following patch.
Reported-by: Bamvor Jian Zhang <bjzhang@suse.com>
Cc: Bamvor Jian Zhang <bjzhang@suse.com>
Cc: Ian Campbell <Ian.Campbell@citrix.com>
Tested-by: Jim Fehlig <jfehlig@suse.com>
Acked-by: Jim Fehlig <jfehlig@suse.com>
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Committed-by: Ian Campbell <ian.campbell@citrix.com>
When deregistering a timeout, the address of application data was
being passed to the timeout_deregister hook instead of the data
itself.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
Committed-by: Ian Campbell <ian.campbell@citrix.com>
This was ommited in the majority of cases. Add as a parameter to
libxl__event_new and the NEW_EVENT wrapper to help prevent it being
forgotten in the future.
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Committed-by: Ian Campbell <ian.campbell@citrix.com>
It was convenient to invent $(CFLAGS_LIBXL) to do this.
Various renamings to avoid shadowing standard functions:
- index(3)
- listen(2)
- link(2)
- abort(3)
- abs(3)
Reduced the scope of some variables to avoid conflicts.
Change to libxc is due to the nested hypercall buf macros in
set_xen_guest_handle (used in libxl) using the same local private vars.
Build tested only.
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Committed-by: Ian Campbell <ian.campbell@citrix.com>
Previously we only initialised it if an explicit keyvar_init_val was
given but not if the default was implicitly 0.
In the generated code this only changes the unused libxl_event_init_type
function:
void libxl_event_init_type(libxl_event *p, libxl_event_type type)
{
+ assert(!p->type);
+ p->type = type;
switch (p->type) {
case LIBXL_EVENT_TYPE_DOMAIN_SHUTDOWN:
break;
However I think it is wrong that this function is unused, this and
libxl_event_init should be used by libxl__event_new. As it happens
both are just memset to zero but for correctness we should use the
init functions (in case the IDL changes).
In the generator we also need to properly handle init_var == 0 which
the current if statements incorrectly treat as False. This doesn't
actually have any impact on the generated code.
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Committed-by: Ian Campbell <ian.campbell@citrix.com>
In afterpoll_internal, the callback functions may register and
deregister events arbitrarily. This means that we need to consider
the reentrancy-safety of the event machinery state variables.
Most of the code is safe but the fd handling is not. Fix this by
arranging to restart the fd scan loop every time we call one of these
callback functions.
For this loop to terminate, we modify afterpoll_check_fd so that it
returns only once for each of afterpoll's efds.
Another possible solution would be simply to return from
afterpoll_internal after calling efd->func. That would be a small and
more obviously correct change but would prevent the process from
handling more than one fd event with a single call to poll.
This is apropos of a report from Roger Pau Monne to me (pers.comm.)
of this crash on NetBSD:
Program terminated with signal 11, Segmentation fault.
#0 0x00007f7ff743131b in afterpoll_check_fd (poller=<optimized out>, fds=0x7f7ff7b241c0, nfds=7, fd=-1, events=1)
at libxl_event.c:856
856 if (fds[slot].fd != fd)
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Reported-by: Roger Pau Monne <roger.pau@citrix.com>
Tested-by: Roger Pau Monne <roger.pau@citrix.com>
Acked-by: Ian Campbell <Ian.Campbell@citrix.com>
Committed-by: Ian Campbell <ian.campbell@citrix.com>
Cedric Bosdonnat
Ian Campbell
Wei Liu
Ian Jackson
Andrew Cooper
Matthew Daley
Jim Fehlig