Today the CODING_STYLE contains a section "Handling unexpected
conditions" specific to the hypervisor. This section is kind of
misplaced for a coding style. It should rather be part of a "Coding
best practices" guide.
Add such a guide as docs/process/coding-best-practices.pandoc and
move the mentioned section from CODING_STYLE to the new file, while
converting the format to pandoc.
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Julien Grall <jgrall@amazon.com>
When running as stubdom, map the stubdom's Xenstore ring page in order
to support using the 9pfs frontend.
Use the same pattern as in dom0_init() when running as daemon in dom0
(introduce the own domain, then send an event to the client side to
signal Xenstore is ready to communicate).
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Jason Andryuk <jandryuk@gmail.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
Today domain_init() is called either just before calling dom0_init()
in case no live update is being performed, or it is called after
reading the global state from read_state_global(), as the event
channel fd is needed.
Split up domain_init() into a preparation part which can be called
unconditionally, and in a part setting up the event channel handle.
Note that there is no chance that chk_domain_generation() can be
called now before xc_handle has been setup, so there is no need for
the related special case anymore.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Jason Andryuk <jandryuk@gmail.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
When [un]mapping the ring page of a Xenstore client, different actions
are required for "normal" guests and dom0. Today this distinction is
made at call site.
Move this distinction into [un]map_interface() instead, avoiding code
duplication and preparing special handling for [un]mapping the stub
domain's ring page.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Jason Andryuk <jandryuk@gmail.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
All of the socket handling is needed only when running as daemon.
Move it into posix.c, allowing to remove the NO_SOCKETS macro.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
All of the log-pipe handling is needed only when running as daemon.
Move it into posix.c. This requires to have a service function in the
main event loop for handling the related requests and one for setting
the fds[] array, which is renamed to poll_fds to have a more specific
name. Use a generic name for those functions, as socket handling can
be added to them later, too.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
Move systemd handling to a new late_init() function in posix.c.
This prepares a future removal of the NO_SOCKETS macro.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Jason Andryuk <jandryuk@gmail.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
Some xenstored initialization needs to be done in the daemon case only,
so split it out into a new early_init() function being a stub in the
stubdom case.
Remove the call of talloc_enable_leak_report_full(), as it serves no
real purpose: the daemon only ever exits due to a crash, in which case
a log of talloc()ed memory hardly has any value.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Jason Andryuk <jandryuk@gmail.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
Rename the xenbus_evtchn() function to get_xenbus_evtchn() in order to
avoid two externally visible symbols with the same name when Xenstore-
stubdom is being built with a Mini-OS with CONFIG_XENBUS set.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Jason Andryuk <jandryuk@gmail.com>
In order to prepare support of PV frontends in xenstore-stubdom, add
allocation of a Xenstore event channel to init-xenstore-domain.c.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Jason Andryuk <jandryuk@gmail.com>
Add some optional additional backend paths for 9pfs PV devices. Those
paths will be supported by the new xen-9pfsd 9pfs backend.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Jason Andryuk <jandryuk@gmail.com>
With include/xen/unaligned.h now dealing properly with unaligned
accesses for all architectures, asm/unaligned.h can be removed and
users can be switched to include xen/unaligned.h instead.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Instead of defining get_unaligned() and put_unaligned() in a way that
is only supporting architectures allowing unaligned accesses, use the
same approach as the Linux kernel and let the compiler do the
decision how to generate the code for probably unaligned data accesses.
Update include/xen/unaligned.h from include/asm-generic/unaligned.h of
the Linux kernel.
The generated code has been checked to be the same on x86.
Modify the Linux variant to not use underscore prefixed identifiers,
avoid unneeded parentheses and drop the 24-bit accessors.
Add the definition of __packed to xg_dom_decompress_unsafe_zstd.c in
libxenguest as it is using a cruel hack to reuse the hypervisor's
decompressing code for stubdom.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Origin: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 803f4e1eab7a
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
When adding locking to tools/libs/evtchn/minios.c a semaphore was
used. This can result in deadlocks, as the lock is taken inside the
event handler, which can interrupt an already locked region.
The fix is rather simple, as Mini-OS is supporting a single vcpu
only. So instead of the semaphore it is enough to disable interrupts
when operating on the port list.
Fixes: bc4fe94a69 ("ools/libs/evtchn: replace assert()s in stubdom with proper locking")
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Anthony PERARD <anthony.perard@citrix.com>
I've been the main contributor to Mini-OS since several years now.
Add me as a maintainer.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
Use named member initializers instead of positional ones for the macros
used to initialize structures.
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
With some small adjustments to the LOCK_PROFILE_* macros some #ifdefs
can be dropped from spinlock.c.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Alejandro Vallejo <alejandro.vallejo@cloud.com>
Acked-by: Julien Grall <jgrall@amazon.com>
With include/xen/unaligned.h now dealing properly with unaligned
accesses for all architectures, asm/unaligned.h can be removed and
users can be switched to include xen/unaligned.h instead.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Instead of defining get_unaligned() and put_unaligned() in a way that
is only supporting architectures allowing unaligned accesses, use the
same approach as the Linux kernel and let the compiler do the
decision how to generate the code for probably unaligned data accesses.
Update include/xen/unaligned.h from include/asm-generic/unaligned.h of
the Linux kernel.
The generated code has been checked to be the same on x86.
Modify the Linux variant to not use underscore prefixed identifiers,
avoid unneeded parentheses and drop the 24-bit accessors.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Origin: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 803f4e1eab7a
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
As the hypervisor is disabling unaligned accesses for Arm32, set the
-mno-unaligned-access compiler option for building. This will prohibit
unaligned accesses when e.g. accessing 2- or 4-byte data items in
packed data structures.
Backport: 4.15+
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Julien Grall <jgrall@amazon.com>
Today write_messages() will call trace_io() after having written the
complete message to the ring buffer or socket.
In case the message can't be written in one go, split it by writing
one trace entry when starting the write and one when finishing it.
In order to distinguish a complete OUT message from a split one, let the
caller of trace_io specify the prefix string ("IN", "OUT", "OUT(START)",
"OUT(END)") directly instead via an int.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
In tools/libs/evtchn/minios.c there are assert()s for the current
thread being the main thread when binding an event channel.
As Mini-OS is supporting multiple threads, there is no real reason
why the binding shouldn't be allowed to happen in any other thread.
Drop the assert()s and replace them with proper locking of the
port_list.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Jason Andryuk <jandryuk@gmail.com>
Do some minor cleanups:
- Move setting of old_domdata and old_units next to each other
- Drop incrementing unit_idx in the final loop of sched_move_domain()
as it isn't used afterwards
- Rename new_p to new_cpu and unit_p to unit_cpu
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: George Dunlap <george.dunlap@cloud.com>
Do cleanup in sched_move_domain() in a dedicated service function,
which is called either in error case with newly allocated data, or in
success case with the old data to be freed.
This will at once fix some subtle bugs which sneaked in due to
forgetting to overwrite some pointers in the error case.
Fixes: 70fadc4163 ("xen/cpupool: support moving domain between cpupools with different granularity")
Reported-by: René Winther Højgaard <renewin@proton.me>
Initial-fix-by: Jan Beulich <jbeulich@suse.com>
Initial-fix-by: George Dunlap <george.dunlap@cloud.com>
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Acked-by: George Dunlap <george.dunlap@cloud.com>
The "-R" (no recovery) command line option enables to omit fixing the
node store in case of detected inconsistencies.
This might have been of interest in the past, when the node data base
was kept in a file, but now the usability of this option is zero.
Remove the "-R" option.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
The "-P" command line option just results in printing the PID of the
xenstored daemon to stdout before stdout is being closed. The same
information can be retrieved from the PID file via the "-F" option.
Remove the redundant "-P" option.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
The "-V" (verbose) command line option is nearly completely redundant
with "io" tracing. Just the time of the printed data is a little bit
different, while the tracing is more informative.
Remove the verbose option.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
Remove the "-D" command parameter, which is disabling initialization of
the mandatory domain data handling.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
Trying to add an offline cpu to a cpupool can crash the hypervisor,
as the probably non-existing percpu area of the cpu is accessed before
the availability of the cpu is being tested. This can happen in case
the cpupool's granularity is "core" or "socket".
Fix that by testing the cpu to be online.
Fixes: cb563d7665 ("xen/sched: support core scheduling for moving cpus to/from cpupools")
Reported-by: René Winther Højgaard <renewin@proton.me>
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Flexible arrays in public headers can be problematic with some
compilers.
With XEN_FLEX_ARRAY_DIM there is a mechanism available to deal with
this issue, but care must be taken to not change the affected structs
in an incompatible way.
So bump __XEN_LATEST_INTERFACE_VERSION__ and introduce a new macro
XENPV_FLEX_ARRAY_DIM which will be XENPV_FLEX_ARRAY_DIM with the
interface version being new enough and "1" (the value used today in
the affected headers) when the interface version is an old one.
Replace the arr[1] instances (this includes the ones seen to be
problematic in recent Linux kernels [1]) with arr[XENPV_FLEX_ARRAY_DIM]
in order to avoid compilation errors.
[1]: https://bugzilla.kernel.org/show_bug.cgi?id=217693
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Henry Wang <Henry.Wang@arm.com> # CHANGELOG
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Julien Grall <jgrall@amazon.com>
Add a local .gitignore file for tools/python.
As at least on some systems (e.g. OpenSUSE Leap 15.5) the build will
produce a tools/python/xen.egg-info directory, add it to the new
.gitignore file, too.
Use "/dir/" as pattern for ignoring a directory and its contents, as
the "/dir/*" pattern won't ignore the directory itself (git just
doesn't complain when seeing a directory without any not ignored file
in it).
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Add a local .gitignore file for tools/pygrub.
As at least on some systems (e.g. OpenSUSE Leap 15.5) the build will
produce a tools/pygrub/pygrub.egg-info directory, add it to the new
.gitignore file, too.
Use "/dir/" as pattern for ignoring a directory and its contents, as
the "/dir/*" pattern won't ignore the directory itself (git just
doesn't complain when seeing a directory without any not ignored file
in it).
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Update the used commit for Mini-OS in order to be able to use the
9pfs frontend.
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
In order to build caml-stubdom, it must be explicitly enabled via
"configure --enable-caml-stubdom". The build process is failing due to
stubdom/ocaml.patch failing to apply. Since the patched file has been
modified in 2014 the last time, it seems nobody cares for caml-stubdom
since at least then.
Remove caml-stubdom from the build system.
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Christian Lindig <christian.lindig@cloud.com>
Acked-by: Henry Wang <Henry.Wang@arm.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
The version in SUPPORT.md is still "4.18-rc".
Change it to "4.19-unstable".
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Fix some coding style issues in spinlock coding:
- Instead of u64 etc. use the preferred uint64_t etc. types
- Remove some trailing spaces
- Add missing spaces in "if" statements
- Add some missing blank lines
While modifying the line, add const for the observed_head() parameter.
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
When moving a domain out of a cpupool running with the credit2
scheduler and having multiple run-queues, the following ASSERT() can
be observed:
(XEN) Xen call trace:
(XEN) [<ffff82d04023a700>] R credit2.c#csched2_unit_remove+0xe3/0xe7
(XEN) [<ffff82d040246adb>] S sched_move_domain+0x2f3/0x5b1
(XEN) [<ffff82d040234cf7>] S cpupool.c#cpupool_move_domain_locked+0x1d/0x3b
(XEN) [<ffff82d040236025>] S cpupool_move_domain+0x24/0x35
(XEN) [<ffff82d040206513>] S domain_kill+0xa5/0x116
(XEN) [<ffff82d040232b12>] S do_domctl+0xe5f/0x1951
(XEN) [<ffff82d0402276ba>] S timer.c#timer_lock+0x69/0x143
(XEN) [<ffff82d0402dc71b>] S pv_hypercall+0x44e/0x4a9
(XEN) [<ffff82d0402012b7>] S lstar_enter+0x137/0x140
(XEN)
(XEN)
(XEN) ****************************************
(XEN) Panic on CPU 1:
(XEN) Assertion 'svc->rqd == c2rqd(sched_unit_master(unit))' failed at common/sched/credit2.c:1159
(XEN) ****************************************
This is happening as sched_move_domain() is setting a different cpu
for a scheduling unit without telling the scheduler. When this unit is
removed from the scheduler, the ASSERT() will trigger.
In non-debug builds the result is usually a clobbered pointer, leading
to another crash a short time later.
Fix that by swapping the two involved actions (setting another cpu and
removing the unit from the scheduler).
Link: https://github.com/Dasharo/dasharo-issues/issues/488
Fixes: 70fadc4163 ("xen/cpupool: support moving domain between cpupools with different granularity")
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: George Dunlap <george.dunlap@cloud.com>
The extras directory is used only as a download target for Mini-OS
sources. Instead of special handling extras/mini-os* in .gitignore and
the clean targets, just use extras for that purpose.
So add "extras" to .gitignore and remove it when doing a
"make distclean".
Signed-off-by: Juergen Gross <jgross@suse.com>
In tools/xenstore there are living xenstored and xenstore clients.
They are no longer sharing anything apart from the "xenstore" in their
names.
Move the xenstored sources into a new directory tools/xenstored while
dropping the "xenstored_" prefix from their names. This will make it
clearer that xenstore clients and xenstored are independent from each
other.
In order to avoid two very similar named directories below tools,
rename tools/xenstore to tools/xs-clients.
Drop the make targets [un]install-clients as those are not used in
the Xen tree.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
Acked-by: Anthony PERARD <anthony.perard@citrix.com>
When introducing polarssl into stubdom building the clean targets of
stubdom/Makefile gained actions for removing openssl directories and
files additional to polarssl ones.
As those openssl files are never downloaded or created during build,
the related actions can be dropped.
Fixes: bdd516dc6b ("vtpm/vtpmmgr and required libs to stubdom/Makefile")
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>
Instead of adding the definition of XEN_RUN_STORED to CFLAGS in
multiple Makefiles, let configure add it to tools/config.h instead.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>
Add a variant of get_node() returning a const struct node pointer.
Note that all callers of this new variant don't supply a pointer where
to store the canonical node name, while all callers needing a non-const
node do supply this pointer. This results in an asymmetric
simplification of the two variants.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
Today get_node_canonicalized() is the only caller of get_node().
In order to prepare introducing a get_node() variant returning a
pointer to const struct node, do the following restructuring:
- move the call of read_node() from get_node() into
get_node_canonicalized()
- rename get_node() to get_node_chk_perm()
- rename get_node_canonicalized() to get_node()
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
Today is_valid_nodename() is always called directly after calling
canonicalize(), with the exception of do_unwatch(), where the call
is missing (which is not correct, but results just in a wrong error
reason being returned).
Merge is_valid_nodename() into canonicalize().
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
Add a "allow_special" parameter to get_node_canonicalized() allowing
to merge get_spec_node() into get_node_canonicalized().
Add the same parameter to is_valid_nodename(), as this will simplify
check_watch_path().
This is done in preparation to introducing a get_node() variant
returning a pointer to const struct node.
Note that this will change how special node names are going to be
validated, as now the normal restrictions for node names will be
applied:
- they can't end with "/"
- they can't contain "//"
- they can't contain characters other than the ones allowed for normal
nodes
- the length of the node name is restricted by the max path length
quota
For defined special node names this isn't any real restriction, though.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
Introduce a read_node() variant returning a pointer to const struct
node, which doesn't do a copy of the node data after retrieval from
the data base.
Call this variant where appropriate.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
In order to avoid modifying the node data in the data base in case a
domain is gone, let domain_adjust_node_perms() allocate new memory for
the permissions in case they need to be modified. As this should
happen only in very rare cases, it is fine to do this even when having
copied the node data already.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
Replace some individual fields in struct node with struct node_hdr.
This allows to add a helper for calculating the accounted memory size
of a node.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
Open code struct node_perms in struct node in order to prepare using
struct node_hdr in struct node.
Add two helpers to transfer permissions between struct node and struct
node_perms and a helper to directly get connection base permissions
from a node.
Let perms_to_strings() take a struct node as parameter and rename it
to node_perms_to_strings().
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
Struct xs_tdb_record_hdr is used for nodes stored in the data base.
When working on a node, struct node is being used, which is including
the same information as struct xs_tdb_record_hdr, but in a different
format. Rework struct xs_tdb_record_hdr in order to prepare including
it in struct node.
Do the following modifications:
- move its definition to xenstored_core.h, as the reason to put it into
utils.h are no longer existing
- rename it to struct node_hdr, as the "tdb" in its name has only
historical reasons
- replace the empty permission array at the end with a comment about
the layout of data in the data base (concatenation of header,
permissions, node contents, and children list)
- use narrower types for num_perms and datalen, as those are naturally
limited to XENSTORE_PAYLOAD_MAX (childlen is different here, as it is
in theory basically unlimited)
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
Today the node data is copied in db_fetch() on each data base read in
order to avoid accidental data base modifications when working on a
node.
read_node() is the only caller of db_fetch() which isn't freeing the
returned data area immediately after using it. The other callers don't
modify the returned data, so they don't need the data to be copied.
Move copying of the data into read_node(), resulting in a speedup of
the other callers due to no memory allocation and no copying being
needed anymore.
This allows to let db_fetch() return a pointer to const data.
As db_fetch() can't return any error other than ENOENT now, error
handling for the callers can be simplified.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
With talloc_free() and related functions not taking a pointer to const
it is tedious to use the const attribute for talloc()-ed memory in
many cases.
Change the related prototypes to use "const void *" instead of
"void *".
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
db_delete() only ever is returning 0. Switch it to return void and
remove all the error handling dealing wit a non-zero return value.
Suggested-by: Julien Grall <julien@xen.org>
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
Today all Xenstore nodes are stored in a TDB data base. This data base
has several disadvantages:
- It is using a fixed sized hash table, resulting in high memory
overhead for small installations with only very few VMs, and a rather
large performance hit for systems with lots of VMs due to many
collisions.
The hash table size today is 7919 entries. This means that e.g. in
case of a simple desktop use case with 2 or 3 VMs probably far less
than 10% of the entries will be used (assuming roughly 100 nodes per
VM). OTOH a setup on a large server with 500 VMs would result in
heavy conflicts in the hash list with 5-10 nodes per hash table entry.
- TDB is using a single large memory area for storing the nodes. It
only ever increases this area and will never shrink it afterwards.
This will result in more memory usage than necessary after a peak of
Xenstore usage.
- Xenstore is only single-threaded, while TDB is designed to be fit
for multi-threaded use cases, resulting in much higher code
complexity than needed.
- Special use cases of Xenstore are not possible to implement with TDB
in an effective way, while an implementation of a data base tailored
for Xenstore could simplify some handling (e.g. transactions) a lot.
So drop using TDB and store the nodes directly in memory making them
easily accessible. Use a hash-based lookup mechanism for fast lookup
of nodes by their full path.
For now only replace TDB keeping the current access functions.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
For an effective way to replace a hashtable entry add a new function
hashtable_replace().
This is in preparation to replace TDB with a more simple data storage.
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Julien Grall <jgrall@amazon.com>
Silently adding another entry with the same key to a hashtable is a
perfect receipt for later failure with hard to diagnose symptoms.
Let hashtable_add() fail in case another entry with the same key is
already existing.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
The key is never modified by hashtable code, so it should be marked as
const.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
In case Xen has been configured with "--disable-pygrub", don't accept
the domain config option "bootloader=pygrub".
Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Anthony PERARD <anthony.perard@citrix.com>
The only in-tree user of libfsimage is pygrub. Now that it is possible
to disable the build of pygrub, the same should be possible for
libfsimage.
Add an option for controlling the build of libfsimage. The default is
on if pygrub is being built, and off if it isn't. Without pygrub the
build of libfsimage can be enabled via --enable-libfsimage.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>
Add a "--disable-pygrub" option for being able to disable the build
and installation of pygrub.
There are two main reasons to do so:
- A main reason to use pygrub is to allow a PV guest to choose its
bitness (32- or 64-bit). Pygrub allows that by looking into the boot
image and to start the guest in the correct mode depending on the
kernel selected. With 32-bit PV guests being deprecated and the
possibility to even build a hypervisor without 32-bit PV support,
this use case is gone for at least some configurations.
- Pygrub is running in dom0 with root privileges. As it is operating
on guest controlled data (the boot image) and taking decisions based
on this data, there is a higher security risk. Not being possible
to use pygrub is thus a step towards a reduction of attack surface.
Default is still to build and install pygrub.
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Anthony PERARD <anthony.perard@citrix.com>
Add a wrapper function for tdb_fetch taking the name of the node in
the data base as a parameter. Let it return a data pointer and the
length of the data via a length pointer provided as additional
parameter.
Move logging of the TDB access from the callers into the wrapper.
This enables to make set_tdb_key() and tdb_ctx static.
This is in preparation to replace TDB with a more simple data storage.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
Eliminate further TDB_DATA usage by switching get_acc_data() and
get_acc_domid() from a TDB key to the name of the node in the data base
as a parameter.
This is in preparation to replace TDB with a more simple data storage.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
Rename do_tdb_write() to db_write() and replace the key parameter with
db_name specifying the name of the node in the data base, and the data
parameter with a data pointer and a length.
Do the same key parameter type change for write_node_raw(), too.
This is in preparation to replace TDB with a more simple data storage.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
Rename do_tdb_delete() to db_delete() and replace the key parameter
with db_name specifying the name of the node in the data base.
This is in preparation to replace TDB with a more simple data storage.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
Instead of setting the TDB key for accessing the node in the data base,
let transaction_prepend() return the associated name instead.
This is in preparation to replace TDB with a more simple data storage.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
Instead of storing the TDB key in struct node, only store the name of
the node used to access it in the data base.
Associated with that change replace the key parameter of access_node()
with the equivalent db_name.
This is in preparation to replace TDB with a more simple data storage.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
Instead of using TDB_REPLACE for either creating or modifying a TDB
entry, use either TDB_INSERT or TDB_MODIFY when calling tdb_store().
At higher function levels use the abstract mode values NODE_CREATE
and NODE_MODIFY.
This is for preparing to get rid of TDB, even if it is beneficial
while using TDB, too.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
In case get_spec_node() is being called for a special node starting
with '@' it won't set *canonical_name. This can result in a crash of
xenstored due to dereferencing the uninitialized name in
fire_watches().
This is no security issue as it requires either a privileged caller or
ownership of the special node in question by an unprivileged caller
(which is questionable, as this would make the owner privileged in some
way).
Fixes: d6bb63924f ("tools/xenstore: introduce dummy nodes for special watch paths")
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
The return type of canonicalize() can be modified to const char *.
This avoids the need to cast the const away from the input parameter.
There need to be quite some other functions modified to take const
parameters in order to avoid further casts.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
The fix for XSA-417 had a bug: domain_alloc_permrefs() will not return
a negative value in case of an error, but a plain errno value.
Note this is not considered to be a security issue, as the only case
where domain_alloc_permrefs() will return an error is a failed memory
allocation. As a guest should not be able to drive Xenstore out of
memory, this is NOT a problem a guest can trigger at will.
Fixes: ab12821822 ("tools/xenstore: fix checking node permissions")
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Julien Grall <jgrall@amazon.com>
Remove the hlist defines/functions and the rcu related functions from
tools/xenstore/list.h, as they are not used.
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Julien Grall <jgrall@amazon.com>
Move the rest of live update related code from xenstored_control.c to
a dedicated new source file.
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Julien Grall <jgrall@amazon.com>
Instead of using #ifdef in xenstored_control.c split out the code of
environment specific functions (daemon or Mini-OS) to dedicated source
files.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
Instead of including the same small C source in multiple binaries from
2 source directories, use a header file with inline functions as a
replacement.
As some of the functions are exported by libxenstore, rename the inline
functions from xs_*() do xenstore_*() and add xs_*() wrappers to
libxenstore.
With that no sources required to build libxenstore are left in
tools/xenstore, so the file COPYING can be removed now.
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Julien Grall <jgrall@amazon.com>
xs_daemon_tdb() in xs_lib.c is no longer used at all, so it can be
removed. xs_domain_dev() and xs_write_all() are not used by xenstored,
so they can be moved to tools/libs/store/xs.c.
xs_daemon_rootdir() is used by xenstored only and it only calls
xs_daemon_rundir(), so replace its use cases with xs_daemon_rundir()
and remove it from xs_lib.c.
xs_daemon_socket_ro() is needed in libxenstore only, so move it to
tools/libs/store/xs.c.
Move functions used by xenstore-client only to xenstore_client.c.
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Julien Grall <jgrall@amazon.com>
There is no real need for including tools/xenstore/utils.h from
libxenstore, as only streq() and ARRAY_SIZE() are obtained via that
header.
streq() is just !strcmp(), and ARRAY_SIZE() is brought in via
xen-tools/common-macros.h.
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Julien Grall <jgrall@amazon.com>
Replace the usage of the xenstore private list.h header with the
common xen_list.h one.
Use the XEN_TAILQ type list, as it allows to directly swap the
related macros/functions without having to change the logic.
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Julien Grall <jgrall@amazon.com>
In order to prepare the replacement of TDB with direct accessible nodes
in memory, remove the support for a file backed data base.
This allows to remove xs_tdb_dump, too.
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Julien Grall <jgrall@amazon.com>
The TODO file is not really helpful any longer. It contains only
entries which no longer apply or it is unknown what they are meant
for ("Dynamic/supply nodes", "Remove assumption that rename doesn't
fail").
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Julien Grall <jgrall@amazon.com>
There is no need to keep struct wrl_timestampt, as it serves the same
purpose as the more simple time base provided by get_now().
Move some more stuff from xenstored_domain.h into xenstored_domain.c
as it is being used nowhere else.
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Julien Grall <jgrall@amazon.com>
Some wrl_*() functions are only used in xenstored_domain.c, so make
them static. In order to avoid the need of forward declarations, move
the whole function block to the start of the file.
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Julien Grall <jgrall@amazon.com>
Today hashtable_insert() returns 0 in case of an error. Change that to
let it return an errno value in the error case and 0 in case of success.
In order to avoid any missed return value checks or related future
backport errors, rename hashtable_insert() to hashtable_add().
Even if not used today, do the same switch for the return value of
hashtable_expand().
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
The minsize parameter of create_hashtable() doesn't have any real use
case for Xenstore, so drop it.
For better talloc_report_full() diagnostic output add a name parameter
to create_hashtable().
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
Do the following cleanups:
- hashtable_count() isn't used at all, so remove it
- replace prime_table_length and max_load_factor with macros
- make hash() static
- add a loadlimit() helper function
- remove the /***/ lines between functions
- do some style corrections
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
Instead of having individual quota variables switch to a table based
approach like the generic accounting. Include all the related data in
the same table and add accessor functions.
This enables to use the command line --quota parameter for setting all
possible quota values, keeping the previous parameters for
compatibility.
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Julien Grall <jgrall@amazon.com>
Let get_optval_int() return an unsigned value and rename it
accordingly.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
The maxrequests, node size, number of node permissions, and path length
quota are a little bit special, as they are either active in
transactions only (maxrequests), or they are just per item instead of
count values. Nevertheless being able to know the maximum number of
those quota related values per domain would be beneficial, so add them
to the generic accounting.
The per domain value will never show current numbers other than zero,
but the maximum number seen can be gathered the same way as the number
of nodes during a transaction.
To be able to use the const qualifier for a new function switch
domain_is_unprivileged() to take a const pointer, too.
For printing the quota/max values, adapt the print format string to
the longest quota name (now 17 characters long).
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Julien Grall <jgrall@amazon.com>
Add saving the maximum values of the different accounting data seen
per domain and (for unprivileged domains) globally, and print those
values via the xenstore-control quota command. Add a sub-command for
resetting the global maximum values seen.
This should help for a decision how to set the related quotas.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
As transaction accounting is active for unprivileged domains only, it
can easily be added to the generic per-domain accounting.
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Julien Grall <jgrall@amazon.com>
Add a new trace switch "tdb" and the related trace calls.
The "tdb" switch is off per default.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
Add a new trace switch "acc" and the related trace calls.
The "acc" switch is off per default.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
Add the accounting of per-domain usage of Xenstore memory, watches, and
outstanding requests to the array based mechanism.
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Julien Grall <jgrall@amazon.com>
In order to enable switching memory accounting to the generic array
based accounting, add the current connection to the parameters of
domain_memory_add().
This requires to add the connection to some other functions, too.
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Julien Grall <jgrall@amazon.com>
Juergen Gross