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>
Add the node accounting to the accounting information buffering in
order to avoid having to undo it in case of failure.
This requires to call domain_nbentry_dec() before any changes to the
data base, as it can return an error now.
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Julien Grall <jgrall@amazon.com>
Instead of modifying accounting data and undo those modifications in
case of an error during further processing, add a framework for
collecting the needed changes and commit them only when the whole
operation has succeeded.
This scheme can reuse large parts of the per transaction accounting.
The changed_domain handling can be reused, but the array size of the
accounting data should be possible to be different for both use cases.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
Introduce the scheme of an accounting data array for per-domain
accounting data and use it initially for the number of nodes owned by
a domain.
Make the accounting data type to be unsigned int, as no data is allowed
to be negative at any time.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
In order to prepare keeping accounting data in an array instead of
using independent fields, switch the struct changed_domain accounting
data to that scheme, for now only using an array with one element.
In order to be able to extend this scheme add the needed indexing enum
to xenstored_domain.h.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
The accounting for the number of nodes of a domain in an active
transaction is not working correctly, as it is checking the node quota
only against the number of nodes outside the transaction.
This can result in the transaction finally failing, as node quota is
checked at the end of the transaction again.
On the other hand even in a transaction deleting many nodes, new nodes
might not be creatable, in case the node quota was already reached at
the start of the transaction.
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Julien Grall <jgrall@amazon.com>
There is no need to have the Xenstore headers listed in the LIBS
section now that they have been added to the XENSTORE section.
Suggested-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Anthony PERARD <anthony.perard@citrix.com>
In xen/include/public/io/9pfs.h the name of the Xenstore backend node
"security-model" should be "security_model", as this is how the Xen
tools are creating it and qemu is reading it.
Fixes: ad58142e73 ("xen/public: move xenstore related doc into 9pfs.h")
Fixes: cf1d2d22fd ("docs/misc: Xen transport for 9pfs")
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Jason Andryuk <jandryuk@gmail.com>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
In case XSM is active, the handling of XEN_SYSCTL_getdomaininfolist
can fail if the last domain scanned isn't allowed to be accessed by
the calling domain (i.e. xsm_getdomaininfo(XSM_HOOK, d) is failing).
Fix that by just ignoring scanned domains where xsm_getdomaininfo()
is returning an error, like it is effectively done when such a
situation occurs for a domain not being the last one scanned.
Fixes: d046f361dc ("Xen Security Modules: XSM")
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Today it is not possible to analyse crash dumps of a system in
hypervisor mode when it had been booted via EFI, as the crash utility
doesn't understand the file format of xen.efi.
This can easily be solved by creating an ELF file from xen.efi via
objcopy. Using that file as name list for crash enables the user to
analyse the dump in hypervisor mode. Note that crash isn't happy with
a file containing no text and data, so using --only-keep-debug is not
an option.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Update the help text of the CONFIG_DEBUG_INFO option to be a little
bit more specific.
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
In order to support hypervisor analysis of crash dumps, xen-syms needs
to contain debug_info. It should be allowed to configure the hypervisor
to be built with CONFIG_DEBUG_INFO in non-debug builds without having
to enable EXPERT.
Using a rather oldish gcc (7.5) it was verified that code generation
doesn't really differ between CONFIG_DEBUG_INFO on or off without
CONFIG_DEBUG being set (only observed differences were slightly
different symbol addresses, verified via "objdump -d", resulting from
the different config.gz in the binary). The old gcc version selection
was based on the assumption, that newer gcc won't regress in this
regard.
So move CONFIG_DEBUG_INFO out of the section guarded by EXPERT.
It should be mentioned that there have been reports that the linking
of the xen.efi might take considerably longer with CONFIG_DEBUG_INFO
selected when using newer binutils.
Signed-off-by: Juergen Gross <jgross@suse.com>
Tested-by: Dietmar Hahn <dietmar.hahn@fujitsu.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Today when finalizing a transaction the number of node quota is checked
to not being exceeded after the transaction. This check is always done,
even if the transaction is being performed by a privileged connection,
or if there were no nodes created in the transaction.
Correct that by checking quota only if:
- the transaction is being performed by an unprivileged guest, and
- at least one node was created in the transaction
Reported-by: Julien Grall <julien@xen.org>
Fixes: f2bebf72c4 ("xenstore: rework of transaction handling")
Signed-off-by: Juergen Gross <jgross@suse.com>
There is a part of a comment in create_node() which should have been
deleted when modifying the related coding.
Fixes: 1cd3cc7ea2 ("tools/xenstore: create_node: Don't defer work to undo any changes on failure")
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
vchan/init.c is defining offsetof privately. Remove that definition
and just use stddef.h instead.
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Anthony PERARD <anthony.perard@citrix.com>
xfs/fsys_xfs.c is defining offsetof privately. Remove that definition
and just use stddef.h instead.
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Anthony PERARD <anthony.perard@citrix.com>
util.h contains a definition of offsetof(), which isn't needed, as
firmware/include/stddef.h's doesn't really need overriding.
Remove it.
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Instead of having 3 identical copies of the definition of a
container_of() macro in different tools header files, add that macro
to xen-tools/common-macros.h and use that instead.
Delete the other copies of that macro.
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Anthony PERARD <anthony.perard@citrix.com>
Defining min(), min_t(), max() and max_t() at other places than
xen-tools/common-macros.h isn't needed, as the definitions in said
header can be used instead.
Same applies to BUILD_BUG_ON() in hvmloader.
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Anthony PERARD <anthony.perard@citrix.com>
Rework the config parsing of a p9 device to use the
split_string_into_pair() function instead of open coding it.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Jason Andryuk <jandryuk@gmail.com>
Acked-by: Anthony PERARD <anthony.perard@citrix.com>
Today split_string_into_pair() will not really do what its name is
suggesting: instead of splitting a string into a pair of strings using
a delimiter, it will return the first two strings of the initial string
by using the delimiter.
This is never what the callers want, so modify split_string_into_pair()
to split the string only at the first delimiter found, resulting in
something like "x=a=b" to be split into "x" and "a=b" when being called
with "=" as the delimiter. Today the returned strings would be "x" and
"a".
At the same time switch the delimiter from "const char *" (allowing
multiple delimiter characters) to "char" (a single character only), as
this makes the function more simple without breaking any use cases.
Suggested-by: Anthony PERARD <anthony.perard@citrix.com>
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Jason Andryuk <jandryuk@gmail.com>
Acked-by: Anthony PERARD <anthony.perard@citrix.com>
Instead of duplicating libxl__device_console_add() work in
init-xenstore-domain.c, just use libxenlight.
This requires to add a small wrapper function to libxenlight, as
libxl__device_console_add() is an internal function.
This at once removes a theoretical race between starting xenconsoled
and xenstore-stubdom, as the old code wasn't using a single
transaction for writing all the entries, leading to the possibility
that xenconsoled would see only some of the entries being written.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>
Most use cases of split_string_into_pair() are requiring the returned
strings to be white space trimmed.
In order to avoid the same code pattern multiple times, add a predicate
parameter to split_string_into_pair() which can be specified to call
trim() with that predicate for the string pair returned. Specifying
NULL for the predicate will avoid the call of trim().
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Jason Andryuk <jandryuk@gmail.com>
Acked-by: Anthony PERARD <anthony.perard@citrix.com>
In order to better reflect the contents of the header and to make it
more appropriate to use it for different runtime environments like
programs, libraries, and firmware, rename the libs.h include file to
common-macros.h. Additionally add a comment pointing out the need to be
self-contained.
Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> # tools/python/xen/lowlevel/xc/xc.c
Acked-by: Christian Lindig <christian.lindig@cloud.com>
Acked-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
tools/tests/vhpet tests don't build since ages (at least since 4.10)
and they can't be activated from outside of tools/tests/vhpet.
Remove them as they seem to be irrelevant.
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
The documented connection sequence in xen/include/public/io/9pfs.h has
a bug: the frontend needs to wait for the backend to have published its
features before being able to allocate its rings and event-channels.
While correcting that, make it clear that there might be multiple
rings and event-channels by adding "(s)".
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
In xen/include/public/io/9pfs.h the documentation regarding the
Xenstore entries isn't reflecting reality: the "tag" Xenstore entry
is on the frontend side, not on the backend one.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
In docs/misc/xenstore.txt the description of the Xenstore node access
permissions is missing one important aspect, which can be found only
in the code or in the wiki [1]:
The first permission entry is defining the owner of the node via the
domid, and the access rights for all domains NOT having a dedicated
permission entry.
Make that aspect clear in the official documentation.
[1]: https://wiki.xenproject.org/wiki/XenBus#Permissions
Reported-by: Edwin Török <edwin.torok@cloud.com>
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
The current version is no longer 4.17-rc. Update it to "4.18-unstable".
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
When loading a Xenstore stubdom the loader doesn't know whether the
lo be loaded kernel is a PVH or a PV one. So it tries to load it as
a PVH one first, and if this fails it is loading it as a PV kernel.
This results in errors being logged in case the stubdom is a PV kernel.
Suppress those errors by setting the minimum logging level to
"critical" while trying to load the kernel as PVH.
In case PVH mode and PV mode loading fails, retry PVH mode loading
without changing the log level in order to get the error messages
logged.
Fixes: f89955449c ("tools/init-xenstore-domain: support xenstore pvh stubdom")
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
The Xenstore related documentation is currently to be found in
docs/misc/9pfs.pandoc, instead of the related header file
xen/include/public/io/9pfs.h like for most other paravirtualized
device protocols.
There is a comment in the header pointing at the document, but the
given file name is wrong. Additionally such headers are meant to be
copied into consuming projects (Linux kernel, qemu, etc.), so pointing
at a doc file in the Xen git repository isn't really helpful for the
consumers of the header.
This situation is far from ideal, which is already being proved by the
fact that neither qemu nor the Linux kernel are implementing the
device attach/detach protocol correctly.
Change that by moving the Xenstore related 9pfs documentation from
docs/misc/9pfs.pandoc into xen/include/public/io/9pfs.h.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
Using a tab for separating the command from the options in the output
of "xenstore-control help" results in a rather ugly list.
Use a fixed size for the command instead.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
Today check_store() is only testing the correctness of the node tree.
Add verification of the accounting data (number of nodes) and correct
the data if it is wrong.
Do the initial check_store() call only after Xenstore entries of a
live update have been read. This is wanted to make sure the accounting
data is correct after a live update.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
Make the xenstored internal trace configurable by adding classes
which can be switched on and off independently from each other.
Define the following classes:
- obj: Creation and deletion of interesting "objects" (watch,
transaction, connection)
- io: incoming requests and outgoing responses
- wrl: write limiting
Per default "obj" and "io" are switched on.
Entries written via trace() will always be printed (if tracing is on
at all).
Add the capability to control the trace settings via the "log"
command and via a new "--log-control" command line option.
Add a missing trace_create() call for creating a transaction.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
Instead of using malloc() and friends, let the hashtable implementation
use the talloc framework.
This is more consistent with the rest of xenstored and it allows to
track memory usage via "xenstore-control memreport".
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
Letting hashtable_remove() return the value of the removed element is
not used anywhere in Xenstore, and it conflicts with a hashtable
created specifying the HASHTABLE_FREE_VALUE flag.
So just drop returning the value.
This of course requires to free the value if the HASHTABLE_FREE_VALUE
was specified, as otherwise it would be a memory leak.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
Instead of returning 0 or 1 let chk_domain_generation() return a
boolean value.
Simplify the only caller by removing the ret variable.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
clang 14 is complaining about a NULL dereference for constructs like:
domain_is_unprivileged(conn) ? conn->in : NULL
as it can't know that domain_is_unprivileged(conn) will return false
if conn is NULL.
Fix that by making domain_is_unprivileged() an inline function (and
related to that domid_is_unprivileged(), too).
In order not having to make struct domain public, use conn->id instead
of conn->domain->domid for the test.
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Julien Grall <jgrall@amazon.com>
There are some places left where dom0 is associated with domid 0.
Use dom0_domid instead.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
Rework the interface and the internals of the per-domain node
accounting:
- rename the functions to domain_nbentry_*() in order to better match
the related counter name
- switch from node pointer to domid as interface, as all nodes have the
owner filled in
- use a common internal function for adding a value to the counter
For the transaction case add a helper function to get the list head
of the per-transaction changed domains, enabling to eliminate the
transaction_entry_*() functions.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
Move all code related to struct changed_domain from
xenstored_transaction.c to xenstored_domain.c.
This will be needed later in order to simplify the accounting data
updates in cases of errors during a request.
Split the code to have a more generic base framework.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
Instead of storing a pointer to the path which is prepended to
relative paths in struct watch, just use the length of the prepended
path.
It should be noted that the now removed special case of the
relative path being "" in get_watch_path() can't happen at all.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
Juergen Gross