From mboxrd@z Thu Jan 1 00:00:00 1970 From: Mats Petersson Subject: Re: [PATCH][xend] Fix/cleanup destoryDevice code path Date: Thu, 02 Aug 2007 20:25:39 +0100 Message-ID: <46b22fc8.0422300a.6b61.fffffe9a@mx.google.com> References: <46B22A04.3000106@novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; format=flowed Return-path: In-Reply-To: <46B22A04.3000106@novell.com> References: <46B22A04.3000106@novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Jim Fehlig , xen-devel@lists.xensource.com List-Id: xen-devel@lists.xenproject.org Jim Can you check if this removes the device nodes within xenstore when doing xm save [1] - as I modified this section of code to fix up a problem with just that not so long ago. Unfortunately, I'm not able to test it myself, as I'm "without a xen-capable machine at the moment". I have no particular reason to believe it DOESN'T do that, I'm just asking to make sure that it's destroyed properly, because I found that leaving lots of device info in xenstore bloats the database in xenstore, and eventually Dom0 runs out of physical memory.... :-( The easiest way to do this is to repeatedly save and restore a domain, whilst whatching the size of xenstore, e.g. "xenstore-ls | wc" -- Mats At 20:01 02/08/2007, Jim Fehlig wrote: >More subtle problems with destroyDevice code path in xend. See attached >patch > >Regards, >Jim > ># HG changeset patch ># User Jim Fehlig ># Date 1186081049 21600 ># Node ID 430ae0d3a333ff4d212df7c2313caa03e8f4dd51 ># Parent 88bb0d305308a2cab31fd8559a6a2719db1ea55a >Fix/cleanup destroyDevice code path in xend. > >When calling destroyDevice code path (e.g. xm block-detach dom devid), >allow specifying an integer device id or a device name such as xvdN or >/dev/xvdN. Allowing the /dev/xvdN form is useful when detaching devices >from dom0. Bootloaders may do this to unmount a disk previously >mounted in dom0. > >Move examination of device ID format into the DevController, permitting >device controllers to determine a valid device ID instead of higher >level code. > >Signed-off-by: Jim Fehlig > >diff -r 88bb0d305308 -r 430ae0d3a333 tools/python/xen/xend/XendDomainInfo.py >--- a/tools/python/xen/xend/XendDomainInfo.py Wed Aug 01 15:47:54 2007 +0100 >+++ b/tools/python/xen/xend/XendDomainInfo.py Thu Aug 02 12:57:29 2007 -0600 >@@ -559,18 +559,8 @@ class XendDomainInfo: > self.getDeviceController(devclass).waitForDevices() > > def destroyDevice(self, deviceClass, devid, force = False): >- try: >- dev = int(devid) >- except ValueError: >- # devid is not a number but a string containing either device >- # name (e.g. xvda) or device_type/device_id (e.g. vbd/51728) >- dev = type(devid) is str and devid.split('/')[-1] or None >- if dev == None: >- log.debug("Could not find the device %s", devid) >- return None >- >- log.debug("dev = %s", dev) >- return >self.getDeviceController(deviceClass).destroyDevice(dev, force) >+ log.debug("dev = %s", devid) >+ return >self.getDeviceController(deviceClass).destroyDevice(devid, force) > > def getDeviceSxprs(self, deviceClass): > if self._stateGet() in (DOM_STATE_RUNNING, DOM_STATE_PAUSED): >diff -r 88bb0d305308 -r 430ae0d3a333 >tools/python/xen/xend/server/DevController.py >--- a/tools/python/xen/xend/server/DevController.py Wed Aug 01 >15:47:54 2007 +0100 >+++ b/tools/python/xen/xend/server/DevController.py Thu Aug 02 >12:57:29 2007 -0600 >@@ -203,27 +203,32 @@ class DevController: > > The implementation here simply deletes the appropriate > paths from the > store. This may be overridden by subclasses who need to > perform other >- tasks on destruction. Further, the implementation here can only >- accept integer device IDs, or values that can be converted to >- integers. Subclasses may accept other values and convert them to >- integers before passing them here. >- """ >- >- devid = int(devid) >+ tasks on destruction. The implementation here accepts integer device >+ IDs or paths containg integer deviceIDs, e.g. vfb/0. Subclasses may >+ accept other values and convert them to integers before passing them >+ here. >+ """ >+ >+ try: >+ dev = int(devid) >+ except ValueError: >+ # Does devid contain devicetype/deviceid? >+ # Propogate exception if unable to find an integer devid >+ dev = int(type(devid) is str and devid.split('/')[-1] or None) > > # Modify online status /before/ updating state (latter is watched by > # drivers, so this ordering avoids a race). >- self.writeBackend(devid, 'online', "0") >- self.writeBackend(devid, 'state', str(xenbusState['Closing'])) >+ self.writeBackend(dev, 'online', "0") >+ self.writeBackend(dev, 'state', str(xenbusState['Closing'])) > > if force: >- frontpath = self.frontendPath(devid) >+ frontpath = self.frontendPath(dev) > backpath = xstransact.Read(frontpath, "backend") > if backpath: > xstransact.Remove(backpath) > xstransact.Remove(frontpath) > >- self.vm._removeVm("device/%s/%d" % (self.deviceClass, devid)) >+ self.vm._removeVm("device/%s/%d" % (self.deviceClass, dev)) > > def configurations(self): > return map(self.configuration, self.deviceIDs()) >diff -r 88bb0d305308 -r 430ae0d3a333 tools/python/xen/xend/server/blkif.py >--- a/tools/python/xen/xend/server/blkif.py Wed Aug 01 15:47:54 2007 +0100 >+++ b/tools/python/xen/xend/server/blkif.py Thu Aug 02 12:57:29 2007 -0600 >@@ -149,13 +149,16 @@ class BlkifController(DevController): > def destroyDevice(self, devid, force): > """@see DevController.destroyDevice""" > >- # If we are given a device name, then look up the device ID from it, >- # and destroy that ID instead. If what we are given is an integer, >- # then assume it's a device ID and pass it straight through to our >- # superclass's method. >- >+ # vbd device IDs can be either string or integer. Further, the >+ # following string values are possible: >+ # - devicetype/deviceid (vbd/51728) >+ # - devicetype/devicename (/dev/xvdb) >+ # - devicename (xvdb) >+ # Let our superclass handle integer or devicetype/deviceid forms. >+ # If we are given a device name form, then look up the device ID >+ # from it, and destroy that ID instead. > try: >- DevController.destroyDevice(self, int(devid), force) >+ DevController.destroyDevice(self, devid, force) > except ValueError: > devid_end = type(devid) is str and devid.split('/')[-1] or None > > >_______________________________________________ >Xen-devel mailing list >Xen-devel@lists.xensource.com >http://lists.xensource.com/xen-devel