tools/xenstored: Ignore domain we were unable to restore

Commit 939775cfd3 "handle dying domains in live update" was meant to
handle gracefully dying domain. However, the @releaseDomain watch
will end up to be sent as soon as we finished to restore Xenstored
state.

This may be before Xen reports the domain to be dying (such as if
the guest decided to revoke access to the xenstore page). Consequently
daemon like xenconsoled will not clean-up the domain and it will be
left as a zombie.

To avoid the problem, mark the connection as ignored. This also
requires to tweak conn_can_write() and conn_can_read() to prevent
dereferencing a NULL pointer (the interface will not mapped).

The check conn->is_ignored was originally added after the callbacks
because the helpers for a socket connection may close the fd. However,
ignore_connection() will close a socket connection directly. So it is
fine to do the re-order.

Signed-off-by: Julien Grall <jgrall@amazon.com>
Release-Acked-by: Ian Jackson <iwj@xenproject.org>
Reviewed-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Luca Fancellu <luca.fancellu@arm.com>

diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
index 0d4c73d6e20c31183bd83e10cacf80851e652d0a..91d093a12ea6840684fa5b85fd9fb0a360480dc7 100644 (file)
--- a/tools/xenstore/xenstored_core.c
+++ b/tools/xenstore/xenstored_core.c
@@ -338,10 +338,10 @@ static int destroy_conn(void *_conn)
 
 static bool conn_can_read(struct connection *conn)
 {
-       if (!conn->funcs->can_read(conn))
+       if (conn->is_ignored)
                return false;
 
-       if (conn->is_ignored)
+       if (!conn->funcs->can_read(conn))
                return false;
 
        /*
@@ -356,7 +356,7 @@ static bool conn_can_read(struct connection *conn)
 
 static bool conn_can_write(struct connection *conn)
 {
-       return conn->funcs->can_write(conn) && !conn->is_ignored;
+       return !conn->is_ignored && conn->funcs->can_write(conn);
 }
 
 /* This function returns index inside the array if succeed, -1 if fail */
@@ -1466,7 +1466,7 @@ static struct {
  *
  * All watches, transactions, buffers will be freed.
  */
-static void ignore_connection(struct connection *conn)
+void ignore_connection(struct connection *conn)
 {
        struct buffered_data *out, *tmp;
 

diff --git a/tools/xenstore/xenstored_core.h b/tools/xenstore/xenstored_core.h
index 258f6ff38279cc2e2c7e205ad5f544c49028a386..07d861d9249952b1b75cbe7aa107bb709ded7b9a 100644 (file)
--- a/tools/xenstore/xenstored_core.h
+++ b/tools/xenstore/xenstored_core.h
@@ -206,6 +206,7 @@ struct node *read_node(struct connection *conn, const void *ctx,
 
 struct connection *new_connection(const struct interface_funcs *funcs);
 struct connection *get_connection_by_id(unsigned int conn_id);
+void ignore_connection(struct connection *conn);
 void check_store(void);
 void corrupt(struct connection *conn, const char *fmt, ...);
 

diff --git a/tools/xenstore/xenstored_domain.c b/tools/xenstore/xenstored_domain.c
index 47e9107c144ea2fd00d1c8f43e0c4911fa4db492..d03c7d93a9e767dd8f193239f1d98eec862ea5a8 100644 (file)
--- a/tools/xenstore/xenstored_domain.c
+++ b/tools/xenstore/xenstored_domain.c
@@ -268,14 +268,7 @@ void check_domains(void)
                                domain->shutdown = true;
                                notify = 1;
                        }
-                       /*
-                        * On Restore, we may have been unable to remap the
-                        * interface and the port. As we don't know whether
-                        * this was because of a dying domain, we need to
-                        * check if the interface and port are still valid.
-                        */
-                       if (!dominfo.dying && domain->port &&
-                           domain->interface)
+                       if (!dominfo.dying)
                                continue;
                }
                if (domain->conn) {
@@ -1303,6 +1296,17 @@ void read_state_connection(const void *ctx, const void *state)
                if (!domain)
                        barf("domain allocation error");
 
+               conn = domain->conn;
+
+               /*
+                * We may not have been able to restore the domain (for
+                * instance because it revoked the Xenstore grant). We need
+                * to keep it around to send @releaseDomain when it is
+                * dead. So mark it as ignored.
+                */
+               if (!domain->port || !domain->interface)
+                       ignore_connection(conn);
+
                if (sc->spec.ring.tdomid != DOMID_INVALID) {
                        tdomain = find_or_alloc_domain(ctx,
                                                       sc->spec.ring.tdomid);
@@ -1311,7 +1315,6 @@ void read_state_connection(const void *ctx, const void *state)
                        talloc_reference(domain->conn, tdomain->conn);
                        domain->conn->target = tdomain->conn;
                }
-               conn = domain->conn;
        }
 
        conn->conn_id = sc->conn_id;