[Xen-devel] [PATCH v3 5/5] xenstore: use temporary memory context for firing watches

[Juergen Gross <jgross@xxxxxxxx>]

Use a temporary memory context for memory allocations when firing
watches. This will avoid leaking memory in case of long living
connections and/or xenstore entries.

This requires adding a new parameter to fire_watches() and add_event()
to specify the memory context to use for allocations.

Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
Reviewed-by: Wei Liu <wei.liu2@xxxxxxxxxx>
Acked-by: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
---
 tools/xenstore/xenstored_core.c        |  8 ++++----
 tools/xenstore/xenstored_domain.c      |  6 +++---
 tools/xenstore/xenstored_transaction.c |  2 +-
 tools/xenstore/xenstored_watch.c       | 22 ++++++++++++++++------
 tools/xenstore/xenstored_watch.h       |  3 ++-
 5 files changed, 26 insertions(+), 15 deletions(-)

diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
index 4239410..1232169 100644
--- a/tools/xenstore/xenstored_core.c
+++ b/tools/xenstore/xenstored_core.c
@@ -961,7 +961,7 @@ static void do_write(struct connection *conn, struct 
buffered_data *in)
        }
 
        add_change_node(conn->transaction, name, false);
-       fire_watches(conn, name, false);
+       fire_watches(conn, in, name, false);
        send_ack(conn, XS_WRITE);
 }
 
@@ -986,7 +986,7 @@ static void do_mkdir(struct connection *conn, struct 
buffered_data *in)
                        return;
                }
                add_change_node(conn->transaction, name, false);
-               fire_watches(conn, name, false);
+               fire_watches(conn, in, name, false);
        }
        send_ack(conn, XS_MKDIR);
 }
@@ -1112,7 +1112,7 @@ static void do_rm(struct connection *conn, struct 
buffered_data *in)
 
        if (_rm(conn, node, name)) {
                add_change_node(conn->transaction, name, true);
-               fire_watches(conn, name, true);
+               fire_watches(conn, in, name, true);
                send_ack(conn, XS_RM);
        }
 }
@@ -1188,7 +1188,7 @@ static void do_set_perms(struct connection *conn, struct 
buffered_data *in)
        }
 
        add_change_node(conn->transaction, name, false);
-       fire_watches(conn, name, false);
+       fire_watches(conn, in, name, false);
        send_ack(conn, XS_SET_PERMS);
 }
 
diff --git a/tools/xenstore/xenstored_domain.c 
b/tools/xenstore/xenstored_domain.c
index c66539a..5de93d4 100644
--- a/tools/xenstore/xenstored_domain.c
+++ b/tools/xenstore/xenstored_domain.c
@@ -204,7 +204,7 @@ static int destroy_domain(void *_domain)
                        unmap_interface(domain->interface);
        }
 
-       fire_watches(NULL, "@releaseDomain", false);
+       fire_watches(NULL, domain, "@releaseDomain", false);
 
        return 0;
 }
@@ -232,7 +232,7 @@ static void domain_cleanup(void)
        }
 
        if (notify)
-               fire_watches(NULL, "@releaseDomain", false);
+               fire_watches(NULL, NULL, "@releaseDomain", false);
 }
 
 /* We scan all domains rather than use the information given here. */
@@ -389,7 +389,7 @@ void do_introduce(struct connection *conn, struct 
buffered_data *in)
                /* Now domain belongs to its connection. */
                talloc_steal(domain->conn, domain);
 
-               fire_watches(NULL, "@introduceDomain", false);
+               fire_watches(NULL, in, "@introduceDomain", false);
        } else if ((domain->mfn == mfn) && (domain->conn != conn)) {
                /* Use XS_INTRODUCE for recreating the xenbus event-channel. */
                if (domain->port)
diff --git a/tools/xenstore/xenstored_transaction.c 
b/tools/xenstore/xenstored_transaction.c
index 3cde26e..34720fa 100644
--- a/tools/xenstore/xenstored_transaction.c
+++ b/tools/xenstore/xenstored_transaction.c
@@ -227,7 +227,7 @@ void do_transaction_end(struct connection *conn, struct 
buffered_data *in)
 
                /* Fire off the watches for everything that changed. */
                list_for_each_entry(i, &trans->changes, list)
-                       fire_watches(conn, i->node, i->recurse);
+                       fire_watches(conn, in, i->node, i->recurse);
                generation++;
        }
        send_ack(conn, XS_TRANSACTION_END);
