[Xen-tools] [PATCH 2/3] Delay deletion of failed transactions slightly

[Rusty Russell <rusty@xxxxxxxxxxxxxxx>]

# HG changeset patch
# User Rusty Russell <rusty@xxxxxxxxxxxxxxx>
# Node ID 703e0e8e7efce364f62a79b209af0d48195b6825
# Parent  6d2f8b0a18c4713286a16d22fd2a11d7ab4525b4
Delay delete of transaction file when transaction fails with EAGAIN.

Randomized testing with killing xenstored reveals a flaw: if it dies before 
client gets response from xs_transaction_end (ie. transaction commit), we can't 
tell if commit failed (EAGAIN) or succeeded.  This is the only non-repeatable 
operation.

Note that this won't happen on a clean shutdown, but delaying delete allows us 
to handle the unexpected death case: the client will reconnect, attach to 
transaction again, the second commit attempt will fail.

Signed-off-by: Rusty Russell <rusty@xxxxxxxxxxxxxxx>

diff -r 6d2f8b0a18c4 -r 703e0e8e7efc tools/xenstore/xenstored_core.c
--- a/tools/xenstore/xenstored_core.c   Mon Sep 26 04:30:14 2005
+++ b/tools/xenstore/xenstored_core.c   Mon Sep 26 10:53:13 2005
@@ -1185,6 +1185,12 @@
        if (type != XS_WATCH_ACK)
                conn->waiting_for_ack = NULL;
 
+       /* Now we're sure they know transaction is over. */
+       if (conn->last_transaction) {
+               talloc_free(conn->last_transaction);
+               conn->last_transaction = NULL;
+       }
+
        /* Careful: process_message may free connection.  We detach
         * "in" beforehand and allocate the new buffer to avoid
         * touching conn after process_message.
@@ -1282,7 +1288,7 @@
        new->fd = -1;
        new->id = 0;
        new->domain = NULL;
-       new->transaction = NULL;
+       new->transaction = new->last_transaction = NULL;
        new->write = write;
        new->read = read;
        new->can_write = true;
diff -r 6d2f8b0a18c4 -r 703e0e8e7efc tools/xenstore/xenstored_core.h
--- a/tools/xenstore/xenstored_core.h   Mon Sep 26 04:30:14 2005
+++ b/tools/xenstore/xenstored_core.h   Mon Sep 26 10:53:13 2005
@@ -84,8 +84,8 @@
        /* If we had a watch fire outgoing when we needed to reply... */
        struct buffered_data *waiting_reply;
 
-       /* My transaction, if any. */
-       struct transaction *transaction;
+       /* My transaction, if any, and aborted one. */
+       struct transaction *transaction, *last_transaction;
 
        /* The domain I'm associated with, if any. */
        struct domain *domain;
diff -r 6d2f8b0a18c4 -r 703e0e8e7efc tools/xenstore/xenstored_transaction.c
--- a/tools/xenstore/xenstored_transaction.c    Mon Sep 26 04:30:14 2005
+++ b/tools/xenstore/xenstored_transaction.c    Mon Sep 26 10:53:13 2005
@@ -219,6 +219,8 @@
        if (streq(arg, "T")) {
                /* FIXME: Merge, rather failing on any change. */
                if (trans->generation != generation) {
+                       /* Keep transaction around: more robust if we die. */
+                       conn->last_transaction = talloc_steal(conn, trans);
                        send_error(conn, EAGAIN);
                        return;
                }
@@ -248,7 +250,7 @@
 
        transdir = talloc_strdup(talloc_autofree_context(), xs_daemon_tdb());
        if (strrchr(transdir, '/'))
-               *strchr(transdir, '/') = '\0';
+               *strrchr(transdir, '/') = '\0';
        else
                transdir = talloc_strdup(transdir, ".");
 
@@ -267,6 +269,7 @@
                if (!trans->tdb) {
                        eprintf("Could not reopen transaction %i: corrupt?",
                                id);
+                       unlink(trans->tdb_name);
                        talloc_free(trans);
                }
        }

-- 
A bad analogy is like a leaky screwdriver -- Richard Braakman


_______________________________________________
Xen-tools mailing list
Xen-tools@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-tools