[Xen-devel] [PATCH] Fix xenstore entry accounting

[john.levon@xxxxxxx]

# HG changeset patch
# User john.levon@xxxxxxx
# Date 1180453350 25200
# Node ID 169ed8c1e7c79bd55d07110d9e784945b60da561
# Parent  d39ebf946dff51d436f1d8393a9abf97f9654bf9
Fix xenstore entry accounting

xenstored is incorrectly accounting domain nodes when transactions fail. Store
pending count changes in the transaction structure, and apply at transaction
completion, instead of directly applying the changes.

Signed-off-by: Max Zhen <max.zhen@xxxxxxx>

diff --git a/tools/xenstore/xenstored_domain.c 
b/tools/xenstore/xenstored_domain.c
--- a/tools/xenstore/xenstored_domain.c
+++ b/tools/xenstore/xenstored_domain.c
@@ -605,12 +605,21 @@ void domain_entry_inc(struct connection 
                return;
 
        if (node->perms && node->perms[0].id != conn->id) {
-               d = find_domain_by_domid(node->perms[0].id);
-               if (d)
-                       d->nbentry++;
-       }
-       else if (conn->domain) {
-               conn->domain->nbentry++;
+               if (conn->transaction) {
+                       transaction_entry_inc(conn->transaction,
+                               node->perms[0].id);
+               } else {
+                       d = find_domain_by_domid(node->perms[0].id);
+                       if (d)
+                               d->nbentry++;
+               }
+       } else if (conn->domain) {
+               if (conn->transaction) {
+                       transaction_entry_inc(conn->transaction,
+                               conn->domain->domid);
+               } else {
+                       conn->domain->nbentry++;
+               }
        }
 }
 
@@ -622,11 +631,36 @@ void domain_entry_dec(struct connection 
                return;
 
        if (node->perms && node->perms[0].id != conn->id) {
-               d = find_domain_by_domid(node->perms[0].id);
-               if (d && d->nbentry)
-                       d->nbentry--;
-       } else if (conn->domain && conn->domain->nbentry)
-               conn->domain->nbentry--;
+               if (conn->transaction) {
+                       transaction_entry_dec(conn->transaction,
+                               node->perms[0].id);
+               } else {
+                       d = find_domain_by_domid(node->perms[0].id);
+                       if (d && d->nbentry)
+                               d->nbentry--;
+               }
+       } else if (conn->domain && conn->domain->nbentry) {
+               if (conn->transaction) {
+                       transaction_entry_dec(conn->transaction,
+                               conn->domain->domid);
+               } else {
+                       conn->domain->nbentry--;
+               }
+       }
+}
+
+void domain_entry_fix(unsigned int domid, int num)
+{
+       struct domain *d;
+
+       d = find_domain_by_domid(domid);
+       if (d) {
+               if ((d->nbentry += num) < 0) {
+                       eprintf("invalid domain entry number %d",
+                               d->nbentry);
+                       d->nbentry = 0;
+               }
+       }
 }
 
 int domain_entry(struct connection *conn)
diff --git a/tools/xenstore/xenstored_domain.h 
b/tools/xenstore/xenstored_domain.h
--- a/tools/xenstore/xenstored_domain.h
+++ b/tools/xenstore/xenstored_domain.h
@@ -55,6 +55,7 @@ bool domain_is_unprivileged(struct conne
 /* 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(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
--- a/tools/xenstore/xenstored_transaction.c
+++ b/tools/xenstore/xenstored_transaction.c
@@ -32,6 +32,7 @@
 #include "list.h"
 #include "xenstored_transaction.h"
 #include "xenstored_watch.h"
+#include "xenstored_domain.h"
 #include "xs_lib.h"
 #include "utils.h"
 #include "xenstored_test.h"
@@ -48,6 +49,18 @@ struct changed_node
        bool recurse;
 };
 
+struct changed_domain
+{
+       /* List of all changed domains in the context of this transaction. */
+       struct list_head list;
+
+       /* id of the changed domain */
+       unsigned int domid;
+
+       /* changed domain entry number */
+       int nbentry;
+};
+
 struct transaction
 {
        /* List of all transactions active on this connection. */
@@ -65,6 +78,9 @@ struct transaction
 
        /* List of changed nodes. */
        struct list_head changes;
+
+       /* List of changed domains - to record the changed domain entry number 
*/
+       struct list_head changed_domains;
 };
 
 extern int quota_max_transaction;
@@ -142,6 +158,7 @@ void do_transaction_start(struct connect
        /* Attach transaction to input for autofree until it's complete */
        trans = talloc(in, struct transaction);
        INIT_LIST_HEAD(&trans->changes);
+       INIT_LIST_HEAD(&trans->changed_domains);
        trans->generation = generation;
        trans->tdb_name = talloc_asprintf(trans, "%s.%p",
                                          xs_daemon_tdb(), trans);
@@ -172,6 +189,7 @@ void do_transaction_end(struct connectio
 void do_transaction_end(struct connection *conn, const char *arg)
 {
        struct changed_node *i;
+       struct changed_domain *d;
        struct transaction *trans;
 
        if (!arg || (!streq(arg, "T") && !streq(arg, "F"))) {
@@ -204,12 +222,54 @@ void do_transaction_end(struct connectio
                /* 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, i->node, i->recurse);
                generation++;
        }
        send_ack(conn, XS_TRANSACTION_END);
+}
+
+void transaction_entry_inc(struct transaction *trans, unsigned int domid)
+{
+       struct changed_domain *d;
+
+       if (!trans)
+               return;
+
+       list_for_each_entry(d, &trans->changed_domains, list)
+               if (d->domid == domid) {
+                       d->nbentry++;
+                       return;
+               }
+
+       d = talloc(trans, struct changed_domain);
+       d->domid = domid;
+       d->nbentry = 1;
+       list_add_tail(&d->list, &trans->changed_domains);
+}
+
+void transaction_entry_dec(struct transaction *trans, unsigned int domid)
+{
+       struct changed_domain *d;
+
+       if (!trans)
+               return;
+
+       list_for_each_entry(d, &trans->changed_domains, list)
+               if (d->domid == domid) {
+                       d->nbentry--;
+                       return;
+               }
+
+       d = talloc(trans, struct changed_domain);
+       d->domid = domid;
+       d->nbentry = -1;
+       list_add_tail(&d->list, &trans->changed_domains);
 }
 
 void conn_delete_all_transactions(struct connection *conn)
diff --git a/tools/xenstore/xenstored_transaction.h 
b/tools/xenstore/xenstored_transaction.h
--- a/tools/xenstore/xenstored_transaction.h
+++ b/tools/xenstore/xenstored_transaction.h
@@ -27,6 +27,10 @@ void do_transaction_end(struct connectio
 
 struct transaction *transaction_lookup(struct connection *conn, uint32_t id);
 
+/* inc/dec entry number local to trans while changing a node */
+void transaction_entry_inc(struct transaction *trans, unsigned int domid);
+void transaction_entry_dec(struct transaction *trans, unsigned int domid);
+
 /* This node was changed: can fail and longjmp. */
 void add_change_node(struct transaction *trans, const char *node,
                      bool recurse);

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