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@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
(cherry picked from commit 3a96013a3e17baa07410b1b9776225d1d9a74297)

diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
index 5d54779d409b09eb7e3bc46fb35c2f2ff78ca2e6..53d003aebffb3c2c1bf570e3d166536c38ef6fff 100644 (file)
--- a/tools/xenstore/xenstored_core.c
+++ b/tools/xenstore/xenstored_core.c
@@ -1182,7 +1182,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;
 
@@ -1194,7 +1194,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",
@@ -1206,7 +1206,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);
@@ -1232,13 +1237,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 4ffa18311120d5f940ea8ea3d327780156ab8688..6fbdb29dcdd71a804046f41c5b84efede665b38a 100644 (file)
--- 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
@@ -329,6 +333,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.
@@ -383,15 +410,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 14062730e3c99021854fe249bd02697fdaf5fec4..0093cac807e3da48588dab849e543d70f3386590 100644 (file)
--- 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 6d8097376e478ce9e503f8221dc9fa54c71d7e14..2f9367767e442ea9ccb98cf5d97da2bd3d82197c 100644 (file)
--- 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) {