On 02/03/2023 15:34, David Woodhouse wrote:
From: David Woodhouse <d...@amazon.co.uk>
Starts out fairly simple: a hash table of watches based on the path.
Except there can be multiple watches on the same path, so the watch ends
up being a simple linked list, and the head of that list is in the hash
table. Which makes removal a bit of a PITA but it's not so bad; we just
special-case "I had to remove the head of the list and now I have to
replace it in / remove it from the hash table". And if we don't remove
the head, it's a simple linked-list operation.
We do need to fire watches on *deleted* nodes, so instead of just a simple
xs_node_unref() on the topmost victim, we need to recurse down and fire
watches on them all.
Signed-off-by: David Woodhouse <d...@amazon.co.uk>
---
hw/i386/kvm/xenstore_impl.c | 253 +++++++++++++++++++++++++++++++++---
tests/unit/test-xs-node.c | 85 ++++++++++++
2 files changed, 323 insertions(+), 15 deletions(-)
Reviewed-by: Paul Durrant <p...@xen.org>
... with one suggestion...
[snip]
+ /* Check for duplicates */
+ w = g_hash_table_lookup(s->watches, abspath);
+ while (w) {
+ if (!g_strcmp0(token, w->token) && opaque == w->cb_opaque &&
+ fn == w->cb && dom_id == w->dom_id) {
+ return EEXIST;
+ }
+ w = w->next;
I think you could stash a tail pointer here...
+ }
+
+ if (dom_id && s->nr_domu_watches >= XS_MAX_WATCHES) {
+ return E2BIG;
+ }
+
+ w = g_new0(XsWatch, 1);
+ w->token = g_strdup(token);
+ w->cb = fn;
+ w->cb_opaque = opaque;
+ w->dom_id = dom_id;
+ w->rel_prefix = strlen(abspath) - strlen(path);
+
+ l = g_hash_table_lookup(s->watches, abspath);
... to avoid the duplicate hash lookup here.
+ if (l) {
+ w->next = l->next;
+ l->next = w;
+ } else {
+ g_hash_table_insert(s->watches, g_strdup(abspath), w);
+ }
+ if (dom_id) {
+ s->nr_domu_watches++;
+ }
+
+ /* A new watch should fire immediately */
+ fn(opaque, path, token);
+
+ return 0;
+}
