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 <jgr...@suse.com>
---
V5:
- add error handling after domain_nbentry_dec() calls (Julien Grall)
---
 tools/xenstore/xenstored_core.c   | 29 +++++++----------------------
 tools/xenstore/xenstored_domain.h |  4 ++--
 2 files changed, 9 insertions(+), 24 deletions(-)

diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
index 8392bdec9b..22da434e2a 100644
--- a/tools/xenstore/xenstored_core.c
+++ b/tools/xenstore/xenstored_core.c
@@ -1454,7 +1454,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.
@@ -1645,6 +1644,9 @@ static int delnode_sub(const void *ctx, struct connection 
*conn,
        if (ret > 0)
                return WALK_TREE_SUCCESS_STOP;
 
+       if (domain_nbentry_dec(conn, get_node_owner(node)))
+               return WALK_TREE_ERROR_STOP;
+
        /* In case of error stop the walk. */
        if (!ret && do_tdb_delete(conn, &key, &node->acc))
                return WALK_TREE_SUCCESS_STOP;
@@ -1657,8 +1659,6 @@ 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_nbentry_dec(conn, get_node_owner(node));
-
        return WALK_TREE_RM_CHILDENTRY;
 }
 
@@ -1797,29 +1797,14 @@ static int do_set_perms(const void *ctx, struct 
connection *conn,
                return EPERM;
 
        old_perms = node->perms;
-       domain_nbentry_dec(conn, get_node_owner(node));
+       if (domain_nbentry_dec(conn, get_node_owner(node)))
+               return ENOMEM;
        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 e40657216b..466549709f 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_CHD_N = ACC_TR_N,   /* max(ACC_REQ_N, ACC_TR_N), for changed dom. */
        ACC_N = ACC_TR_N,       /* Number of elements per domain. */
 };
-- 
2.35.3


Reply via email to