[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [xen master] tools/xenstore: change per-domain node accounting interface
commit 4c1f92d664c49442af8c743bd06360bda3412b3c Author: Juergen Gross <jgross@xxxxxxxx> AuthorDate: Wed Jan 18 10:50:07 2023 +0100 Commit: Julien Grall <jgrall@xxxxxxxxxx> CommitDate: Fri Jan 20 09:23:51 2023 +0000 tools/xenstore: change per-domain node accounting interface 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@xxxxxxxx> Reviewed-by: Julien Grall <jgrall@xxxxxxxxxx> --- tools/xenstore/xenstored_core.c | 28 ++++---- tools/xenstore/xenstored_core.h | 6 ++ tools/xenstore/xenstored_domain.c | 126 +++++++++++++++------------------ tools/xenstore/xenstored_domain.h | 10 ++- tools/xenstore/xenstored_transaction.c | 15 +--- tools/xenstore/xenstored_transaction.h | 7 +- 6 files changed, 87 insertions(+), 105 deletions(-) diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c index c82fb6e3d5..4582ee39e1 100644 --- a/tools/xenstore/xenstored_core.c +++ b/tools/xenstore/xenstored_core.c @@ -738,13 +738,13 @@ struct node *read_node(struct connection *conn, const void *ctx, /* Permissions are struct xs_permissions. */ node->perms.p = hdr->perms; - node->acc.domid = node->perms.p[0].id; + node->acc.domid = get_node_owner(node); node->acc.memory = data.dsize; if (domain_adjust_node_perms(node)) goto error; /* If owner is gone reset currently accounted memory size. */ - if (node->acc.domid != node->perms.p[0].id) + if (node->acc.domid != get_node_owner(node)) node->acc.memory = 0; /* Data is binary blob (usually ascii, no nul). */ @@ -1445,7 +1445,7 @@ static void destroy_node_rm(struct connection *conn, struct node *node) static int destroy_node(struct connection *conn, struct node *node) { destroy_node_rm(conn, node); - domain_entry_dec(conn, node); + domain_nbentry_dec(conn, get_node_owner(node)); /* * It is not possible to easily revert the changes in a transaction. @@ -1484,7 +1484,7 @@ static struct node *create_node(struct connection *conn, const void *ctx, for (i = node; i; i = i->parent) { /* i->parent is set for each new node, so check quota. */ if (i->parent && - domain_entry(conn) >= quota_nb_entry_per_domain) { + domain_nbentry(conn) >= quota_nb_entry_per_domain) { ret = ENOSPC; goto err; } @@ -1495,7 +1495,7 @@ static struct node *create_node(struct connection *conn, const void *ctx, /* Account for new node */ if (i->parent) { - if (domain_entry_inc(conn, i)) { + if (domain_nbentry_inc(conn, get_node_owner(i))) { destroy_node_rm(conn, i); return NULL; } @@ -1648,7 +1648,7 @@ static int delnode_sub(const void *ctx, struct connection *conn, watch_exact = strcmp(root, node->name); fire_watches(conn, ctx, node->name, node, watch_exact, NULL); - domain_entry_dec(conn, node); + domain_nbentry_dec(conn, get_node_owner(node)); return WALK_TREE_RM_CHILDENTRY; } @@ -1784,29 +1784,29 @@ static int do_set_perms(const void *ctx, struct connection *conn, /* Unprivileged domains may not change the owner. */ if (domain_is_unprivileged(conn) && - perms.p[0].id != node->perms.p[0].id) + perms.p[0].id != get_node_owner(node)) return EPERM; old_perms = node->perms; - domain_entry_dec(conn, node); + domain_nbentry_dec(conn, get_node_owner(node)); node->perms = perms; - if (domain_entry_inc(conn, node)) { + if (domain_nbentry_inc(conn, get_node_owner(node))) { node->perms = old_perms; /* * This should never fail because we had a reference on the * domain before and Xenstored is single-threaded. */ - domain_entry_inc(conn, node); + domain_nbentry_inc(conn, get_node_owner(node)); return ENOMEM; } if (write_node(conn, node, false)) { int saved_errno = errno; - domain_entry_dec(conn, node); + domain_nbentry_dec(conn, get_node_owner(node)); node->perms = old_perms; /* No failure possible as above. */ - domain_entry_inc(conn, node); + domain_nbentry_inc(conn, get_node_owner(node)); errno = saved_errno; return errno; @@ -2378,7 +2378,7 @@ void setup_structure(bool live_update) manual_node("/tool/xenstored", NULL); manual_node("@releaseDomain", NULL); manual_node("@introduceDomain", NULL); - domain_entry_fix(dom0_domid, 5, true); + domain_nbentry_fix(dom0_domid, 5, true); } check_store(); @@ -3386,7 +3386,7 @@ void read_state_node(const void *ctx, const void *state) if (write_node_raw(NULL, &key, node, true)) barf("write node error restoring node"); - if (domain_entry_inc(&conn, node)) + if (domain_nbentry_inc(&conn, get_node_owner(node))) barf("node accounting error restoring node"); talloc_free(node); diff --git a/tools/xenstore/xenstored_core.h b/tools/xenstore/xenstored_core.h index 89055cbb21..62d8ee96bd 100644 --- a/tools/xenstore/xenstored_core.h +++ b/tools/xenstore/xenstored_core.h @@ -232,6 +232,12 @@ char *canonicalize(struct connection *conn, const void *ctx, const char *node); unsigned int perm_for_conn(struct connection *conn, const struct node_perms *perms); +/* Get owner of a node. */ +static inline unsigned int get_node_owner(const struct node *node) +{ + return node->perms.p[0].id; +} + /* Write a node to the tdb data base. */ int write_node_raw(struct connection *conn, TDB_DATA *key, struct node *node, bool no_quota_check); diff --git a/tools/xenstore/xenstored_domain.c b/tools/xenstore/xenstored_domain.c index 1d765ceffa..703ddeec4e 100644 --- a/tools/xenstore/xenstored_domain.c +++ b/tools/xenstore/xenstored_domain.c @@ -249,7 +249,7 @@ static int domain_tree_remove_sub(const void *ctx, struct connection *conn, domain->nbentry--; node->perms.p[0].id = priv_domid; node->acc.memory = 0; - domain_entry_inc(NULL, node); + domain_nbentry_inc(NULL, priv_domid); if (write_node_raw(NULL, &key, node, true)) { /* That's unfortunate. We only can try to continue. */ syslog(LOG_ERR, @@ -561,7 +561,7 @@ int acc_fix_domains(struct list_head *head, bool update) int cnt; list_for_each_entry(cd, head, list) { - cnt = domain_entry_fix(cd->domid, cd->nbentry, update); + cnt = domain_nbentry_fix(cd->domid, cd->nbentry, update); if (!update) { if (cnt >= quota_nb_entry_per_domain) return ENOSPC; @@ -606,8 +606,8 @@ static struct changed_domain *acc_get_changed_domain(const void *ctx, return cd; } -int acc_add_dom_nbentry(const void *ctx, struct list_head *head, int val, - unsigned int domid) +static int acc_add_dom_nbentry(const void *ctx, struct list_head *head, int val, + unsigned int domid) { struct changed_domain *cd; @@ -991,30 +991,6 @@ void domain_deinit(void) xenevtchn_unbind(xce_handle, virq_port); } -int domain_entry_inc(struct connection *conn, struct node *node) -{ - struct domain *d; - unsigned int domid; - - if (!node->perms.p) - return 0; - - domid = node->perms.p[0].id; - - if (conn && conn->transaction) { - transaction_entry_inc(conn->transaction, domid); - } else { - d = (conn && domid == conn->id && conn->domain) ? conn->domain - : find_or_alloc_existing_domain(domid); - if (d) - d->nbentry++; - else - return ENOMEM; - } - - return 0; -} - /* * Check whether a domain was created before or after a specific generation * count (used for testing whether a node permission is older than a domain). @@ -1082,62 +1058,76 @@ int domain_adjust_node_perms(struct node *node) return 0; } -void domain_entry_dec(struct connection *conn, struct node *node) +static int domain_nbentry_add(struct connection *conn, unsigned int domid, + int add, bool no_dom_alloc) { struct domain *d; - unsigned int domid; - - if (!node->perms.p) - return; + struct list_head *head; + int ret; - domid = node->perms.p ? node->perms.p[0].id : conn->id; + if (conn && domid == conn->id && conn->domain) + d = conn->domain; + else if (no_dom_alloc) { + d = find_domain_struct(domid); + if (!d) { + errno = ENOENT; + corrupt(conn, "Missing domain %u\n", domid); + return -1; + } + } else { + d = find_or_alloc_existing_domain(domid); + if (!d) { + errno = ENOMEM; + return -1; + } + } if (conn && conn->transaction) { - transaction_entry_dec(conn->transaction, domid); - } else { - d = (conn && domid == conn->id && conn->domain) ? conn->domain - : find_domain_struct(domid); - if (d) { - d->nbentry--; - } else { - errno = ENOENT; - corrupt(conn, - "Node \"%s\" owned by non-existing domain %u\n", - node->name, domid); + head = transaction_get_changed_domains(conn->transaction); + ret = acc_add_dom_nbentry(conn->transaction, head, add, domid); + if (errno) { + fail_transaction(conn->transaction); + return -1; } + /* + * In a transaction when a node is being added/removed AND the + * same node has been added/removed outside the transaction in + * parallel, the resulting number of nodes will be wrong. This + * is no problem, as the transaction will fail due to the + * resulting conflict. + * In the node remove case the resulting number can be even + * negative, which should be avoided. + */ + return max(d->nbentry + ret, 0); } + + d->nbentry += add; + + return d->nbentry; } -int domain_entry_fix(unsigned int domid, int num, bool update) +int domain_nbentry_inc(struct connection *conn, unsigned int domid) { - struct domain *d; - int cnt; + return (domain_nbentry_add(conn, domid, 1, false) < 0) ? errno : 0; +} - if (update) { - d = find_domain_struct(domid); - assert(d); - } else { - /* - * We are called first with update == false in order to catch - * any error. So do a possible allocation and check for error - * only in this case, as in the case of update == true nothing - * can go wrong anymore as the allocation already happened. - */ - d = find_or_alloc_existing_domain(domid); - if (!d) - return -1; - } +int domain_nbentry_dec(struct connection *conn, unsigned int domid) +{ + return (domain_nbentry_add(conn, domid, -1, true) < 0) ? errno : 0; +} - cnt = d->nbentry + num; - assert(cnt >= 0); +int domain_nbentry_fix(unsigned int domid, int num, bool update) +{ + int ret; - if (update) - d->nbentry = cnt; + ret = domain_nbentry_add(NULL, domid, update ? num : 0, update); + if (ret < 0 || update) + return ret; - return domid_is_unprivileged(domid) ? cnt : 0; + return domid_is_unprivileged(domid) ? ret + num : 0; } -int domain_entry(struct connection *conn) +int domain_nbentry(struct connection *conn) { return (domain_is_unprivileged(conn)) ? conn->domain->nbentry diff --git a/tools/xenstore/xenstored_domain.h b/tools/xenstore/xenstored_domain.h index 9e20d2b17d..1e402f2609 100644 --- a/tools/xenstore/xenstored_domain.h +++ b/tools/xenstore/xenstored_domain.h @@ -66,10 +66,10 @@ int domain_adjust_node_perms(struct node *node); int domain_alloc_permrefs(struct node_perms *perms); /* Quota manipulation */ -int domain_entry_inc(struct connection *conn, struct node *); -void domain_entry_dec(struct connection *conn, struct node *); -int domain_entry_fix(unsigned int domid, int num, bool update); -int domain_entry(struct connection *conn); +int domain_nbentry_inc(struct connection *conn, unsigned int domid); +int domain_nbentry_dec(struct connection *conn, unsigned int domid); +int domain_nbentry_fix(unsigned int domid, int num, bool update); +int domain_nbentry(struct connection *conn); int domain_memory_add(unsigned int domid, int mem, bool no_quota_check); /* @@ -99,8 +99,6 @@ void domain_outstanding_domid_dec(unsigned int domid); int domain_get_quota(const void *ctx, struct connection *conn, unsigned int domid); int acc_fix_domains(struct list_head *head, bool update); -int acc_add_dom_nbentry(const void *ctx, struct list_head *head, int val, - unsigned int domid); /* Write rate limiting */ diff --git a/tools/xenstore/xenstored_transaction.c b/tools/xenstore/xenstored_transaction.c index c009c67989..82e5e66c18 100644 --- a/tools/xenstore/xenstored_transaction.c +++ b/tools/xenstore/xenstored_transaction.c @@ -548,20 +548,9 @@ int do_transaction_end(const void *ctx, struct connection *conn, return 0; } -void transaction_entry_inc(struct transaction *trans, unsigned int domid) +struct list_head *transaction_get_changed_domains(struct transaction *trans) { - if (!acc_add_dom_nbentry(trans, &trans->changed_domains, 1, domid)) { - /* Let the transaction fail. */ - trans->fail = true; - } -} - -void transaction_entry_dec(struct transaction *trans, unsigned int domid) -{ - if (!acc_add_dom_nbentry(trans, &trans->changed_domains, -1, domid)) { - /* Let the transaction fail. */ - trans->fail = true; - } + return &trans->changed_domains; } void fail_transaction(struct transaction *trans) diff --git a/tools/xenstore/xenstored_transaction.h b/tools/xenstore/xenstored_transaction.h index 3417303f94..b6f8cb7d0a 100644 --- a/tools/xenstore/xenstored_transaction.h +++ b/tools/xenstore/xenstored_transaction.h @@ -36,10 +36,6 @@ int do_transaction_end(const void *ctx, struct connection *conn, struct transaction *transaction_lookup(struct connection *conn, uint32_t id); -/* inc/dec entry number local to trans while changing a node */ -void transaction_entry_inc(struct transaction *trans, unsigned int domid); -void transaction_entry_dec(struct transaction *trans, unsigned int domid); - /* This node was accessed. */ int __must_check access_node(struct connection *conn, struct node *node, enum node_access_type type, TDB_DATA *key); @@ -54,6 +50,9 @@ void transaction_prepend(struct connection *conn, const char *name, /* Mark the transaction as failed. This will prevent it to be committed. */ void fail_transaction(struct transaction *trans); +/* Get the list head of the changed domains. */ +struct list_head *transaction_get_changed_domains(struct transaction *trans); + void conn_delete_all_transactions(struct connection *conn); int check_transactions(struct hashtable *hash); -- generated by git-patchbot for /home/xen/git/xen.git#master
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |