Ryan O''Connor
2009-Jun-23 17:10 UTC
[Xen-devel] [PATCH] xend: fix vbd/tapdisk device destruction
blktap2 devices attach as a regular ''vbd'' rather than a ''tap'' device. Accordingly, Xend should use ''vbd'' as the device class of blktap2 devices. This patch determines the appropriate device class by reading the ''type'' field of the backend''s xenstore entry. Signed-off-by: Ryan O''Connor <rjo@cs.ubc.ca> diff -r 3c7536d6b583 -r 3f100fc620fd tools/python/xen/xend/XendDomainInfo.py --- a/tools/python/xen/xend/XendDomainInfo.py Tue Jun 23 17:27:01 2009 +0100 +++ b/tools/python/xen/xend/XendDomainInfo.py Tue Jun 23 10:09:20 2009 -0700 @@ -1244,13 +1244,33 @@ class XendDomainInfo: dev_num += 1 return sxprs - def getBlockDeviceClass(self, devid): - # To get a device number from the devid, - # we temporarily use the device controller of VBD. - dev = self.getDeviceController(''vbd'').convertToDeviceNumber(devid) - dev_info = self._getDeviceInfo_vbd(dev) - if dev_info: - return dev_info[0] + def getBlockDeviceClass(self, devid, deviceClass=False): + backendType = ''vbd'' + + # For backwards compatability, we may have to get deviceClass ourselves + if not deviceClass: + t = xstransact("%s/device" % self.dompath) + for devclass in XendDevices.valid_devices(): + if devid in t.list(devclass): + deviceClass = devclass + break + t.abort() + + try: + # Note: deviceClass may be incorrect, so we cannot trust readBackend + # or readFrontend from the Device Controller to succeed + backendPath = xstransact.Read("%s/device/%s/%s" + % (self.dompath, deviceClass, devid), + "backend") + backendType = xstransact.Read(backendPath, "type") + except Exception, ex: + # we can probably just assume the block device is a VBD + log.debug("error reading backend for device (%s): %s", devid, str(ex)) + + if backendType == ''tap'': + return ''tap'' + else: + return ''vbd'' def _getDeviceInfo_vif(self, mac): for dev_type, dev_info in self.info.all_devices_sxpr(): @@ -2217,7 +2237,7 @@ class XendDomainInfo: # may possibly be "tap". Just in case, verify # device class. devid = dev.split(''/'')[-1] - true_devclass = self.getBlockDeviceClass(devid) + true_devclass = self.getBlockDeviceClass(devid, devclass) log.debug("Removing %s", dev); self.destroyDevice(true_devclass, dev, False); except: _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Masaki Kanno
2009-Jun-24 04:09 UTC
Re: [Xen-devel] [PATCH] xend: fix vbd/tapdisk device destruction
Hi Ryan, I detected problems by testing your patch. In the case of inactive domains: xm block-detach failed. # xm block-list vm2 Vdev BE handle state evt-ch ring-ref BE-path 769 -1 0 -1 -1 -1 ?? 833 -1 0 -1 -1 -1 ?? # xm block-detach vm2 833 Error: (22, ''Invalid argument'') Usage: xm block-detach <Domain> <DevId> [-f|--force] Destroy a domain''s virtual block device In the case of active domains: xm block-deatch succeeded. But I saw the following message in xend.log. # xm block-list vm2 Vdev BE handle state evt-ch ring-ref BE-path 769 0 0 4 10 8 /local/domain/0/backend/vbd/3/769 833 0 0 4 11 9 /local/domain/0/backend/vbd/3/833 # xm block-detach vm2 833 # xm block-list vm2 Vdev BE handle state evt-ch ring-ref BE-path 769 0 0 4 10 8 /local/domain/0/backend/vbd/3/769 [2009-06-24 12:49:00 4433] DEBUG (XendDomainInfo:1268) error reading backend for device (833): To solve all problems, I will send a patch instead of your patch. Best regards, Kan Tue, 23 Jun 2009 10:10:39 -0700, "Ryan O''Connor" wrote:>blktap2 devices attach as a regular ''vbd'' rather than a ''tap'' device. >Accordingly, Xend should use ''vbd'' as the device class of blktap2 devices. >This >patch determines the appropriate device class by reading the ''type'' field >of the >backend''s xenstore entry. > >Signed-off-by: Ryan O''Connor <rjo@cs.ubc.ca> > >diff -r 3c7536d6b583 -r 3f100fc620fd tools/python/xen/xend/XendDomainInfo.py >--- a/tools/python/xen/xend/XendDomainInfo.py Tue Jun 23 17:27:01 2009 +0100 >+++ b/tools/python/xen/xend/XendDomainInfo.py Tue Jun 23 10:09:20 2009 -0700 >@@ -1244,13 +1244,33 @@ class XendDomainInfo: > dev_num += 1 > return sxprs > >- def getBlockDeviceClass(self, devid): >- # To get a device number from the devid, >- # we temporarily use the device controller of VBD. >- dev = self.getDeviceController(''vbd'').convertToDeviceNumber(devid) >- dev_info = self._getDeviceInfo_vbd(dev) >- if dev_info: >- return dev_info[0] >+ def getBlockDeviceClass(self, devid, deviceClass=False): >+ backendType = ''vbd'' >+ >+ # For backwards compatability, we may have to get deviceClass >ourselves >+ if not deviceClass: >+ t = xstransact("%s/device" % self.dompath) >+ for devclass in XendDevices.valid_devices(): >+ if devid in t.list(devclass): >+ deviceClass = devclass >+ break >+ t.abort() >+ >+ try: >+ # Note: deviceClass may be incorrect, so we cannot trust >readBackend >+ # or readFrontend from the Device Controller to succeed >+ backendPath = xstransact.Read("%s/device/%s/%s" >+ % (self.dompath, deviceClass, >devid), >+ "backend") >+ backendType = xstransact.Read(backendPath, "type") >+ except Exception, ex: >+ # we can probably just assume the block device is a VBD >+ log.debug("error reading backend for device (%s): %s", devid, >str(ex)) >+ >+ if backendType == ''tap'': >+ return ''tap'' >+ else: >+ return ''vbd'' > > def _getDeviceInfo_vif(self, mac): > for dev_type, dev_info in self.info.all_devices_sxpr(): >@@ -2217,7 +2237,7 @@ class XendDomainInfo: > # may possibly be "tap". Just in case, verify > # device class. > devid = dev.split(''/'')[-1] >- true_devclass = self.getBlockDeviceClass(devid) >+ true_devclass = self.getBlockDeviceClass(devid >, devclass) > log.debug("Removing %s", dev); > self.destroyDevice(true_devclass, dev, False); > except: > >_______________________________________________ >Xen-devel mailing list >Xen-devel@lists.xensource.com >http://lists.xensource.com/xen-devel_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Masaki Kanno
2009-Jun-24 04:35 UTC
[Xen-devel] [PATCH] xend: Fix vbd/tapdisk device destruction
Hi, When I repeated creating and shutting off a guest domain, I detected that processes of tapdisk2 were left. Then I saw the following error message in xend.log. # xm create vm2 Using config file "/etc/xen/vm2". Started domain vm2 (id=1) # xm shutdown vm2 # xm create vm2 Using config file "/etc/xen/vm2". Started domain vm2 (id=2) # xm shutdown vm2 # xm create vm2 Using config file "/etc/xen/vm2". Started domain vm2 (id=3) # xm shutdown vm2 # xm create vm2 Using config file "/etc/xen/vm2". Started domain vm2 (id=4) # xm shutdown vm2 # ps aux | grep tapdisk2 root 3814 0.0 0.3 2748 2748 ? SL 16:13 0:00 /usr/sbin/tapdisk2 -n aio:/xen/root-vm2.img root 4021 0.0 0.3 2752 2752 ? SL 16:17 0:00 /usr/sbin/tapdisk2 -n aio:/xen/root-vm2.img root 4193 0.0 0.3 2748 2748 ? SL 16:20 0:00 /usr/sbin/tapdisk2 -n aio:/xen/root-vm2.img root 4366 0.0 0.3 2748 2748 ? SL 16:25 0:00 /usr/sbin/tapdisk2 -n aio:/xen/root-vm2.img root 4537 0.0 0.0 3892 668 pts/0 S+ 16:33 0:00 grep tapdisk2 [2009-06-22 14:36:21 3626] DEBUG (XendDomainInfo:2221) Removing vbd/769 [2009-06-22 14:36:21 3626] DEBUG (XendDomainInfo:1137) XendDomainInfo.destroyDevice: deviceClass = tap, device = vbd/769 [2009-06-22 14:36:21 3626] ERROR (XendDomainInfo:2228) Device release failed: vm2; tap; vbd/769 Traceback (most recent call last): File "usr/lib/python2.4/site-packages/xen/xend/XendDomainInfo.py", line 2222, in _releaseDevices self.destroyDevice(true_devclass, dev, False); File "usr/lib/python2.4/site-packages/xen/xend/XendDomainInfo.py", line 1154, in destroyDevice path = self.getDeviceController(deviceClass).readBackend(dev, \047params\047) File "usr/lib/python2.4/site-packages/xen/xend/server/DevController.py", line 467, in readBackend raise VmError("Device %s not connected" % devid) VmError: Device 769 not connected This patch solves the problem. And the patch solves the detected problem by Ryan. Signed-off-by: Masaki Kanno <kanno.masaki@jp.fujitsu.com> Best regards, Kan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ryan O''Connor
2009-Jun-26 09:33 UTC
[Xen-devel] Re: [PATCH] xend: Fix vbd/tapdisk device destruction
Hi Kan, On Wed, Jun 24, 2009 at 01:35:51PM +0900, Masaki Kanno wrote: Content-Description: Mail message body> Hi, > > When I repeated creating and shutting off a guest domain, I detected > that processes of tapdisk2 were left. Then I saw the following error > message in xend.log.<snip>> [2009-06-22 14:36:21 3626] DEBUG (XendDomainInfo:2221) Removing vbd/769 > [2009-06-22 14:36:21 3626] DEBUG (XendDomainInfo:1137) XendDomainInfo.destroyDevice: deviceClass = tap, device = vbd/769 > [2009-06-22 14:36:21 3626] ERROR (XendDomainInfo:2228) Device release failed: vm2; tap; vbd/769 > Traceback (most recent call last): > File "usr/lib/python2.4/site-packages/xen/xend/XendDomainInfo.py", line 2222, in _releaseDevices > self.destroyDevice(true_devclass, dev, False); > File "usr/lib/python2.4/site-packages/xen/xend/XendDomainInfo.py", line 1154, in destroyDevice > path = self.getDeviceController(deviceClass).readBackend(dev, \047params\047) > File "usr/lib/python2.4/site-packages/xen/xend/server/DevController.py", line 467, in readBackend > raise VmError("Device %s not connected" % devid) > VmError: Device 769 not connected > > > This patch solves the problem. And the patch solves the detected > problem by Ryan.Your attached patch does indeed solve this problem, however it breaks save/restore for blktap1. Taking a few steps back, I''m not sure hard-coding the device path to device/vbd is the right way to fix this. Xend seems to trust that devices in /vm/<uuid>/<deivceClass>/ actually belong to <devClass>. I think this points to a bigger problem. Bllktap2 devices need their own device controller for their entire life-cycle, but xend currently treats the blktap2 device as a vbd after it is created. As a result we have blktap2 specific code in DevController, and we switch between using the vbd and tap controller on the blktap2 device. Further, if we want to get save/restore working for blktap2 we''ll need to add more blktap2-specific code to xend (to store and retrieve the original uname). For these reasons I don''t think blktap2''s current approach of creating a vanilla vbd device will work in the long run. Instead I think we need a dedicated blktap2 controller class. I have a patch that does just this, which I will post later today after a bit more testing. Thanks, Ryan -- Ryan O''Connor <rjo@cs.ubc.ca> _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Masaki Kanno
2009-Jun-29 00:01 UTC
Re: [Xen-devel] Re: [PATCH] xend: Fix vbd/tapdisk device destruction
Fri, 26 Jun 2009 02:33:53 -0700, Ryan O''Connor wrote:>Hi Kan, > >On Wed, Jun 24, 2009 at 01:35:51PM +0900, Masaki Kanno wrote: >Content-Description: Mail message body >> Hi, >> >> When I repeated creating and shutting off a guest domain, I detected >> that processes of tapdisk2 were left. Then I saw the following error >> message in xend.log. ><snip> >> [2009-06-22 14:36:21 3626] DEBUG (XendDomainInfo:2221) Removing vbd/769 >> [2009-06-22 14:36:21 3626] DEBUG (XendDomainInfo:1137) XendDomainInfo. >> destroyDevice: deviceClass = tap, device = vbd/769 >> [2009-06-22 14:36:21 3626] ERROR (XendDomainInfo:2228) Device release >> failed: vm2; tap; vbd/769 >> Traceback (most recent call last): >> File "usr/lib/python2.4/site-packages/xen/xend/XendDomainInfo.py", line >> 2222, in _releaseDevices >> self.destroyDevice(true_devclass, dev, False); >> File "usr/lib/python2.4/site-packages/xen/xend/XendDomainInfo.py", line >> 1154, in destroyDevice >> path = self.getDeviceController(deviceClass).readBackend(dev, \ >> 047params\047) >> File "usr/lib/python2.4/site-packages/xen/xend/server/DevController.py" >> , line 467, in readBackend >> raise VmError("Device %s not connected" % devid) >> VmError: Device 769 not connected >> >> >> This patch solves the problem. And the patch solves the detected >> problem by Ryan. > >Your attached patch does indeed solve this problem, however it breaks >save/restore for blktap1. Taking a few steps back, I''m not sure >hard-coding the device path to device/vbd is the right way to fix this. >Xend seems to trust that devices in /vm/<uuid>/<deivceClass>/ actually >belong to <devClass>. > >I think this points to a bigger problem. Bllktap2 devices need their >own device controller for their entire life-cycle, but xend currently >treats the blktap2 device as a vbd after it is created. As a result we >have blktap2 specific code in DevController, and we switch between using >the vbd and tap controller on the blktap2 device. Further, if we want to >get save/restore working for blktap2 we''ll need to add more >blktap2-specific code to xend (to store and retrieve the original >uname). > >For these reasons I don''t think blktap2''s current approach of creating a >vanilla vbd device will work in the long run. Instead I think we need a >dedicated blktap2 controller class. I have a patch that does just this, >which I will post later today after a bit more testing.Hi Ryan, I also think we need the dedicated blktap2 controller class. I''m really looking forward to the patch from you. Best regards, Kan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel