Re: [Xen-devel] [PATCH][xend] Fix/cleanup destoryDevice code path

[Mats Petersson <mats@xxxxxxxxxxxxxxxxx>]

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 <jfehlig@xxxxxxxxxx>
> # 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 <jfehlig@xxxxxxxxxx>
>
> 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@xxxxxxxxxxxxxxxxxxx
> http://lists.xensource.com/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel