Consume and ignore all XS_DEBUG packets, and pass the response type back to
the caller of xenbus_recv() so the caller can take appropriate action if an
unexpected reply was received.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
The functions ring_read and ring_write() have logic to try and deal with
partial reads and writes.
However, in all cases where the "while (len)" loop executed twice, data
corruption would occur as the second memcpy() starts from the beginning of
"data" again, rather than from where it got to.
This bug manifested itself as protocol corruption when a reply header crossed
the first wrap of the response ring. However, similar corruption would also
occur if hvmloader observed xenstored performing partial writes of the block
in question, or if hvmloader had to wait for xenstored to make space in either
ring.
Reported-by: Adam Kucia <djexit@o2.pl>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
hvm_info->signature is not a string, but an 64 bit int, and is not
NULL terminated. The use of strncpy to populate it is inappropriate and
potentially misleading. A cursory glance might have you thinking someone
had miscounted the length of the string literal - not realising it was
intentionally cropping of the null termination.
Also, since we wish to initialise all of hvm_info->signature, and
certainly no more, the use of sizeof is safer.
Coverity-ID: 1198710
Signed-off-by: Jennifer Herbert <jennifer.herbert@citrix.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
domain config output goes to
before after
xl create nowhere nowhere
xl create -d stderr stderr
xl -N create stdout stdout
xl -N create -d stderr stdout
It is not sensible that adding -d would cause different output on
stdout. And that -N would produce less debug output is hardly
surprising in general and not really a problem in this case.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
v2: New patch in this version of the mini-series.
Currently, xl -N create produces:
{
"domid": null,
"config": {
"c_info": {
"type": "pv",
[etc]
}
The domid is always NULL (as the domain has not been created at this
stage).
This is annoying if you want to take this output and use it for some
actually useful purpose like domain creation: either it needs to be
massaged, or the the consuming tool needs to be taught to look inside
the json object for the `config' element (which IMO makes no sense as
an interface).
We would like to be able to pass libxl json configs around sensibly.
In the future maybe xl will grow an option to create a domain from a
json config, and this is currently something I want to be able to have
a test tool do.
Note that this change is NOT BACKWARDS COMPATIBLE. But it would only
adversely affects anyone who uses `xl -N create' and then saves and
processes the JSON. (The output from xl list et al is not changed; it
normally needs the domid.) Such a user should probably have already
have complained about the infelicitous output. If they haven't it
would be simple enough for them to bookend the output so as to provide
compatible output.
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: Euan Harris <euan.harris@citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
v2: Print json output to correct filehandle
(Using newly introduced flush_stream.)
We are going to want to reuse this. Adjust the code slightly to
detect right away call sites that pass something other than stdout or
stderr.
No resulting functional change.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
v2: New patch in this version of the mini-series
According to ARM CPUs bindings, the reg field should match the MPIDR's
affinity bits. We will use AFF0 and AFF1 when constructing the reg value
of the guest at the moment, for it is enough for the current max vcpu
number.
Signed-off-by: Chen Baozi <baozich@gmail.com>
Reviewed-by: Julien Grall <julien.grall@citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
so that, if the same config is reused later, the following
two (good) things happen:
- we do not trip over warnings because node and/or vcpu
soft affinity now exist (as a consequence of the
successful placement), but numa_placement is still
true;
- we end up always using the results of the original
execution of the placement algorithm, rather than
re-running it at each re-use of the same config,
which is what most users expects and wants.
This fixes the bug reported here:
http://lists.xenproject.org/archives/html/xen-devel/2015-06/msg04454.html
Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
instead than errors. More specifically, in libxl,
b_info->numa_autoplacement is meant as a way to
disable automatic NUMA placement, if one does not
want it to happen. It is, however, useful for
consistency checking as well, i.e., to ensure that
the user provided configuration (such as, for instance,
vcpu hard or soft affinity) and NUMA placement itself
will not clash.
However, right now, if such a clash happens we abort
domain creation and error out, which is too much! It
is, in fact, enough to infom the user/caller that NUMA
placement won't be performed, with a WARN, and that's
what this commit does.
Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Fix example code in comment. libxl_event_check() can return
ERROR_NOT_READY; LIBXL_NOT_READY does not exist.
Signed-off-by: Euan Harris <euan.harris@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
c3c8da9 "libxl: ao: datacopier callback gets an rc" caused
libxl__domain_save_device_model() to pass its rc directly into the
callback.
However in the preexisting code, there were 3 "goto out;" paths which
left rc uninitialised. This causes a build failure with GCC 4.8's
-Wmaybe-uninitialized.
Set the rc explicitly on each goto out path.
Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Tested-by: Andrew Cooper <andrew.cooper3@citrix.com>
If we already have an rc (eg from ao abort), keep it.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
---
v4: New patch in this version of the series.
The callbacks actually ignore this except for logging, but we should
log the correct pid.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
---
v4: New patch in this version of the series.
Otherwise the debug log is strangely devoid of an explanation for the
spawn completing.
We decorate `what', as otherwise the logged message is rather alarming
(especially if the death is due to us sending SIGKILL, which even
happens on the success path).
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
---
v4: New patch in this version of the series.
Make it log something if the xenstore path does not exist.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
---
v4: New patch in this version of the series.
libxl__datacopier_* can now actually generate a callback with
rc==ABORTED.
This provides abort support during some corner cases, including (at
least) copying the device model data during the end of domain save.
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.
libxl__datacopier_* now provides its caller's callback function with
an rc value. This relieves the caller of the need to figure out an
appropriate rc value.
Arrange that the `other internal failure' cases now get a valid
positive errno value (EIO).
In a few places, assert that errno is nonzero before passing it to our
caller.
Extend the datacopier callback API to permit the dc to signal ABORTED.
(It doesn't actually do this yet, though.)
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
CC: Andrew Cooper <andrew.cooper3@citrix.com>
CC: David Vrabel <david.vrabel@citrix.com>
---
v2: New in this version of the series.
Register the the save/restore helper interface with the abort
machinery. When we are informed that save/restore should be aborted,
we make a note of the that in our rc variable, and send the helper a
SIGTERM. It will die in due course.
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.
During startup of the save/restore helper, set the disposition of
SIGTERM appropriately.
For restore, we can simply die immediately - there is no point trying
to do any kind of cleanup on what is now going to be a trashed domain.
For save, we want to arrange that libxc's cleanup code (eg turning off
logdirty) takes place. So our signal handler replaces the fd with one
on which writes will fail, causing libxc's own loop to fail next time
it actually tries to do a write.
Currently this has only a minor beneficial effect: we don't send the
helper a SIGTERM ourselves, and if someone else contrives to send our
helper a SIGTERM they have probably sent one to libxl too in which
case things are going to be a bit messy anyway.
But in the next patch libxl itself is going to use SIGTERM to
implement ao abort requests.
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.
Two unrelated non-functional changes, broken out into a pre-patch for
easier review:
Break out a function sendsig() in libxl_save_callout.c.
Move io_fd to be a global variable in libxl_save_helper.c.
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.
FILLZERO is a macro for memset(&foo,0,sizeof(foo)). It eliminates the
possiblity to make the error memset(&foo,0,sizeof(&foo)).
No callers yet, but document it in CODING_STYLE. (In accordance with
existing libxl policy, I haven't gone through all existing possible
call sites.)
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 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.
This is currently reported only by the bootloader code, if the domain
is destroyed while the bootloader is running.
In the future it would be nice to return it for other circumstances
where the domain existed when the operation started but subsequently
vanished.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
v4: Add a comment about the new error code's semantics
v2: New in this version of the series.
In practice, aborting this task will cause all subsequent actual
backend operations to fail, but will not actually cause the
libxl_device_events_handler operation to complete.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Roger Pau Monné <roger.pau@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.
If we aborted the domain creation, do not try to tear it down again
Document this.
This is a backwards-compatible API change since old libxl users will
never abort any operations.
In the current code, there is no functional change, because
ERROR_ABORTED is never generated anywhere yet.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
v4: ABORTED not CANCELLED.
Always write out guest domid on completion.
Do not trash rc with libxl__set_domain_configuration on preserve path.
Make spawn provide an rc to its caller, and either pass it through
from the timeout callback, or invent ERROR_FAIL, as applicable.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
---
v4: New patch in this version of the series.
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.
The internal user of libxl__async_exec_start et al now gets an rc as
well as the process's exit status.
For now this is always either 0 or ERROR_FAIL, but with ao
abort requests this will possibly be ABORTED or TIMEDOUT too.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
v4: Improve doc comment as suggested by Ian C.
v2: New patch due to rebause; v1 had changes to device_hotplug_*
scripts instead.
Callback now gets unambiguous information about error situation:
previously, if only thing that went wrong was that child died
badly, rc would be FAILED, which was unambigously; now rc=0.
Add a comment document the meaning of the rc and status parameters
to the callback.
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 introduce ERROR_ABORTED now, so that we can write code to handle
it, and decreee that functions might return it, even though currently
there is nowhere where this error is generated.
While we're here, provide ERROR_NOTFOUND which will also be used
later, but only as part of the public API.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
---
v4: CANCELLED renamed to ABORTED.
No longer introduce ERROR_NOTIMPLEMENTED.
v2: Rebase means new errors have bigger (more negative) numbers.
We are going to introduce application-requested aborts of (ao)
operations, but these suspend failures are something different.
Reword to avoid confusion.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
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>
Replace the separate timeout and xenstore watch with use of
libxl__xswait*.
Different control flow, but no ultimate functional change apart from
slight changes to the text of error messages.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
If a suspend callback fails, it has a libxl error code in its hand.
However we must return to libxc the values that libxc expects. So we
stash the libxl error code in dss->rc and fish it out again after
libxc returns from the suspend call.
While we're here, abolish the now-redundant `ok' variable in
remus_devices_postsuspend_cb.
The overall functional change is that libxl_domain_save now completes
with the correct error code as determined when the underlying failure
happened. (Usually this is, still, ERROR_FAIL.)
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
v2: Add cleanup in remus_devices_postsuspend_cb.
Change the following functions to take a libxl error code rather than
a boolean "ok" value, and translate that value to the boolean expected
by libxc at the last moment:
domain_suspend_callback_common_done } dss->callback_common_done
remus_domain_suspend_callback_common_done }
domain_suspend_common_done
Also, abolish domain_suspend_common_failed as
domain_suspend_common_done can easily do its job and the call sites
now have to supply the right rc value anyway.
In domain_suspend_common_guest_suspended, change "ret" to "rc"
as it contains a libxl error code.
There is no functional change in this patch: the proper rc value now
propagates further, but is still eventually smashed to a boolean.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
v2: Fix a leftover comment referring to domain_suspend_common_failed
switch_logdirty_done used to take the value to pass to
libxl__xc_domain_saverestore_async_callback_done (ie, the return value
from the callback). (This was mistakenly described as "ok" in the
prototype, but in the definition it is "broke" and all the call sites
passed 0 for success or -1 for error.)
Instead, make it take a libxl error code (rc). Convert this to the
suspend callback value at the end.
No functional change in this patch.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
Andrew Cooper
Jennifer Herbert
Ian Jackson
Chen Baozi
Dario Faggioli
Euan Harris
Tiejun Chen
Wen Congyang