Hi Juergen,
On 17/01/2023 09:11, Juergen Gross wrote:
Today check_store() is only testing the correctness of the node tree.
Add verification of the accounting data (number of nodes) and correct
NIT: one too many space before 'and'.
the data if it is wrong.
Do the initial check_store() call only after Xenstore entries of a
live update have been read.
Can you clarify whether this is needed for the rest of the patch, or
simply a nice thing to have in general?
Signed-off-by: Juergen Gross <jgr...@suse.com>
---
tools/xenstore/xenstored_core.c | 62 ++++++++++++++++------
tools/xenstore/xenstored_domain.c | 86 +++++++++++++++++++++++++++++++
tools/xenstore/xenstored_domain.h | 4 ++
3 files changed, 137 insertions(+), 15 deletions(-)
diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
index 3099077a86..e201f14053 100644
--- a/tools/xenstore/xenstored_core.c
+++ b/tools/xenstore/xenstored_core.c
@@ -2389,8 +2389,6 @@ void setup_structure(bool live_update)
manual_node("@introduceDomain", NULL);
domain_nbentry_fix(dom0_domid, 5, true);
}
-
- check_store();
}
static unsigned int hash_from_key_fn(void *k)
@@ -2433,20 +2431,28 @@ int remember_string(struct hashtable *hash, const char
*str)
* As we go, we record each node in the given reachable hashtable. These
* entries will be used later in clean_store.
*/
+
+struct check_store_data {
+ struct hashtable *reachable;
+ struct hashtable *domains;
+};
+
static int check_store_step(const void *ctx, struct connection *conn,
struct node *node, void *arg)
{
- struct hashtable *reachable = arg;
+ struct check_store_data *data = arg;
- if (hashtable_search(reachable, (void *)node->name)) {
+ if (hashtable_search(data->reachable, (void *)node->name)) {
IIUC the cast is only necessary because hashtable_search() expects a
non-const value. I can't think for a reason for the key to be non-const.
So I will look to send a follow-up patch.
+
+void domain_check_acc_add(const struct node *node, struct hashtable *domains)
+{
+ struct domain_acc *dom;
+ unsigned int domid;
+
+ domid = node->perms.p[0].id;
This could be replaced with get_node_owner().
+ dom = hashtable_search(domains, &domid);
+ if (!dom)
+ log("Node %s owned by unknown domain %u", node->name, domid);
+ else
+ dom->nodes++;
+}
+
+static int domain_check_acc_sub(const void *k, void *v, void *arg)
I find the suffix 'sub' misleading because we have a function with a
very similar name (instead suffixed 'sub'). Yet, AFAICT, it is not meant
to substract.
So I would prefix with '_cb' instead.
Cheers,
--
Julien Grall
