[Qemu-devel] [PATCH v2 05/18] xen: add xenstore watcher infrastructure

[Paul Durrant]

A Xen PV frontend communicates its state to the PV backend by writing to
the 'state' key in the frontend area in xenstore. It is therefore
necessary for a XenDevice implementation to be notified whenever the
value of this key changes.

This patch adds code to do this as follows:

- an 'fd handler' is registered on the libxenstore handle which will be
  triggered whenever a 'watch' event occurs
- primitives are added to xen-bus-helper to add or remove watch events
- a list of Notifier objects is added to XenBus to provide a mechanism
  to call the appropriate 'watch handler' when its associated event
  occurs

The xen-block implementation is extended with a 'frontend_changed' method,
which calls as-yet stub 'connect' and 'disconnect' functions when the
relevant frontend state transitions occur. A subsequent patch will supply
a full implementation for these functions.

Signed-off-by: Paul Durrant <address@hidden>
---
Cc: Kevin Wolf <address@hidden>
Cc: Max Reitz <address@hidden>
Cc: Stefano Stabellini <address@hidden>
Cc: Anthony Perard <address@hidden>

v2:
 - Don't crash when xen_block_disconnect() fails
 - Check xs_unwatch() for error
 - Add new_watch() and free_watch() utility functions
 - Use xs_check_watch() rather than xs_read_watch()
---
 hw/block/trace-events           |   2 +
 hw/block/xen-block.c            |  73 ++++++++++++++
 hw/xen/trace-events             |   6 ++
 hw/xen/xen-bus-helper.c         |  34 +++++++
 hw/xen/xen-bus.c                | 217 +++++++++++++++++++++++++++++++++++++++-
 include/hw/xen/xen-bus-helper.h |   6 ++
 include/hw/xen/xen-bus.h        |  15 +++
 7 files changed, 351 insertions(+), 2 deletions(-)

diff --git a/hw/block/trace-events b/hw/block/trace-events
index 4afbd62..89e2583 100644
--- a/hw/block/trace-events
+++ b/hw/block/trace-events
@@ -130,6 +130,8 @@ xen_disk_free(char *name) "%s"
 
 # hw/block/xen-block.c
 xen_block_realize(const char *type, uint32_t disk, uint32_t partition) "%s 
d%up%u"
+xen_block_connect(const char *type, uint32_t disk, uint32_t partition) "%s 
d%up%u"
+xen_block_disconnect(const char *type, uint32_t disk, uint32_t partition) "%s 
d%up%u"
 xen_block_unrealize(const char *type, uint32_t disk, uint32_t partition) "%s 
d%up%u"
 xen_disk_realize(void) ""
 xen_disk_unrealize(void) ""
diff --git a/hw/block/xen-block.c b/hw/block/xen-block.c
index 440bec2..d2334ef 100644
--- a/hw/block/xen-block.c
+++ b/hw/block/xen-block.c
@@ -21,6 +21,24 @@ static char *xen_block_get_name(XenDevice *xendev, Error 
**errp)
     return g_strdup_printf("%lu", vdev->number);
 }
 
