[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [Xen-changelog] Added further integrity checking, this time checking for duplicate directory
# HG changeset patch # User emellor@xxxxxxxxxxxxxxxxxxxxxx # Node ID f68f6f711efc1774b96eee63839eaf0fc6ccd3dd # Parent bac71019f6aacc8393eddfb7f28a3972d253ea50 Added further integrity checking, this time checking for duplicate directory entries and for orphaned nodes in the database. Added two flags, -R and -L, to disable the recovery code and the remove of /local at start-up. This makes it much easier to analyse corrupted tdb files. Added some missing talloc_free calls in the previous integrity checking code. Removed the transaction handle from the trace_io message -- unfortunately, the transaction is always null at this point, as it's not yet been looked up. Signed-off-by: Ewan Mellor <ewan@xxxxxxxxxxxxx> diff -r bac71019f6aa -r f68f6f711efc tools/xenstore/Makefile --- a/tools/xenstore/Makefile Wed Mar 8 22:57:34 2006 +++ b/tools/xenstore/Makefile Wed Mar 8 22:57:40 2006 @@ -34,7 +34,7 @@ testcode: xs_test xenstored_test xs_random -xenstored: xenstored_core.o xenstored_watch.o xenstored_domain.o xenstored_transaction.o xs_lib.o talloc.o utils.o tdb.o +xenstored: xenstored_core.o xenstored_watch.o xenstored_domain.o xenstored_transaction.o xs_lib.o talloc.o utils.o tdb.o hashtable.o $(LINK.o) $^ $(LOADLIBES) $(LDLIBS) -lxenctrl -o $@ $(CLIENTS): xenstore-%: xenstore_%.o libxenstore.so diff -r bac71019f6aa -r f68f6f711efc tools/xenstore/xenstored_core.c --- a/tools/xenstore/xenstored_core.c Wed Mar 8 22:57:34 2006 +++ b/tools/xenstore/xenstored_core.c Wed Mar 8 22:57:40 2006 @@ -51,11 +51,16 @@ #include "xenctrl.h" #include "tdb.h" +#include "hashtable.h" + + extern int eventchn_fd; /* in xenstored_domain.c */ -static bool verbose; +static bool verbose = false; LIST_HEAD(connections); static int tracefd = -1; +static bool recovery = true; +static bool remove_local = true; static int reopen_log_pipe[2]; static char *tracefile = NULL; static TDB_CONTEXT *tdb_ctx; @@ -201,8 +206,8 @@ now = time(NULL); tm = localtime(&now); - trace("%s %p %p %04d%02d%02d %02d:%02d:%02d %s (", prefix, conn, - conn->transaction, tm->tm_year + 1900, tm->tm_mon + 1, + trace("%s %p %04d%02d%02d %02d:%02d:%02d %s (", prefix, conn, + tm->tm_year + 1900, tm->tm_mon + 1, tm->tm_mday, tm->tm_hour, tm->tm_min, tm->tm_sec, sockmsg_string(data->hdr.msg.type)); @@ -947,12 +952,24 @@ } } + /* Delete memory using memmove. */ static void memdel(void *mem, unsigned off, unsigned len, unsigned total) { memmove(mem + off, mem + off + len, total - off - len); } + +static bool 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); +} + + static bool delete_child(struct connection *conn, struct node *node, const char *childname) { @@ -960,10 +977,7 @@ for (i = 0; i < node->childlen; i += strlen(node->children+i) + 1) { if (streq(node->children+i, childname)) { - memdel(node->children, i, strlen(childname) + 1, - node->childlen); - node->childlen -= strlen(childname) + 1; - return write_node(conn, node); + return remove_child_entry(conn, node, i); } } corrupt(conn, "Can't find child '%s' in %s", childname, node->name); @@ -998,6 +1012,7 @@ struct node *node = read_node(NULL, tname); if (node) _rm(NULL, node, tname); + talloc_free(node); talloc_free(tname); } @@ -1429,12 +1444,14 @@ check_store(); - internal_rm("/local"); - create_node(NULL, tlocal, NULL, 0); + if (remove_local) { + internal_rm("/local"); + create_node(NULL, tlocal, NULL, 0); + + check_store(); + } talloc_free(tlocal); - - check_store(); } else { tdb_ctx = tdb_open(tdbname, 7919, TDB_FLAGS, O_RDWR|O_CREAT, @@ -1450,6 +1467,26 @@ } } + +static unsigned int hash_from_key_fn(void *k) +{ + char *str = k; + unsigned int hash = 5381; + char c; + + while ((c = *str++)) + hash = ((hash << 5) + hash) + (unsigned int)c; + + return hash; +} + + +static int keys_equal_fn(void *key1, void *key2) +{ + return 0 == strcmp((char *)key1, (char *)key2); +} + + static char *child_name(const char *s1, const char *s2) { if (strcmp(s1, "/")) { @@ -1460,12 +1497,39 @@ } } -static void check_store_(const char *name) + +static void remember_string(struct hashtable *hash, const char *str) +{ + char *k = malloc(strlen(str) + 1); + strcpy(k, str); + hashtable_insert(hash, k, (void *)1); +} + + +/** + * A node has a children field that names the children of the node, separated + * by NULs. We check whether there are entries in there that are duplicated + * (and if so, delete the second one), and whether there are any that do not + * have a corresponding child node (and if so, delete them). Each valid child + * is then recursively checked. + * + * No deleting is performed if the recovery flag is cleared (i.e. -R was + * passed on the command line). + * + * As we go, we record each node in the given reachable hashtable. These + * entries will be used later in clean_store. + */ +static void check_store_(const char *name, struct hashtable *reachable) { struct node *node = read_node(NULL, name); if (node) { size_t i = 0; + + struct hashtable * children = + create_hashtable(16, hash_from_key_fn, keys_equal_fn); + + remember_string(reachable, name); while (i < node->childlen) { size_t childlen = strlen(node->children + i); @@ -1474,21 +1538,39 @@ struct node *childnode = read_node(NULL, childname); if (childnode) { - check_store_(childname); - i += childlen + 1; + if (hashtable_search(children, childname)) { + log("check_store: '%s' is duplicated!", + childname); + + if (recovery) { + remove_child_entry(NULL, node, + i); + i -= childlen + 1; + } + } + else { + remember_string(children, childname); + check_store_(childname, reachable); + } } else { log("check_store: No child '%s' found!\n", childname); - memdel(node->children, i, childlen + 1, - node->childlen); - node->childlen -= childlen + 1; - write_node(NULL, node); + if (recovery) { + remove_child_entry(NULL, node, i); + i -= childlen + 1; + } } + talloc_free(childnode); talloc_free(childname); + i += childlen + 1; } + + hashtable_destroy(children, 0 /* Don't free values (they are + all (void *)1) */); + talloc_free(node); } else { /* Impossible, because no database should ever be without the @@ -1500,12 +1582,51 @@ } +/** + * Helper to clean_store below. + */ +static int clean_store_(TDB_CONTEXT *tdb, TDB_DATA key, TDB_DATA val, + void *private) +{ + struct hashtable *reachable = private; + char * name = talloc_strndup(NULL, key.dptr, key.dsize); + + if (!hashtable_search(reachable, name)) { + log("clean_store: '%s' is orphaned!", name); + if (recovery) { + tdb_delete(tdb, key); + } + } + + talloc_free(name); + + return 0; +} + + +/** + * Given the list of reachable nodes, iterate over the whole store, and + * remove any that were not reached. + */ +static void clean_store(struct hashtable *reachable) +{ + tdb_traverse(tdb_ctx, &clean_store_, reachable); +} + + static void check_store() { char * root = talloc_strdup(NULL, "/"); + struct hashtable * reachable = + create_hashtable(16, hash_from_key_fn, keys_equal_fn); + log("Checking store ..."); - check_store_(root); + check_store_(root, reachable); + clean_store(reachable); log("Checking store complete."); + + hashtable_destroy(reachable, 0 /* Don't free values (they are all + (void *)1) */); talloc_free(root); } @@ -1594,6 +1715,9 @@ " --no-fork to request that the daemon does not fork,\n" " --output-pid to request that the pid of the daemon is output,\n" " --trace-file <file> giving the file for logging, and\n" +" --no-recovery to request that no recovery should be attempted when\n" +" the store is corrupted (debug only),\n" +" --preserve-local to request that /local is preserved on start-up,\n" " --verbose to request verbose execution.\n"); } @@ -1605,6 +1729,8 @@ { "no-fork", 0, NULL, 'N' }, { "output-pid", 0, NULL, 'P' }, { "trace-file", 1, NULL, 'T' }, + { "no-recovery", 0, NULL, 'R' }, + { "preserve-local", 0, NULL, 'L' }, { "verbose", 0, NULL, 'V' }, { NULL, 0, NULL, 0 } }; @@ -1620,7 +1746,7 @@ bool no_domain_init = false; const char *pidfile = NULL; - while ((opt = getopt_long(argc, argv, "DF:HNPT:V", options, + while ((opt = getopt_long(argc, argv, "DF:HNPT:RLV", options, NULL)) != -1) { switch (opt) { case 'D': @@ -1638,6 +1764,12 @@ case 'P': outputpid = true; break; + case 'R': + recovery = false; + break; + case 'L': + remove_local = false; + break; case 'T': tracefile = optarg; break; _______________________________________________ Xen-changelog mailing list Xen-changelog@xxxxxxxxxxxxxxxxxxx http://lists.xensource.com/xen-changelog
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |