# HG changeset patch
# User Keir Fraser <keir.fraser@xxxxxxxxxx>
# Date 1198758454 0
# Node ID 9fe92a88912b45c0c44184d7e3b16983bf5aed47
# Parent d5f0afb58589e137f100dda9d22ba99dfc29aef5
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@xxxxxxx>
---
tools/python/xen/xend/XendDomainInfo.py | 41 ++++++++++++++-------------
tools/python/xen/xend/server/pciif.py | 2 -
tools/python/xen/xend/xenstore/xstransact.py | 13 +++++++-
3 files changed, 33 insertions(+), 23 deletions(-)
diff -r d5f0afb58589 -r 9fe92a88912b tools/python/xen/xend/XendDomainInfo.py
--- a/tools/python/xen/xend/XendDomainInfo.py Thu Dec 27 12:03:02 2007 +0000
+++ b/tools/python/xen/xend/XendDomainInfo.py Thu Dec 27 12:27:34 2007 +0000
@@ -1525,18 +1525,19 @@ class XendDomainInfo:
log.debug("Releasing devices")
t = xstransact("%s/device" % self.dompath)
- for devclass in XendDevices.valid_devices():
- for dev in t.list(devclass):
- try:
- log.debug("Removing %s", dev);
- self.destroyDevice(devclass, dev, False);
- except:
- # Log and swallow any exceptions in removal --
- # there's nothing more we can do.
+ try:
+ for devclass in XendDevices.valid_devices():
+ for dev in t.list(devclass):
+ try:
+ log.debug("Removing %s", dev);
+ self.destroyDevice(devclass, dev, False);
+ except:
+ # Log and swallow any exceptions in removal --
+ # there's nothing more we can do.
log.exception("Device release failed: %s; %s; %s",
self.info['name_label'], devclass, dev)
-
-
+ finally:
+ t.abort()
def getDeviceController(self, name):
"""Get the device controller for this domain, and if it
@@ -1848,16 +1849,18 @@ class XendDomainInfo:
# build list of phantom devices to be removed after normal devices
plist = []
if self.domid is not None:
- from xen.xend.xenstore.xstransact import xstransact
t = xstransact("%s/device/vbd" % GetDomainPath(self.domid))
- for dev in t.list():
- backend_phantom_vbd =
xstransact.Read("%s/device/vbd/%s/phantom_vbd" \
- % (self.dompath, dev))
- if backend_phantom_vbd is not None:
- frontend_phantom_vbd = xstransact.Read("%s/frontend" \
- % backend_phantom_vbd)
- plist.append(backend_phantom_vbd)
- plist.append(frontend_phantom_vbd)
+ try:
+ for dev in t.list():
+ backend_phantom_vbd =
xstransact.Read("%s/device/vbd/%s/phantom_vbd" \
+ % (self.dompath, dev))
+ if backend_phantom_vbd is not None:
+ frontend_phantom_vbd = xstransact.Read("%s/frontend" \
+ % backend_phantom_vbd)
+ plist.append(backend_phantom_vbd)
+ plist.append(frontend_phantom_vbd)
+ finally:
+ t.abort()
return plist
def _cleanup_phantom_devs(self, plist):
diff -r d5f0afb58589 -r 9fe92a88912b tools/python/xen/xend/server/pciif.py
--- a/tools/python/xen/xend/server/pciif.py Thu Dec 27 12:03:02 2007 +0000
+++ b/tools/python/xen/xend/server/pciif.py Thu Dec 27 12:27:34 2007 +0000
@@ -22,8 +22,6 @@ from xen.xend import sxp
from xen.xend import sxp
from xen.xend.XendError import VmError
from xen.xend.XendLogging import log
-
-from xen.xend.xenstore.xstransact import xstransact
from xen.xend.server.DevController import DevController
diff -r d5f0afb58589 -r 9fe92a88912b
tools/python/xen/xend/xenstore/xstransact.py
--- a/tools/python/xen/xend/xenstore/xstransact.py Thu Dec 27 12:03:02
2007 +0000
+++ b/tools/python/xen/xend/xenstore/xstransact.py Thu Dec 27 12:27:34
2007 +0000
@@ -7,8 +7,16 @@
from xen.xend.xenstore.xsutil import xshandle
-
class xstransact:
+ """WARNING: Be very careful if you're instantiating an xstransact object
+ yourself (i.e. not using the capitalized static helpers like .Read().
+ It is essential that you clean up the object in place via
+ t.commit/abort(): GC can happen at any time, including contexts where
+ it's not safe to to use the shared xenstore socket fd. In particular,
+ if xend forks, and GC occurs, we can have two processes trying to
+ use the same xenstore fd, and all hell breaks loose.
+ """
+
def __init__(self, path = ""):
@@ -22,8 +30,9 @@ class xstransact:
self.in_transaction = True
def __del__(self):
+ # see above.
if self.in_transaction:
- xshandle().transaction_end(self.transaction, True)
+ raise RuntimeError("ERROR: GC of live transaction")
def commit(self):
if not self.in_transaction:
_______________________________________________
Xen-changelog mailing list
Xen-changelog@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-changelog
|
Home