[PATCH 0/2] tools/xenstore: don't use old tdb file in xenstored

[PATCH 0/2] tools/xenstore: don't use old tdb file in xenstored

[Juergen Gross]

Today xenstored tries to open an existing tdb file when started. As
this is known to be problematic the start script of xenstored is
deleting any old tdb file.

This can be made easier and less error prone by letting xenstored use
a new file.

Juergen Gross (2):
  tools/xenstore: start with empty data base
  tools: don't remove tdb data base file before starting xenstored

 tools/hotplug/Linux/launch-xenstore.in |  1 -
 tools/xenstore/xenstored_core.c        | 60 +++++-----------------------------
 2 files changed, 9 insertions(+), 52 deletions(-)

-- 
2.10.2


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

[PATCH 1/2] tools/xenstore: start with empty data base

[Juergen Gross]

Today xenstored tries to open a tdb data base file on disk when it is
started. As this is problematic in most cases the scripts used to start
xenstored ensure xenstored won't find such a file in order to start
with an empty xenstore.

A tdb data base file can't be used to restore all Xenstore state as
e.g. Xenstore watches are not kept in the tdb data base. The file is
meant to be used for debugging purposes after a xenstored crash only.

Instead of opening a Xenstore data base file found on disk always start
with an empty data base. This will avoid problems in case someone is
testing multiple xenstored versions without rebooting (which is not
supported but helps debugging in some cases).

Signed-off-by: Juergen Gross <jgross@suse.com>
---
 tools/xenstore/xenstored_core.c | 60 +++++++----------------------------------
 1 file changed, 9 insertions(+), 51 deletions(-)

diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
index 3770056..3dc06d4 100644
--- a/tools/xenstore/xenstored_core.c
+++ b/tools/xenstore/xenstored_core.c
@@ -1157,17 +1157,6 @@ static int _rm(struct connection *conn, const void *ctx, struct node *node,
 }
 
 
-static void internal_rm(const char *name)
-{
-	char *tname = talloc_strdup(NULL, name);
-	struct node *node = read_node(NULL, tname, tname);
-	if (node)
-		_rm(NULL, tname, node, tname);
-	talloc_free(node);
-	talloc_free(tname);
-}
-
-
 static int do_rm(struct connection *conn, struct buffered_data *in)
 {
 	struct node *node;
@@ -1582,49 +1571,18 @@ static void setup_structure(void)
 		barf_perror("Could not create tdbname");
 
 	if (!(tdb_flags & TDB_INTERNAL))
-		tdb_ctx = tdb_open_ex(tdbname, 0, tdb_flags, O_RDWR, 0,
-				      &tdb_logger, NULL);
+		unlink(tdbname);
 
-	if (tdb_ctx) {
-		/* XXX When we make xenstored able to restart, this will have
-		   to become cleverer, checking for existing domains and not
-		   removing the corresponding entries, but for now xenstored
-		   cannot be restarted without losing all the registered
-		   watches, which breaks all the backend drivers anyway.  We
-		   can therefore get away with just clearing /local and
-		   expecting Xend to put the appropriate entries back in.
+	tdb_ctx = tdb_open_ex(tdbname, 7919, tdb_flags, O_RDWR|O_CREAT|O_EXCL,
+			      0640, &tdb_logger, NULL);
+	if (!tdb_ctx)
+		barf_perror("Could not create tdb file %s", tdbname);
 
-		   When this change is made it is important to note that
-		   dom0's entries must be cleaned up on reboot _before_ this
-		   daemon starts, otherwise the backend drivers and dom0's
-		   balloon driver will pick up stale entries.  In the case of
-		   the balloon driver, this can be fatal.
-		*/
-		char *tlocal = talloc_strdup(NULL, "/local");
+	manual_node("/", "tool");
+	manual_node("/tool", "xenstored");
+	manual_node("/tool/xenstored", NULL);
 
-		check_store();
-
-		if (remove_local) {
-			internal_rm("/local");
-			create_node(NULL, NULL, tlocal, NULL, 0);
-
-			check_store();
-		}
-
-		talloc_free(tlocal);
-	}
-	else {
-		tdb_ctx = tdb_open_ex(tdbname, 7919, tdb_flags, O_RDWR|O_CREAT,
-				      0640, &tdb_logger, NULL);
-		if (!tdb_ctx)
-			barf_perror("Could not create tdb file %s", tdbname);
-
-		manual_node("/", "tool");
-		manual_node("/tool", "xenstored");
-		manual_node("/tool/xenstored", NULL);
-
-		check_store();
-	}
+	check_store();
 }
 
 
-- 
2.10.2


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

Re: [PATCH 1/2] tools/xenstore: start with empty data base

[Wei Liu]

On Tue, Jan 10, 2017 at 05:13:38PM +0100, Juergen Gross wrote:
> Today xenstored tries to open a tdb data base file on disk when it is
> started. As this is problematic in most cases the scripts used to start
> xenstored ensure xenstored won't find such a file in order to start
> with an empty xenstore.
> 
> A tdb data base file can't be used to restore all Xenstore state as
> e.g. Xenstore watches are not kept in the tdb data base. The file is
> meant to be used for debugging purposes after a xenstored crash only.
> 
> Instead of opening a Xenstore data base file found on disk always start
> with an empty data base. This will avoid problems in case someone is
> testing multiple xenstored versions without rebooting (which is not
> supported but helps debugging in some cases).
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>

