[PATCH v3 0/4] xenstore: rework of transaction handling

[PATCH v3 0/4] xenstore: rework of transaction handling

[Juergen Gross]

Rework the transaction handling of xenstored to no longer raise
conflicts so often.

This series has been sent for pre-review to some reviewers before as the
series is related to XSA 206 which has been disclosed only today. So V1
and V2 have been non-public in order to speed up review process without
disclosing the XSA.

Changes in V3:
- don't always return EAGAIN in case of a failed transaction:
  it can be ENOMEM or ENOSPC, too.

Changes in V2:
- Rebase on top of those patches
- split patch 1 in two patches as suggested by Ian

Juergen Gross (4):
  xenstore: let write_node() and some callers return errno
  xenstore: undo function rename
  xenstore: rework of transaction handling
  xenstore: cleanup tdb.c

 tools/xenstore/tdb.c                   | 439 +--------------------------------
 tools/xenstore/tdb.h                   |  22 --
 tools/xenstore/xenstored_core.c        | 173 ++++++-------
 tools/xenstore/xenstored_core.h        |  17 +-
 tools/xenstore/xenstored_domain.c      |  24 +-
 tools/xenstore/xenstored_domain.h      |   2 +-
 tools/xenstore/xenstored_transaction.c | 429 ++++++++++++++++++++++++++------
 tools/xenstore/xenstored_transaction.h |  18 +-
 8 files changed, 481 insertions(+), 643 deletions(-)

-- 
2.10.2


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

[PATCH v3 1/4] xenstore: let write_node() and some callers return errno

[Juergen Gross]

Instead of setting errno and returning true or false return the error
value directly.

In order to ensure all call sites have been changed according to the
modification rename the functions to xs_*.

Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
---
 tools/xenstore/xenstored_core.c | 34 +++++++++++++++++-----------------
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
index 739e8b2..4bcaac0 100644
--- a/tools/xenstore/xenstored_core.c
+++ b/tools/xenstore/xenstored_core.c
@@ -433,7 +433,7 @@ static struct node *read_node(struct connection *conn, const void *ctx,
 	return node;
 }
 
-static bool write_node(struct connection *conn, struct node *node)
+static int xs_write_node(struct connection *conn, struct node *node)
 {
 	/*
 	 * conn will be null when this is called from manual_node.
@@ -475,10 +475,10 @@ static bool write_node(struct connection *conn, struct node *node)
 		corrupt(conn, "Write of %s failed", key.dptr);
 		goto error;
 	}
-	return true;
+	return 0;
  error:
 	errno = ENOSPC;
-	return false;
+	return errno;
 }
 
 static enum xs_perm_type perm_for_conn(struct connection *conn,
@@ -1022,7 +1022,7 @@ static struct node *create_node(struct connection *conn, const void *ctx,
 	/* We write out the nodes down, setting destructor in case
 	 * something goes wrong. */
 	for (i = node; i; i = i->parent) {
-		if (!write_node(conn, i)) {
+		if (xs_write_node(conn, i)) {
 			domain_entry_dec(conn, i);
 			return NULL;
 		}
@@ -1062,7 +1062,7 @@ static int do_write(struct connection *conn, struct buffered_data *in)
 	} else {
 		node->data = in->buffer + offset;
 		node->datalen = datalen;
-		if (!write_node(conn, node))
+		if (xs_write_node(conn, node))
 			return errno;
 	}
 
@@ -1133,28 +1133,28 @@ static void memdel(void *mem, unsigned off, unsigned len, unsigned total)
 }
 
 
-static bool remove_child_entry(struct connection *conn, struct node *node,
-			       size_t offset)
+static int xs_remove_child_entry(struct connection *conn, struct node *node,
+			      size_t offset)
 {
 	size_t childlen = strlen(node->children + offset);
 	memdel(node->children, offset, childlen + 1, node->childlen);
 	node->childlen -= childlen + 1;
-	return write_node(conn, node);
+	return xs_write_node(conn, node);
 }
 
 
-static bool delete_child(struct connection *conn,
-			 struct node *node, const char *childname)
+static int xs_delete_child(struct connection *conn,
+			struct node *node, const char *childname)
 {
 	unsigned int i;
 
 	for (i = 0; i < node->childlen; i += strlen(node->children+i) + 1) {
 		if (streq(node->children+i, childname)) {
-			return remove_child_entry(conn, node, i);
+			return xs_remove_child_entry(conn, node, i);
 		}
 	}
 	corrupt(conn, "Can't find child '%s' in %s", childname, node->name);
-	return false;
+	return ENOENT;
 }
 
 
@@ -1174,7 +1174,7 @@ static int _rm(struct connection *conn, const void *ctx, struct node *node,
 	if (!parent)
 		return (errno == ENOMEM) ? ENOMEM : EINVAL;
 
-	if (!delete_child(conn, parent, basename(name)))
+	if (xs_delete_child(conn, parent, basename(name)))
 		return EINVAL;
 
 	delete_node(conn, node, true);
@@ -1278,7 +1278,7 @@ static int do_set_perms(struct connection *conn, struct buffered_data *in)
 	node->num_perms = num;
 	domain_entry_inc(conn, node);
 
-	if (!write_node(conn, node))
+	if (xs_write_node(conn, node))
 		return errno;
 
 	fire_watches(conn, in, name, false);
@@ -1538,7 +1538,7 @@ static void manual_node(const char *name, const char *child)
 	if (child)
 		node->childlen = strlen(child) + 1;
 
-	if (!write_node(NULL, node))
+	if (xs_write_node(NULL, node))
 		barf_perror("Could not create initial node %s", name);
 	talloc_free(node);
 }
@@ -1677,7 +1677,7 @@ static int check_store_(const char *name, struct hashtable *reachable)
 					    childname);
 
 					if (recovery) {
-						remove_child_entry(NULL, node,
+						xs_remove_child_entry(NULL, node,
 								   i);
 						i -= childlen + 1;
 					}
@@ -1699,7 +1699,7 @@ static int check_store_(const char *name, struct hashtable *reachable)
 				    childname);
 
 				if (recovery) {
-					remove_child_entry(NULL, node, i);
+					xs_remove_child_entry(NULL, node, i);
 					i -= childlen + 1;
 				}
 			} else {
-- 
2.10.2


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

[PATCH v3 2/4] xenstore: undo function rename

[Juergen Gross]

Undo the function rename done in previous patch.

Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
---
 tools/xenstore/xenstored_core.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
index 4bcaac0..ee4c9e1 100644
--- a/tools/xenstore/xenstored_core.c
+++ b/tools/xenstore/xenstored_core.c
@@ -433,7 +433,7 @@ static struct node *read_node(struct connection *conn, const void *ctx,
 	return node;
 }
 
-static int xs_write_node(struct connection *conn, struct node *node)
+static int write_node(struct connection *conn, struct node *node)
 {
 	/*
 	 * conn will be null when this is called from manual_node.
@@ -1022,7 +1022,7 @@ static struct node *create_node(struct connection *conn, const void *ctx,
 	/* We write out the nodes down, setting destructor in case
 	 * something goes wrong. */
 	for (i = node; i; i = i->parent) {
-		if (xs_write_node(conn, i)) {
+		if (write_node(conn, i)) {
 			domain_entry_dec(conn, i);
 			return NULL;
 		}
@@ -1062,7 +1062,7 @@ static int do_write(struct connection *conn, struct buffered_data *in)
 	} else {
 		node->data = in->buffer + offset;
 		node->datalen = datalen;
-		if (xs_write_node(conn, node))
+		if (write_node(conn, node))
 			return errno;
 	}
 
@@ -1133,24 +1133,24 @@ static void memdel(void *mem, unsigned off, unsigned len, unsigned total)
 }
 
 
-static int xs_remove_child_entry(struct connection *conn, struct node *node,
+static int remove_child_entry(struct connection *conn, struct node *node,
 			      size_t offset)
 {
 	size_t childlen = strlen(node->children + offset);
 	memdel(node->children, offset, childlen + 1, node->childlen);
 	node->childlen -= childlen + 1;
-	return xs_write_node(conn, node);
+	return write_node(conn, node);
 }
 
 
-static int xs_delete_child(struct connection *conn,
+static int delete_child(struct connection *conn,
 			struct node *node, const char *childname)
 {
 	unsigned int i;
 
 	for (i = 0; i < node->childlen; i += strlen(node->children+i) + 1) {
 		if (streq(node->children+i, childname)) {
-			return xs_remove_child_entry(conn, node, i);
+			return remove_child_entry(conn, node, i);
 		}
 	}
 	corrupt(conn, "Can't find child '%s' in %s", childname, node->name);
@@ -1174,7 +1174,7 @@ static int _rm(struct connection *conn, const void *ctx, struct node *node,
 	if (!parent)
 		return (errno == ENOMEM) ? ENOMEM : EINVAL;
 
-	if (xs_delete_child(conn, parent, basename(name)))
+	if (delete_child(conn, parent, basename(name)))
 		return EINVAL;
 
 	delete_node(conn, node, true);
@@ -1278,7 +1278,7 @@ static int do_set_perms(struct connection *conn, struct buffered_data *in)
 	node->num_perms = num;
 	domain_entry_inc(conn, node);
 
-	if (xs_write_node(conn, node))
+	if (write_node(conn, node))
 		return errno;
 
 	fire_watches(conn, in, name, false);
@@ -1538,7 +1538,7 @@ static void manual_node(const char *name, const char *child)
 	if (child)
 		node->childlen = strlen(child) + 1;
 
-	if (xs_write_node(NULL, node))
+	if (write_node(NULL, node))
 		barf_perror("Could not create initial node %s", name);
 	talloc_free(node);
 }
@@ -1677,7 +1677,7 @@ static int check_store_(const char *name, struct hashtable *reachable)
 					    childname);
 
 					if (recovery) {
-						xs_remove_child_entry(NULL, node,
+						remove_child_entry(NULL, node,
 								   i);
 						i -= childlen + 1;
 					}
@@ -1699,7 +1699,7 @@ static int check_store_(const char *name, struct hashtable *reachable)
 				    childname);
 
 				if (recovery) {
-					xs_remove_child_entry(NULL, node, i);
+					remove_child_entry(NULL, node, i);
 					i -= childlen + 1;
 				}
 			} else {
-- 
2.10.2


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

[PATCH v3 3/4] xenstore: rework of transaction handling

[Juergen Gross]

The handling of transactions in xenstored is rather clumsy today:

- Each transaction in progress is keeping a local copy of the complete
  xenstore data base
- A transaction will fail as soon as any node is being modified outside
  the transaction

This is leading to a very bad behavior in case of a large xenstore.
Memory consumption of xenstored is much higher than necessary and with
many domains up transactions failures will be more and more common.

Instead of keeping a complete copy of the data base for each
transaction store the transaction data in the same data base as the
normal xenstore entries prepended with the transaction in the single
nodes either read or modified. At the end of the transaction walk
through all nodes accessed and check for conflicting modifications.
In case no conflicts are found write all modified nodes to the data
base without transaction identifier.

Following tests have been performed:
- create/destroy of various domains, including HVM with ioemu-stubdom
  (xenstored and xenstore-stubdom)
- multiple concurrent runs of xs-test over several minutes
  (xenstored and xenstore-stubdom)
- test for memory leaks of xenstored by dumping talloc reports before
  and after the tests

Signed-off-by: Juergen Gross <jgross@suse.com>
---
 tools/xenstore/xenstored_core.c        | 151 ++++++------
 tools/xenstore/xenstored_core.h        |  17 +-
 tools/xenstore/xenstored_domain.c      |  24 +-
 tools/xenstore/xenstored_domain.h      |   2 +-
 tools/xenstore/xenstored_transaction.c | 429 +++++++++++++++++++++++++++------
 tools/xenstore/xenstored_transaction.h |  18 +-
 6 files changed, 469 insertions(+), 172 deletions(-)

diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
index ee4c9e1..f6571b0 100644
--- a/tools/xenstore/xenstored_core.c
+++ b/tools/xenstore/xenstored_core.c
@@ -81,9 +81,8 @@ static bool recovery = true;
 static int reopen_log_pipe[2];
 static int reopen_log_pipe0_pollfd_idx = -1;
 char *tracefile = NULL;
-static TDB_CONTEXT *tdb_ctx = NULL;
+TDB_CONTEXT *tdb_ctx = NULL;
 
-static void corrupt(struct connection *conn, const char *fmt, ...);
 static const char *sockmsg_string(enum xsd_sockmsg_type type);
 
 #define log(...)							\
@@ -105,24 +104,6 @@ int quota_nb_watch_per_domain = 128;
 int quota_max_entry_size = 2048; /* 2K */
 int quota_max_transaction = 10;
 
-TDB_CONTEXT *tdb_context(struct connection *conn)
-{
-	/* conn = NULL used in manual_node at setup. */
-	if (!conn || !conn->transaction)
-		return tdb_ctx;
-	return tdb_transaction_context(conn->transaction);
-}
-
-bool replace_tdb(const char *newname, TDB_CONTEXT *newtdb)
-{
-	if (!(tdb_ctx->flags & TDB_INTERNAL))
-		if (rename(newname, xs_daemon_tdb()) != 0)
-			return false;
-	tdb_close(tdb_ctx);
-	tdb_ctx = talloc_steal(talloc_autofree_context(), newtdb);
-	return true;
-}
-
 void trace(const char *fmt, ...)
 {
 	va_list arglist;
@@ -385,35 +366,38 @@ static struct node *read_node(struct connection *conn, const void *ctx,
 	TDB_DATA key, data;
 	struct xs_tdb_record_hdr *hdr;
 	struct node *node;
-	TDB_CONTEXT * context = tdb_context(conn);
 
-	key.dptr = (void *)name;
-	key.dsize = strlen(name);
-	data = tdb_fetch(context, key);
+	node = talloc(ctx, struct node);
+	if (!node) {
+		errno = ENOMEM;
+		return NULL;
+	}
+	node->name = talloc_strdup(node, name);
+	if (!node->name) {
+		talloc_free(node);
+		errno = ENOMEM;
+		return NULL;
+	}
+
+	if (transaction_prepend(conn, name, NODE_ACCESS_READ, &key))
+		return NULL;
+
+	data = tdb_fetch(tdb_ctx, key);
 
 	if (data.dptr == NULL) {
-		if (tdb_error(context) == TDB_ERR_NOEXIST)
+		if (tdb_error(tdb_ctx) == TDB_ERR_NOEXIST) {
+			node->generation = NO_GENERATION;
+			access_node(conn, node, NODE_ACCESS_READ, NULL);
 			errno = ENOENT;
-		else {
-			log("TDB error on read: %s", tdb_errorstr(context));
+		} else {
+			log("TDB error on read: %s", tdb_errorstr(tdb_ctx));
 			errno = EIO;
 		}
-		return NULL;
-	}
-
-	node = talloc(ctx, struct node);
-	if (!node) {
-		errno = ENOMEM;
-		return NULL;
-	}
-	node->name = talloc_strdup(node, name);
-	if (!node->name) {
 		talloc_free(node);
-		errno = ENOMEM;
 		return NULL;
 	}
+
 	node->parent = NULL;
-	node->tdb = tdb_context(conn);
 	talloc_steal(node, data.dptr);
 
 	/* Datalen, childlen, number of permissions */
@@ -430,32 +414,26 @@ static struct node *read_node(struct connection *conn, const void *ctx,
 	/* Children is strings, nul separated. */
 	node->children = node->data + node->datalen;
 
+	access_node(conn, node, NODE_ACCESS_READ, NULL);
+
 	return node;
 }
 
-static int write_node(struct connection *conn, struct node *node)
+int write_node_raw(struct connection *conn, TDB_DATA *key, struct node *node)
 {
-	/*
-	 * conn will be null when this is called from manual_node.
-	 * tdb_context copes with this.
-	 */
-
-	TDB_DATA key, data;
+	TDB_DATA data;
 	void *p;
 	struct xs_tdb_record_hdr *hdr;
 
-	key.dptr = (void *)node->name;
-	key.dsize = strlen(node->name);
-
 	data.dsize = sizeof(*hdr)
 		+ node->num_perms*sizeof(node->perms[0])
 		+ node->datalen + node->childlen;
 
-	if (domain_is_unprivileged(conn) && data.dsize >= quota_max_entry_size)
-		goto error;
-
-	add_change_node(conn, node, false);
-	wrl_apply_debit_direct(conn);
+	if (domain_is_unprivileged(conn) &&
+	    data.dsize >= quota_max_entry_size) {
+		errno = ENOSPC;
+		return errno;
+	}
 
 	data.dptr = talloc_size(node, data.dsize);
 	hdr = (void *)data.dptr;
@@ -471,14 +449,27 @@ static int write_node(struct connection *conn, struct node *node)
 	memcpy(p, node->children, node->childlen);
 
 	/* TDB should set errno, but doesn't even set ecode AFAICT. */
-	if (tdb_store(tdb_context(conn), key, data, TDB_REPLACE) != 0) {
-		corrupt(conn, "Write of %s failed", key.dptr);
-		goto error;
+	if (tdb_store(tdb_ctx, *key, data, TDB_REPLACE) != 0) {
+		corrupt(conn, "Write of %s failed", key->dptr);
+		errno = EIO;
+		return errno;
 	}
 	return 0;
- error:
-	errno = ENOSPC;
-	return errno;
+}
+
+static int write_node(struct connection *conn, struct node *node)
+{
+	/*
+	 * conn will be null when this is called from manual_node.
+	 * tdb_context copes with this.
+	 */
+
+	TDB_DATA key;
+
+	if (access_node(conn, node, NODE_ACCESS_WRITE, &key))
+		return errno;
+
+	return write_node_raw(conn, &key, node);
 }
 
 static enum xs_perm_type perm_for_conn(struct connection *conn,
@@ -900,24 +891,18 @@ static int do_read(struct connection *conn, struct buffered_data *in)
 	return 0;
 }
 
-static void delete_node_single(struct connection *conn, struct node *node,
-			       bool changed)
+static void delete_node_single(struct connection *conn, struct node *node)
 {
 	TDB_DATA key;
 
-	key.dptr = (void *)node->name;
-	key.dsize = strlen(node->name);
+	if (access_node(conn, node, NODE_ACCESS_DELETE, &key))
+		return;
 
-	if (tdb_delete(tdb_context(conn), key) != 0) {
+	if (tdb_delete(tdb_ctx, key) != 0) {
 		corrupt(conn, "Could not delete '%s'", node->name);
 		return;
 	}
 
-	if (changed) {
-		add_change_node(conn, node, true);
-		wrl_apply_debit_direct(conn);
-	}
-
 	domain_entry_dec(conn, node);
 }
 
@@ -965,7 +950,6 @@ static struct node *construct_node(struct connection *conn, const void *ctx,
 	node = talloc(ctx, struct node);
 	if (!node)
 		goto nomem;
-	node->tdb = tdb_context(conn);
 	node->name = talloc_strdup(node, name);
 	if (!node->name)
 		goto nomem;
@@ -1002,7 +986,7 @@ static int destroy_node(void *_node)
 	key.dptr = (void *)node->name;
 	key.dsize = strlen(node->name);
 
-	tdb_delete(node->tdb, key);
+	tdb_delete(tdb_ctx, key);
 	return 0;
 }
 
@@ -1095,8 +1079,7 @@ static int do_mkdir(struct connection *conn, struct buffered_data *in)
 	return 0;
 }
 
-static void delete_node(struct connection *conn, struct node *node,
-			bool changed)
+static void delete_node(struct connection *conn, struct node *node)
 {
 	unsigned int i;
 	char *name;
@@ -1104,7 +1087,7 @@ static void delete_node(struct connection *conn, struct node *node,
 	/* Delete self, then delete children.  If we crash, then the worst
 	   that can happen is the children will continue to take up space, but
 	   will otherwise be unreachable. */
-	delete_node_single(conn, node, changed);
+	delete_node_single(conn, node);
 
 	/* Delete children, too. */
 	for (i = 0; i < node->childlen; i += strlen(node->children+i) + 1) {
@@ -1114,7 +1097,7 @@ static void delete_node(struct connection *conn, struct node *node,
 				       node->children + i);
 		child = name ? read_node(conn, node, name) : NULL;
 		if (child) {
-			delete_node(conn, child, false);
+			delete_node(conn, child);
 		}
 		else {
 			trace("delete_node: Error deleting child '%s/%s'!\n",
@@ -1177,7 +1160,7 @@ static int _rm(struct connection *conn, const void *ctx, struct node *node,
 	if (delete_child(conn, parent, basename(name)))
 		return EINVAL;
 
-	delete_node(conn, node, true);
+	delete_node(conn, node);
 	return 0;
 }
 
@@ -1617,8 +1600,9 @@ static char *child_name(const char *s1, const char *s2)
 }
 
 
-static int remember_string(struct hashtable *hash, const char *str)
+static int remember_string(void *par, const char *str)
 {
+	struct hashtable *hash = par;
 	char *k = malloc(strlen(str) + 1);
 
 	if (!k)
@@ -1737,6 +1721,7 @@ static int clean_store_(TDB_CONTEXT *tdb, TDB_DATA key, TDB_DATA val,
 			void *private)
 {
 	struct hashtable *reachable = private;
+	char *slash;
 	char * name = talloc_strndup(NULL, key.dptr, key.dsize);
 
 	if (!name) {
@@ -1744,6 +1729,11 @@ static int clean_store_(TDB_CONTEXT *tdb, TDB_DATA key, TDB_DATA val,
 		return 1;
 	}
 
+	if (name[0] != '/') {
+		slash = strchr(name, '/');
+		if (slash)
+			*slash = 0;
+	}
 	if (!hashtable_search(reachable, name)) {
 		log("clean_store: '%s' is orphaned!", name);
 		if (recovery) {
@@ -1779,7 +1769,8 @@ void check_store(void)
 	}
 
 	log("Checking store ...");
-	if (!check_store_(root, reachable))
+	if (!check_store_(root, reachable) &&
+	    !check_transactions(remember_string, reachable))
 		clean_store(reachable);
 	log("Checking store complete.");
 
@@ -1790,7 +1781,7 @@ void check_store(void)
 
 
 /* Something is horribly wrong: check the store. */
-static void corrupt(struct connection *conn, const char *fmt, ...)
+void corrupt(struct connection *conn, const char *fmt, ...)
 {
 	va_list arglist;
 	char *str;
diff --git a/tools/xenstore/xenstored_core.h b/tools/xenstore/xenstored_core.h
index 0580827..1183adb 100644
--- a/tools/xenstore/xenstored_core.h
+++ b/tools/xenstore/xenstored_core.h
@@ -113,14 +113,12 @@ extern struct list_head connections;
 struct node {
 	const char *name;
 
-	/* Database I came from */
-	TDB_CONTEXT *tdb;
-
 	/* Parent (optional) */
 	struct node *parent;
 
 	/* Generation count. */
 	uint64_t generation;
+#define NO_GENERATION ~((uint64_t)0)
 
 	/* Permissions. */
 	unsigned int num_perms;
@@ -151,20 +149,18 @@ void send_ack(struct connection *conn, enum xsd_sockmsg_type type);
 /* Canonicalize this path if possible. */
 char *canonicalize(struct connection *conn, const void *ctx, const char *node);
 
+/* Write a node to the tdb data base. */
+int write_node_raw(struct connection *conn, TDB_DATA *key, struct node *node);
+
 /* Get this node, checking we have permissions. */
 struct node *get_node(struct connection *conn,
 		      const void *ctx,
 		      const char *name,
 		      enum xs_perm_type perm);
 
-/* Get TDB context for this connection */
-TDB_CONTEXT *tdb_context(struct connection *conn);
-
-/* Replace the tdb: required for transaction code */
-bool replace_tdb(const char *newname, TDB_CONTEXT *newtdb);
-
 struct connection *new_connection(connwritefn_t *write, connreadfn_t *read);
 void check_store(void);
+void corrupt(struct connection *conn, const char *fmt, ...);
 
 /* Is this a valid node name? */
 bool is_valid_nodename(const char *node);
@@ -179,9 +175,12 @@ void close_log(void);
 
 extern char *tracefile;
 extern int tracefd;
+
+extern TDB_CONTEXT *tdb_ctx;
 extern int dom0_domid;
 extern int dom0_event;
 extern int priv_domid;
+extern int quota_nb_entry_per_domain;
 
 /* Map the kernel's xenstore page. */
 void *xenbus_map(void);
diff --git a/tools/xenstore/xenstored_domain.c b/tools/xenstore/xenstored_domain.c
index 6af219e..4edd14e 100644
--- a/tools/xenstore/xenstored_domain.c
+++ b/tools/xenstore/xenstored_domain.c
@@ -268,9 +268,15 @@ bool domain_can_read(struct connection *conn)
 	return (intf->req_cons != intf->req_prod);
 }
 
+static bool domid_is_unprivileged(unsigned int domid)
+{
+	return domid != 0 && domid != priv_domid;
+}
+
 bool domain_is_unprivileged(struct connection *conn)
 {
-	return (conn && conn->domain && conn->domain->domid != 0 && conn->domain->domid != priv_domid);
+	return conn && conn->domain &&
+	       domid_is_unprivileged(conn->domain->domid);
 }
 
 bool domain_can_write(struct connection *conn)
@@ -699,13 +705,23 @@ void domain_entry_dec(struct connection *conn, struct node *node)
 	}
 }
 
-void domain_entry_fix(unsigned int domid, int num)
+int domain_entry_fix(unsigned int domid, int num, bool update)
 {
 	struct domain *d;
+	int cnt;
 
 	d = find_domain_by_domid(domid);
-	if (d && ((d->nbentry += num) < 0))
-		d->nbentry = 0;
+	if (!d)
+		return 0;
+
+	cnt = d->nbentry + num;
+	if (cnt < 0)
+		cnt = 0;
+
+	if (update)
+		d->nbentry = cnt;
+
+	return domid_is_unprivileged(domid) ? cnt : 0;
 }
 
 int domain_entry(struct connection *conn)
diff --git a/tools/xenstore/xenstored_domain.h b/tools/xenstore/xenstored_domain.h
index 4aa80db..56ae015 100644
--- a/tools/xenstore/xenstored_domain.h
+++ b/tools/xenstore/xenstored_domain.h
@@ -59,7 +59,7 @@ bool domain_is_unprivileged(struct connection *conn);
 /* Quota manipulation */
 void domain_entry_inc(struct connection *conn, struct node *);
 void domain_entry_dec(struct connection *conn, struct node *);
-void domain_entry_fix(unsigned int domid, int num);
+int domain_entry_fix(unsigned int domid, int num, bool update);
 int domain_entry(struct connection *conn);
 void domain_watch_inc(struct connection *conn);
 void domain_watch_dec(struct connection *conn);
diff --git a/tools/xenstore/xenstored_transaction.c b/tools/xenstore/xenstored_transaction.c
index a01f8cf..c54fe75 100644
--- a/tools/xenstore/xenstored_transaction.c
+++ b/tools/xenstore/xenstored_transaction.c
@@ -16,6 +16,7 @@
     along with this program; If not, see <http://www.gnu.org/licenses/>.
 */
 
+#include <inttypes.h>
 #include <stdio.h>
 #include <sys/types.h>
 #include <sys/stat.h>
@@ -35,7 +36,74 @@
 #include "xenstore_lib.h"
 #include "utils.h"
 
-struct changed_node
+/*
+ * Some notes regarding detection and handling of transaction conflicts:
+ *
+ * Basic source of reference is the 'generation' count. Each writing access
+ * (either normal write or in a transaction) to the tdb data base will set
+ * the node specific generation count to the global generation count.
+ * For being able to identify a transaction the transaction specific generation
+ * count is initialized with the global generation count when starting the
+ * transaction.
+ * Each time the global generation count is copied to either a node or a
+ * transaction it is incremented. This ensures all nodes and/or transactions
+ * are having a unique generation count.
+ *
+ * Transaction conflicts are detected by checking the generation count of all
+ * nodes read in the transaction to match with the generation count in the
+ * global data base at the end of the transaction. Nodes which have been
+ * modified in the transaction don't have to be checked to match even if they
+ * have been read, as the modified node will be globally visible after the
+ * succeeded transaction possibly overwriting another modification which may
+ * have occurred concurrent to the transaction.
+ *
+ * Examples:
+ * ---------
+ * The following notation is used:
+ * I:      initial state
+ * G       global generation count
+ * g(X)    generation count of node X
+ * G(1)    generation count of transaction 1
+ * g(1:Y)  saved generation count of node Y in transaction 1
+ * TA1:    operation in transaction 1
+ * X=1:X   replace global node X with transaction 1 specific value of X
+ *
+ * 1. Simple transaction doing: read node A, write node B
+ *    I: g(A) = 1, g(B) = 2, G = 3
+ *    Start transaction 1: G(1) = 3, G = 4
+ *    TA1: read node A:    g(1:A) = 1
+ *    TA1: write node B:   g(1:B) = 4, G = 5
+ *    End TA1: g(1:A) == g(A) => okay, B = 1:B, g(B) = 5, G = 6
+ *
+ * 2. Transaction with conflicting write
+ *    I: g(A) = 1, g(B) = 2, G = 3
+ *    Start transaction 1: G(1) = 3, G = 4
+ *    TA1: read node A:    g(1:A) = 1
+ *    write node A:        g(A) = 4, G = 5
+ *    TA1: write node B:   g(1:B) = 5, G = 6
+ *    End TA1: g(1:A) != g(A) => EAGAIN
+ *
+ * 3. Transaction with conflicting delete
+ *    I: g(A) = 1, g(B) = 2, G = 3
+ *    Start transaction 1: G(1) = 3, G = 4
+ *    TA1: read node A:    g(1:A) = 1
+ *    delete node A:       g(A) = ~0
+ *    TA1: write node B:   g(1:B) = 4, G = 5
+ *    End TA1: g(1:A) != g(A) => EAGAIN
+ *
+ * 4. Two interfering transactions
+ *    I: g(A) = 1, g(B) = 2, G = 3
+ *    Start transaction 1: G(1) = 3, G = 4
+ *    Start transaction 2: G(2) = 4, G = 5
+ *    TA1: read node A:    g(1:A) = 1
+ *    TA2: read node B:    g(2:B) = 2
+ *    TA1: write node B:   g(1:B) = 5, G = 6
+ *    TA2: write node A:   g(2:A) = 6, G = 7
+ *    End TA1: g(1:A) == g(A) => okay, B = 1:B, g(B) = 7, G = 8
+ *    End TA2: g(2:B) != g(B) => EAGAIN
+ */
+
+struct accessed_node
 {
 	/* List of all changed nodes in the context of this transaction. */
 	struct list_head list;
@@ -43,8 +111,17 @@ struct changed_node
 	/* The name of the node. */
 	char *node;
 
-	/* And the children? (ie. rm) */
-	bool recurse;
+	/* Generation count (or NO_GENERATION) for conflict checking. */
+	uint64_t generation;
+
+	/* Generation count checking required? */
+	bool check_gen;
+
+	/* Modified? */
+	bool modified;
+
+	/* Transaction node in data base? */
+	bool ta_node;
 };
 
 struct changed_domain
@@ -70,80 +147,265 @@ struct transaction
 	/* Generation when transaction started. */
 	uint64_t generation;
 
-	/* Transaction internal generation. */
-	uint64_t trans_gen;
-
-	/* TDB to work on, and filename */
-	TDB_CONTEXT *tdb;
-	char *tdb_name;
-
-	/* List of changed nodes. */
-	struct list_head changes;
+	/* List of accessed nodes. */
+	struct list_head accessed;
 
 	/* List of changed domains - to record the changed domain entry number */
 	struct list_head changed_domains;
+
+	/* Flag for letting transaction fail. */
+	bool fail;
 };
 
 extern int quota_max_transaction;
 static uint64_t generation;
 
-/* Return tdb context to use for this connection. */
-TDB_CONTEXT *tdb_transaction_context(struct transaction *trans)
+static void set_tdb_key(const char *name, TDB_DATA *key)
 {
-	return trans->tdb;
+	key->dptr = (char *)name;
+	key->dsize = strlen(name);
 }
 
-/* Callers get a change node (which can fail) and only commit after they've
- * finished.  This way they don't have to unwind eg. a write. */
-void add_change_node(struct connection *conn, struct node *node, bool recurse)
+static struct accessed_node *find_accessed_node(struct transaction *trans,
+						const char *name)
 {
-	struct changed_node *i;
+	struct accessed_node *i;
+
+	list_for_each_entry(i, &trans->accessed, list)
+		if (streq(i->node, name))
+			return i;
+
+	return NULL;
+}
+
+static char *transaction_get_node_name(void *ctx, struct transaction *trans,
+				       const char *name)
+{
+	return talloc_asprintf(ctx, "%"PRIu64"/%s", trans->generation, name);
+}
+
+/*
+ * Prepend the transaction to name if appropriate.
+ * In a transaction the transaction will be prepended for:
+ * - write accesses
+ * - read accesses of a node which has been written already in
+ *   the same transaction
+ */
+int transaction_prepend(struct connection *conn, const char *name,
+			enum node_access_type type, TDB_DATA *key)
+{
+	char *tdb_name;
+
+	if (!conn || !conn->transaction ||
+	    (type == NODE_ACCESS_READ &&
+	     !find_accessed_node(conn->transaction, name))) {
+		set_tdb_key(name, key);
+		return 0;
+	}
+
+	tdb_name = transaction_get_node_name(conn->transaction,
+					     conn->transaction, name);
+	if (!tdb_name)
+		return errno;
+
+	set_tdb_key(tdb_name, key);
+
+	return 0;
+}
+
+/*
+ * A node has been accessed.
+ *
+ * Modifying accesses (write, delete) always update the generation (global and
+ * node->generation).
+ *
+ * Accesses in a transaction will be added to the list of accessed nodes
+ * if not already done. Read type accesses will copy the node to the
+ * transaction specific data base part, write type accesses go there
+ * anyway.
+ *
+ * If not NULL, key will be supplied with name and length of name of the node
+ * to be accessed in the data base.
+ */
+int access_node(struct connection *conn, struct node *node,
+		enum node_access_type type, TDB_DATA *key)
+{
+	struct accessed_node *i = NULL;
 	struct transaction *trans;
+	TDB_DATA local_key;
+	const char *trans_name = NULL;
+	int ret;
+	bool introduce = false;
 
-	if (!conn || !conn->transaction) {
+	if (type != NODE_ACCESS_READ) {
 		/* They're changing the global database. */
 		node->generation = generation++;
-		return;
+		if (conn && !conn->transaction)
+			wrl_apply_debit_direct(conn);
+	}
+
+	if (!conn || !conn->transaction) {
+		if (key)
+			set_tdb_key(node->name, key);
+		return 0;
 	}
 
 	trans = conn->transaction;
 
-	node->generation = generation + trans->trans_gen++;
+	trans_name = transaction_get_node_name(node, trans, node->name);
+	if (!trans_name)
+		goto nomem;
 
-	list_for_each_entry(i, &trans->changes, list) {
-		if (streq(i->node, node->name)) {
-			if (recurse)
-				i->recurse = recurse;
-			return;
-		}
-	}
-
-	i = talloc(trans, struct changed_node);
+	i = find_accessed_node(trans, node->name);
 	if (!i) {
-		/* All we can do is let the transaction fail. */
-		generation++;
-		return;
+		i = talloc_zero(trans, struct accessed_node);
+		if (!i)
+			goto nomem;
+		i->node = talloc_strdup(i, node->name);
+		if (!i->node)
+			goto nomem;
+
+		introduce = true;
+		i->ta_node = false;
+
+		/* We have to verify read nodes only if we didn't write them. */
+		if (type == NODE_ACCESS_READ) {
+			i->generation = node->generation;
+			i->check_gen = true;
+			if (node->generation != NO_GENERATION) {
+				set_tdb_key(trans_name, &local_key);
+				ret = write_node_raw(conn, &local_key, node);
+				if (ret)
+					goto err;
+				i->ta_node = true;
+			}
+		}
+		list_add_tail(&i->list, &trans->accessed);
 	}
-	i->node = talloc_strdup(i, node->name);
-	if (!i->node) {
-		/* All we can do is let the transaction fail. */
-		generation++;
+
+	if (type != NODE_ACCESS_READ)
+		i->modified = true;
+
+	if (introduce && type == NODE_ACCESS_DELETE)
+		/* Nothing to delete. */
+		return -1;
+
+	if (key) {
+		set_tdb_key(trans_name, key);
+		if (type == NODE_ACCESS_WRITE)
+			i->ta_node = true;
+		if (type == NODE_ACCESS_DELETE)
+			i->ta_node = false;
+	}
+
+	return 0;
+
+nomem:
+	ret = ENOMEM;
+err:
+	talloc_free((void *)trans_name);
+	talloc_free(i);
+	trans->fail = true;
+	errno = ret;
+	return ret;
+}
+
+/*
+ * Finalize transaction:
+ * Walk through accessed nodes and check generation against global data.
+ * If all entries match, read the transaction entries and write them without
+ * transaction prepended. Delete all transaction specific nodes in the data
+ * base.
+ */
+static int finalize_transaction(struct connection *conn,
+				struct transaction *trans)
+{
+	struct accessed_node *i;
+	TDB_DATA key, ta_key, data;
+	struct xs_tdb_record_hdr *hdr;
+	uint64_t gen;
+	char *trans_name;
+	int ret;
+
+	list_for_each_entry(i, &trans->accessed, list) {
+		if (!i->check_gen)
+			continue;
+
+		set_tdb_key(i->node, &key);
+		data = tdb_fetch(tdb_ctx, key);
+		hdr = (void *)data.dptr;
+		if (!data.dptr) {
+			if (tdb_error(tdb_ctx) != TDB_ERR_NOEXIST)
+				return EIO;
+			gen = NO_GENERATION;
+		} else
+			gen = hdr->generation;
+		talloc_free(data.dptr);
+		if (i->generation != gen)
+			return EAGAIN;
+	}
+
+	while ((i = list_top(&trans->accessed, struct accessed_node, list))) {
+		trans_name = transaction_get_node_name(i, trans, i->node);
+		if (!trans_name)
+			/* We are doomed: the transaction is only partial. */
+			goto err;
+
+		set_tdb_key(trans_name, &ta_key);
+
+		if (i->modified) {
+			set_tdb_key(i->node, &key);
+			if (i->ta_node) {
+				data = tdb_fetch(tdb_ctx, ta_key);
+				if (!data.dptr)
+					goto err;
+				hdr = (void *)data.dptr;
+				hdr->generation = generation++;
+				ret = tdb_store(tdb_ctx, key, data,
+						TDB_REPLACE);
+				talloc_free(data.dptr);
+				if (ret)
+					goto err;
+			} else if (tdb_delete(tdb_ctx, key))
+					goto err;
+			fire_watches(conn, trans, i->node, false);
+		}
+
+		if (i->ta_node && tdb_delete(tdb_ctx, ta_key))
+			goto err;
+		list_del(&i->list);
 		talloc_free(i);
-		return;
 	}
-	i->recurse = recurse;
-	list_add_tail(&i->list, &trans->changes);
+
+	return 0;
+
+err:
+	corrupt(conn, "Partial transaction");
+	return EIO;
 }
 
 static int destroy_transaction(void *_transaction)
 {
 	struct transaction *trans = _transaction;
+	struct accessed_node *i;
+	char *trans_name;
+	TDB_DATA key;
 
 	wrl_ntransactions--;
 	trace_destroy(trans, "transaction");
-	if (trans->tdb)
-		tdb_close(trans->tdb);
-	unlink(trans->tdb_name);
+	while ((i = list_top(&trans->accessed, struct accessed_node, list))) {
+		if (i->ta_node) {
+			trans_name = transaction_get_node_name(i, trans,
+							       i->node);
+			if (trans_name) {
+				set_tdb_key(trans_name, &key);
+				tdb_delete(tdb_ctx, key);
+			}
+		}
+		list_del(&i->list);
+		talloc_free(i);
+	}
+
 	return 0;
 }
 
@@ -178,18 +440,10 @@ int do_transaction_start(struct connection *conn, struct buffered_data *in)
 	if (!trans)
 		return ENOMEM;
 
-	INIT_LIST_HEAD(&trans->changes);
+	INIT_LIST_HEAD(&trans->accessed);
 	INIT_LIST_HEAD(&trans->changed_domains);
-	trans->generation = generation;
-	trans->tdb_name = talloc_asprintf(trans, "%s.%p",
-					  xs_daemon_tdb(), trans);
-	if (!trans->tdb_name)
-		return ENOMEM;
-	trans->tdb = tdb_copy(tdb_context(conn), trans->tdb_name);
-	if (!trans->tdb)
-		return errno;
-	/* Make it close if we go away. */
-	talloc_steal(trans, trans->tdb);
+	trans->fail = false;
+	trans->generation = generation++;
 
 	/* Pick an unused transaction identifier. */
 	do {
@@ -210,12 +464,25 @@ int do_transaction_start(struct connection *conn, struct buffered_data *in)
 	return 0;
 }
 
+static int transaction_fix_domains(struct transaction *trans, bool update)
+{
+	struct changed_domain *d;
+	int cnt;
+
+	list_for_each_entry(d, &trans->changed_domains, list) {
+		cnt = domain_entry_fix(d->domid, d->nbentry, update);
+		if (!update && cnt >= quota_nb_entry_per_domain)
+			return ENOSPC;
+	}
+
+	return 0;
+}
+
 int do_transaction_end(struct connection *conn, struct buffered_data *in)
 {
 	const char *arg = onearg(in);
-	struct changed_node *i;
-	struct changed_domain *d;
 	struct transaction *trans;
+	int ret;
 
 	if (!arg || (!streq(arg, "T") && !streq(arg, "F")))
 		return EINVAL;
@@ -231,25 +498,18 @@ int do_transaction_end(struct connection *conn, struct buffered_data *in)
 	talloc_steal(in, trans);
 
 	if (streq(arg, "T")) {
-		/* FIXME: Merge, rather failing on any change. */
-		if (trans->generation != generation)
+		if (trans->fail)
+			return ENOMEM;
+		ret = transaction_fix_domains(trans, false);
+		if (ret)
+			return ret;
+		if (finalize_transaction(conn, trans))
 			return EAGAIN;
 
 		wrl_apply_debit_trans_commit(conn);
 
-		if (!replace_tdb(trans->tdb_name, trans->tdb))
-			return errno;
-		/* Don't close this: we won! */
-		trans->tdb = NULL;
-
 		/* fix domain entry for each changed domain */
-		list_for_each_entry(d, &trans->changed_domains, list)
-			domain_entry_fix(d->domid, d->nbentry);
-
-		/* Fire off the watches for everything that changed. */
-		list_for_each_entry(i, &trans->changes, list)
-			fire_watches(conn, in, i->node, i->recurse);
-		generation += trans->trans_gen;
+		transaction_fix_domains(trans, true);
 	}
 	send_ack(conn, XS_TRANSACTION_END);
 
@@ -269,7 +529,7 @@ void transaction_entry_inc(struct transaction *trans, unsigned int domid)
 	d = talloc(trans, struct changed_domain);
 	if (!d) {
 		/* Let the transaction fail. */
-		generation++;
+		trans->fail = true;
 		return;
 	}
 	d->domid = domid;
@@ -290,7 +550,7 @@ void transaction_entry_dec(struct transaction *trans, unsigned int domid)
 	d = talloc(trans, struct changed_domain);
 	if (!d) {
 		/* Let the transaction fail. */
-		generation++;
+		trans->fail = true;
 		return;
 	}
 	d->domid = domid;
@@ -313,6 +573,29 @@ void conn_delete_all_transactions(struct connection *conn)
 	conn->transaction_started = 0;
 }
 
+int check_transactions(int (*func)(void *, const char *), void *par)
+{
+	struct connection *conn;
+	struct transaction *trans;
+	char *tname;
+
+	list_for_each_entry(conn, &connections, list) {
+		list_for_each_entry(trans, &conn->transaction_list, list) {
+			tname = talloc_asprintf(trans, "%"PRIu64,
+						trans->generation);
+			if (!tname)
+				return ENOMEM;
+
+			if (!func(par, tname))
+				return ENOMEM;
+
+			talloc_free(tname);
+		}
+	}
+
+	return 0;
+}
+
 /*
  * Local variables:
  *  c-file-style: "linux"
diff --git a/tools/xenstore/xenstored_transaction.h b/tools/xenstore/xenstored_transaction.h
index aeeac3d..cfaca69 100644
--- a/tools/xenstore/xenstored_transaction.h
+++ b/tools/xenstore/xenstored_transaction.h
@@ -19,6 +19,12 @@
 #define _XENSTORED_TRANSACTION_H
 #include "xenstored_core.h"
 
+enum node_access_type {
+    NODE_ACCESS_READ,
+    NODE_ACCESS_WRITE,
+    NODE_ACCESS_DELETE
+};
+
 struct transaction;
 
 int do_transaction_start(struct connection *conn, struct buffered_data *node);
@@ -30,13 +36,15 @@ struct transaction *transaction_lookup(struct connection *conn, uint32_t id);
 void transaction_entry_inc(struct transaction *trans, unsigned int domid);
 void transaction_entry_dec(struct transaction *trans, unsigned int domid);
 
-/* This node was changed. */
-void add_change_node(struct connection *conn, struct node *node,
-                     bool recurse);
+/* This node was accessed. */
+int access_node(struct connection *conn, struct node *node,
+                enum node_access_type type, TDB_DATA *key);
 
-/* Return tdb context to use for this connection. */
-TDB_CONTEXT *tdb_transaction_context(struct transaction *trans);
+/* Prepend the transaction to name if appropriate. */
+int transaction_prepend(struct connection *conn, const char *name,
+                        enum node_access_type type, TDB_DATA *key);
 
 void conn_delete_all_transactions(struct connection *conn);
+int check_transactions(int (*func)(void *, const char *), void *par);
 
 #endif /* _XENSTORED_TRANSACTION_H */
-- 
2.10.2


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

Re: [PATCH v3 3/4] xenstore: rework of transaction handling

[Wei Liu]

On Tue, Mar 28, 2017 at 06:26:14PM +0200, Juergen Gross wrote:
[...]
> +
> +static char *transaction_get_node_name(void *ctx, struct transaction *trans,
> +				       const char *name)
> +{
> +	return talloc_asprintf(ctx, "%"PRIu64"/%s", trans->generation, name);
> +}
> +
> +/*
> + * Prepend the transaction to name if appropriate.
> + * In a transaction the transaction will be prepended for:
> + * - write accesses
> + * - read accesses of a node which has been written already in
> + *   the same transaction
> + */
> +int transaction_prepend(struct connection *conn, const char *name,
> +			enum node_access_type type, TDB_DATA *key)

In the code this function is only ever called with type==READ, so either
you miss one callsite for write access or the comment is wrong.

I would like to hear from you before continuing.

Wei.

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

Re: [PATCH v3 3/4] xenstore: rework of transaction handling

[Juergen Gross]

On 30/03/17 13:17, Wei Liu wrote:
> On Tue, Mar 28, 2017 at 06:26:14PM +0200, Juergen Gross wrote:
> [...]
>> +
>> +static char *transaction_get_node_name(void *ctx, struct transaction *trans,
>> +				       const char *name)
>> +{
>> +	return talloc_asprintf(ctx, "%"PRIu64"/%s", trans->generation, name);
>> +}
>> +
>> +/*
>> + * Prepend the transaction to name if appropriate.
>> + * In a transaction the transaction will be prepended for:
>> + * - write accesses
>> + * - read accesses of a node which has been written already in
>> + *   the same transaction
>> + */
>> +int transaction_prepend(struct connection *conn, const char *name,
>> +			enum node_access_type type, TDB_DATA *key)
> 
> In the code this function is only ever called with type==READ, so either
> you miss one callsite for write access or the comment is wrong.
> 
> I would like to hear from you before continuing.

This is a artefact of an earlier version. Now the write and delete
case are handled directly in access_node(). I will remove the access
type in V4.


Juergen

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

Re: [PATCH v3 3/4] xenstore: rework of transaction handling

[Wei Liu]

On Thu, Mar 30, 2017 at 02:36:15PM +0200, Juergen Gross wrote:
> On 30/03/17 13:17, Wei Liu wrote:
> > On Tue, Mar 28, 2017 at 06:26:14PM +0200, Juergen Gross wrote:
> > [...]
> >> +
> >> +static char *transaction_get_node_name(void *ctx, struct transaction *trans,
> >> +				       const char *name)
> >> +{
> >> +	return talloc_asprintf(ctx, "%"PRIu64"/%s", trans->generation, name);
> >> +}
> >> +
> >> +/*
> >> + * Prepend the transaction to name if appropriate.
> >> + * In a transaction the transaction will be prepended for:
> >> + * - write accesses
> >> + * - read accesses of a node which has been written already in
> >> + *   the same transaction
> >> + */
> >> +int transaction_prepend(struct connection *conn, const char *name,
> >> +			enum node_access_type type, TDB_DATA *key)
> > 
> > In the code this function is only ever called with type==READ, so either
> > you miss one callsite for write access or the comment is wrong.
> > 
> > I would like to hear from you before continuing.
> 
> This is a artefact of an earlier version. Now the write and delete
> case are handled directly in access_node(). I will remove the access
> type in V4.
> 

Please send an updated version. Sending just this one is OK.

> 
> Juergen

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

[PATCH v3 4/4] xenstore: cleanup tdb.c

[Juergen Gross]

Remove all unused functions from tdb.c. This will reduce code size of
xenstored and - more important - of xenstore stubdom.

tdb.c hasn't been updated to a newer version since its introduction in
2005. Any backport of bug fixes or update to a new version will need
major work, so there is no real downside to remove not needed code.

Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
---
 tools/xenstore/tdb.c | 439 +--------------------------------------------------
 tools/xenstore/tdb.h |  22 ---
 2 files changed, 1 insertion(+), 460 deletions(-)

diff --git a/tools/xenstore/tdb.c b/tools/xenstore/tdb.c
index e26900e..29593b7 100644
--- a/tools/xenstore/tdb.c
+++ b/tools/xenstore/tdb.c
@@ -103,7 +103,7 @@
 #endif
 
 #define BUCKET(hash) ((hash) % tdb->header.hash_size)
-TDB_DATA tdb_null;
+static TDB_DATA tdb_null;
 
 /* all contexts, to ensure no double-opens (fcntl locks don't nest!) */
 static TDB_CONTEXT *tdbs = NULL;
@@ -510,108 +510,6 @@ static int update_tailer(TDB_CONTEXT *tdb, tdb_off offset,
 			 &totalsize);
 }
 
-static tdb_off tdb_dump_record(TDB_CONTEXT *tdb, tdb_off offset)
-{
-	struct list_struct rec;
-	tdb_off tailer_ofs, tailer;
-
-	if (tdb_read(tdb, offset, (char *)&rec, sizeof(rec), DOCONV()) == -1) {
-		printf("ERROR: failed to read record at %u\n", offset);
-		return 0;
-	}
-
-	printf(" rec: offset=0x%08x next=0x%08x rec_len=%d key_len=%d data_len=%d full_hash=0x%x magic=0x%x\n",
-	       offset, rec.next, rec.rec_len, rec.key_len, rec.data_len, rec.full_hash, rec.magic);
-
-	tailer_ofs = offset + sizeof(rec) + rec.rec_len - sizeof(tdb_off);
-	if (ofs_read(tdb, tailer_ofs, &tailer) == -1) {
-		printf("ERROR: failed to read tailer at %u\n", tailer_ofs);
-		return rec.next;
-	}
-
-	if (tailer != rec.rec_len + sizeof(rec)) {
-		printf("ERROR: tailer does not match record! tailer=%u totalsize=%u\n",
-				(unsigned int)tailer, (unsigned int)(rec.rec_len + sizeof(rec)));
-	}
-	return rec.next;
-}
-
-static int tdb_dump_chain(TDB_CONTEXT *tdb, int i)
-{
-	tdb_off rec_ptr, top;
-
-	top = TDB_HASH_TOP(i);
-
-	if (tdb_lock(tdb, i, F_WRLCK) != 0)
-		return -1;
-
-	if (ofs_read(tdb, top, &rec_ptr) == -1)
-		return tdb_unlock(tdb, i, F_WRLCK);
-
-	if (rec_ptr)
-		printf("hash=%d\n", i);
-
-	while (rec_ptr) {
-		rec_ptr = tdb_dump_record(tdb, rec_ptr);
-	}
-
-	return tdb_unlock(tdb, i, F_WRLCK);
-}
-
-void tdb_dump_all(TDB_CONTEXT *tdb)
-{
-	unsigned int i;
-	for (i=0;i<tdb->header.hash_size;i++) {
-		tdb_dump_chain(tdb, i);
-	}
-	printf("freelist:\n");
-	tdb_dump_chain(tdb, -1);
-}
-
-int tdb_printfreelist(TDB_CONTEXT *tdb)
-{
-	int ret;
-	long total_free = 0;
-	tdb_off offset, rec_ptr;
-	struct list_struct rec;
-
-	if ((ret = tdb_lock(tdb, -1, F_WRLCK)) != 0)
-		return ret;
-
-	offset = FREELIST_TOP;
-
-	/* read in the freelist top */
-	if (ofs_read(tdb, offset, &rec_ptr) == -1) {
-		tdb_unlock(tdb, -1, F_WRLCK);
-		return 0;
-	}
-
-	printf("freelist top=[0x%08x]\n", rec_ptr );
-	while (rec_ptr) {
-		if (tdb_read(tdb, rec_ptr, (char *)&rec, sizeof(rec), DOCONV()) == -1) {
-			tdb_unlock(tdb, -1, F_WRLCK);
-			return -1;
-		}
-
-		if (rec.magic != TDB_FREE_MAGIC) {
-			printf("bad magic 0x%08x in free list\n", rec.magic);
-			tdb_unlock(tdb, -1, F_WRLCK);
-			return -1;
-		}
-
-		printf("entry offset=[0x%08x], rec.rec_len = [0x%08x (%d)] (end = 0x%08x)\n", 
-		       rec_ptr, rec.rec_len, rec.rec_len, rec_ptr + rec.rec_len);
-		total_free += rec.rec_len;
-
-		/* move to the next record */
-		rec_ptr = rec.next;
-	}
-	printf("total rec_len = [0x%08x (%d)]\n", (int)total_free, 
-               (int)total_free);
-
-	return tdb_unlock(tdb, -1, F_WRLCK);
-}
-
 /* Remove an element from the freelist.  Must have alloc lock. */
 static int remove_from_freelist(TDB_CONTEXT *tdb, tdb_off off, tdb_off next)
 {
@@ -1170,12 +1068,6 @@ static int tdb_exists_hash(TDB_CONTEXT *tdb, TDB_DATA key, uint32_t hash)
 	return 1;
 }
 
-int tdb_exists(TDB_CONTEXT *tdb, TDB_DATA key)
-{
-	uint32_t hash = tdb->hash_fn(&key);
-	return tdb_exists_hash(tdb, key, hash);
-}
-
 /* record lock stops delete underneath */
 static int lock_record(TDB_CONTEXT *tdb, tdb_off off)
 {
@@ -1617,131 +1509,6 @@ fail:
 	goto out;
 }
 
-/* Attempt to append data to an entry in place - this only works if the new data size
-   is <= the old data size and the key exists.
-   on failure return -1. Record must be locked before calling.
-*/
-static int tdb_append_inplace(TDB_CONTEXT *tdb, TDB_DATA key, uint32_t hash, TDB_DATA new_dbuf)
-{
-	struct list_struct rec;
-	tdb_off rec_ptr;
-
-	/* find entry */
-	if (!(rec_ptr = tdb_find(tdb, key, hash, &rec)))
-		return -1;
-
-	/* Append of 0 is always ok. */
-	if (new_dbuf.dsize == 0)
-		return 0;
-
-	/* must be long enough for key, old data + new data and tailer */
-	if (rec.rec_len < key.dsize + rec.data_len + new_dbuf.dsize + sizeof(tdb_off)) {
-		/* No room. */
-		tdb->ecode = TDB_SUCCESS; /* Not really an error */
-		return -1;
-	}
-
-	if (tdb_write(tdb, rec_ptr + sizeof(rec) + rec.key_len + rec.data_len,
-		      new_dbuf.dptr, new_dbuf.dsize) == -1)
-		return -1;
-
-	/* update size */
-	rec.data_len += new_dbuf.dsize;
-	return rec_write(tdb, rec_ptr, &rec);
-}
-
-/* Append to an entry. Create if not exist. */
-
-int tdb_append(TDB_CONTEXT *tdb, TDB_DATA key, TDB_DATA new_dbuf)
-{
-	struct list_struct rec;
-	uint32_t hash;
-	tdb_off rec_ptr;
-	char *p = NULL;
-	int ret = 0;
-	size_t new_data_size = 0;
-
-	/* find which hash bucket it is in */
-	hash = tdb->hash_fn(&key);
-	if (tdb_lock(tdb, BUCKET(hash), F_WRLCK) == -1)
-		return -1;
-
-	/* first try in-place. */
-	if (tdb_append_inplace(tdb, key, hash, new_dbuf) == 0)
-		goto out;
-
-	/* reset the error code potentially set by the tdb_append_inplace() */
-	tdb->ecode = TDB_SUCCESS;
-
-	/* find entry */
-	if (!(rec_ptr = tdb_find(tdb, key, hash, &rec))) {
-		if (tdb->ecode != TDB_ERR_NOEXIST)
-			goto fail;
-
-		/* Not found - create. */
-
-		ret = tdb_store(tdb, key, new_dbuf, TDB_INSERT);
-		goto out;
-	}
-
-	new_data_size = rec.data_len + new_dbuf.dsize;
-
-	/* Copy key+old_value+value *before* allocating free space in case malloc
-	   fails and we are left with a dead spot in the tdb. */
-
-	if (!(p = (char *)talloc_size(tdb, key.dsize + new_data_size))) {
-		tdb->ecode = TDB_ERR_OOM;
-		goto fail;
-	}
-
-	/* Copy the key in place. */
-	memcpy(p, key.dptr, key.dsize);
-
-	/* Now read the old data into place. */
-	if (rec.data_len &&
-		tdb_read(tdb, rec_ptr + sizeof(rec) + rec.key_len, p + key.dsize, rec.data_len, 0) == -1)
-			goto fail;
-
-	/* Finally append the new data. */
-	if (new_dbuf.dsize)
-		memcpy(p+key.dsize+rec.data_len, new_dbuf.dptr, new_dbuf.dsize);
-
-	/* delete any existing record - if it doesn't exist we don't
-           care.  Doing this first reduces fragmentation, and avoids
-           coalescing with `allocated' block before it's updated. */
-
-	tdb_delete_hash(tdb, key, hash);
-
-	if (!(rec_ptr = tdb_allocate(tdb, key.dsize + new_data_size, &rec)))
-		goto fail;
-
-	/* Read hash top into next ptr */
-	if (ofs_read(tdb, TDB_HASH_TOP(hash), &rec.next) == -1)
-		goto fail;
-
-	rec.key_len = key.dsize;
-	rec.data_len = new_data_size;
-	rec.full_hash = hash;
-	rec.magic = TDB_MAGIC;
-
-	/* write out and point the top of the hash chain at it */
-	if (rec_write(tdb, rec_ptr, &rec) == -1
-	    || tdb_write(tdb, rec_ptr+sizeof(rec), p, key.dsize+new_data_size)==-1
-	    || ofs_write(tdb, TDB_HASH_TOP(hash), &rec_ptr) == -1) {
-		/* Need to tdb_unallocate() here */
-		goto fail;
-	}
-
- out:
-	SAFE_FREE(p); 
-	tdb_unlock(tdb, BUCKET(hash), F_WRLCK);
-	return ret;
-
-fail:
-	ret = -1;
-	goto out;
-}
-
 static int tdb_already_open(dev_t device,
 			    ino_t ino)
 {
@@ -1756,22 +1523,6 @@ static int tdb_already_open(dev_t device,
 	return 0;
 }
 
-/* open the database, creating it if necessary 
-
-   The open_flags and mode are passed straight to the open call on the
-   database file. A flags value of O_WRONLY is invalid. The hash size
-   is advisory, use zero for a default value.
-
-   Return is NULL on error, in which case errno is also set.  Don't 
-   try to call tdb_error or tdb_errname, just do strerror(errno).
-
-   @param name may be NULL for internal databases. */
-TDB_CONTEXT *tdb_open(const char *name, int hash_size, int tdb_flags,
-		      int open_flags, mode_t mode)
-{
-	return tdb_open_ex(name, hash_size, tdb_flags, open_flags, mode, NULL, NULL);
-}
-
 /* a default logging function */
 static void null_log_fn(TDB_CONTEXT *tdb __attribute__((unused)),
 			int level __attribute__((unused)),
@@ -1995,191 +1746,3 @@ int tdb_close(TDB_CONTEXT *tdb)
 
 	return ret;
 }
-
-/* lock/unlock entire database */
-int tdb_lockall(TDB_CONTEXT *tdb)
-{
-	uint32_t i;
-
-	/* There are no locks on read-only dbs */
-	if (tdb->read_only)
-		return TDB_ERRCODE(TDB_ERR_LOCK, -1);
-	for (i = 0; i < tdb->header.hash_size; i++) 
-		if (tdb_lock(tdb, i, F_WRLCK))
-			break;
-
-	/* If error, release locks we have... */
-	if (i < tdb->header.hash_size) {
-		uint32_t j;
-
-		for ( j = 0; j < i; j++)
-			tdb_unlock(tdb, j, F_WRLCK);
-		return TDB_ERRCODE(TDB_ERR_NOLOCK, -1);
-	}
-
-	return 0;
-}
-void tdb_unlockall(TDB_CONTEXT *tdb)
-{
-	uint32_t i;
-	for (i=0; i < tdb->header.hash_size; i++)
-		tdb_unlock(tdb, i, F_WRLCK);
-}
-
-/* lock/unlock one hash chain. This is meant to be used to reduce
-   contention - it cannot guarantee how many records will be locked */
-int tdb_chainlock(TDB_CONTEXT *tdb, TDB_DATA key)
-{
-	return tdb_lock(tdb, BUCKET(tdb->hash_fn(&key)), F_WRLCK);
-}
-
-int tdb_chainunlock(TDB_CONTEXT *tdb, TDB_DATA key)
-{
-	return tdb_unlock(tdb, BUCKET(tdb->hash_fn(&key)), F_WRLCK);
-}
-
-int tdb_chainlock_read(TDB_CONTEXT *tdb, TDB_DATA key)
-{
-	return tdb_lock(tdb, BUCKET(tdb->hash_fn(&key)), F_RDLCK);
-}
-
-int tdb_chainunlock_read(TDB_CONTEXT *tdb, TDB_DATA key)
-{
-	return tdb_unlock(tdb, BUCKET(tdb->hash_fn(&key)), F_RDLCK);
-}
-
-
-/* register a loging function */
-void tdb_logging_function(TDB_CONTEXT *tdb, void (*fn)(TDB_CONTEXT *, int , const char *, ...))
-{
-	tdb->log_fn = fn?fn:null_log_fn;
-}
-
-
-/* reopen a tdb - this can be used after a fork to ensure that we have an independent
-   seek pointer from our parent and to re-establish locks */
-int tdb_reopen(TDB_CONTEXT *tdb)
-{
-	struct stat st;
-
-	if (tdb->flags & TDB_INTERNAL)
-		return 0; /* Nothing to do. */
-	if (tdb_munmap(tdb) != 0) {
-		TDB_LOG((tdb, 0, "tdb_reopen: munmap failed (%s)\n", strerror(errno)));
-		goto fail;
-	}
-	if (close(tdb->fd) != 0)
-		TDB_LOG((tdb, 0, "tdb_reopen: WARNING closing tdb->fd failed!\n"));
-	tdb->fd = open(tdb->name, tdb->open_flags & ~(O_CREAT|O_TRUNC), 0);
-	if (tdb->fd == -1) {
-		TDB_LOG((tdb, 0, "tdb_reopen: open failed (%s)\n", strerror(errno)));
-		goto fail;
-	}
-	if (fstat(tdb->fd, &st) != 0) {
-		TDB_LOG((tdb, 0, "tdb_reopen: fstat failed (%s)\n", strerror(errno)));
-		goto fail;
-	}
-	if (st.st_ino != tdb->inode || st.st_dev != tdb->device) {
-		TDB_LOG((tdb, 0, "tdb_reopen: file dev/inode has changed!\n"));
-		goto fail;
-	}
-	tdb_mmap(tdb);
-	if ((tdb->flags & TDB_CLEAR_IF_FIRST) && (tdb_brlock(tdb, ACTIVE_LOCK, F_RDLCK, F_SETLKW, 0) == -1)) {
-		TDB_LOG((tdb, 0, "tdb_reopen: failed to obtain active lock\n"));
-		goto fail;
-	}
-
-	return 0;
-
-fail:
-	tdb_close(tdb);
-	return -1;
-}
-
-/* Not general: only works if single writer. */
-TDB_CONTEXT *tdb_copy(TDB_CONTEXT *tdb, const char *outfile)
-{
-	int fd, saved_errno;
-	TDB_CONTEXT *copy;
-
-	if (tdb->flags & TDB_INTERNAL) {
-		struct tdb_header *copydb;
-		
-		copy = talloc_zero(outfile, TDB_CONTEXT);
-		if (copy == NULL) {
-			errno = ENOMEM;
-			goto intfail;
-		}
-		memcpy(copy, tdb, sizeof(TDB_CONTEXT));
-
-		if (copy->name || copy->locked || copy->device || copy->inode) {
-			fprintf(stderr, "tdb_copy assumption(s) failed\n");
-			goto intfail;
-		}
-
-		copydb = talloc_zero_size(copy, copy->map_size);
-		if (copydb == NULL) {
-			errno = ENOMEM;
-			goto intfail;
-		}
-		memcpy(copydb, copy->map_ptr, copy->map_size);
-		copy->map_ptr = (char*) copydb;
-
-		if (tdb_brlock(tdb, GLOBAL_LOCK, F_UNLCK, F_SETLKW, 0) == -1)
-			goto intfail;
-
-		copy->next = tdbs;
-		tdbs = copy;
-
-		return copy;
-intfail:
-		talloc_free(copy);
-		return NULL;
-	}
-
-	fd = open(outfile, O_TRUNC|O_CREAT|O_WRONLY, 0640);
-	if (fd < 0)
-		return NULL;
-	if (tdb->map_ptr) {
-		if (write(fd,tdb->map_ptr,tdb->map_size) != (int)tdb->map_size)
-			goto fail;
-	} else {
-		char buf[65536];
-		int r;
-
-		lseek(tdb->fd, 0, SEEK_SET);
-		while ((r = read(tdb->fd, buf, sizeof(buf))) > 0) {
-			if (write(fd, buf, r) != r)
-				goto fail;
-		}
-		if (r < 0)
-			goto fail;
-	}
-	copy = tdb_open(outfile, 0, 0, O_RDWR, 0);
-	if (!copy)
-		goto fail;
-	close(fd);
-	return copy;
-
-fail:
-	saved_errno = errno;
-	close(fd);
-	unlink(outfile);
-	errno = saved_errno;
-	return NULL;
-}
-
-/* reopen all tdb's */
-int tdb_reopen_all(void)
-{
-	TDB_CONTEXT *tdb;
-
-	for (tdb=tdbs; tdb; tdb = tdb->next) {
-		/* Ensure no clear-if-first. */
-		tdb->flags &= ~TDB_CLEAR_IF_FIRST;
-		if (tdb_reopen(tdb) != 0)
-			return -1;
-	}
-
-	return 0;
-}
diff --git a/tools/xenstore/tdb.h b/tools/xenstore/tdb.h
index 4187274..557cf72 100644
--- a/tools/xenstore/tdb.h
+++ b/tools/xenstore/tdb.h
@@ -112,42 +112,20 @@ typedef int (*tdb_traverse_func)(TDB_CONTEXT *, TDB_DATA, TDB_DATA, void *);
 typedef void (*tdb_log_func)(TDB_CONTEXT *, int , const char *, ...);
 typedef uint32_t (*tdb_hash_func)(TDB_DATA *key);
 
-TDB_CONTEXT *tdb_open(const char *name, int hash_size, int tdb_flags,
-		      int open_flags, mode_t mode);
 TDB_CONTEXT *tdb_open_ex(const char *name, int hash_size, int tdb_flags,
 			 int open_flags, mode_t mode,
 			 tdb_log_func log_fn,
 			 tdb_hash_func hash_fn);
 
-int tdb_reopen(TDB_CONTEXT *tdb);
-int tdb_reopen_all(void);
-void tdb_logging_function(TDB_CONTEXT *tdb, tdb_log_func);
 enum TDB_ERROR tdb_error(TDB_CONTEXT *tdb);
 const char *tdb_errorstr(TDB_CONTEXT *tdb);
 TDB_DATA tdb_fetch(TDB_CONTEXT *tdb, TDB_DATA key);
 int tdb_delete(TDB_CONTEXT *tdb, TDB_DATA key);
 int tdb_store(TDB_CONTEXT *tdb, TDB_DATA key, TDB_DATA dbuf, int flag);
-int tdb_append(TDB_CONTEXT *tdb, TDB_DATA key, TDB_DATA new_dbuf);
 int tdb_close(TDB_CONTEXT *tdb);
 TDB_DATA tdb_firstkey(TDB_CONTEXT *tdb);
 TDB_DATA tdb_nextkey(TDB_CONTEXT *tdb, TDB_DATA key);
 int tdb_traverse(TDB_CONTEXT *tdb, tdb_traverse_func fn, void *);
-int tdb_exists(TDB_CONTEXT *tdb, TDB_DATA key);
-int tdb_lockall(TDB_CONTEXT *tdb);
-void tdb_unlockall(TDB_CONTEXT *tdb);
-
-/* Low level locking functions: use with care */
-int tdb_chainlock(TDB_CONTEXT *tdb, TDB_DATA key);
-int tdb_chainunlock(TDB_CONTEXT *tdb, TDB_DATA key);
-int tdb_chainlock_read(TDB_CONTEXT *tdb, TDB_DATA key);
-int tdb_chainunlock_read(TDB_CONTEXT *tdb, TDB_DATA key);
-TDB_CONTEXT *tdb_copy(TDB_CONTEXT *tdb, const char *outfile);
-
-/* Debug functions. Not used in production. */
-void tdb_dump_all(TDB_CONTEXT *tdb);
-int tdb_printfreelist(TDB_CONTEXT *tdb);
-
-extern TDB_DATA tdb_null;
 
 #ifdef  __cplusplus
 }
-- 
2.10.2


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

Re: [PATCH v3 0/4] xenstore: rework of transaction handling

[Juergen Gross]

Cc-ing Julien, as this series is meant for 4.9.

Juergen

On 28/03/17 18:26, Juergen Gross wrote:
> Rework the transaction handling of xenstored to no longer raise
> conflicts so often.
> 
> This series has been sent for pre-review to some reviewers before as the
> series is related to XSA 206 which has been disclosed only today. So V1
> and V2 have been non-public in order to speed up review process without
> disclosing the XSA.
> 
> Changes in V3:
> - don't always return EAGAIN in case of a failed transaction:
>   it can be ENOMEM or ENOSPC, too.
> 
> Changes in V2:
> - Rebase on top of those patches
> - split patch 1 in two patches as suggested by Ian
> 
> Juergen Gross (4):
>   xenstore: let write_node() and some callers return errno
>   xenstore: undo function rename
>   xenstore: rework of transaction handling
>   xenstore: cleanup tdb.c
> 
>  tools/xenstore/tdb.c                   | 439 +--------------------------------
>  tools/xenstore/tdb.h                   |  22 --
>  tools/xenstore/xenstored_core.c        | 173 ++++++-------
>  tools/xenstore/xenstored_core.h        |  17 +-
>  tools/xenstore/xenstored_domain.c      |  24 +-
>  tools/xenstore/xenstored_domain.h      |   2 +-
>  tools/xenstore/xenstored_transaction.c | 429 ++++++++++++++++++++++++++------
>  tools/xenstore/xenstored_transaction.h |  18 +-
>  8 files changed, 481 insertions(+), 643 deletions(-)
> 


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

Re: [PATCH v3 0/4] xenstore: rework of transaction handling

[Julien Grall]

Hi Juergen,

On 29/03/17 15:10, Juergen Gross wrote:
> Cc-ing Julien, as this series is meant for 4.9.

George asked me a couple of weeks ago a "last posting date" exception 
request for this series and I was happy with that.

Assuming the series gets the proper acked-by/reviewed-by tags:

Release-acked-by: Julien Grall <julien.grall@arm.com>

Cheers,

-- 
Julien Grall

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

[PATCH v3 0/4] xenstore: rework of transaction handling

[Ian Jackson]

Juergen Gross writes ("[PATCH v3 0/4] xenstore: rework of transaction handling"):
> Rework the transaction handling of xenstored to no longer raise
> conflicts so often.
> 
> This series has been sent for pre-review to some reviewers before as the
> series is related to XSA 206 which has been disclosed only today. So V1
> and V2 have been non-public in order to speed up review process without
> disclosing the XSA.

I meant to look over these again and ack them.  I see they have been
committed, but FTR:

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

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