New Xenbus driver structure, bringing an explicit state machine to the table.

This will make the drivers more robust to failure of the other end, and makes
current driver state more visible.  It also allows us to bring very similar
pieces of code from the drivers together into the xenbus driver.  This should
be the start of a fix to the problem with block and net device hotplugging,
because the code was previously confused about the difference between the
connection state of the kernel device layer, and the state of the xenbus
communication.  This confusion is addressed here.

Much of this work is influenced by Rusty Russell's skeleton driver, recently
posted to xen-devel, as that piece made suggestions as to structure and
arrangement that have been taken on board, albeit pushed down into the xenbus
layer.

A new file xenbus_client.c has been added.  This file implements the interface
between the xenbus layer and the layer specific to the driver.  Access to
lower layers is still allowed for those who do not need all the facilities of
the new driver.

xenbus_dev_error has moved from xenbus_xs.c into xenbus_client.c, and
xenbus_dev_ok has been removed, in favour of xenbus_switch_state.

DevController.py now closes down a device by writing a change to the Closing
state on the backend, to allow the frontend time to close down properly.  Its
also writes the frontend and backend details within the same transaction.

The timeout on the hotplug scripts has been cranked down from 2 minutes to 5
seconds.

The diagnose script has been updated to parse the new states.

The blktab and tpm drivers have yet to be ported to this structure.

Signed-off-by: Ewan Mellor <ewan@xensource.com>

tools/python/xen/util/diagnose.py

@@ -22,6 +22,7 @@ from xen.xend import sxp
 from xen.xend.XendClient import server
 from xen.xend.XendError import XendError
 from xen.xend.xenstore.xstransact import xstransact
+from xen.xend.server import DevController
 
 import xen.xend.XendProtocol
 
@@ -107,34 +108,57 @@ def diagnose_devices():
                 print ("Cannot find backend path for device %s, %s." %
                        (deviceClass, device))
             else:
-                backend_error = xstransact.Read(
-                    backendPath.replace('backend/', 'error/backend/'),
-                    'error')
+                frontend_state = xstransact.Read(frontendPath, 'state')
+                backend_state  = xstransact.Read(backendPath,  'state')
 
-                if backend_error:
-                    diagnose_device_error(backend_error)
+                print "Backend is in state %s." %  stateString(backend_state)
+                print "Frontend is in state %s." % stateString(frontend_state)
+
+                check_for_error(True)
+                check_for_error(False)
+
+                diagnose_hotplugging()
 
 
-def diagnose_device_error(err):
-    if re.search("2 reading .*/ring-ref and event-channel", err):
-        print ("Backend is stuck waiting for frontend for device %s, %s." %
-               (deviceClass, device))
-        diagnose_stuck_frontend()
+def check_for_error(backend):
+    if backend:
+        path = backendPath.replace('backend/', 'error/backend/')
     else:
-        print ("Device %s, %s shows error %s." %
-               (deviceClass, device, err))
+        path = frontendPath.replace('device/', 'error/device/')
+
+    err = xstransact.Read(path, 'error')
+
+    if err:
+        print ("%s for device %s, %s shows error %s." %
+               (backend and 'Backend' or 'Frontend', deviceClass, device,
+                err))
 
 
-def diagnose_stuck_frontend():
-    if deviceClass == "vbd":
+def diagnose_hotplugging():
+    if deviceClass == 'vbd':
         phy = xstransact.Read(backendPath, 'physical-device')
 
         if phy:
-            print ("Device %s, %s hotplugging has completed successfully." %
-                   (deviceClass, device))
+            print ('Device %s, %s hotplugging has completed successfully, '
+                   'and is connected to physical device %s.' %
+                   (deviceClass, device, phy))
         else:
