# HG changeset patch
# User cl349@xxxxxxxxxxxxxxxxxxxx
# Node ID 1853a6e966bd808af19568ba3c0c05a4e69750c1
# Parent b9903985e9b65969daabec9cada972e3b956da6c
Remove ill-conceived concept of watches blocking reply on
connection which did write/mkdir/rm/setperm etc.
This causes deadlocks in real life, and I can't see a sane way
of avoiding them: it is reasonable for someone to ignore watch
notifications while doing other actions, and that means that
we can do other writes. These writes can block pending other
watchers; if one of these is the process blocked awaiting our
ack, we deadlock.
Signed-off-by: Rusty Russel <rusty@xxxxxxxxxxxxxxx>
Signed-off-by: Christian Limpach <Christian.Limpach@xxxxxxxxxxxx>
diff -r b9903985e9b6 -r 1853a6e966bd tools/xenstore/xenstored_core.c
--- a/tools/xenstore/xenstored_core.c Tue Jul 26 15:24:28 2005
+++ b/tools/xenstore/xenstored_core.c Tue Jul 26 15:26:32 2005
@@ -976,10 +976,7 @@
}
add_change_node(conn->transaction, node, false);
- if (fire_watches(conn, node, false)) {
- conn->watch_ack = XS_WRITE;
- return;
- }
+ fire_watches(conn, node, false);
send_ack(conn, XS_WRITE);
}
@@ -1005,10 +1002,7 @@
}
add_change_node(conn->transaction, node, false);
- if (fire_watches(conn, node, false)) {
- conn->watch_ack = XS_MKDIR;
- return;
- }
+ fire_watches(conn, node, false);
send_ack(conn, XS_MKDIR);
}
@@ -1046,10 +1040,7 @@
}
add_change_node(conn->transaction, node, true);
- if (fire_watches(conn, node, true)) {
- conn->watch_ack = XS_RM;
- return;
- }
+ fire_watches(conn, node, true);
send_ack(conn, XS_RM);
}
@@ -1121,10 +1112,7 @@
}
add_change_node(conn->transaction, node, false);
- if (fire_watches(conn, node, false)) {
- conn->watch_ack = XS_SET_PERMS;
- return;
- }
+ fire_watches(conn, node, false);
send_ack(conn, XS_SET_PERMS);
}
@@ -1359,12 +1347,6 @@
consider_message(i);
}
break;
- case WATCHED:
- if (i->watches_unacked == 0) {
- i->state = OK;
- send_ack(i, i->watch_ack);
- }
- break;
case OK:
break;
}
@@ -1389,8 +1371,6 @@
new->state = OK;
new->blocked_by = NULL;
- new->watch_ack = XS_ERROR;
- new->watches_unacked = 0;
new->out = new->waiting_reply = NULL;
new->fd = -1;
new->id = 0;
@@ -1471,7 +1451,6 @@
printf(" state = %s\n",
i->state == OK ? "OK"
: i->state == BLOCKED ? "BLOCKED"
- : i->state == WATCHED ? "WATCHED"
: "INVALID");
if (i->id)
printf(" id = %i\n", i->id);
diff -r b9903985e9b6 -r 1853a6e966bd tools/xenstore/xenstored_core.h
--- a/tools/xenstore/xenstored_core.h Tue Jul 26 15:24:28 2005
+++ b/tools/xenstore/xenstored_core.h Tue Jul 26 15:26:32 2005
@@ -51,8 +51,6 @@
{
/* Blocked by transaction. */
BLOCKED,
- /* Waiting for watchers to ack event we caused */
- WATCHED,
/* Completed */
OK,
};
@@ -72,12 +70,6 @@
/* Node we are waiting for (if state == BLOCKED) */
char *blocked_by;
-
- /* Are we waiting for watches to be acked from an event we caused? */
- unsigned int watches_unacked;
-
- /* Type of ack to send once watches fired. */
- enum xsd_sockmsg_type watch_ack;
/* Is this a read-only connection? */
bool can_write;
diff -r b9903985e9b6 -r 1853a6e966bd tools/xenstore/xenstored_transaction.c
--- a/tools/xenstore/xenstored_transaction.c Tue Jul 26 15:24:28 2005
+++ b/tools/xenstore/xenstored_transaction.c Tue Jul 26 15:26:32 2005
@@ -307,7 +307,6 @@
{
struct changed_node *i;
struct transaction *trans;
- bool fired = false;
if (!arg || (!streq(arg, "T") && !streq(arg, "F"))) {
send_error(conn, EINVAL);
@@ -337,12 +336,8 @@
/* Fire off the watches for everything that changed. */
list_for_each_entry(i, &trans->changes, list)
- fired |= fire_watches(conn, i->node, i->recurse);
- }
-
- if (fired)
- conn->watch_ack = XS_TRANSACTION_END;
- else
- send_ack(conn, XS_TRANSACTION_END);
-}
-
+ fire_watches(conn, i->node, i->recurse);
+ }
+ send_ack(conn, XS_TRANSACTION_END);
+}
+
diff -r b9903985e9b6 -r 1853a6e966bd tools/xenstore/xenstored_watch.c
--- a/tools/xenstore/xenstored_watch.c Tue Jul 26 15:24:28 2005
+++ b/tools/xenstore/xenstored_watch.c Tue Jul 26 15:26:32 2005
@@ -41,9 +41,6 @@
/* Data to send (node\0token\0). */
unsigned int len;
char *data;
-
- /* Connection which caused watch event (which we are blocking) */
- struct connection *cause;
};
struct watch
@@ -95,14 +92,10 @@
struct watch_event *event = _event;
trace_destroy(event, "watch_event");
- assert(event->cause->watches_unacked != 0);
- /* If it hits zero, will unblock in unblock_connections. */
- event->cause->watches_unacked--;
return 0;
}
-static void add_event(struct connection *cause, struct watch *watch,
- const char *node)
+static void add_event(struct watch *watch, const char *node)
{
struct watch_event *event;
@@ -117,22 +110,20 @@
event->data = talloc_array(event, char, event->len);
strcpy(event->data, node);
strcpy(event->data + strlen(node) + 1, watch->token);
- event->cause = cause;
- cause->watches_unacked++;
talloc_set_destructor(event, destroy_watch_event);
list_add_tail(&event->list, &watch->events);
trace_create(event, "watch_event");
}
/* FIXME: we fail to fire on out of memory. Should drop connections. */
-bool fire_watches(struct connection *conn, const char *node, bool recurse)
+void fire_watches(struct connection *conn, const char *node, bool recurse)
{
struct connection *i;
struct watch *watch;
/* During transactions, don't fire watches. */
if (conn->transaction)
- return false;
+ return;
/* Create an event for each watch. Don't send to self. */
list_for_each_entry(i, &connections, list) {
@@ -141,18 +132,16 @@
list_for_each_entry(watch, &i->watches, list) {
if (is_child(node, watch->node))
- add_event(conn, watch, node);
+ add_event(watch, node);
else if (recurse && is_child(watch->node, node))
- add_event(conn, watch, watch->node);
+ add_event(watch, watch->node);
else
continue;
- conn->state = WATCHED;
/* If connection not doing anything, queue this. */
if (!i->out)
queue_next_event(i);
}
}
- return conn->state == WATCHED;
}
static int destroy_watch(void *_watch)
diff -r b9903985e9b6 -r 1853a6e966bd tools/xenstore/xenstored_watch.h
--- a/tools/xenstore/xenstored_watch.h Tue Jul 26 15:24:28 2005
+++ b/tools/xenstore/xenstored_watch.h Tue Jul 26 15:26:32 2005
@@ -33,9 +33,8 @@
void queue_next_event(struct connection *conn);
/* Fire all watches: recurse means all the children are effected (ie. rm).
- * Returns true if there were any, meaning connection has to wait.
*/
-bool fire_watches(struct connection *conn, const char *node, bool recurse);
+void fire_watches(struct connection *conn, const char *node, bool recurse);
/* Find shortest timeout: if any, reduce tv (may already be set). */
void shortest_watch_ack_timeout(struct timeval *tv);
_______________________________________________
Xen-changelog mailing list
Xen-changelog@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-changelog
|
Home