Acked-by: Wei Liu <wei.liu2@citrix.com>

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

Re: [PATCH 1/2] tools/xenstore: start with empty data base

[George Dunlap]

On Tue, Jan 10, 2017 at 4:13 PM, Juergen Gross <jgross@suse.com> wrote:

> -       if (tdb_ctx) {
> -               /* XXX When we make xenstored able to restart, this will have

Ah, the optimism of a young project. :-)

 -George

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

Re: [Xen-devel] [PATCH 1/2] tools/xenstore: start with empty data base

[George Dunlap]

On Tue, Jan 10, 2017 at 4:14 PM Juergen Gross <jgross@suse.com> wrote:
>
> Today xenstored tries to open a tdb data base file on disk when it is
> started. As this is problematic in most cases the scripts used to start
> xenstored ensure xenstored won't find such a file in order to start
> with an empty xenstore.

Sorry to respond to such an old thread, just trying to record this in
a useful place: in the distros design session at the recent XenSummit,
it turned out that:
1. Most distros had to mount a tmpfs for the xenstore database to
prevent xenstore from destroying disk performance
2. xenstored already has an `-I` option which creates a memory-only database

At which point it was asked: Why is this option not the default?

This patch seems to imply that the main reason at the moment for an
external db is debugging; in which case, it does seem like the default
should be in-memory, with an external db as a debugging option.

 -George

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

[PATCH 2/2] tools: don't remove tdb data base file before starting xenstored

[Juergen Gross]

As xenstored now is always starting with an empty tdb data base there
is no need any longer to remove the file before starting xenstored.

A rerun of autogen.sh is required.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
 tools/hotplug/Linux/launch-xenstore.in | 1 -
 1 file changed, 1 deletion(-)

diff --git a/tools/hotplug/Linux/launch-xenstore.in b/tools/hotplug/Linux/launch-xenstore.in
index 01193be..991dec8 100644
--- a/tools/hotplug/Linux/launch-xenstore.in
+++ b/tools/hotplug/Linux/launch-xenstore.in
@@ -54,7 +54,6 @@ test -f @CONFIG_DIR@/@CONFIG_LEAF_DIR@/xencommons && . @CONFIG_DIR@/@CONFIG_LEAF
 
 [ "$XENSTORETYPE" = "daemon" ] && {
 	[ -z "$XENSTORED_ROOTDIR" ] && XENSTORED_ROOTDIR="@XEN_LIB_STORED@"
-	/bin/rm -f $XENSTORED_ROOTDIR/tdb* &>/dev/null
 	[ -z "$XENSTORED_TRACE" ] || XENSTORED_ARGS="$XENSTORED_ARGS -T @XEN_LOG_DIR@/xenstored-trace.log"
 	[ -z "$XENSTORED" ] && XENSTORED=@XENSTORED@
 	[ -x "$XENSTORED" ] || {
-- 
2.10.2


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

Re: [PATCH 2/2] tools: don't remove tdb data base file before starting xenstored

[Wei Liu]

On Tue, Jan 10, 2017 at 05:13:39PM +0100, Juergen Gross wrote:
> As xenstored now is always starting with an empty tdb data base there
> is no need any longer to remove the file before starting xenstored.
> 
> A rerun of autogen.sh is required.
> 

Actually no. This file is used when running configure.

I will just delete this sentence while committing.

> Signed-off-by: Juergen Gross <jgross@suse.com>

Acked-by: Wei Liu <wei.liu2@citrix.com>

> ---
>  tools/hotplug/Linux/launch-xenstore.in | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/tools/hotplug/Linux/launch-xenstore.in b/tools/hotplug/Linux/launch-xenstore.in
> index 01193be..991dec8 100644
> --- a/tools/hotplug/Linux/launch-xenstore.in
> +++ b/tools/hotplug/Linux/launch-xenstore.in
> @@ -54,7 +54,6 @@ test -f @CONFIG_DIR@/@CONFIG_LEAF_DIR@/xencommons && . @CONFIG_DIR@/@CONFIG_LEAF
>  
>  [ "$XENSTORETYPE" = "daemon" ] && {
>  	[ -z "$XENSTORED_ROOTDIR" ] && XENSTORED_ROOTDIR="@XEN_LIB_STORED@"
> -	/bin/rm -f $XENSTORED_ROOTDIR/tdb* &>/dev/null
>  	[ -z "$XENSTORED_TRACE" ] || XENSTORED_ARGS="$XENSTORED_ARGS -T @XEN_LOG_DIR@/xenstored-trace.log"
>  	[ -z "$XENSTORED" ] && XENSTORED=@XENSTORED@
>  	[ -x "$XENSTORED" ] || {
> -- 
> 2.10.2
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> https://lists.xen.org/xen-devel

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

Re: [PATCH 0/2] tools/xenstore: don't use old tdb file in xenstored

[Wei Liu]

On Tue, Jan 10, 2017 at 05:13:37PM +0100, Juergen Gross wrote:
> Today xenstored tries to open an existing tdb file when started. As
> this is known to be problematic the start script of xenstored is
> deleting any old tdb file.
> 
> This can be made easier and less error prone by letting xenstored use
> a new file.
> 
> Juergen Gross (2):
>   tools/xenstore: start with empty data base
>   tools: don't remove tdb data base file before starting xenstored

Pushed.

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