-            print ("Device %s, %s hotplugging failed." %
+            print ('Device %s, %s hotplugging failed.' %
                    (deviceClass, device))
+    elif deviceClass == 'vif':
+        handle = xstransact.Read(backendPath, 'handle')
+
+        if handle:
+            print ('Device %s, %s hotplugging has completed successfully, '
+                   'and is using handle %s.' %
+                   (deviceClass, device, handle))
+        else:
+            print ('Device %s, %s hotplugging failed.' %
+                   (deviceClass, device))
+
+
+def stateString(state):
+    return state and DevController.xenbusState[int(state)] or '<None>'
 
 
 def main(argv = None):

tools/python/xen/xend/server/DevController.py

@@ -25,10 +25,23 @@ from xen.xend.XendLogging import log
 from xen.xend.xenstore.xstransact import xstransact
 from xen.xend.xenstore.xswatch import xswatch
 
-DEVICE_CREATE_TIMEOUT = 120
+DEVICE_CREATE_TIMEOUT = 5
 HOTPLUG_STATUS_NODE = "hotplug-status"
 HOTPLUG_STATUS_ERROR = "error"
 
+xenbusState = {
+    'Unknown'      : 0,
+    'Initialising' : 1,
+    'InitWait'     : 2,
+    'Initialised'  : 3,
+    'Connected'    : 4,
+    'Closing'      : 5,
+    'Closed'       : 6,
+    }
+
+xenbusState.update(dict(zip(xenbusState.values(), xenbusState.keys())))
+
+
 class DevController:
     """Abstract base class for a device controller.  Device controllers create
     appropriate entries in the store to trigger the creation, reconfiguration,
@@ -116,10 +129,8 @@ class DevController:
         frontpath = self.frontendPath(devid)
         backpath = xstransact.Read(frontpath, "backend")
 
-        xstransact.Remove(frontpath)
-
         if backpath:
-            xstransact.Remove(backpath)
+            xstransact.Write(backpath, 'state', str(xenbusState['Closing']))
         else:
             raise VmError("Device %s not connected" % devid)
            
@@ -257,18 +268,18 @@ class DevController:
         frontpath = self.frontendPath(devid)
         backpath  = self.backendPath(backdom, devid)
         
-        xstransact.Remove(backpath, HOTPLUG_STATUS_NODE)
-
         frontDetails.update({
             'backend' : backpath,
-            'backend-id' : "%i" % backdom.getDomid()
+            'backend-id' : "%i" % backdom.getDomid(),
+            'state' : str(xenbusState['Initialising'])
             })
 
 
         backDetails.update({
             'domain' : self.vm.getName(),
             'frontend' : frontpath,
-            'frontend-id' : "%i" % self.vm.getDomid()
+            'frontend-id' : "%i" % self.vm.getDomid(),
+            'state' : str(xenbusState['Initialising'])
             })
 
         log.debug('DevController: writing %s to %s.', str(frontDetails),
@@ -276,8 +287,20 @@ class DevController:
         log.debug('DevController: writing %s to %s.', str(backDetails),
                   backpath)
 
-        xstransact.Write(frontpath, frontDetails)
-        xstransact.Write(backpath, backDetails)
+        while True:
+            t = xstransact()
+            try:
+                t.remove2(backpath, HOTPLUG_STATUS_NODE)
+
+                t.write2(frontpath, frontDetails)
+                t.write2(backpath,  backDetails)
+
+                if t.commit():
+                    return
+            except:
+                t.abort()
+                raise
+
 
     def waitForBackend(self,devid):
         ev = Event()

tools/python/xen/xend/xenstore/xstransact.py

@@ -10,7 +10,7 @@ from xen.xend.xenstore.xsutil import xshandle
 
 class xstransact:
 
-    def __init__(self, path):
+    def __init__(self, path = ""):
         assert path is not None
         
         self.in_transaction = False # Set this temporarily -- if this
@@ -41,7 +41,7 @@ class xstransact:
         return rc
 
     def _read(self, key):
-        path = "%s/%s" % (self.path, key)
+        path = self.prependPath(key)
         try:
             return xshandle().read(self.transaction, path)
         except RuntimeError, ex:
@@ -66,7 +66,7 @@ class xstransact:
         return ret
 
     def _write(self, key, data):
-        path = "%s/%s" % (self.path, key)
+        path = self.prependPath(key)
         try:
             xshandle().write(self.transaction, path, data)
         except RuntimeError, ex:
@@ -99,7 +99,7 @@ class xstransact:
             raise TypeError
 
     def _remove(self, key):
-        path = "%s/%s" % (self.path, key)
+        path = self.prependPath(key)
         return xshandle().rm(self.transaction, path)
 
     def remove(self, *args):
@@ -114,7 +114,7 @@ class xstransact:
                 self._remove(key)
 
     def _list(self, key):
-        path = "%s/%s" % (self.path, key)
+        path = self.prependPath(key)
         l = xshandle().ls(self.transaction, path)
         if l:
             return map(lambda x: key + "/" + x, l)
@@ -215,6 +215,30 @@ class xstransact:
                 self._write(key, fmt % val)
 
 
+    def remove2(self, middlePath, *args):
+        self.callRebased(middlePath, self.remove, *args)
+
+
+    def write2(self, middlePath, *args):
+        self.callRebased(middlePath, self.write, *args)
+
+
+    def callRebased(self, middlePath, func, *args):
+        oldpath = self.path
+        self.path = self.prependPath(middlePath)
+        try:
+            func(*args)
+        finally:
+            self.path = oldpath
+
+
+    def prependPath(self, key):
+        if self.path:
+            return self.path + '/' + key
+        else:
+            return key
+
+
     def Read(cls, path, *args):
         """If only one argument is given (path), return the value stored at
         that path.  If two arguments are given, treat the second argument as a