Hi Juergen, On 05/04/2023 08:03, Juergen Gross wrote:
Add the node accounting to the accounting information buffering in order to avoid having to undo it in case of failure.Signed-off-by: Juergen Gross <jgr...@suse.com> --- tools/xenstore/xenstored_core.c | 21 ++------------------- tools/xenstore/xenstored_domain.h | 4 ++-- 2 files changed, 4 insertions(+), 21 deletions(-) diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c index 84335f5f3d..92a40ccf3f 100644 --- a/tools/xenstore/xenstored_core.c +++ b/tools/xenstore/xenstored_core.c @@ -1452,7 +1452,6 @@ 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_nbentry_dec(conn, get_node_owner(node));/** It is not possible to easily revert the changes in a transaction. @@ -1797,27 +1796,11 @@ static int do_set_perms(const void *ctx, struct connection *conn, old_perms = node->perms; domain_nbentry_dec(conn, get_node_owner(node));
IIRC, we originally said that domain_nbentry_dec() could never fail in a non-transaction case. But with your current rework, the function can now fail because of an allocation failure.
Therefore, shouldn't we now check the error? (Possibly in a patch beforehand).
node->perms = perms; - 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_nbentry_inc(conn, get_node_owner(node)); + if (domain_nbentry_inc(conn, get_node_owner(node))) return ENOMEM; - }- if (write_node(conn, node, false)) {- int saved_errno = errno; - - domain_nbentry_dec(conn, get_node_owner(node)); - node->perms = old_perms; - /* No failure possible as above. */ - domain_nbentry_inc(conn, get_node_owner(node)); - - errno = saved_errno; + if (write_node(conn, node, false)) return errno; - }fire_watches(conn, ctx, name, node, false, &old_perms);send_ack(conn, XS_SET_PERMS); diff --git a/tools/xenstore/xenstored_domain.h b/tools/xenstore/xenstored_domain.h index 6355ad4f37..e669f57b80 100644 --- a/tools/xenstore/xenstored_domain.h +++ b/tools/xenstore/xenstored_domain.h @@ -25,9 +25,9 @@ * a per transaction array. */ enum accitem { + ACC_NODES, ACC_REQ_N, /* Number of elements per request. */ - ACC_NODES = ACC_REQ_N, - ACC_TR_N, /* Number of elements per transaction. */ + ACC_TR_N = ACC_REQ_N, /* Number of elements per transaction. */ ACC_N = ACC_TR_N, /* Number of elements per domain. */ };
Cheers, -- Julien Grall