+static void xen_block_disconnect(XenDevice *xendev, Error **errp)
+{
+    XenBlockDevice *blockdev = XEN_BLOCK_DEVICE(xendev);
+    const char *type = object_get_typename(OBJECT(blockdev));
+    XenBlockVdev *vdev = &blockdev->vdev;
+
+    trace_xen_block_disconnect(type, vdev->disk, vdev->partition);
+}
+
+static void xen_block_connect(XenDevice *xendev, Error **errp)
+{
+    XenBlockDevice *blockdev = XEN_BLOCK_DEVICE(xendev);
+    const char *type = object_get_typename(OBJECT(blockdev));
+    XenBlockVdev *vdev = &blockdev->vdev;
+
+    trace_xen_block_connect(type, vdev->disk, vdev->partition);
+}
+
 static void xen_block_unrealize(XenDevice *xendev, Error **errp)
 {
     XenBlockDevice *blockdev = XEN_BLOCK_DEVICE(xendev);
@@ -36,6 +54,12 @@ static void xen_block_unrealize(XenDevice *xendev, Error 
**errp)
 
     trace_xen_block_unrealize(type, vdev->disk, vdev->partition);
 
+    /* Disconnect from the frontend in case this has not already happened */
+    xen_block_disconnect(xendev, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+    }
+
     if (blockdev_class->unrealize) {
         blockdev_class->unrealize(blockdev, &local_err);
         if (local_err) {
@@ -68,6 +92,54 @@ static void xen_block_realize(XenDevice *xendev, Error 
**errp)
     }
 }
 
+static void xen_block_frontend_changed(XenDevice *xendev,
+                                       enum xenbus_state frontend_state,
+                                       Error **errp)
+{
+    enum xenbus_state backend_state = xen_device_backend_get_state(xendev);
+    Error *local_err = NULL;
+
+    switch (frontend_state) {
+    case XenbusStateInitialised:
+    case XenbusStateConnected:
+        if (backend_state == XenbusStateConnected) {
+            break;
+        }
+
+        xen_block_disconnect(xendev, &local_err);
+        if (local_err) {
+            error_propagate(errp, local_err);
+            break;
+        }
+
+        xen_block_connect(xendev, &local_err);
+        if (local_err) {
+            error_propagate(errp, local_err);
+            break;
+        }
+
+        xen_device_backend_set_state(xendev, XenbusStateConnected);
+        break;
+
+    case XenbusStateClosing:
+        xen_device_backend_set_state(xendev, XenbusStateClosing);
+        break;
+
+    case XenbusStateClosed:
+        xen_block_disconnect(xendev, &local_err);
+        if (local_err) {
+            error_propagate(errp, local_err);
+            break;
+        }
+
+        xen_device_backend_set_state(xendev, XenbusStateClosed);
+        break;
+
+    default:
+        break;
+    }
+}
+
 static char *disk_to_vbd_name(unsigned int disk)
 {
     char *name, *prefix = (disk >= 26) ?
@@ -276,6 +348,7 @@ static void xen_block_class_init(ObjectClass *class, void 
*data)
 
     xendev_class->get_name = xen_block_get_name;
     xendev_class->realize = xen_block_realize;
+    xendev_class->frontend_changed = xen_block_frontend_changed;
     xendev_class->unrealize = xen_block_unrealize;
 
     dev_class->props = xen_block_props;
diff --git a/hw/xen/trace-events b/hw/xen/trace-events
index 75dc226..22055b5 100644
--- a/hw/xen/trace-events
+++ b/hw/xen/trace-events
@@ -16,13 +16,19 @@ xen_domid_restrict(int err) "err: %u"
 # include/hw/xen/xen-bus.c
 xen_bus_realize(void) ""
 xen_bus_unrealize(void) ""
+xen_bus_add_watch(const char *node, const char *key, char *token) "node: %s 
key: %s token: %s"
+xen_bus_remove_watch(const char *node, const char *key, char *token) "node: %s 
key: %s token: %s"
+xen_bus_watch(const char *token) "token: %s"
 xen_device_realize(const char *type, char *name) "type: %s name: %s"
 xen_device_unrealize(const char *type, char *name) "type: %s name: %s"
 xen_device_backend_state(const char *type, char *name, const char *state) 
"type: %s name: %s -> %s"
 xen_device_frontend_state(const char *type, char *name, const char *state) 
"type: %s name: %s -> %s"
+xen_device_frontend_changed(const char *type, char *name) "type: %s name: %s"
 
 # include/hw/xen/xen-bus-helper.c
 xs_node_create(const char *node) "%s"
 xs_node_destroy(const char *node) "%s"
 xs_node_vprintf(char *path, char *value) "%s %s"
 xs_node_vscanf(char *path, char *value) "%s %s"
+xs_node_watch(char *path) "%s"
+xs_node_unwatch(char *path) "%s"
diff --git a/hw/xen/xen-bus-helper.c b/hw/xen/xen-bus-helper.c
index 2304f8e..d1d5512 100644
--- a/hw/xen/xen-bus-helper.c
+++ b/hw/xen/xen-bus-helper.c
@@ -145,3 +145,37 @@ int xs_node_scanf(struct xs_handle *xsh, const char *node, 
const char *key,
 
     return rc;
 }
+
+void xs_node_watch(struct xs_handle *xsh, const char *node, const char *key,
+                   char *token, Error **errp)
+{
+    char *path;
+
+    path = (strlen(node) != 0) ? g_strdup_printf("%s/%s", node, key) :
+        g_strdup(key);
+
+    trace_xs_node_watch(path);
+
+    if (!xs_watch(xsh, path, token)) {
+        error_setg_errno(errp, errno, "failed to watch node '%s'", path);
+    }
+
+    g_free(path);
+}
+
+void xs_node_unwatch(struct xs_handle *xsh, const char *node,
+                     const char *key, const char *token, Error **errp)
+{
+    char *path;
+
+    path = (strlen(node) != 0) ? g_strdup_printf("%s/%s", node, key) :
+        g_strdup(key);
+
+    trace_xs_node_unwatch(path);
+
+    if (!xs_unwatch(xsh, path, token)) {
+        error_setg_errno(errp, errno, "failed to unwatch node '%s'", path);
+    }
+
+    g_free(path);
+}
diff --git a/hw/xen/xen-bus.c b/hw/xen/xen-bus.c
index 3bd5e4e..f0732f8 100644
--- a/hw/xen/xen-bus.c
+++ b/hw/xen/xen-bus.c
@@ -6,6 +6,8 @@
  */
 
 #include "qemu/osdep.h"
+#include "qemu/main-loop.h"
+#include "qemu/uuid.h"
 #include "hw/hw.h"
 #include "hw/sysbus.h"
 #include "hw/xen/xen.h"
@@ -59,6 +61,93 @@ static char *xen_bus_get_dev_path(DeviceState *dev)
     return xen_device_get_backend_path(XEN_DEVICE(dev));
 }
 
+struct XenWatch {
+    char *node, *key;
+    char *token;
+    XenWatchHandler handler;
+    void *opaque;
+    Notifier notifier;
+};
+
+static void watch_notify(Notifier *n, void *data)
+{
+    XenWatch *watch = container_of(n, XenWatch, notifier);
+    const char *token = data;
+
+    if (!strcmp(watch->token, token)) {
+        watch->handler(watch->opaque);
+    }
+}
+
+static XenWatch *new_watch(const char *node, const char *key,
+                           XenWatchHandler handler, void *opaque)
+{
+    XenWatch *watch = g_new0(XenWatch, 1);
+    QemuUUID uuid;
+
+    qemu_uuid_generate(&uuid);
+
+    watch->token = qemu_uuid_unparse_strdup(&uuid);
+    watch->node = g_strdup(node);
+    watch->key = g_strdup(key);
+    watch->handler = handler;
+    watch->opaque = opaque;
+    watch->notifier.notify = watch_notify;
+
+    return watch;
+}
+
+static void free_watch(XenWatch *watch)
+{
+    g_free(watch->token);
+    g_free(watch->key);
+    g_free(watch->node);
+
+    g_free(watch);
+}
+
+static XenWatch *xen_bus_add_watch(XenBus *xenbus, const char *node,
+                                   const char *key, XenWatchHandler handler,
+                                   void *opaque, Error **errp)
+{
+    XenWatch *watch = new_watch(node, key, handler, opaque);
+    Error *local_err = NULL;
+
+    trace_xen_bus_add_watch(watch->node, watch->key, watch->token);
+
+    notifier_list_add(&xenbus->watch_notifiers, &watch->notifier);
+
+    xs_node_watch(xenbus->xsh, node, key, watch->token, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+
+        notifier_remove(&watch->notifier);
+        free_watch(watch);
+
+        return NULL;
+    }
+
+    return watch;
+}
+
+static void xen_bus_remove_watch(XenBus *xenbus, XenWatch *watch,
+                                 Error **errp)
+{
+    Error *local_err = NULL;
+
+    trace_xen_bus_remove_watch(watch->node, watch->key, watch->token);
+
+    xs_node_unwatch(xenbus->xsh, watch->node, watch->key, watch->token,
+                    &local_err);
+
+    notifier_remove(&watch->notifier);
+    free_watch(watch);
+
+    if (local_err) {
+        error_propagate(errp, local_err);
+    }
+}
+
 static void xen_bus_unrealize(BusState *bus, Error **errp)
 {
     XenBus *xenbus = XEN_BUS(bus);
@@ -69,9 +158,33 @@ static void xen_bus_unrealize(BusState *bus, Error **errp)
         return;
     }
 
+    qemu_set_fd_handler(xs_fileno(xenbus->xsh), NULL, NULL, NULL);
+
     xs_close(xenbus->xsh);
 }
 
+static void xen_bus_watch(void *opaque)
+{
+    XenBus *xenbus = opaque;
+    char **v;
+    const char *token;
+
+    g_assert(xenbus->xsh);
+
+    v = xs_check_watch(xenbus->xsh);
+    if (!v) {
+        return;
+    }
+
+    token = v[XS_WATCH_TOKEN];
+
+    trace_xen_bus_watch(token);
+
+    notifier_list_notify(&xenbus->watch_notifiers, (void *)token);
+
+    free(v);
+}
+
 static void xen_bus_realize(BusState *bus, Error **errp)
 {
     XenBus *xenbus = XEN_BUS(bus);
@@ -92,6 +205,9 @@ static void xen_bus_realize(BusState *bus, Error **errp)
         xenbus->backend_id = 0; /* Assume lack of node means dom0 */
     }
 
+    notifier_list_init(&xenbus->watch_notifiers);
+    qemu_set_fd_handler(xs_fileno(xenbus->xsh), xen_bus_watch, NULL,
+                        xenbus);
     return;
 
 fail:
@@ -139,8 +255,25 @@ static void xen_device_backend_printf(XenDevice *xendev, 
const char *key,
     }
 }
 
-static void xen_device_backend_set_state(XenDevice *xendev,
-                                         enum xenbus_state state)
+static int xen_device_backend_scanf(XenDevice *xendev, const char *key,
+                                    const char *fmt, ...)
+{
+    XenBus *xenbus = XEN_BUS(qdev_get_parent_bus(DEVICE(xendev)));
+    va_list ap;
+    int rc;
+
+    g_assert(xenbus->xsh);
+
+    va_start(ap, fmt);
+    rc = xs_node_vscanf(xenbus->xsh, xendev->backend_path, key, NULL,
+                        fmt, ap);
+    va_end(ap);
+
+    return rc;
+}
+
+void xen_device_backend_set_state(XenDevice *xendev,
+                                  enum xenbus_state state)
 {
     const char *type = object_get_typename(OBJECT(xendev));
 
@@ -155,6 +288,11 @@ static void xen_device_backend_set_state(XenDevice *xendev,
     xen_device_backend_printf(xendev, "state", "%u", state);
 }
 
+enum xenbus_state xen_device_backend_get_state(XenDevice *xendev)
+{
+    return xendev->backend_state;
+}
+
 static void xen_device_backend_create(XenDevice *xendev, Error **errp)
 {
     XenBus *xenbus = XEN_BUS(qdev_get_parent_bus(DEVICE(xendev)));
@@ -217,6 +355,23 @@ static void xen_device_frontend_printf(XenDevice *xendev, 
const char *key,
     }
 }
 
+static int xen_device_frontend_scanf(XenDevice *xendev, const char *key,
+                                     const char *fmt, ...)
+{
+    XenBus *xenbus = XEN_BUS(qdev_get_parent_bus(DEVICE(xendev)));
+    va_list ap;
+    int rc;
+
+    g_assert(xenbus->xsh);
+
+    va_start(ap, fmt);
+    rc = xs_node_vscanf(xenbus->xsh, xendev->frontend_path, key, NULL,
+                        fmt, ap);
+    va_end(ap);
+
+    return rc;
+}
+
 static void xen_device_frontend_set_state(XenDevice *xendev,
                                           enum xenbus_state state)
 {
@@ -233,6 +388,50 @@ static void xen_device_frontend_set_state(XenDevice 
*xendev,
     xen_device_frontend_printf(xendev, "state", "%u", state);
 }
 
+static void xen_device_frontend_changed(void *opaque)
+{
+    XenDevice *xendev = opaque;
+    XenDeviceClass *xendev_class = XEN_DEVICE_GET_CLASS(xendev);
+    const char *type = object_get_typename(OBJECT(xendev));
+    enum xenbus_state state;
+
+    trace_xen_device_frontend_changed(type, xendev->name);
+
+    if (xen_device_frontend_scanf(xendev, "state", "%u", &state) != 1) {
+        state = XenbusStateUnknown;
+    }
+
+    xen_device_frontend_set_state(xendev, state);
+
+    if (xendev_class->frontend_changed) {
+        Error *local_err = NULL;
+
+        xendev_class->frontend_changed(xendev, state, &local_err);
+
+        if (local_err) {
+            error_reportf_err(local_err, "frontend change error: ");
+        }
+    }
+
+    /*
+     * If a backend is still 'online' then its state should be cycled
+     * back round to InitWait in order for a new frontend instance to
+     * connect. This may happen when, for example, a frontend driver is
+     * re-installed or updated.
+     */
+    if (xendev->backend_state == XenbusStateClosed) {
+        unsigned int online;
+
+        if (xen_device_backend_scanf(xendev, "online", "%u", &online) != 1) {
+            online = 0;
+        }
+
+        if (online) {
+            xen_device_backend_set_state(xendev, XenbusStateInitWait);
+        }
+    }
+}
+
 static void xen_device_frontend_create(XenDevice *xendev, Error **errp)
 {
     XenBus *xenbus = XEN_BUS(qdev_get_parent_bus(DEVICE(xendev)));
@@ -253,6 +452,15 @@ static void xen_device_frontend_create(XenDevice *xendev, 
Error **errp)
     if (local_err) {
         error_propagate_prepend(errp, local_err,
                                 "failed to create frontend: ");
+        return;
+    }
+
+    xendev->frontend_state_watch =
+        xen_bus_add_watch(xenbus, xendev->frontend_path, "state",
+                          xen_device_frontend_changed, xendev, &local_err);
+    if (local_err) {
+        error_propagate_prepend(errp, local_err,
+                                "failed to watch frontend state: ");
     }
 }
 
@@ -261,6 +469,11 @@ static void xen_device_frontend_destroy(XenDevice *xendev)
     XenBus *xenbus = XEN_BUS(qdev_get_parent_bus(DEVICE(xendev)));
     Error *local_err = NULL;
 
+    if (xendev->frontend_state_watch) {
+        xen_bus_remove_watch(xenbus, xendev->frontend_state_watch, NULL);
+        xendev->frontend_state_watch = NULL;
+    }
+
     if (!xendev->frontend_path) {
         return;
     }
diff --git a/include/hw/xen/xen-bus-helper.h b/include/hw/xen/xen-bus-helper.h
index 083e4de..57935ff 100644
--- a/include/hw/xen/xen-bus-helper.h
+++ b/include/hw/xen/xen-bus-helper.h
@@ -31,4 +31,10 @@ int xs_node_vscanf(struct xs_handle *xsh, const char *node, 
const char *key,
 int xs_node_scanf(struct xs_handle *xsh, const char *node, const char *key,
                   Error **errp, const char *fmt, ...);
 
+/* Watch node/key unless node is empty, in which case watch key */
+void xs_node_watch(struct xs_handle *xsh, const char *node, const char *key,
+                   char *token, Error **errp);
+void xs_node_unwatch(struct xs_handle *xsh, const char *node, const char *key,
+                     const char *token, Error **errp);
+
 #endif /* HW_XEN_BUS_HELPER_H */
diff --git a/include/hw/xen/xen-bus.h b/include/hw/xen/xen-bus.h
index 85a75d8..df73674 100644
--- a/include/hw/xen/xen-bus.h
+++ b/include/hw/xen/xen-bus.h
@@ -10,6 +10,11 @@
 
 #include "hw/xen/xen_common.h"
 #include "hw/sysbus.h"
+#include "qemu/notify.h"
+
+typedef void (*XenWatchHandler)(void *opaque);
+
+typedef struct XenWatch XenWatch;
 
 typedef struct XenDevice {
     DeviceState qdev;
@@ -18,10 +23,14 @@ typedef struct XenDevice {
     char *backend_path, *frontend_path;
     enum xenbus_state backend_state, frontend_state;
     Notifier exit;
+    XenWatch *frontend_state_watch;
 } XenDevice;
 
 typedef char *(*XenDeviceGetName)(XenDevice *xendev, Error **errp);
 typedef void (*XenDeviceRealize)(XenDevice *xendev, Error **errp);
+typedef void (*XenDeviceFrontendChanged)(XenDevice *xendev,
+                                         enum xenbus_state frontend_state,
+                                         Error **errp);
 typedef void (*XenDeviceUnrealize)(XenDevice *xendev, Error **errp);
 
 typedef struct XenDeviceClass {
@@ -32,6 +41,7 @@ typedef struct XenDeviceClass {
     const char *device;
     XenDeviceGetName get_name;
     XenDeviceRealize realize;
+    XenDeviceFrontendChanged frontend_changed;
     XenDeviceUnrealize unrealize;
 } XenDeviceClass;
 
@@ -47,6 +57,7 @@ typedef struct XenBus {
     BusState qbus;
     domid_t backend_id;
     struct xs_handle *xsh;
+    NotifierList watch_notifiers;
 } XenBus;
 
 typedef struct XenBusClass {
@@ -64,4 +75,8 @@ typedef struct XenBusClass {
 
 void xen_bus_init(void);
 
+void xen_device_backend_set_state(XenDevice *xendev,
+                                  enum xenbus_state state);
+enum xenbus_state xen_device_backend_get_state(XenDevice *xendev);
+
 #endif /* HW_XEN_BUS_H */
-- 
2.1.4