ia64/xen-unstable

changeset 16664:9fe92a88912b

Fix xend xenstore handling.

xend can get into a situation where two processes are attempting to
interact with the xenstore socket, with disastrous results. Fix the
two bad users of xstransact, add a big warning, and fix the destructor
so future mistakes will be detected earlier.

Signed-off-by: John Levon <john.levon@sun.com>
author Keir Fraser <keir.fraser@citrix.com>
date Thu Dec 27 12:27:34 2007 +0000 (2007-12-27)
parents d5f0afb58589
children 003542d9ab77
files tools/python/xen/xend/XendDomainInfo.py tools/python/xen/xend/server/pciif.py tools/python/xen/xend/xenstore/xstransact.py
line diff
     1.1 --- a/tools/python/xen/xend/XendDomainInfo.py	Thu Dec 27 12:03:02 2007 +0000
     1.2 +++ b/tools/python/xen/xend/XendDomainInfo.py	Thu Dec 27 12:27:34 2007 +0000
     1.3 @@ -1525,18 +1525,19 @@ class XendDomainInfo:
     1.4  
     1.5          log.debug("Releasing devices")
     1.6          t = xstransact("%s/device" % self.dompath)
     1.7 -        for devclass in XendDevices.valid_devices():
     1.8 -            for dev in t.list(devclass):
     1.9 -                try:
    1.10 -                    log.debug("Removing %s", dev);
    1.11 -                    self.destroyDevice(devclass, dev, False);
    1.12 -                except:
    1.13 -                    # Log and swallow any exceptions in removal --
    1.14 -                    # there's nothing more we can do.
    1.15 +        try:
    1.16 +            for devclass in XendDevices.valid_devices():
    1.17 +                for dev in t.list(devclass):
    1.18 +                    try:
    1.19 +                        log.debug("Removing %s", dev);
    1.20 +                        self.destroyDevice(devclass, dev, False);
    1.21 +                    except:
    1.22 +                        # Log and swallow any exceptions in removal --
    1.23 +                        # there's nothing more we can do.
    1.24                          log.exception("Device release failed: %s; %s; %s",
    1.25                                        self.info['name_label'], devclass, dev)
    1.26 -
    1.27 -            
    1.28 +        finally:
    1.29 +            t.abort()
    1.30  
    1.31      def getDeviceController(self, name):
    1.32          """Get the device controller for this domain, and if it
    1.33 @@ -1848,16 +1849,18 @@ class XendDomainInfo:
    1.34          # build list of phantom devices to be removed after normal devices
    1.35          plist = []
    1.36          if self.domid is not None:
    1.37 -            from xen.xend.xenstore.xstransact import xstransact
    1.38              t = xstransact("%s/device/vbd" % GetDomainPath(self.domid))
    1.39 -            for dev in t.list():
    1.40 -                backend_phantom_vbd = xstransact.Read("%s/device/vbd/%s/phantom_vbd" \
    1.41 -                                      % (self.dompath, dev))
    1.42 -                if backend_phantom_vbd is not None:
    1.43 -                    frontend_phantom_vbd =  xstransact.Read("%s/frontend" \
    1.44 -                                      % backend_phantom_vbd)
    1.45 -                    plist.append(backend_phantom_vbd)
    1.46 -                    plist.append(frontend_phantom_vbd)
    1.47 +            try:
    1.48 +                for dev in t.list():
    1.49 +                    backend_phantom_vbd = xstransact.Read("%s/device/vbd/%s/phantom_vbd" \
    1.50 +                                          % (self.dompath, dev))
    1.51 +                    if backend_phantom_vbd is not None:
    1.52 +                        frontend_phantom_vbd =  xstransact.Read("%s/frontend" \
    1.53 +                                          % backend_phantom_vbd)
    1.54 +                        plist.append(backend_phantom_vbd)
    1.55 +                        plist.append(frontend_phantom_vbd)
    1.56 +            finally:
    1.57 +                t.abort()
    1.58          return plist
    1.59  
    1.60      def _cleanup_phantom_devs(self, plist):
     2.1 --- a/tools/python/xen/xend/server/pciif.py	Thu Dec 27 12:03:02 2007 +0000
     2.2 +++ b/tools/python/xen/xend/server/pciif.py	Thu Dec 27 12:27:34 2007 +0000
     2.3 @@ -23,8 +23,6 @@ from xen.xend import sxp
     2.4  from xen.xend.XendError import VmError
     2.5  from xen.xend.XendLogging import log
     2.6  
     2.7 -from xen.xend.xenstore.xstransact import xstransact
     2.8 -
     2.9  from xen.xend.server.DevController import DevController
    2.10  
    2.11  import xen.lowlevel.xc
     3.1 --- a/tools/python/xen/xend/xenstore/xstransact.py	Thu Dec 27 12:03:02 2007 +0000
     3.2 +++ b/tools/python/xen/xend/xenstore/xstransact.py	Thu Dec 27 12:27:34 2007 +0000
     3.3 @@ -7,8 +7,16 @@
     3.4  
     3.5  from xen.xend.xenstore.xsutil import xshandle
     3.6  
     3.7 +class xstransact:
     3.8 +    """WARNING: Be very careful if you're instantiating an xstransact object
     3.9 +       yourself (i.e. not using the capitalized static helpers like .Read().
    3.10 +       It is essential that you clean up the object in place via
    3.11 +       t.commit/abort(): GC can happen at any time, including contexts where
    3.12 +       it's not safe to to use the shared xenstore socket fd. In particular,
    3.13 +       if xend forks, and GC occurs, we can have two processes trying to
    3.14 +       use the same xenstore fd, and all hell breaks loose.
    3.15 +       """
    3.16  
    3.17 -class xstransact:
    3.18  
    3.19      def __init__(self, path = ""):
    3.20          
    3.21 @@ -22,8 +30,9 @@ class xstransact:
    3.22          self.in_transaction = True
    3.23  
    3.24      def __del__(self):
    3.25 +        # see above.
    3.26          if self.in_transaction:
    3.27 -            xshandle().transaction_end(self.transaction, True)
    3.28 +            raise RuntimeError("ERROR: GC of live transaction")
    3.29  
    3.30      def commit(self):
    3.31          if not self.in_transaction: