[xen stable-4.16] tools/xenstore: reduce number of watch events

[patchbot@xxxxxxx]

commit e26d6f4d1b389b859fb5a6570421e80e0213f92b
Author:     Juergen Gross <jgross@xxxxxxxx>
AuthorDate: Tue Sep 13 07:35:07 2022 +0200
Commit:     Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
CommitDate: Tue Nov 1 14:07:24 2022 +0000

    tools/xenstore: reduce number of watch events
    
    When removing a watched node outside of a transaction, two watch events
    are being produced instead of just a single one.
    
    When finalizing a transaction watch events can be generated for each
    node which is being modified, even if outside a transaction such
    modifications might not have resulted in a watch event.
    
    This happens e.g.:
    
    - for nodes which are only modified due to added/removed child entries
    - for nodes being removed or created implicitly (e.g. creation of a/b/c
      is implicitly creating a/b, resulting in watch events for a, a/b and
      a/b/c instead of a/b/c only)
    
    Avoid these additional watch events, in order to reduce the needed
    memory inside Xenstore for queueing them.
    
    This is being achieved by adding event flags to struct accessed_node
    specifying whether an event should be triggered, and whether it should
    be an exact match of the modified path. Both flags can be set from
    fire_watches() instead of implying them only.
    
    This is part of XSA-326.
    
    Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
    Reviewed-by: Julien Grall <jgrall@xxxxxxxxxx>
    (cherry picked from commit 3a96013a3e17baa07410b1b9776225d1d9a74297)
---
 tools/xenstore/xenstored_core.c        | 19 ++++++++--------
 tools/xenstore/xenstored_transaction.c | 41 ++++++++++++++++++++++++++++------
 tools/xenstore/xenstored_transaction.h |  3 +++
 tools/xenstore/xenstored_watch.c       |  7 ++++--
 4 files changed, 51 insertions(+), 19 deletions(-)

diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
index 527a1ebded..bf22438739 100644
--- a/tools/xenstore/xenstored_core.c
+++ b/tools/xenstore/xenstored_core.c
@@ -1295,7 +1295,7 @@ static void delete_child(struct connection *conn,
 }
 
 static int delete_node(struct connection *conn, const void *ctx,
-                      struct node *parent, struct node *node)
+                      struct node *parent, struct node *node, bool watch_exact)
 {
        char *name;
 
@@ -1307,7 +1307,7 @@ static int delete_node(struct connection *conn, const 
void *ctx,
                                       node->children);
                child = name ? read_node(conn, node, name) : NULL;
                if (child) {
-                       if (delete_node(conn, ctx, node, child))
+                       if (delete_node(conn, ctx, node, child, true))
                                return errno;
                } else {
                        trace("delete_node: Error deleting child '%s/%s'!\n",
@@ -1319,7 +1319,12 @@ static int delete_node(struct connection *conn, const 
void *ctx,
                talloc_free(name);
        }
 
-       fire_watches(conn, ctx, node->name, node, true, NULL);
+       /*
+        * Fire the watches now, when we can still see the node permissions.
+        * This fine as we are single threaded and the next possible read will
+        * be handled only after the node has been really removed.
+        */
+       fire_watches(conn, ctx, node->name, node, watch_exact, NULL);
        delete_node_single(conn, node);
        delete_child(conn, parent, basename(node->name));
        talloc_free(node);
@@ -1345,13 +1350,7 @@ static int _rm(struct connection *conn, const void *ctx, 
struct node *node,
                return (errno == ENOMEM) ? ENOMEM : EINVAL;
        node->parent = parent;
 
-       /*
-        * Fire the watches now, when we can still see the node permissions.
-        * This fine as we are single threaded and the next possible read will
-        * be handled only after the node has been really removed.
-        */
-       fire_watches(conn, ctx, name, node, false, NULL);
-       return delete_node(conn, ctx, parent, node);
+       return delete_node(conn, ctx, parent, node, false);
 }
 
 
diff --git a/tools/xenstore/xenstored_transaction.c 
b/tools/xenstore/xenstored_transaction.c
index faf6c930e4..54432907fc 100644
--- a/tools/xenstore/xenstored_transaction.c
+++ b/tools/xenstore/xenstored_transaction.c
@@ -130,6 +130,10 @@ struct accessed_node
 
        /* Transaction node in data base? */
        bool ta_node;
+
+       /* Watch event flags. */
+       bool fire_watch;
+       bool watch_exact;
 };
 
 struct changed_domain
@@ -323,6 +327,29 @@ err:
        return ret;
 }
 
+/*
+ * A watch event should be fired for a node modified inside a transaction.
+ * Set the corresponding information. A non-exact event is replacing an exact
+ * one, but not the other way round.
+ */
+void queue_watches(struct connection *conn, const char *name, bool watch_exact)
+{
+       struct accessed_node *i;
+
+       i = find_accessed_node(conn->transaction, name);
+       if (!i) {
+               conn->transaction->fail = true;
+               return;
+       }
+
+       if (!i->fire_watch) {
+               i->fire_watch = true;
+               i->watch_exact = watch_exact;
+       } else if (!watch_exact) {
+               i->watch_exact = false;
+       }
+}
+
 /*
  * Finalize transaction:
  * Walk through accessed nodes and check generation against global data.
@@ -377,15 +404,15 @@ static int finalize_transaction(struct connection *conn,
                                ret = tdb_store(tdb_ctx, key, data,
                                                TDB_REPLACE);
                                talloc_free(data.dptr);
-                               if (ret)
-                                       goto err;
-                               fire_watches(conn, trans, i->node, NULL, false,
-                                            i->perms.p ? &i->perms : NULL);
                        } else {
-                               fire_watches(conn, trans, i->node, NULL, false,
+                               ret = tdb_delete(tdb_ctx, key);
+                       }
+                       if (ret)
+                               goto err;
+                       if (i->fire_watch) {
+                               fire_watches(conn, trans, i->node, NULL,
+                                            i->watch_exact,
                                             i->perms.p ? &i->perms : NULL);
-                               if (tdb_delete(tdb_ctx, key))
-                                       goto err;
                        }
                }
 
diff --git a/tools/xenstore/xenstored_transaction.h 
b/tools/xenstore/xenstored_transaction.h
index 14062730e3..0093cac807 100644
--- a/tools/xenstore/xenstored_transaction.h
+++ b/tools/xenstore/xenstored_transaction.h
@@ -42,6 +42,9 @@ void transaction_entry_dec(struct transaction *trans, 
unsigned int domid);
 int access_node(struct connection *conn, struct node *node,
                 enum node_access_type type, TDB_DATA *key);
 
+/* Queue watches for a modified node. */
+void queue_watches(struct connection *conn, const char *name, bool 
watch_exact);
+
 /* Prepend the transaction to name if appropriate. */
 int transaction_prepend(struct connection *conn, const char *name,
                         TDB_DATA *key);
diff --git a/tools/xenstore/xenstored_watch.c b/tools/xenstore/xenstored_watch.c
index 99a2c266b2..205d9d8ea1 100644
--- a/tools/xenstore/xenstored_watch.c
+++ b/tools/xenstore/xenstored_watch.c
@@ -29,6 +29,7 @@
 #include "xenstore_lib.h"
 #include "utils.h"
 #include "xenstored_domain.h"
+#include "xenstored_transaction.h"
 
 extern int quota_nb_watch_per_domain;
 
@@ -143,9 +144,11 @@ void fire_watches(struct connection *conn, const void 
*ctx, const char *name,
        struct connection *i;
        struct watch *watch;
 
-       /* During transactions, don't fire watches. */
-       if (conn && conn->transaction)
+       /* During transactions, don't fire watches, but queue them. */
+       if (conn && conn->transaction) {
+               queue_watches(conn, name, exact);
                return;
+       }
 
        /* Create an event for each watch. */
        list_for_each_entry(i, &connections, list) {
--
generated by git-patchbot for /home/xen/git/xen.git#stable-4.16