[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH 04/18] xen/pvcalls: xenbus state handling





On 05/15/2017 04:35 PM, Stefano Stabellini wrote:
Introduce the code to handle xenbus state changes.

Implement the probe function for the pvcalls backend. Write the
supported versions, max-page-order and function-calls nodes to xenstore,
as required by the protocol.

Introduce stub functions for disconnecting/connecting to a frontend.

Signed-off-by: Stefano Stabellini <stefano@xxxxxxxxxxx>
CC: boris.ostrovsky@xxxxxxxxxx
CC: jgross@xxxxxxxx
---
 drivers/xen/pvcalls-back.c | 133 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 133 insertions(+)

diff --git a/drivers/xen/pvcalls-back.c b/drivers/xen/pvcalls-back.c
index 46a889a..86eca19 100644
--- a/drivers/xen/pvcalls-back.c
+++ b/drivers/xen/pvcalls-back.c
@@ -25,6 +25,9 @@
 #include <xen/xenbus.h>
 #include <xen/interface/io/pvcalls.h>

+#define PVCALLS_VERSIONS "1"
+#define MAX_RING_ORDER XENBUS_MAX_RING_GRANT_ORDER
+
 struct pvcalls_ioworker {
        struct work_struct register_work;
        atomic_t io;
@@ -45,15 +48,145 @@ static void pvcalls_back_ioworker(struct work_struct *work)
 {
 }

+static int backend_connect(struct xenbus_device *dev)
+{
+       return 0;
+}
+
+static int backend_disconnect(struct xenbus_device *dev)
+{
+       return 0;
+}
+
 static int pvcalls_back_probe(struct xenbus_device *dev,
                              const struct xenbus_device_id *id)
 {
+       int err;
+
+       err = xenbus_printf(XBT_NIL, dev->nodename, "versions", "%s",
+                           PVCALLS_VERSIONS);
+       if (err) {
+               pr_warn("%s write out 'version' failed\n", __func__);
+               return -EINVAL;

Why not return err? (below too)


+       }
+
+       err = xenbus_printf(XBT_NIL, dev->nodename, "max-page-order", "%u",
+                           MAX_RING_ORDER);
+       if (err) {
+               pr_warn("%s write out 'max-page-order' failed\n", __func__);
+               return -EINVAL;
+       }
+
+       /* "1" means socket, connect, release, bind, listen, accept and poll*/
+       err = xenbus_printf(XBT_NIL, dev->nodename, "function-calls", "1");


Should "1" be defined in the (public) header file?


+       if (err) {
+               pr_warn("%s write out 'function-calls' failed\n", __func__);
+               return -EINVAL;
+       }
+
+       err = xenbus_switch_state(dev, XenbusStateInitWait);
+       if (err)
+               return err;
+
        return 0;
 }

+static void set_backend_state(struct xenbus_device *dev,
+                             enum xenbus_state state)
+{
+       while (dev->state != state) {
+               switch (dev->state) {
+               case XenbusStateClosed:
+                       switch (state) {
+                       case XenbusStateInitWait:
+                       case XenbusStateConnected:
+                               xenbus_switch_state(dev, XenbusStateInitWait);
+                               break;
+                       case XenbusStateClosing:
+                               xenbus_switch_state(dev, XenbusStateClosing);
+                               break;
+                       default:
+                               __WARN();
+                       }
+                       break;
+               case XenbusStateInitWait:
+               case XenbusStateInitialised:
+                       switch (state) {
+                       case XenbusStateConnected:
+                               backend_connect(dev);
+                               xenbus_switch_state(dev, XenbusStateConnected);
+                               break;
+                       case XenbusStateClosing:
+                       case XenbusStateClosed:
+                               xenbus_switch_state(dev, XenbusStateClosing);
+                               break;
+                       default:
+                               __WARN();
+                       }
+                       break;
+               case XenbusStateConnected:
+                       switch (state) {
+                       case XenbusStateInitWait:
+                       case XenbusStateClosing:
+                       case XenbusStateClosed:
+                               down_write(&pvcalls_back_global.privs_lock);
+                               backend_disconnect(dev);
+                               up_write(&pvcalls_back_global.privs_lock);


Unless you plan to have more stuff under the semaphore, I'd consider putting them in backend_disconnect().


+                               xenbus_switch_state(dev, XenbusStateClosing);
+                               break;
+                       default:
+                               __WARN();
+                       }
+                       break;
+               case XenbusStateClosing:
+                       switch (state) {
+                       case XenbusStateInitWait:
+                       case XenbusStateConnected:
+                       case XenbusStateClosed:
+                               xenbus_switch_state(dev, XenbusStateClosed);
+                               break;
+                       default:
+                               __WARN();
+                       }
+                       break;
+               default:
+                       __WARN();
+               }
+       }
+}
+
 static void pvcalls_back_changed(struct xenbus_device *dev,
                                 enum xenbus_state frontend_state)
 {
+       switch (frontend_state) {
+       case XenbusStateInitialising:
+               set_backend_state(dev, XenbusStateInitWait);
+               break;
+
+       case XenbusStateInitialised:
+       case XenbusStateConnected:
+               set_backend_state(dev, XenbusStateConnected);
+               break;
+
+       case XenbusStateClosing:
+               set_backend_state(dev, XenbusStateClosing);
+               break;
+
+       case XenbusStateClosed:
+               set_backend_state(dev, XenbusStateClosed);
+               if (xenbus_dev_is_online(dev))
+                       break;
+               /* fall through if not online */
+       case XenbusStateUnknown:
+               set_backend_state(dev, XenbusStateClosed);


You are setting XenbusStateClosed twice in case of fallthrough.

-boris


+               device_unregister(&dev->dev);
+               break;
+
+       default:
+               xenbus_dev_fatal(dev, -EINVAL, "saw state %d at frontend",
+                                frontend_state);
+               break;
+       }
 }

 static int pvcalls_back_remove(struct xenbus_device *dev)


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.