diff --git a/tools/xenstore/xenstored_watch.c b/tools/xenstore/xenstored_watch.c
index beefd6c..856750e 100644
--- a/tools/xenstore/xenstored_watch.c
+++ b/tools/xenstore/xenstored_watch.c
@@ -47,7 +47,12 @@ struct watch
        char *node;
 };
 
+/*
+ * Send a watch event.
+ * Temporary memory allocations are done with ctx.
+ */
 static void add_event(struct connection *conn,
+                     void *ctx,
                      struct watch *watch,
                      const char *name)
 {
@@ -57,7 +62,7 @@ static void add_event(struct connection *conn,
 
        if (!check_event_node(name)) {
                /* Can this conn load node, or see that it doesn't exist? */
-               struct node *node = get_node(conn, name, name, XS_PERM_READ);
+               struct node *node = get_node(conn, ctx, name, XS_PERM_READ);
                /*
                 * XXX We allow EACCES here because otherwise a non-dom0
                 * backend driver cannot watch for disappearance of a frontend
@@ -78,14 +83,19 @@ static void add_event(struct connection *conn,
        }
 
        len = strlen(name) + 1 + strlen(watch->token) + 1;
-       data = talloc_array(watch, char, len);
+       data = talloc_array(ctx, char, len);
        strcpy(data, name);
        strcpy(data + strlen(name) + 1, watch->token);
        send_reply(conn, XS_WATCH_EVENT, data, len);
        talloc_free(data);
 }
 
-void fire_watches(struct connection *conn, const char *name, bool recurse)
+/*
+ * Check whether any watch events are to be sent.
+ * Temporary memory allocations are done with ctx.
+ */
+void fire_watches(struct connection *conn, void *ctx, const char *name,
+                 bool recurse)
 {
        struct connection *i;
        struct watch *watch;
@@ -98,9 +108,9 @@ void fire_watches(struct connection *conn, const char *name, 
bool recurse)
        list_for_each_entry(i, &connections, list) {
                list_for_each_entry(watch, &i->watches, list) {
                        if (is_child(name, watch->node))
-                               add_event(i, watch, name);
+                               add_event(i, ctx, watch, name);
                        else if (recurse && is_child(watch->node, name))
-                               add_event(i, watch, watch->node);
+                               add_event(i, ctx, watch, watch->node);
                }
        }
 }
@@ -169,7 +179,7 @@ void do_watch(struct connection *conn, struct buffered_data 
*in)
        send_ack(conn, XS_WATCH);
 
        /* We fire once up front: simplifies clients and restart. */
-       add_event(conn, watch, watch->node);
+       add_event(conn, in, watch, watch->node);
 }
 
 void do_unwatch(struct connection *conn, struct buffered_data *in)
diff --git a/tools/xenstore/xenstored_watch.h b/tools/xenstore/xenstored_watch.h
index 5bc4f88..8ed1dde 100644
--- a/tools/xenstore/xenstored_watch.h
+++ b/tools/xenstore/xenstored_watch.h
@@ -25,7 +25,8 @@ void do_watch(struct connection *conn, struct buffered_data 
*in);
 void do_unwatch(struct connection *conn, struct buffered_data *in);
 
 /* Fire all watches: recurse means all the children are affected (ie. rm). */
-void fire_watches(struct connection *conn, const char *name, bool recurse);
+void fire_watches(struct connection *conn, void *tmp, const char *name,
+                 bool recurse);
 
 void dump_watches(struct connection *conn);
 
-- 
2.6.6


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel