On 02/03/2023 15:34, David Woodhouse wrote:
From: David Woodhouse <d...@amazon.co.uk>
Firing watches on the nodes that still exist is relatively easy; just
walk the tree and look at the nodes with refcount of one.
Firing watches on *deleted* nodes is more fun. We add 'modified_in_tx'
and 'deleted_in_tx' flags to each node. Nodes with those flags cannot
be shared, as they will always be unique to the transaction in which
they were created.
When xs_node_walk would need to *create* a node as scaffolding and it
encounters a deleted_in_tx node, it can resurrect it simply by clearing
its deleted_in_tx flag. If that node originally had any *data*, they're
gone, and the modified_in_tx flag will have been set when it was first
deleted.
We then attempt to send appropriate watches when the transaction is
committed, properly delete the deleted_in_tx nodes, and remove the
modified_in_tx flag from the others.
Signed-off-by: David Woodhouse <d...@amazon.co.uk>
---
hw/i386/kvm/xenstore_impl.c | 151 ++++++++++++++++++++++-
tests/unit/test-xs-node.c | 231 +++++++++++++++++++++++++++++++++++-
2 files changed, 380 insertions(+), 2 deletions(-)
Reviewed-by: Paul Durrant <p...@xen.org>
... with a couple of nits in comments called out below...
[snip]
+static gboolean tx_commit_walk(gpointer key, gpointer value,
+ gpointer user_data)
+{
+ struct walk_op *op = user_data;
+ int path_len = strlen(op->path);
+ int key_len = strlen(key);
+ bool fire_parents = true;
+ XsWatch *watch;
+ XsNode *n = value;
+
+ if (n->ref != 1) {
+ return false;
+ }
+
+ if (n->deleted_in_tx) {
+ /*
+ * We first watches on our parents if we are the *first* node
We first *fire* watches on our parents...
+ * to be deleted (the topmost one). This matches the behaviour
+ * when deleting in the live tree.
+ */
+ fire_parents = !op->deleted_in_tx;
+
+ /* Only used on the way down so no need to clear it later */
+ op->deleted_in_tx = true;
+ }
+
+ assert(key_len + path_len + 2 <= sizeof(op->path));
+ op->path[path_len] = '/';
+ memcpy(op->path + path_len + 1, key, key_len + 1);
+
+ watch = g_hash_table_lookup(op->s->watches, op->path);
+ if (watch) {
+ op->watches = g_list_append(op->watches, watch);
+ }
+
+ if (n->children) {
+ g_hash_table_foreach_remove(n->children, tx_commit_walk, op);
+ }
+
+ if (watch) {
+ op->watches = g_list_remove(op->watches, watch);
+ }
+
+ /*
+ * Don't fire watches if this node was only copied because a
+ * descendent was changed. The modifieD_in_tx flag indicates the
s/modifieD/modified
+ * ones which were really changed.
+ */
+ if (n->modified_in_tx || n->deleted_in_tx) {
+ fire_watches(op, fire_parents);
+ n->modified_in_tx = false;
+ }
+ op->path[path_len] = '\0';
+
+ /* Deleted nodes really do get expunged when we commit */
+ return n->deleted_in_tx;
+}