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