1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
|
From 8999db805e5ef55172a85d67695429edc3d78771 Mon Sep 17 00:00:00 2001
From: Juergen Gross <jgross@suse.com>
Date: Tue, 13 Sep 2022 07:35:07 +0200
Subject: [PATCH 087/126] 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)
---
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 6498bf603666..5157a7527f58 100644
--- a/tools/xenstore/xenstored_core.c
+++ b/tools/xenstore/xenstored_core.c
@@ -1261,7 +1261,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;
@@ -1273,7 +1273,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",
@@ -1285,7 +1285,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);
@@ -1311,13 +1316,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 faf6c930e42a..54432907fc76 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 14062730e3c9..0093cac807e3 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 a116f967dc66..bc6d833028a3 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) {
--
2.37.4
|
GitWeb