Xen devel - Jul 2009 - [PATCH] blktap2: add blktap2 device class and device controller

# HG changeset patch
# User Ryan O''Connor <rjo@cs.ubc.ca>
# Date 1246409081 25200
# Node ID 6295171d4ccc60f80f0411217d9b1c7d8543c804
# Parent  7397608bce877b151ee507413a8e228092947ee4
blktap2: add blktap2 device class and device controller

blktap2 devices must be handled differently than blktap2 devices. blktap2
devices require a sysfs write to close the underlying device, as well as extra
sysfs writes when the domU is paused/unpaused. The differences between blktap1
and blktap2 are great enough to warrant the creation of a new device class,
''tap2'', and device controller for blktap2 devices.

  * add a new device controller (Blktap2Controller) and device class (tap2) for
    blktap2 devices
  * move blktap2 specific code from DevController to Blktap2Controller
  * if possible, check xenstore to determine block device class
  * use vmpath (/vm/<uuid>/) when releasing devices
  * modify linux hotplug cleanup script to handle blktap2 device removal

Signed-off-by: Ryan O''Connor <rjo@cs.ubc.ca>

diff -r 7397608bce87 -r 6295171d4ccc tools/hotplug/Linux/xen-hotplug-cleanup
--- a/tools/hotplug/Linux/xen-hotplug-cleanup	Mon Jun 29 15:50:32 2009 +0100
+++ b/tools/hotplug/Linux/xen-hotplug-cleanup	Tue Jun 30 17:44:41 2009 -0700
@@ -18,6 +18,13 @@ vm=$(xenstore_read_default "/local/domai
 # construct /vm/UUID/device/DEVCLASS/DEVID
 if [ "$vm" != "" ]; then
   vm_dev="$vm/device/${path_array[1]}/${path_array[3]}"
+
+  # if the vm path does not exist and the device class is
''vbd'' then we may have
+  # a tap2 device
+  if [ ! $(xenstore-read "vm_dev" 2>/dev/null) ] \
+       && [ "${path_array[1]}" = "vbd" ]; then
+    vm_dev="$vm/device/tap2/${path_array[3]}"
+  fi
 else
   vm_dev fi
diff -r 7397608bce87 -r 6295171d4ccc tools/python/xen/xend/XendConfig.py
--- a/tools/python/xen/xend/XendConfig.py	Mon Jun 29 15:50:32 2009 +0100
+++ b/tools/python/xen/xend/XendConfig.py	Tue Jun 30 17:44:41 2009 -0700
@@ -1114,7 +1114,7 @@ class XendConfig(dict):
                             controller = domain.getDeviceController(cls)
                             configs = controller.configurations(txn)
                             for config in configs:
-                                if sxp.name(config) in
(''vbd'', ''tap''):
+                                if sxp.name(config) in
(''vbd'', ''tap'', ''tap2''):
                                     dev_uuid = sxp.child_value(config,
''uuid'')
                                     dev_type, dev_cfg =
self[''devices''][dev_uuid]
                                     if sxp.child_value(config,
''bootable'', None) is None:
@@ -1179,7 +1179,7 @@ class XendConfig(dict):
     def device_duplicate_check(self, dev_type, dev_info, defined_config,
config):
         defined_devices_sxpr = self.all_devices_sxpr(target = defined_config)
         
-        if dev_type == ''vbd'' or dev_type ==
''tap'':
+        if dev_type == ''vbd'' or dev_type ==
''tap'' or dev_type == ''tap2'':
             dev_uname = dev_info.get(''uname'')
             blkdev_name = dev_info.get(''dev'')
             devid = self._blkdev_name_to_number(blkdev_name)
@@ -1187,7 +1187,7 @@ class XendConfig(dict):
                 return
             
             for o_dev_type, o_dev_info in defined_devices_sxpr:
-                if o_dev_type == ''vbd'' or o_dev_type ==
''tap'':
+                if o_dev_type == ''vbd'' or o_dev_type ==
''tap'' or o_dev_type == ''tap2'':
                     blkdev_file = blkdev_uname_to_file(dev_uname)
                     o_dev_uname = sxp.child_value(o_dev_info,
''uname'')
                     if o_dev_uname != None:
@@ -1369,7 +1369,7 @@ class XendConfig(dict):
                 else:
                     dev_info[''driver''] =
''paravirtualised''
 
-            if dev_type == ''tap'':
+            if dev_type == ''tap'' or dev_type ==
''tap2'':
                 if
dev_info[''uname''].split('':'')[1] not in
blktap_disk_types:
                     raise XendConfigError("tap:%s not a valid disk
type" %
                                    
dev_info[''uname''].split('':'')[1])
@@ -1406,7 +1406,7 @@ class XendConfig(dict):
                         # Compat hack -- mark first disk bootable
                         dev_info[''bootable''] = int(not
target[param])
                     target[param].append(dev_uuid)
-            elif dev_type == ''tap'':
+            elif dev_type == ''tap'' or dev_type ==
''tap2'':
                 if ''vbd_refs'' not in target:
                     target[''vbd_refs''] = []
                 if dev_uuid not in target[''vbd_refs'']:
@@ -1504,7 +1504,7 @@ class XendConfig(dict):
                 target[''devices''][dev_uuid] = (dev_type,
dev_info)
                 target[''vif_refs''].append(dev_uuid)
             
-            elif dev_type in (''vbd'',
''tap''):
+            elif dev_type in (''vbd'', ''tap'',
''tap2''):
                 dev_info[''type''] =
cfg_xenapi.get(''type'', ''Disk'')
                 if dev_info[''type''] ==
''CD'':
                     old_vbd_type = ''cdrom''
diff -r 7397608bce87 -r 6295171d4ccc tools/python/xen/xend/XendDevices.py
--- a/tools/python/xen/xend/XendDevices.py	Mon Jun 29 15:50:32 2009 +0100
+++ b/tools/python/xen/xend/XendDevices.py	Tue Jun 30 17:44:41 2009 -0700
@@ -20,7 +20,7 @@
 #
 
 from xen.xend.server import blkif, netif, tpmif, pciif, iopif, irqif, vfbif,
vscsiif
-from xen.xend.server.BlktapController import BlktapController
+from xen.xend.server.BlktapController import BlktapController,
Blktap2Controller
 from xen.xend.server.ConsoleController import ConsoleController
 
 
@@ -42,6 +42,7 @@ class XendDevices:
         ''ioports'': iopif.IOPortsController,
         ''irq'': irqif.IRQController,
         ''tap'': BlktapController,
+        ''tap2'': Blktap2Controller,
         ''vfb'': vfbif.VfbifController,
         ''vkbd'': vfbif.VkbdifController,
         ''console'': ConsoleController,
diff -r 7397608bce87 -r 6295171d4ccc tools/python/xen/xend/XendDomainInfo.py
--- a/tools/python/xen/xend/XendDomainInfo.py	Mon Jun 29 15:50:32 2009 +0100
+++ b/tools/python/xen/xend/XendDomainInfo.py	Tue Jun 30 17:44:41 2009 -0700
@@ -538,12 +538,11 @@ class XendDomainInfo:
         @raise XendError: Failed pausing a domain
         """
         try:
-            bepath="/local/domain/0/backend/"
             if(self.domid):
-                
-                dev =  xstransact.List(bepath + ''vbd'' +
"/%d" % (self.domid,))
+                # get all blktap2 devices
+                dev =  xstransact.List(self.vmpath +
''device/tap2'')
                 for x in dev:
-                    path =
self.getDeviceController(''vbd'').readBackend(x,
''params'')
+                    path =
self.getDeviceController(''tap2'').readBackend(x,
''params'')
                     if path and
path.startswith(''/dev/xen/blktap-2''):
                         #Figure out the sysfs path.
                         pattern =
re.compile(''/dev/xen/blktap-2/tapdev(\d+)$'')
@@ -569,11 +568,10 @@ class XendDomainInfo:
         @raise XendError: Failed unpausing a domain
         """
         try:
-            bepath="/local/domain/0/backend/"
             if(self.domid):
-                dev =  xstransact.List(bepath + "vbd" +
"/%d" % (self.domid,))
+                dev =  xstransact.List(self.vmpath +
''device/tap2'')
                 for x in dev:
-                    path =
self.getDeviceController(''vbd'').readBackend(x,
''params'')
+                    path =
self.getDeviceController(''tap2'').readBackend(x,
''params'')
                     if path and
path.startswith(''/dev/xen/blktap-2''):
                         #Figure out the sysfs path.
                         pattern =
re.compile(''/dev/xen/blktap-2/tapdev(\d+)$'')
@@ -812,6 +810,11 @@ class XendDomainInfo:
             try:
                 dev_config_dict[''devid''] = devid = \
                     self._createDevice(dev_type, dev_config_dict)
+                if dev_type == ''tap2'':
+                    # createDevice may create a blktap1 device if blktap2 is
not
+                    # installed or if the blktap driver is not supported in
+                    # blktap1
+                    dev_type = self.getBlockDeviceClass(devid)
                 self._waitForDevice(dev_type, devid)
             except VmError, ex:
                 del self.info[''devices''][dev_uuid]
@@ -821,7 +824,7 @@ class XendDomainInfo:
                 elif dev_type == ''vscsi'':
                     for dev in dev_config_dict[''devs'']:
                         XendAPIStore.deregister(dev[''uuid''],
''DSCSI'')
-                elif dev_type == ''tap'':
+                elif dev_type == ''tap'' or dev_type ==
''tap2'':
                     self.info[''vbd_refs''].remove(dev_uuid)
                 else:
                     self.info[''%s_refs'' %
dev_type].remove(dev_uuid)
@@ -1200,9 +1203,10 @@ class XendDomainInfo:
         if self.domid is not None:
             
             #new blktap implementation may need a sysfs write after everything
is torn down.
-            dev =
self.getDeviceController(deviceClass).convertToDeviceNumber(devid)
-            path = self.getDeviceController(deviceClass).readBackend(dev,
''params'')
-            if path and path.startswith(''/dev/xen/blktap-2''):
+
+            if deviceClass == ''tap2'':
+                dev =
self.getDeviceController(deviceClass).convertToDeviceNumber(devid)
+                path = self.getDeviceController(deviceClass).readBackend(dev,
''params'')
                 frontpath =
self.getDeviceController(deviceClass).frontendPath(dev)
                 backpath = xstransact.Read(frontpath, "backend")
                
thread.start_new_thread(self.getDeviceController(deviceClass).finishDeviceCleanup,
(backpath, path))
@@ -1238,7 +1242,7 @@ class XendDomainInfo:
                     dev_info = self._getDeviceInfo_vif(mac)
                 else:
                     _, dev_info = sxprs[dev]
-            else:  # ''vbd'' or ''tap''
+            else:  # ''vbd'' or ''tap'' or
''tap2''
                 dev_info = self._getDeviceInfo_vbd(dev)
                 # To remove the UUID of the device from refs,
                 # deviceClass must be always ''vbd''.
@@ -1267,7 +1271,7 @@ class XendDomainInfo:
             sxprs = []
             dev_num = 0
             for dev_type, dev_info in self.info.all_devices_sxpr():
-                if (deviceClass == ''vbd'' and dev_type not in
[''vbd'', ''tap'']) or \
+                if (deviceClass == ''vbd'' and dev_type not in
[''vbd'', ''tap'', ''tap2'']) or
\
                    (deviceClass != ''vbd'' and dev_type !=
deviceClass):
                     continue
 
@@ -1295,12 +1299,27 @@ class XendDomainInfo:
             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]
+        # if the domain is running we can get the device class from xenstore.
+        # This is more accurate, as blktap1 devices show up as blktap2 devices
+        # in the config.
+        if self._stateGet() in (DOM_STATE_RUNNING, DOM_STATE_PAUSED,
DOM_STATE_CRASHED):
+            # All block devices have a vbd frontend, so we know the frontend
path
+            dev =
self.getDeviceController(''vbd'').convertToDeviceNumber(devid)
+            frontendPath = "%s/device/vbd/%s" % (self.dompath, dev)
+            for devclass in XendDevices.valid_devices():
+                for dev in xstransact.List("%s/device/%s" %
(self.vmpath, devclass)):
+                    devFrontendPath =
xstransact.Read("%s/device/%s/%s/frontend" % (self.vmpath, devclass,
dev))
+                    if frontendPath == devFrontendPath:
+                        return devclass
+            
+        else: # the domain is not active so we must get the device class
+              # from the config
+            # 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 _getDeviceInfo_vif(self, mac):
         for dev_type, dev_info in self.info.all_devices_sxpr():
@@ -1311,7 +1330,7 @@ class XendDomainInfo:
 
     def _getDeviceInfo_vbd(self, devid):
         for dev_type, dev_info in self.info.all_devices_sxpr():
-            if dev_type != ''vbd'' and dev_type !=
''tap'':
+            if dev_type != ''vbd'' and dev_type !=
''tap'' and dev_type != ''tap2'':
                 continue
             dev = sxp.child_value(dev_info, ''dev'')
             dev = dev.split('':'')[0]
@@ -2256,26 +2275,19 @@ class XendDomainInfo:
             log.debug("No device model")
 
         log.debug("Releasing devices")
-        t = xstransact("%s/device" % self.dompath)
+        t = xstransact("%s/device" % self.vmpath)
         try:
             for devclass in XendDevices.valid_devices():
                 for dev in t.list(devclass):
                     try:
-                        true_devclass = devclass
-                        if devclass == ''vbd'':
-                            # In the case of "vbd", the true device
class
-                            # may possibly be "tap". Just in case,
verify
-                            # device class.
-                            devid = dev.split(''/'')[-1]
-                            true_devclass = self.getBlockDeviceClass(devid)
                         log.debug("Removing %s", dev);
-                        self.destroyDevice(true_devclass, dev, False);
+                        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''],
-                                      true_devclass, dev)
+                                      devclass, dev)
         finally:
             t.abort()
 
@@ -2948,7 +2960,7 @@ class XendDomainInfo:
 
             fn = blkdev_uname_to_file(disk)
             taptype = blkdev_uname_to_taptype(disk)
-            mounted = devtype == ''tap'' and taptype !=
''aio'' and taptype != ''sync'' and not
os.stat(fn).st_rdev
+            mounted = devtype in [''tap'',
''tap2''] and taptype != ''aio'' and taptype !=
''sync'' and not os.stat(fn).st_rdev
             if mounted:
                 # This is a file, not a device.  pygrub can cope with a
                 # file if it''s raw, but if it''s QCOW or other
such formats
@@ -3052,7 +3064,8 @@ class XendDomainInfo:
             diff = time.time() - start
             vbds =
self.getDeviceController(''vbd'').deviceIDs()
             taps =
self.getDeviceController(''tap'').deviceIDs()
-            for i in vbds + taps:
+            tap2s =
self.getDeviceController(''tap2'').deviceIDs()
+            for i in vbds + taps + tap2s:
                 test = 1
                 log.info("Dev %s still active, looping...", i)
                 time.sleep(0.1)
@@ -3635,7 +3648,7 @@ class XendDomainInfo:
         """
         xenapi_vbd[''image''] = vdi_image_path
         if vdi_image_path.startswith(''tap''):
-            dev_uuid = self.info.device_add(''tap'', cfg_xenapi
= xenapi_vbd)
+            dev_uuid = self.info.device_add(''tap2'',
cfg_xenapi = xenapi_vbd)
         else:
             dev_uuid = self.info.device_add(''vbd'', cfg_xenapi
= xenapi_vbd)
             
@@ -3647,7 +3660,7 @@ class XendDomainInfo:
             _, config = self.info[''devices''][dev_uuid]
             
             if vdi_image_path.startswith(''tap''):
-                dev_control = self.getDeviceController(''tap'')
+                dev_control =
self.getDeviceController(''tap2'')
             else:
                 dev_control = self.getDeviceController(''vbd'')
 
diff -r 7397608bce87 -r 6295171d4ccc
tools/python/xen/xend/server/BlktapController.py
--- a/tools/python/xen/xend/server/BlktapController.py	Mon Jun 29 15:50:32 2009
+0100
+++ b/tools/python/xen/xend/server/BlktapController.py	Tue Jun 30 17:44:41 2009
-0700
@@ -24,7 +24,7 @@ blktap_disk_types = [
     ''ioemu'',
     ''tapdisk'',
     ]
- 
+
 def doexec(args, inputtext=None):
     """Execute a subprocess, then return its return code, stdout
and stderr"""
     proc = popen2.Popen3(args, True)
@@ -49,8 +49,7 @@ def parseDeviceString(device):
 
     return minor, device, control
 
-
-
+# blktap1 device controller
 class BlktapController(BlkifController):
     def __init__(self, vm):
         BlkifController.__init__(self, vm)
@@ -59,11 +58,6 @@ class BlktapController(BlkifController):
         """@see DevController#frontendRoot"""
         
         return "%s/device/vbd" % self.vm.getDomainPath()
-
-    def devicePath(self, devid):
-        """@see DevController#devicePath"""
-        
-        return "%s/device/vbd/%s" % (self.vm.vmpath, devid)
 
     def getDeviceDetails(self, config):
         (devid, back, front) = BlkifController.getDeviceDetails(self, config)
@@ -123,6 +117,20 @@ class BlktapController(BlkifController):
 
         return (devid, back, front)
 
+class Blktap2Controller(BlktapController):
+    def __init__(self, vm):
+        BlktapController.__init__(self, vm)
+
+    def backendPath(self, backdom, devid):
+        if self.deviceClass == ''tap2'':
+            deviceClass = ''vbd''
+        else:
+            deviceClass = ''tap''
+        return "%s/backend/%s/%s/%d" % (backdom.getDomainPath(),
+                                        deviceClass,
+                                        self.vm.getDomid(), devid)
+
+
     def createDevice(self, config):
 
         uname = config.get(''required_uname'',
'''')
@@ -140,12 +148,16 @@ class BlktapController(BlkifController):
         stderr.close();
         if( out.find("blktap2") >= 0 ):
             blktap2_installed=1;
-           
+
         if typ in (''tap''):
-            if subtyp in (''tapdisk''):
+            if subtyp in (''tapdisk''):
                 if params in (''ioemu'',
''qcow2'', ''vmdk'', ''sync'')
or not blktap2_installed:
-                    log.warn(''WARNING: using deprecated blktap
module'');
-                    return BlkifController.createDevice(self, config);
+                    # pass this device off to BlktapController
+                    log.warn(''WARNING: using deprecated blktap
module'')
+                    self.deviceClass = ''tap''
+                    devid = BlktapController.createDevice(self, config)
+                    self.deviceClass = ''tap2''
+                    return devid
 
         cmd = [ TAPDISK_BINARY, ''-n'',
''%s:%s'' % (params, file) ]
         (rc,stdout,stderr) = doexec(cmd)
@@ -165,7 +177,32 @@ class BlktapController(BlkifController):
         #device is configured.  Then continue to create the device
         config.update({''uname'' : ''phy:'' +
device.rstrip()})
 
-        self.deviceClass=''vbd''
         devid = BlkifController.createDevice(self, config)
-        self.deviceClass=''tap''
         return devid
+
+    # The new blocktap implementation requires a sysfs signal to close
+    # out disks.  This function is called from a thread when the
+    # domain is detached from the disk.
+    def finishDeviceCleanup(self, backpath, path):
+        """Perform any device specific cleanup
+
+        @backpath backend xenstore path.
+        @path frontend device path
+
+        """
+
+        #Figure out what we''re going to wait on.
+        self.waitForBackend_destroy(backpath)
+
+        #Figure out the sysfs path.
+        pattern =
re.compile(''/dev/xen/blktap-2/tapdev(\d+)$'')
+        ctrlid = pattern.search(path)
+        ctrl = ''/sys/class/blktap2/blktap'' + ctrlid.group(1)
+
+        #Close out the disk
+        f = open(ctrl + ''/remove'', ''w'')
+        f.write(''remove'');
+        f.close()
+
+        return
+
diff -r 7397608bce87 -r 6295171d4ccc
tools/python/xen/xend/server/DevController.py
--- a/tools/python/xen/xend/server/DevController.py	Mon Jun 29 15:50:32 2009
+0100
+++ b/tools/python/xen/xend/server/DevController.py	Tue Jun 30 17:44:41 2009
-0700
@@ -238,34 +238,6 @@ class DevController:
             # xstransact.Remove(self.devicePath()) ?? Below is the same ?
             self.vm._removeVm("device/%s/%d" % (self.deviceClass,
dev))
 
-    # The new blocktap implementation requires a sysfs signal to close
-    # out disks.  This function is called from a thread when the
-    # domain is detached from the disk.
-    def finishDeviceCleanup(self, backpath, path):
-        """Perform any device specific cleanup
-
-        @backpath backend xenstore path.
-        @path frontend device path
-
-        """
-        
-        if path and path.startswith(''/dev/xen/blktap-2''):
-            
-            #Figure out what we''re going to wait on.
-            self.waitForBackend_destroy(backpath)            
-
-            #Figure out the sysfs path.
-            pattern =
re.compile(''/dev/xen/blktap-2/tapdev(\d+)$'')
-            ctrlid = pattern.search(path)
-            ctrl = ''/sys/class/blktap2/blktap'' +
ctrlid.group(1)
-            
-            #Close out the disk
-            f = open(ctrl + ''/remove'', ''w'')
-            f.write(''remove'');
-            f.close()
-
-        return
-
     def configurations(self, transaction = None):
         return map(lambda x: self.configuration(x, transaction),
self.deviceIDs(transaction))
 
@@ -575,7 +547,6 @@ class DevController:
             
         backpath = self.readVm(devid, "backend")
 
-
         if backpath:
             statusPath = backpath + ''/'' + HOTPLUG_STATUS_NODE
             ev = Event()
diff -r 7397608bce87 -r 6295171d4ccc tools/python/xen/xm/create.py
--- a/tools/python/xen/xm/create.py	Mon Jun 29 15:50:32 2009 +0100
+++ b/tools/python/xen/xm/create.py	Tue Jun 30 17:44:41 2009 -0700
@@ -718,7 +718,7 @@ def configure_disks(config_devs, vals):
     """
     for (uname, dev, mode, backend, protocol) in vals.disk:
         if uname.startswith(''tap:''):
-            cls = ''tap''
+            cls = ''tap2''
         else:
             cls = ''vbd''
 
diff -r 7397608bce87 -r 6295171d4ccc tools/python/xen/xm/main.py
--- a/tools/python/xen/xm/main.py	Mon Jun 29 15:50:32 2009 +0100
+++ b/tools/python/xen/xm/main.py	Tue Jun 30 17:44:41 2009 -0700
@@ -2371,7 +2371,7 @@ def parse_block_configuration(args):
     dom = args[0]
 
     if args[1].startswith(''tap:''):
-        cls = ''tap''
+        cls = ''tap2''
     else:
         cls = ''vbd''
 
@@ -2706,7 +2706,9 @@ def xm_block_detach(args):
         dom = args[0]
         dev = args[1]
         dc = server.xend.domain.getBlockDeviceClass(dom, dev)
-        if dc == "tap":
+        if dc == "tap2":
+            detach(args, ''tap2'')
+        elif dc == "tap":
             detach(args, ''tap'')
         else:
             detach(args, ''vbd'')

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Hi Everyone,

Two quick things:

 1) This patch uses a regular ''vbd'' blkback backend for the
blktap2
    device. This is why the xen-hotplug-cleanup script must first check
    to see if the devices entry in <vmpath> exists (the hotplug scripts
    only see ''vbd'' even if the device is actually a blktap2
device).
    
    An alternative to this is to modify blkback to watch xenstore for
    ''tap2'' devices as well as ''vbd'' devices.
In this case the hotplug
    scripts see the tap2 devices as ''tap2'', so no
modifications to the
    hotplug scripts are required. I have a patch that does this as well,
    and I can post it if needed.

 2) I currently do not have the ability to test this on HVM, so this is
    untested for HVM...

Thanks,
Ryan

On Tue, Jun 30, 2009 at 05:49:44PM -0700, Ryan O''Connor
wrote:
> # HG changeset patch
> # User Ryan O''Connor <rjo@cs.ubc.ca>
> # Date 1246409081 25200
> # Node ID 6295171d4ccc60f80f0411217d9b1c7d8543c804
> # Parent  7397608bce877b151ee507413a8e228092947ee4
> blktap2: add blktap2 device class and device controller
> 
> blktap2 devices must be handled differently than blktap2 devices. blktap2
> devices require a sysfs write to close the underlying device, as well as
extra
> sysfs writes when the domU is paused/unpaused. The differences between
blktap1
> and blktap2 are great enough to warrant the creation of a new device class,
> ''tap2'', and device controller for blktap2 devices.
> 
>   * add a new device controller (Blktap2Controller) and device class (tap2)
for
>     blktap2 devices
>   * move blktap2 specific code from DevController to Blktap2Controller
>   * if possible, check xenstore to determine block device class
>   * use vmpath (/vm/<uuid>/) when releasing devices
>   * modify linux hotplug cleanup script to handle blktap2 device removal
> 
> Signed-off-by: Ryan O''Connor <rjo@cs.ubc.ca>
> 
> diff -r 7397608bce87 -r 6295171d4ccc
tools/hotplug/Linux/xen-hotplug-cleanup
> --- a/tools/hotplug/Linux/xen-hotplug-cleanup	Mon Jun 29 15:50:32 2009
+0100
> +++ b/tools/hotplug/Linux/xen-hotplug-cleanup	Tue Jun 30 17:44:41 2009
-0700
> @@ -18,6 +18,13 @@ vm=$(xenstore_read_default "/local/domai
>  # construct /vm/UUID/device/DEVCLASS/DEVID
>  if [ "$vm" != "" ]; then
>    vm_dev="$vm/device/${path_array[1]}/${path_array[3]}"
> +
> +  # if the vm path does not exist and the device class is
''vbd'' then we may have
> +  # a tap2 device
> +  if [ ! $(xenstore-read "vm_dev" 2>/dev/null) ] \
> +       && [ "${path_array[1]}" = "vbd" ]; then
> +    vm_dev="$vm/device/tap2/${path_array[3]}"
> +  fi
>  else
>    vm_dev>  fi
> diff -r 7397608bce87 -r 6295171d4ccc tools/python/xen/xend/XendConfig.py
> --- a/tools/python/xen/xend/XendConfig.py	Mon Jun 29 15:50:32 2009 +0100
> +++ b/tools/python/xen/xend/XendConfig.py	Tue Jun 30 17:44:41 2009 -0700
> @@ -1114,7 +1114,7 @@ class XendConfig(dict):
>                              controller = domain.getDeviceController(cls)
>                              configs = controller.configurations(txn)
>                              for config in configs:
> -                                if sxp.name(config) in
(''vbd'', ''tap''):
> +                                if sxp.name(config) in
(''vbd'', ''tap'', ''tap2''):
>                                      dev_uuid = sxp.child_value(config,
''uuid'')
>                                      dev_type, dev_cfg =
self[''devices''][dev_uuid]
>                                      if sxp.child_value(config,
''bootable'', None) is None:
> @@ -1179,7 +1179,7 @@ class XendConfig(dict):
>      def device_duplicate_check(self, dev_type, dev_info, defined_config,
config):
>          defined_devices_sxpr = self.all_devices_sxpr(target =
defined_config)
>          
> -        if dev_type == ''vbd'' or dev_type ==
''tap'':
> +        if dev_type == ''vbd'' or dev_type ==
''tap'' or dev_type == ''tap2'':
>              dev_uname = dev_info.get(''uname'')
>              blkdev_name = dev_info.get(''dev'')
>              devid = self._blkdev_name_to_number(blkdev_name)
> @@ -1187,7 +1187,7 @@ class XendConfig(dict):
>                  return
>              
>              for o_dev_type, o_dev_info in defined_devices_sxpr:
> -                if o_dev_type == ''vbd'' or o_dev_type ==
''tap'':
> +                if o_dev_type == ''vbd'' or o_dev_type ==
''tap'' or o_dev_type == ''tap2'':
>                      blkdev_file = blkdev_uname_to_file(dev_uname)
>                      o_dev_uname = sxp.child_value(o_dev_info,
''uname'')
>                      if o_dev_uname != None:
> @@ -1369,7 +1369,7 @@ class XendConfig(dict):
>                  else:
>                      dev_info[''driver''] =
''paravirtualised''
>  
> -            if dev_type == ''tap'':
> +            if dev_type == ''tap'' or dev_type ==
''tap2'':
>                  if
dev_info[''uname''].split('':'')[1] not in
blktap_disk_types:
>                      raise XendConfigError("tap:%s not a valid disk
type" %
>                                     
dev_info[''uname''].split('':'')[1])
> @@ -1406,7 +1406,7 @@ class XendConfig(dict):
>                          # Compat hack -- mark first disk bootable
>                          dev_info[''bootable''] = int(not
target[param])
>                      target[param].append(dev_uuid)
> -            elif dev_type == ''tap'':
> +            elif dev_type == ''tap'' or dev_type ==
''tap2'':
>                  if ''vbd_refs'' not in target:
>                      target[''vbd_refs''] = []
>                  if dev_uuid not in target[''vbd_refs'']:
> @@ -1504,7 +1504,7 @@ class XendConfig(dict):
>                  target[''devices''][dev_uuid] = (dev_type,
dev_info)
>                  target[''vif_refs''].append(dev_uuid)
>              
> -            elif dev_type in (''vbd'',
''tap''):
> +            elif dev_type in (''vbd'',
''tap'', ''tap2''):
>                  dev_info[''type''] =
cfg_xenapi.get(''type'', ''Disk'')
>                  if dev_info[''type''] ==
''CD'':
>                      old_vbd_type = ''cdrom''
> diff -r 7397608bce87 -r 6295171d4ccc tools/python/xen/xend/XendDevices.py
> --- a/tools/python/xen/xend/XendDevices.py	Mon Jun 29 15:50:32 2009 +0100
> +++ b/tools/python/xen/xend/XendDevices.py	Tue Jun 30 17:44:41 2009 -0700
> @@ -20,7 +20,7 @@
>  #
>  
>  from xen.xend.server import blkif, netif, tpmif, pciif, iopif, irqif,
vfbif, vscsiif
> -from xen.xend.server.BlktapController import BlktapController
> +from xen.xend.server.BlktapController import BlktapController,
Blktap2Controller
>  from xen.xend.server.ConsoleController import ConsoleController
>  
>  
> @@ -42,6 +42,7 @@ class XendDevices:
>          ''ioports'': iopif.IOPortsController,
>          ''irq'': irqif.IRQController,
>          ''tap'': BlktapController,
> +        ''tap2'': Blktap2Controller,
>          ''vfb'': vfbif.VfbifController,
>          ''vkbd'': vfbif.VkbdifController,
>          ''console'': ConsoleController,
> diff -r 7397608bce87 -r 6295171d4ccc
tools/python/xen/xend/XendDomainInfo.py
> --- a/tools/python/xen/xend/XendDomainInfo.py	Mon Jun 29 15:50:32 2009
+0100
> +++ b/tools/python/xen/xend/XendDomainInfo.py	Tue Jun 30 17:44:41 2009
-0700
> @@ -538,12 +538,11 @@ class XendDomainInfo:
>          @raise XendError: Failed pausing a domain
>          """
>          try:
> -            bepath="/local/domain/0/backend/"
>              if(self.domid):
> -                
> -                dev =  xstransact.List(bepath + ''vbd'' +
"/%d" % (self.domid,))
> +                # get all blktap2 devices
> +                dev =  xstransact.List(self.vmpath +
''device/tap2'')
>                  for x in dev:
> -                    path =
self.getDeviceController(''vbd'').readBackend(x,
''params'')
> +                    path =
self.getDeviceController(''tap2'').readBackend(x,
''params'')
>                      if path and
path.startswith(''/dev/xen/blktap-2''):
>                          #Figure out the sysfs path.
>                          pattern =
re.compile(''/dev/xen/blktap-2/tapdev(\d+)$'')
> @@ -569,11 +568,10 @@ class XendDomainInfo:
>          @raise XendError: Failed unpausing a domain
>          """
>          try:
> -            bepath="/local/domain/0/backend/"
>              if(self.domid):
> -                dev =  xstransact.List(bepath + "vbd" +
"/%d" % (self.domid,))
> +                dev =  xstransact.List(self.vmpath +
''device/tap2'')
>                  for x in dev:
> -                    path =
self.getDeviceController(''vbd'').readBackend(x,
''params'')
> +                    path =
self.getDeviceController(''tap2'').readBackend(x,
''params'')
>                      if path and
path.startswith(''/dev/xen/blktap-2''):
>                          #Figure out the sysfs path.
>                          pattern =
re.compile(''/dev/xen/blktap-2/tapdev(\d+)$'')
> @@ -812,6 +810,11 @@ class XendDomainInfo:
>              try:
>                  dev_config_dict[''devid''] = devid = \
>                      self._createDevice(dev_type, dev_config_dict)
> +                if dev_type == ''tap2'':
> +                    # createDevice may create a blktap1 device if blktap2
is not
> +                    # installed or if the blktap driver is not supported
in
> +                    # blktap1
> +                    dev_type = self.getBlockDeviceClass(devid)
>                  self._waitForDevice(dev_type, devid)
>              except VmError, ex:
>                  del self.info[''devices''][dev_uuid]
> @@ -821,7 +824,7 @@ class XendDomainInfo:
>                  elif dev_type == ''vscsi'':
>                      for dev in dev_config_dict[''devs'']:
>                         
XendAPIStore.deregister(dev[''uuid''],
''DSCSI'')
> -                elif dev_type == ''tap'':
> +                elif dev_type == ''tap'' or dev_type ==
''tap2'':
>                     
self.info[''vbd_refs''].remove(dev_uuid)
>                  else:
>                      self.info[''%s_refs'' %
dev_type].remove(dev_uuid)
> @@ -1200,9 +1203,10 @@ class XendDomainInfo:
>          if self.domid is not None:
>              
>              #new blktap implementation may need a sysfs write after
everything is torn down.
> -            dev =
self.getDeviceController(deviceClass).convertToDeviceNumber(devid)
> -            path = self.getDeviceController(deviceClass).readBackend(dev,
''params'')
> -            if path and
path.startswith(''/dev/xen/blktap-2''):
> +
> +            if deviceClass == ''tap2'':
> +                dev =
self.getDeviceController(deviceClass).convertToDeviceNumber(devid)
> +                path =
self.getDeviceController(deviceClass).readBackend(dev,
''params'')
>                  frontpath =
self.getDeviceController(deviceClass).frontendPath(dev)
>                  backpath = xstransact.Read(frontpath, "backend")
>                 
thread.start_new_thread(self.getDeviceController(deviceClass).finishDeviceCleanup,
(backpath, path))
> @@ -1238,7 +1242,7 @@ class XendDomainInfo:
>                      dev_info = self._getDeviceInfo_vif(mac)
>                  else:
>                      _, dev_info = sxprs[dev]
> -            else:  # ''vbd'' or ''tap''
> +            else:  # ''vbd'' or ''tap'' or
''tap2''
>                  dev_info = self._getDeviceInfo_vbd(dev)
>                  # To remove the UUID of the device from refs,
>                  # deviceClass must be always ''vbd''.
> @@ -1267,7 +1271,7 @@ class XendDomainInfo:
>              sxprs = []
>              dev_num = 0
>              for dev_type, dev_info in self.info.all_devices_sxpr():
> -                if (deviceClass == ''vbd'' and dev_type
not in [''vbd'', ''tap'']) or \
> +                if (deviceClass == ''vbd'' and dev_type
not in [''vbd'', ''tap'',
''tap2'']) or \
>                     (deviceClass != ''vbd'' and dev_type !=
deviceClass):
>                      continue
>  
> @@ -1295,12 +1299,27 @@ class XendDomainInfo:
>              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]
> +        # if the domain is running we can get the device class from
xenstore.
> +        # This is more accurate, as blktap1 devices show up as blktap2
devices
> +        # in the config.
> +        if self._stateGet() in (DOM_STATE_RUNNING, DOM_STATE_PAUSED,
DOM_STATE_CRASHED):
> +            # All block devices have a vbd frontend, so we know the
frontend path
> +            dev =
self.getDeviceController(''vbd'').convertToDeviceNumber(devid)
> +            frontendPath = "%s/device/vbd/%s" % (self.dompath,
dev)
> +            for devclass in XendDevices.valid_devices():
> +                for dev in xstransact.List("%s/device/%s" %
(self.vmpath, devclass)):
> +                    devFrontendPath =
xstransact.Read("%s/device/%s/%s/frontend" % (self.vmpath, devclass,
dev))
> +                    if frontendPath == devFrontendPath:
> +                        return devclass
> +            
> +        else: # the domain is not active so we must get the device class
> +              # from the config
> +            # 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 _getDeviceInfo_vif(self, mac):
>          for dev_type, dev_info in self.info.all_devices_sxpr():
> @@ -1311,7 +1330,7 @@ class XendDomainInfo:
>  
>      def _getDeviceInfo_vbd(self, devid):
>          for dev_type, dev_info in self.info.all_devices_sxpr():
> -            if dev_type != ''vbd'' and dev_type !=
''tap'':
> +            if dev_type != ''vbd'' and dev_type !=
''tap'' and dev_type != ''tap2'':
>                  continue
>              dev = sxp.child_value(dev_info, ''dev'')
>              dev = dev.split('':'')[0]
> @@ -2256,26 +2275,19 @@ class XendDomainInfo:
>              log.debug("No device model")
>  
>          log.debug("Releasing devices")
> -        t = xstransact("%s/device" % self.dompath)
> +        t = xstransact("%s/device" % self.vmpath)
>          try:
>              for devclass in XendDevices.valid_devices():
>                  for dev in t.list(devclass):
>                      try:
> -                        true_devclass = devclass
> -                        if devclass == ''vbd'':
> -                            # In the case of "vbd", the true
device class
> -                            # may possibly be "tap". Just in
case, verify
> -                            # device class.
> -                            devid = dev.split(''/'')[-1]
> -                            true_devclass =
self.getBlockDeviceClass(devid)
>                          log.debug("Removing %s", dev);
> -                        self.destroyDevice(true_devclass, dev, False);
> +                        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''],
> -                                      true_devclass, dev)
> +                                      devclass, dev)
>          finally:
>              t.abort()
>  
> @@ -2948,7 +2960,7 @@ class XendDomainInfo:
>  
>              fn = blkdev_uname_to_file(disk)
>              taptype = blkdev_uname_to_taptype(disk)
> -            mounted = devtype == ''tap'' and taptype !=
''aio'' and taptype != ''sync'' and not
os.stat(fn).st_rdev
> +            mounted = devtype in [''tap'',
''tap2''] and taptype != ''aio'' and taptype !=
''sync'' and not os.stat(fn).st_rdev
>              if mounted:
>                  # This is a file, not a device.  pygrub can cope with a
>                  # file if it''s raw, but if it''s QCOW or
other such formats
> @@ -3052,7 +3064,8 @@ class XendDomainInfo:
>              diff = time.time() - start
>              vbds =
self.getDeviceController(''vbd'').deviceIDs()
>              taps =
self.getDeviceController(''tap'').deviceIDs()
> -            for i in vbds + taps:
> +            tap2s =
self.getDeviceController(''tap2'').deviceIDs()
> +            for i in vbds + taps + tap2s:
>                  test = 1
>                  log.info("Dev %s still active, looping...", i)
>                  time.sleep(0.1)
> @@ -3635,7 +3648,7 @@ class XendDomainInfo:
>          """
>          xenapi_vbd[''image''] = vdi_image_path
>          if vdi_image_path.startswith(''tap''):
> -            dev_uuid = self.info.device_add(''tap'',
cfg_xenapi = xenapi_vbd)
> +            dev_uuid = self.info.device_add(''tap2'',
cfg_xenapi = xenapi_vbd)
>          else:
>              dev_uuid = self.info.device_add(''vbd'',
cfg_xenapi = xenapi_vbd)
>              
> @@ -3647,7 +3660,7 @@ class XendDomainInfo:
>              _, config = self.info[''devices''][dev_uuid]
>              
>              if vdi_image_path.startswith(''tap''):
> -                dev_control =
self.getDeviceController(''tap'')
> +                dev_control =
self.getDeviceController(''tap2'')
>              else:
>                  dev_control =
self.getDeviceController(''vbd'')
>  
> diff -r 7397608bce87 -r 6295171d4ccc
tools/python/xen/xend/server/BlktapController.py
> --- a/tools/python/xen/xend/server/BlktapController.py	Mon Jun 29 15:50:32
2009 +0100
> +++ b/tools/python/xen/xend/server/BlktapController.py	Tue Jun 30 17:44:41
2009 -0700
> @@ -24,7 +24,7 @@ blktap_disk_types = [
>      ''ioemu'',
>      ''tapdisk'',
>      ]
> - 
> +
>  def doexec(args, inputtext=None):
>      """Execute a subprocess, then return its return code,
stdout and stderr"""
>      proc = popen2.Popen3(args, True)
> @@ -49,8 +49,7 @@ def parseDeviceString(device):
>  
>      return minor, device, control
>  
> -
> -
> +# blktap1 device controller
>  class BlktapController(BlkifController):
>      def __init__(self, vm):
>          BlkifController.__init__(self, vm)
> @@ -59,11 +58,6 @@ class BlktapController(BlkifController):
>          """@see
DevController#frontendRoot"""
>          
>          return "%s/device/vbd" % self.vm.getDomainPath()
> -
> -    def devicePath(self, devid):
> -        """@see DevController#devicePath"""
> -        
> -        return "%s/device/vbd/%s" % (self.vm.vmpath, devid)
>  
>      def getDeviceDetails(self, config):
>          (devid, back, front) = BlkifController.getDeviceDetails(self,
config)
> @@ -123,6 +117,20 @@ class BlktapController(BlkifController):
>  
>          return (devid, back, front)
>  
> +class Blktap2Controller(BlktapController):
> +    def __init__(self, vm):
> +        BlktapController.__init__(self, vm)
> +
> +    def backendPath(self, backdom, devid):
> +        if self.deviceClass == ''tap2'':
> +            deviceClass = ''vbd''
> +        else:
> +            deviceClass = ''tap''
> +        return "%s/backend/%s/%s/%d" % (backdom.getDomainPath(),
> +                                        deviceClass,
> +                                        self.vm.getDomid(), devid)
> +
> +
>      def createDevice(self, config):
>  
>          uname = config.get(''required_uname'',
'''')
> @@ -140,12 +148,16 @@ class BlktapController(BlkifController):
>          stderr.close();
>          if( out.find("blktap2") >= 0 ):
>              blktap2_installed=1;
> -           
> +
>          if typ in (''tap''):
> -            if subtyp in (''tapdisk''):
> +            if subtyp in (''tapdisk''):
>                  if params in (''ioemu'',
''qcow2'', ''vmdk'', ''sync'')
or not blktap2_installed:
> -                    log.warn(''WARNING: using deprecated blktap
module'');
> -                    return BlkifController.createDevice(self, config);
> +                    # pass this device off to BlktapController
> +                    log.warn(''WARNING: using deprecated blktap
module'')
> +                    self.deviceClass = ''tap''
> +                    devid = BlktapController.createDevice(self, config)
> +                    self.deviceClass = ''tap2''
> +                    return devid
>  
>          cmd = [ TAPDISK_BINARY, ''-n'',
''%s:%s'' % (params, file) ]
>          (rc,stdout,stderr) = doexec(cmd)
> @@ -165,7 +177,32 @@ class BlktapController(BlkifController):
>          #device is configured.  Then continue to create the device
>          config.update({''uname'' :
''phy:'' + device.rstrip()})
>  
> -        self.deviceClass=''vbd''
>          devid = BlkifController.createDevice(self, config)
> -        self.deviceClass=''tap''
>          return devid
> +
> +    # The new blocktap implementation requires a sysfs signal to close
> +    # out disks.  This function is called from a thread when the
> +    # domain is detached from the disk.
> +    def finishDeviceCleanup(self, backpath, path):
> +        """Perform any device specific cleanup
> +
> +        @backpath backend xenstore path.
> +        @path frontend device path
> +
> +        """
> +
> +        #Figure out what we''re going to wait on.
> +        self.waitForBackend_destroy(backpath)
> +
> +        #Figure out the sysfs path.
> +        pattern =
re.compile(''/dev/xen/blktap-2/tapdev(\d+)$'')
> +        ctrlid = pattern.search(path)
> +        ctrl = ''/sys/class/blktap2/blktap'' +
ctrlid.group(1)
> +
> +        #Close out the disk
> +        f = open(ctrl + ''/remove'',
''w'')
> +        f.write(''remove'');
> +        f.close()
> +
> +        return
> +
> diff -r 7397608bce87 -r 6295171d4ccc
tools/python/xen/xend/server/DevController.py
> --- a/tools/python/xen/xend/server/DevController.py	Mon Jun 29 15:50:32
2009 +0100
> +++ b/tools/python/xen/xend/server/DevController.py	Tue Jun 30 17:44:41
2009 -0700
> @@ -238,34 +238,6 @@ class DevController:
>              # xstransact.Remove(self.devicePath()) ?? Below is the same ?
>              self.vm._removeVm("device/%s/%d" %
(self.deviceClass, dev))
>  
> -    # The new blocktap implementation requires a sysfs signal to close
> -    # out disks.  This function is called from a thread when the
> -    # domain is detached from the disk.
> -    def finishDeviceCleanup(self, backpath, path):
> -        """Perform any device specific cleanup
> -
> -        @backpath backend xenstore path.
> -        @path frontend device path
> -
> -        """
> -        
> -        if path and
path.startswith(''/dev/xen/blktap-2''):
> -            
> -            #Figure out what we''re going to wait on.
> -            self.waitForBackend_destroy(backpath)            
> -
> -            #Figure out the sysfs path.
> -            pattern =
re.compile(''/dev/xen/blktap-2/tapdev(\d+)$'')
> -            ctrlid = pattern.search(path)
> -            ctrl = ''/sys/class/blktap2/blktap'' +
ctrlid.group(1)
> -            
> -            #Close out the disk
> -            f = open(ctrl + ''/remove'',
''w'')
> -            f.write(''remove'');
> -            f.close()
> -
> -        return
> -
>      def configurations(self, transaction = None):
>          return map(lambda x: self.configuration(x, transaction),
self.deviceIDs(transaction))
>  
> @@ -575,7 +547,6 @@ class DevController:
>              
>          backpath = self.readVm(devid, "backend")
>  
> -
>          if backpath:
>              statusPath = backpath + ''/'' +
HOTPLUG_STATUS_NODE
>              ev = Event()
> diff -r 7397608bce87 -r 6295171d4ccc tools/python/xen/xm/create.py
> --- a/tools/python/xen/xm/create.py	Mon Jun 29 15:50:32 2009 +0100
> +++ b/tools/python/xen/xm/create.py	Tue Jun 30 17:44:41 2009 -0700
> @@ -718,7 +718,7 @@ def configure_disks(config_devs, vals):
>      """
>      for (uname, dev, mode, backend, protocol) in vals.disk:
>          if uname.startswith(''tap:''):
> -            cls = ''tap''
> +            cls = ''tap2''
>          else:
>              cls = ''vbd''
>  
> diff -r 7397608bce87 -r 6295171d4ccc tools/python/xen/xm/main.py
> --- a/tools/python/xen/xm/main.py	Mon Jun 29 15:50:32 2009 +0100
> +++ b/tools/python/xen/xm/main.py	Tue Jun 30 17:44:41 2009 -0700
> @@ -2371,7 +2371,7 @@ def parse_block_configuration(args):
>      dom = args[0]
>  
>      if args[1].startswith(''tap:''):
> -        cls = ''tap''
> +        cls = ''tap2''
>      else:
>          cls = ''vbd''
>  
> @@ -2706,7 +2706,9 @@ def xm_block_detach(args):
>          dom = args[0]
>          dev = args[1]
>          dc = server.xend.domain.getBlockDeviceClass(dom, dev)
> -        if dc == "tap":
> +        if dc == "tap2":
> +            detach(args, ''tap2'')
> +        elif dc == "tap":
>              detach(args, ''tap'')
>          else:
>              detach(args, ''vbd'')
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
-- 
 Ryan O''Connor <rjo@cs.ubc.ca>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Hi Ryan,

Good job!
I will review and test the patch as soon as possible.

Best regards,
 Kan

Tue, 30 Jun 2009 17:49:44 -0700, "Ryan O''Connor" wrote:
># HG changeset patch
># User Ryan O''Connor <rjo@cs.ubc.ca>
># Date 1246409081 25200
># Node ID 6295171d4ccc60f80f0411217d9b1c7d8543c804
># Parent  7397608bce877b151ee507413a8e228092947ee4
>blktap2: add blktap2 device class and device controller
>
>blktap2 devices must be handled differently than blktap2 devices. blktap2
>devices require a sysfs write to close the underlying device, as well as 
>extra
>sysfs writes when the domU is paused/unpaused. The differences between 
>blktap1
>and blktap2 are great enough to warrant the creation of a new device class,
>''tap2'', and device controller for blktap2 devices.
>
>  * add a new device controller (Blktap2Controller) and device class (tap2)
> for
>    blktap2 devices
>  * move blktap2 specific code from DevController to Blktap2Controller
>  * if possible, check xenstore to determine block device class
>  * use vmpath (/vm/<uuid>/) when releasing devices
>  * modify linux hotplug cleanup script to handle blktap2 device removal
>
>Signed-off-by: Ryan O''Connor <rjo@cs.ubc.ca>
>
>diff -r 7397608bce87 -r 6295171d4ccc tools/hotplug/Linux/xen-hotplug-cleanup
>--- a/tools/hotplug/Linux/xen-hotplug-cleanup	Mon Jun 29 15:50:32 2009 +0100
>+++ b/tools/hotplug/Linux/xen-hotplug-cleanup	Tue Jun 30 17:44:41 2009 -0700
>@@ -18,6 +18,13 @@ vm=$(xenstore_read_default "/local/domai
> # construct /vm/UUID/device/DEVCLASS/DEVID
> if [ "$vm" != "" ]; then
>   vm_dev="$vm/device/${path_array[1]}/${path_array[3]}"
>+
>+  # if the vm path does not exist and the device class is
''vbd'' then we
>may have
>+  # a tap2 device
>+  if [ ! $(xenstore-read "vm_dev" 2>/dev/null) ] \
>+       && [ "${path_array[1]}" = "vbd" ]; then
>+    vm_dev="$vm/device/tap2/${path_array[3]}"
>+  fi
> else
>   vm_dev> fi
>diff -r 7397608bce87 -r 6295171d4ccc tools/python/xen/xend/XendConfig.py
>--- a/tools/python/xen/xend/XendConfig.py	Mon Jun 29 15:50:32 2009 +0100
>+++ b/tools/python/xen/xend/XendConfig.py	Tue Jun 30 17:44:41 2009 -0700
>@@ -1114,7 +1114,7 @@ class XendConfig(dict):
>                             controller = domain.getDeviceController(cls)
>                             configs = controller.configurations(txn)
>                             for config in configs:
>-                                if sxp.name(config) in
(''vbd'', ''tap''):
>+                                if sxp.name(config) in
(''vbd'', ''tap'',
>''tap2''):
>                                     dev_uuid = sxp.child_value(config, 
>''uuid'')
>                                     dev_type, dev_cfg =
self[''devices''][
>dev_uuid]
>                                     if sxp.child_value(config,
''bootable'',
> None) is None:
>@@ -1179,7 +1179,7 @@ class XendConfig(dict):
>     def device_duplicate_check(self, dev_type, dev_info, defined_config, 
>config):
>         defined_devices_sxpr = self.all_devices_sxpr(target = 
>defined_config)
>         
>-        if dev_type == ''vbd'' or dev_type ==
''tap'':
>+        if dev_type == ''vbd'' or dev_type ==
''tap'' or dev_type == ''tap2'':
>             dev_uname = dev_info.get(''uname'')
>             blkdev_name = dev_info.get(''dev'')
>             devid = self._blkdev_name_to_number(blkdev_name)
>@@ -1187,7 +1187,7 @@ class XendConfig(dict):
>                 return
>             
>             for o_dev_type, o_dev_info in defined_devices_sxpr:
>-                if o_dev_type == ''vbd'' or o_dev_type ==
''tap'':
>+                if o_dev_type == ''vbd'' or o_dev_type ==
''tap'' or
>o_dev_type == ''tap2'':
>                     blkdev_file = blkdev_uname_to_file(dev_uname)
>                     o_dev_uname = sxp.child_value(o_dev_info,
''uname'')
>                     if o_dev_uname != None:
>@@ -1369,7 +1369,7 @@ class XendConfig(dict):
>                 else:
>                     dev_info[''driver''] =
''paravirtualised''
> 
>-            if dev_type == ''tap'':
>+            if dev_type == ''tap'' or dev_type ==
''tap2'':
>                 if
dev_info[''uname''].split('':'')[1] not in
blktap_disk_types:
>                     raise XendConfigError("tap:%s not a valid disk
type" %
>                                    
dev_info[''uname''].split('':'')[1])
>@@ -1406,7 +1406,7 @@ class XendConfig(dict):
>                         # Compat hack -- mark first disk bootable
>                         dev_info[''bootable''] = int(not
target[param])
>                     target[param].append(dev_uuid)
>-            elif dev_type == ''tap'':
>+            elif dev_type == ''tap'' or dev_type ==
''tap2'':
>                 if ''vbd_refs'' not in target:
>                     target[''vbd_refs''] = []
>                 if dev_uuid not in target[''vbd_refs'']:
>@@ -1504,7 +1504,7 @@ class XendConfig(dict):
>                 target[''devices''][dev_uuid] = (dev_type,
dev_info)
>                 target[''vif_refs''].append(dev_uuid)
>             
>-            elif dev_type in (''vbd'',
''tap''):
>+            elif dev_type in (''vbd'',
''tap'', ''tap2''):
>                 dev_info[''type''] =
cfg_xenapi.get(''type'', ''Disk'')
>                 if dev_info[''type''] ==
''CD'':
>                     old_vbd_type = ''cdrom''
>diff -r 7397608bce87 -r 6295171d4ccc tools/python/xen/xend/XendDevices.py
>--- a/tools/python/xen/xend/XendDevices.py	Mon Jun 29 15:50:32 2009 +0100
>+++ b/tools/python/xen/xend/XendDevices.py	Tue Jun 30 17:44:41 2009 -0700
>@@ -20,7 +20,7 @@
> #
> 
> from xen.xend.server import blkif, netif, tpmif, pciif, iopif, irqif, 
>vfbif, vscsiif
>-from xen.xend.server.BlktapController import BlktapController
>+from xen.xend.server.BlktapController import BlktapController, 
>Blktap2Controller
> from xen.xend.server.ConsoleController import ConsoleController
> 
> 
>@@ -42,6 +42,7 @@ class XendDevices:
>         ''ioports'': iopif.IOPortsController,
>         ''irq'': irqif.IRQController,
>         ''tap'': BlktapController,
>+        ''tap2'': Blktap2Controller,
>         ''vfb'': vfbif.VfbifController,
>         ''vkbd'': vfbif.VkbdifController,
>         ''console'': ConsoleController,
>diff -r 7397608bce87 -r 6295171d4ccc tools/python/xen/xend/XendDomainInfo.py
>--- a/tools/python/xen/xend/XendDomainInfo.py	Mon Jun 29 15:50:32 2009 +0100
>+++ b/tools/python/xen/xend/XendDomainInfo.py	Tue Jun 30 17:44:41 2009 -0700
>@@ -538,12 +538,11 @@ class XendDomainInfo:
>         @raise XendError: Failed pausing a domain
>         """
>         try:
>-            bepath="/local/domain/0/backend/"
>             if(self.domid):
>-                
>-                dev =  xstransact.List(bepath + ''vbd'' +
"/%d" % (self.
>domid,))
>+                # get all blktap2 devices
>+                dev =  xstransact.List(self.vmpath +
''device/tap2'')
>                 for x in dev:
>-                    path =
self.getDeviceController(''vbd'').readBackend(x,
>''params'')
>+                    path =
self.getDeviceController(''tap2'').readBackend(x,
> ''params'')
>                     if path and
path.startswith(''/dev/xen/blktap-2''):
>                         #Figure out the sysfs path.
>                         pattern =
re.compile(''/dev/xen/blktap-2/tapdev(\d+
>)$'')
>@@ -569,11 +568,10 @@ class XendDomainInfo:
>         @raise XendError: Failed unpausing a domain
>         """
>         try:
>-            bepath="/local/domain/0/backend/"
>             if(self.domid):
>-                dev =  xstransact.List(bepath + "vbd" +
"/%d" % (self.
>domid,))
>+                dev =  xstransact.List(self.vmpath +
''device/tap2'')
>                 for x in dev:
>-                    path =
self.getDeviceController(''vbd'').readBackend(x,
>''params'')
>+                    path =
self.getDeviceController(''tap2'').readBackend(x,
> ''params'')
>                     if path and
path.startswith(''/dev/xen/blktap-2''):
>                         #Figure out the sysfs path.
>                         pattern =
re.compile(''/dev/xen/blktap-2/tapdev(\d+
>)$'')
>@@ -812,6 +810,11 @@ class XendDomainInfo:
>             try:
>                 dev_config_dict[''devid''] = devid = \
>                     self._createDevice(dev_type, dev_config_dict)
>+                if dev_type == ''tap2'':
>+                    # createDevice may create a blktap1 device if blktap2 
>is not
>+                    # installed or if the blktap driver is not supported in
>+                    # blktap1
>+                    dev_type = self.getBlockDeviceClass(devid)
>                 self._waitForDevice(dev_type, devid)
>             except VmError, ex:
>                 del self.info[''devices''][dev_uuid]
>@@ -821,7 +824,7 @@ class XendDomainInfo:
>                 elif dev_type == ''vscsi'':
>                     for dev in dev_config_dict[''devs'']:
>                        
XendAPIStore.deregister(dev[''uuid''],
''DSCSI'')
>-                elif dev_type == ''tap'':
>+                elif dev_type == ''tap'' or dev_type ==
''tap2'':
>                    
self.info[''vbd_refs''].remove(dev_uuid)
>                 else:
>                     self.info[''%s_refs'' %
dev_type].remove(dev_uuid)
>@@ -1200,9 +1203,10 @@ class XendDomainInfo:
>         if self.domid is not None:
>             
>             #new blktap implementation may need a sysfs write after 
>everything is torn down.
>-            dev = self.getDeviceController(deviceClass).
>convertToDeviceNumber(devid)
>-            path = self.getDeviceController(deviceClass).readBackend(dev, 
>''params'')                
>-            if path and
path.startswith(''/dev/xen/blktap-2''):
>+
>+            if deviceClass == ''tap2'':
>+                dev = self.getDeviceController(deviceClass).
>convertToDeviceNumber(devid)
>+                path = self.getDeviceController(deviceClass).readBackend(
>dev, ''params'')
>                 frontpath = self.getDeviceController(deviceClass).
>frontendPath(dev)
>                 backpath = xstransact.Read(frontpath, "backend")
>                 thread.start_new_thread(self.getDeviceController(
>deviceClass).finishDeviceCleanup, (backpath, path))
>@@ -1238,7 +1242,7 @@ class XendDomainInfo:
>                     dev_info = self._getDeviceInfo_vif(mac)
>                 else:
>                     _, dev_info = sxprs[dev]
>-            else:  # ''vbd'' or ''tap''
>+            else:  # ''vbd'' or ''tap'' or
''tap2''
>                 dev_info = self._getDeviceInfo_vbd(dev)
>                 # To remove the UUID of the device from refs,
>                 # deviceClass must be always ''vbd''.
>@@ -1267,7 +1271,7 @@ class XendDomainInfo:
>             sxprs = []
>             dev_num = 0
>             for dev_type, dev_info in self.info.all_devices_sxpr():
>-                if (deviceClass == ''vbd'' and dev_type not
in [''vbd'', ''tap''
>]) or \
>+                if (deviceClass == ''vbd'' and dev_type not
in [''vbd'', ''tap''
>, ''tap2'']) or \
>                    (deviceClass != ''vbd'' and dev_type !=
deviceClass):
>                     continue
> 
>@@ -1295,12 +1299,27 @@ class XendDomainInfo:
>             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]
>+        # if the domain is running we can get the device class from 
>xenstore.
>+        # This is more accurate, as blktap1 devices show up as blktap2 
>devices
>+        # in the config.
>+        if self._stateGet() in (DOM_STATE_RUNNING, DOM_STATE_PAUSED, 
>DOM_STATE_CRASHED):
>+            # All block devices have a vbd frontend, so we know the 
>frontend path
>+            dev =
self.getDeviceController(''vbd'').convertToDeviceNumber(
>devid)
>+            frontendPath = "%s/device/vbd/%s" % (self.dompath,
dev)
>+            for devclass in XendDevices.valid_devices():
>+                for dev in xstransact.List("%s/device/%s" %
(self.vmpath,
>devclass)):
>+                    devFrontendPath =
xstransact.Read("%s/device/%s/%s/
>frontend" % (self.vmpath, devclass, dev))
>+                    if frontendPath == devFrontendPath:
>+                        return devclass
>+            
>+        else: # the domain is not active so we must get the device class
>+              # from the config
>+            # 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 _getDeviceInfo_vif(self, mac):
>         for dev_type, dev_info in self.info.all_devices_sxpr():
>@@ -1311,7 +1330,7 @@ class XendDomainInfo:
> 
>     def _getDeviceInfo_vbd(self, devid):
>         for dev_type, dev_info in self.info.all_devices_sxpr():
>-            if dev_type != ''vbd'' and dev_type !=
''tap'':
>+            if dev_type != ''vbd'' and dev_type !=
''tap'' and dev_type !=
>''tap2'':
>                 continue
>             dev = sxp.child_value(dev_info, ''dev'')
>             dev = dev.split('':'')[0]
>@@ -2256,26 +2275,19 @@ class XendDomainInfo:
>             log.debug("No device model")
> 
>         log.debug("Releasing devices")
>-        t = xstransact("%s/device" % self.dompath)
>+        t = xstransact("%s/device" % self.vmpath)
>         try:
>             for devclass in XendDevices.valid_devices():
>                 for dev in t.list(devclass):
>                     try:
>-                        true_devclass = devclass
>-                        if devclass == ''vbd'':
>-                            # In the case of "vbd", the true
device class
>-                            # may possibly be "tap". Just in
case, verify
>-                            # device class.
>-                            devid = dev.split(''/'')[-1]
>-                            true_devclass = self.getBlockDeviceClass(devid)
>                         log.debug("Removing %s", dev);
>-                        self.destroyDevice(true_devclass, dev, False);
>+                        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''],
>-                                      true_devclass, dev)
>+                                      devclass, dev)
>         finally:
>             t.abort()
> 
>@@ -2948,7 +2960,7 @@ class XendDomainInfo:
> 
>             fn = blkdev_uname_to_file(disk)
>             taptype = blkdev_uname_to_taptype(disk)
>-            mounted = devtype == ''tap'' and taptype !=
''aio'' and taptype !> ''sync'' and not
os.stat(fn).st_rdev
>+            mounted = devtype in [''tap'',
''tap2''] and taptype != ''aio'' and
>taptype != ''sync'' and not os.stat(fn).st_rdev
>             if mounted:
>                 # This is a file, not a device.  pygrub can cope with a
>                 # file if it''s raw, but if it''s QCOW or
other such formats
>@@ -3052,7 +3064,8 @@ class XendDomainInfo:
>             diff = time.time() - start
>             vbds =
self.getDeviceController(''vbd'').deviceIDs()
>             taps =
self.getDeviceController(''tap'').deviceIDs()
>-            for i in vbds + taps:
>+            tap2s =
self.getDeviceController(''tap2'').deviceIDs()
>+            for i in vbds + taps + tap2s:
>                 test = 1
>                 log.info("Dev %s still active, looping...", i)
>                 time.sleep(0.1)
>@@ -3635,7 +3648,7 @@ class XendDomainInfo:
>         """
>         xenapi_vbd[''image''] = vdi_image_path
>         if vdi_image_path.startswith(''tap''):
>-            dev_uuid = self.info.device_add(''tap'',
cfg_xenapi = xenapi_vbd)
>+            dev_uuid = self.info.device_add(''tap2'',
cfg_xenapi = xenapi_vbd)
>         else:
>             dev_uuid = self.info.device_add(''vbd'',
cfg_xenapi = xenapi_vbd)
>             
>@@ -3647,7 +3660,7 @@ class XendDomainInfo:
>             _, config = self.info[''devices''][dev_uuid]
>             
>             if vdi_image_path.startswith(''tap''):
>-                dev_control =
self.getDeviceController(''tap'')
>+                dev_control =
self.getDeviceController(''tap2'')
>             else:
>                 dev_control =
self.getDeviceController(''vbd'')
> 
>diff -r 7397608bce87 -r 6295171d4ccc tools/python/xen/xend/server/
>BlktapController.py
>--- a/tools/python/xen/xend/server/BlktapController.py	Mon Jun 29 15:50:32 
>2009 +0100
>+++ b/tools/python/xen/xend/server/BlktapController.py	Tue Jun 30 17:44:41 
>2009 -0700
>@@ -24,7 +24,7 @@ blktap_disk_types = [
>     ''ioemu'',
>     ''tapdisk'',
>     ]
>- 
>+
> def doexec(args, inputtext=None):
>     """Execute a subprocess, then return its return code,
stdout and
>stderr"""
>     proc = popen2.Popen3(args, True)
>@@ -49,8 +49,7 @@ def parseDeviceString(device):
> 
>     return minor, device, control
> 
>-
>-
>+# blktap1 device controller
> class BlktapController(BlkifController):
>     def __init__(self, vm):
>         BlkifController.__init__(self, vm)
>@@ -59,11 +58,6 @@ class BlktapController(BlkifController):
>         """@see DevController#frontendRoot"""
>         
>         return "%s/device/vbd" % self.vm.getDomainPath()
>-
>-    def devicePath(self, devid):
>-        """@see DevController#devicePath"""
>-        
>-        return "%s/device/vbd/%s" % (self.vm.vmpath, devid)
> 
>     def getDeviceDetails(self, config):
>         (devid, back, front) = BlkifController.getDeviceDetails(self, 
>config)
>@@ -123,6 +117,20 @@ class BlktapController(BlkifController):
> 
>         return (devid, back, front)
> 
>+class Blktap2Controller(BlktapController):
>+    def __init__(self, vm):
>+        BlktapController.__init__(self, vm)
>+
>+    def backendPath(self, backdom, devid):
>+        if self.deviceClass == ''tap2'':
>+            deviceClass = ''vbd''
>+        else:
>+            deviceClass = ''tap''
>+        return "%s/backend/%s/%s/%d" % (backdom.getDomainPath(),
>+                                        deviceClass,
>+                                        self.vm.getDomid(), devid)
>+
>+
>     def createDevice(self, config):
> 
>         uname = config.get(''required_uname'',
'''')
>@@ -140,12 +148,16 @@ class BlktapController(BlkifController):
>         stderr.close();
>         if( out.find("blktap2") >= 0 ):
>             blktap2_installed=1;
>-           
>+
>         if typ in (''tap''):
>-            if subtyp in (''tapdisk''):
>     
>+            if subtyp in (''tapdisk''):
>                 if params in (''ioemu'',
''qcow2'', ''vmdk'', ''sync'')
or not
>blktap2_installed:
>-                    log.warn(''WARNING: using deprecated blktap
module'');
>-                    return BlkifController.createDevice(self, config);
>+                    # pass this device off to BlktapController
>+                    log.warn(''WARNING: using deprecated blktap
module'')
>+                    self.deviceClass = ''tap''
>+                    devid = BlktapController.createDevice(self, config)
>+                    self.deviceClass = ''tap2''
>+                    return devid
> 
>         cmd = [ TAPDISK_BINARY, ''-n'',
''%s:%s'' % (params, file) ]
>         (rc,stdout,stderr) = doexec(cmd)
>@@ -165,7 +177,32 @@ class BlktapController(BlkifController):
>         #device is configured.  Then continue to create the device
>         config.update({''uname'' : ''phy:''
+ device.rstrip()})
> 
>-        self.deviceClass=''vbd''
>         devid = BlkifController.createDevice(self, config)
>-        self.deviceClass=''tap''
>         return devid
>+
>+    # The new blocktap implementation requires a sysfs signal to close
>+    # out disks.  This function is called from a thread when the
>+    # domain is detached from the disk.
>+    def finishDeviceCleanup(self, backpath, path):
>+        """Perform any device specific cleanup
>+
>+        @backpath backend xenstore path.
>+        @path frontend device path
>+
>+        """
>+
>+        #Figure out what we''re going to wait on.
>+        self.waitForBackend_destroy(backpath)
>+
>+        #Figure out the sysfs path.
>+        pattern =
re.compile(''/dev/xen/blktap-2/tapdev(\d+)$'')
>+        ctrlid = pattern.search(path)
>+        ctrl = ''/sys/class/blktap2/blktap'' +
ctrlid.group(1)
>+
>+        #Close out the disk
>+        f = open(ctrl + ''/remove'', ''w'')
>+        f.write(''remove'');
>+        f.close()
>+
>+        return
>+
>diff -r 7397608bce87 -r 6295171d4ccc tools/python/xen/xend/server/
>DevController.py
>--- a/tools/python/xen/xend/server/DevController.py	Mon Jun 29 15:50:32 
>2009 +0100
>+++ b/tools/python/xen/xend/server/DevController.py	Tue Jun 30 17:44:41 
>2009 -0700
>@@ -238,34 +238,6 @@ class DevController:
>             # xstransact.Remove(self.devicePath()) ?? Below is the same ?
>             self.vm._removeVm("device/%s/%d" % (self.deviceClass,
dev))
> 
>-    # The new blocktap implementation requires a sysfs signal to close
>-    # out disks.  This function is called from a thread when the
>-    # domain is detached from the disk.
>-    def finishDeviceCleanup(self, backpath, path):
>-        """Perform any device specific cleanup
>-
>-        @backpath backend xenstore path.
>-        @path frontend device path
>-
>-        """
>-        
>-        if path and path.startswith(''/dev/xen/blktap-2''):
>-            
>-            #Figure out what we''re going to wait on.
>-            self.waitForBackend_destroy(backpath)            
>-
>-            #Figure out the sysfs path.
>-            pattern =
re.compile(''/dev/xen/blktap-2/tapdev(\d+)$'')
>-            ctrlid = pattern.search(path)
>-            ctrl = ''/sys/class/blktap2/blktap'' +
ctrlid.group(1)
>-            
>-            #Close out the disk
>-            f = open(ctrl + ''/remove'',
''w'')
>-            f.write(''remove'');
>-            f.close()
>-
>-        return
>-
>     def configurations(self, transaction = None):
>         return map(lambda x: self.configuration(x, transaction), self.
>deviceIDs(transaction))
> 
>@@ -575,7 +547,6 @@ class DevController:
>             
>         backpath = self.readVm(devid, "backend")
> 
>-
>         if backpath:
>             statusPath = backpath + ''/'' +
HOTPLUG_STATUS_NODE
>             ev = Event()
>diff -r 7397608bce87 -r 6295171d4ccc tools/python/xen/xm/create.py
>--- a/tools/python/xen/xm/create.py	Mon Jun 29 15:50:32 2009 +0100
>+++ b/tools/python/xen/xm/create.py	Tue Jun 30 17:44:41 2009 -0700
>@@ -718,7 +718,7 @@ def configure_disks(config_devs, vals):
>     """
>     for (uname, dev, mode, backend, protocol) in vals.disk:
>         if uname.startswith(''tap:''):
>-            cls = ''tap''
>+            cls = ''tap2''
>         else:
>             cls = ''vbd''
> 
>diff -r 7397608bce87 -r 6295171d4ccc tools/python/xen/xm/main.py
>--- a/tools/python/xen/xm/main.py	Mon Jun 29 15:50:32 2009 +0100
>+++ b/tools/python/xen/xm/main.py	Tue Jun 30 17:44:41 2009 -0700
>@@ -2371,7 +2371,7 @@ def parse_block_configuration(args):
>     dom = args[0]
> 
>     if args[1].startswith(''tap:''):
>-        cls = ''tap''
>+        cls = ''tap2''
>     else:
>         cls = ''vbd''
> 
>@@ -2706,7 +2706,9 @@ def xm_block_detach(args):
>         dom = args[0]
>         dev = args[1]
>         dc = server.xend.domain.getBlockDeviceClass(dom, dev)
>-        if dc == "tap":
>+        if dc == "tap2":
>+            detach(args, ''tap2'')
>+        elif dc == "tap":
>             detach(args, ''tap'')
>         else:
>             detach(args, ''vbd'')
>
>_______________________________________________
>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

Hi Ryan,
>blktap2 devices must be handled differently than blktap2 devices. blktap2
                                                  ^^^^^^^
                                                  blktap1?  typo?


Basically questions:

How should I define a disk parameter into VM configuration files to use 
blktap1 devices?

  e.g. disk = [ ''tap:aio:root-vm1.img,xvda,w'' ]


How should I define a disk parameter into VM configuration files to use 
blktap2 devices?

  e.g. disk = [ ''tap2:aio:root-vm2.img,xvda,w'' ]


If you distinguish the device class of blktap2 devices from the device 
class of blktap1 devices, should you modify it as follows?
I think that the device class of blktap1 devices should be
''tap'' if we
use blktap1 devices as before.

diff -r 7397608bce87 -r 6295171d4ccc tools/python/xen/xm/create.py
--- a/tools/python/xen/xm/create.py	Mon Jun 29 15:50:32 2009 +0100
+++ b/tools/python/xen/xm/create.py	Tue Jun 30 17:44:41 2009 -0700
@@ -718,7 +718,7 @@ def configure_disks(config_devs, vals):
     """
     for (uname, dev, mode, backend, protocol) in vals.disk:
         if uname.startswith(''tap:''):
             cls = ''tap''
+        elif uname.startswith(''tap2:''):
+            cls = ''tap2''
         else:
             cls = ''vbd''
 

Best regards,
 Kan

Tue, 30 Jun 2009 17:49:44 -0700, "Ryan O''Connor" wrote:
># HG changeset patch
># User Ryan O''Connor <rjo@cs.ubc.ca>
># Date 1246409081 25200
># Node ID 6295171d4ccc60f80f0411217d9b1c7d8543c804
># Parent  7397608bce877b151ee507413a8e228092947ee4
>blktap2: add blktap2 device class and device controller
>
>blktap2 devices must be handled differently than blktap2 devices. blktap2
>devices require a sysfs write to close the underlying device, as well as 
>extra
>sysfs writes when the domU is paused/unpaused. The differences between 
>blktap1
>and blktap2 are great enough to warrant the creation of a new device class,
>''tap2'', and device controller for blktap2 devices.
>
>  * add a new device controller (Blktap2Controller) and device class (tap2)
> for
>    blktap2 devices
>  * move blktap2 specific code from DevController to Blktap2Controller
>  * if possible, check xenstore to determine block device class
>  * use vmpath (/vm/<uuid>/) when releasing devices
>  * modify linux hotplug cleanup script to handle blktap2 device removal
>
>Signed-off-by: Ryan O''Connor <rjo@cs.ubc.ca>
>
>diff -r 7397608bce87 -r 6295171d4ccc tools/hotplug/Linux/xen-hotplug-cleanup
>--- a/tools/hotplug/Linux/xen-hotplug-cleanup	Mon Jun 29 15:50:32 2009 +0100
>+++ b/tools/hotplug/Linux/xen-hotplug-cleanup	Tue Jun 30 17:44:41 2009 -0700
>@@ -18,6 +18,13 @@ vm=$(xenstore_read_default "/local/domai
> # construct /vm/UUID/device/DEVCLASS/DEVID
> if [ "$vm" != "" ]; then
>   vm_dev="$vm/device/${path_array[1]}/${path_array[3]}"
>+
>+  # if the vm path does not exist and the device class is
''vbd'' then we
>may have
>+  # a tap2 device
>+  if [ ! $(xenstore-read "vm_dev" 2>/dev/null) ] \
>+       && [ "${path_array[1]}" = "vbd" ]; then
>+    vm_dev="$vm/device/tap2/${path_array[3]}"
>+  fi
> else
>   vm_dev> fi
>diff -r 7397608bce87 -r 6295171d4ccc tools/python/xen/xend/XendConfig.py
>--- a/tools/python/xen/xend/XendConfig.py	Mon Jun 29 15:50:32 2009 +0100
>+++ b/tools/python/xen/xend/XendConfig.py	Tue Jun 30 17:44:41 2009 -0700
>@@ -1114,7 +1114,7 @@ class XendConfig(dict):
>                             controller = domain.getDeviceController(cls)
>                             configs = controller.configurations(txn)
>                             for config in configs:
>-                                if sxp.name(config) in
(''vbd'', ''tap''):
>+                                if sxp.name(config) in
(''vbd'', ''tap'',
>''tap2''):
>                                     dev_uuid = sxp.child_value(config, 
>''uuid'')
>                                     dev_type, dev_cfg =
self[''devices''][
>dev_uuid]
>                                     if sxp.child_value(config,
''bootable'',
> None) is None:
>@@ -1179,7 +1179,7 @@ class XendConfig(dict):
>     def device_duplicate_check(self, dev_type, dev_info, defined_config, 
>config):
>         defined_devices_sxpr = self.all_devices_sxpr(target = 
>defined_config)
>         
>-        if dev_type == ''vbd'' or dev_type ==
''tap'':
>+        if dev_type == ''vbd'' or dev_type ==
''tap'' or dev_type == ''tap2'':
>             dev_uname = dev_info.get(''uname'')
>             blkdev_name = dev_info.get(''dev'')
>             devid = self._blkdev_name_to_number(blkdev_name)
>@@ -1187,7 +1187,7 @@ class XendConfig(dict):
>                 return
>             
>             for o_dev_type, o_dev_info in defined_devices_sxpr:
>-                if o_dev_type == ''vbd'' or o_dev_type ==
''tap'':
>+                if o_dev_type == ''vbd'' or o_dev_type ==
''tap'' or
>o_dev_type == ''tap2'':
>                     blkdev_file = blkdev_uname_to_file(dev_uname)
>                     o_dev_uname = sxp.child_value(o_dev_info,
''uname'')
>                     if o_dev_uname != None:
>@@ -1369,7 +1369,7 @@ class XendConfig(dict):
>                 else:
>                     dev_info[''driver''] =
''paravirtualised''
> 
>-            if dev_type == ''tap'':
>+            if dev_type == ''tap'' or dev_type ==
''tap2'':
>                 if
dev_info[''uname''].split('':'')[1] not in
blktap_disk_types:
>                     raise XendConfigError("tap:%s not a valid disk
type" %
>                                    
dev_info[''uname''].split('':'')[1])
>@@ -1406,7 +1406,7 @@ class XendConfig(dict):
>                         # Compat hack -- mark first disk bootable
>                         dev_info[''bootable''] = int(not
target[param])
>                     target[param].append(dev_uuid)
>-            elif dev_type == ''tap'':
>+            elif dev_type == ''tap'' or dev_type ==
''tap2'':
>                 if ''vbd_refs'' not in target:
>                     target[''vbd_refs''] = []
>                 if dev_uuid not in target[''vbd_refs'']:
>@@ -1504,7 +1504,7 @@ class XendConfig(dict):
>                 target[''devices''][dev_uuid] = (dev_type,
dev_info)
>                 target[''vif_refs''].append(dev_uuid)
>             
>-            elif dev_type in (''vbd'',
''tap''):
>+            elif dev_type in (''vbd'',
''tap'', ''tap2''):
>                 dev_info[''type''] =
cfg_xenapi.get(''type'', ''Disk'')
>                 if dev_info[''type''] ==
''CD'':
>                     old_vbd_type = ''cdrom''
>diff -r 7397608bce87 -r 6295171d4ccc tools/python/xen/xend/XendDevices.py
>--- a/tools/python/xen/xend/XendDevices.py	Mon Jun 29 15:50:32 2009 +0100
>+++ b/tools/python/xen/xend/XendDevices.py	Tue Jun 30 17:44:41 2009 -0700
>@@ -20,7 +20,7 @@
> #
> 
> from xen.xend.server import blkif, netif, tpmif, pciif, iopif, irqif, 
>vfbif, vscsiif
>-from xen.xend.server.BlktapController import BlktapController
>+from xen.xend.server.BlktapController import BlktapController, 
>Blktap2Controller
> from xen.xend.server.ConsoleController import ConsoleController
> 
> 
>@@ -42,6 +42,7 @@ class XendDevices:
>         ''ioports'': iopif.IOPortsController,
>         ''irq'': irqif.IRQController,
>         ''tap'': BlktapController,
>+        ''tap2'': Blktap2Controller,
>         ''vfb'': vfbif.VfbifController,
>         ''vkbd'': vfbif.VkbdifController,
>         ''console'': ConsoleController,
>diff -r 7397608bce87 -r 6295171d4ccc tools/python/xen/xend/XendDomainInfo.py
>--- a/tools/python/xen/xend/XendDomainInfo.py	Mon Jun 29 15:50:32 2009 +0100
>+++ b/tools/python/xen/xend/XendDomainInfo.py	Tue Jun 30 17:44:41 2009 -0700
>@@ -538,12 +538,11 @@ class XendDomainInfo:
>         @raise XendError: Failed pausing a domain
>         """
>         try:
>-            bepath="/local/domain/0/backend/"
>             if(self.domid):
>-                
>-                dev =  xstransact.List(bepath + ''vbd'' +
"/%d" % (self.
>domid,))
>+                # get all blktap2 devices
>+                dev =  xstransact.List(self.vmpath +
''device/tap2'')
>                 for x in dev:
>-                    path =
self.getDeviceController(''vbd'').readBackend(x,
>''params'')
>+                    path =
self.getDeviceController(''tap2'').readBackend(x,
> ''params'')
>                     if path and
path.startswith(''/dev/xen/blktap-2''):
>                         #Figure out the sysfs path.
>                         pattern =
re.compile(''/dev/xen/blktap-2/tapdev(\d+
>)$'')
>@@ -569,11 +568,10 @@ class XendDomainInfo:
>         @raise XendError: Failed unpausing a domain
>         """
>         try:
>-            bepath="/local/domain/0/backend/"
>             if(self.domid):
>-                dev =  xstransact.List(bepath + "vbd" +
"/%d" % (self.
>domid,))
>+                dev =  xstransact.List(self.vmpath +
''device/tap2'')
>                 for x in dev:
>-                    path =
self.getDeviceController(''vbd'').readBackend(x,
>''params'')
>+                    path =
self.getDeviceController(''tap2'').readBackend(x,
> ''params'')
>                     if path and
path.startswith(''/dev/xen/blktap-2''):
>                         #Figure out the sysfs path.
>                         pattern =
re.compile(''/dev/xen/blktap-2/tapdev(\d+
>)$'')
>@@ -812,6 +810,11 @@ class XendDomainInfo:
>             try:
>                 dev_config_dict[''devid''] = devid = \
>                     self._createDevice(dev_type, dev_config_dict)
>+                if dev_type == ''tap2'':
>+                    # createDevice may create a blktap1 device if blktap2 
>is not
>+                    # installed or if the blktap driver is not supported in
>+                    # blktap1
>+                    dev_type = self.getBlockDeviceClass(devid)
>                 self._waitForDevice(dev_type, devid)
>             except VmError, ex:
>                 del self.info[''devices''][dev_uuid]
>@@ -821,7 +824,7 @@ class XendDomainInfo:
>                 elif dev_type == ''vscsi'':
>                     for dev in dev_config_dict[''devs'']:
>                        
XendAPIStore.deregister(dev[''uuid''],
''DSCSI'')
>-                elif dev_type == ''tap'':
>+                elif dev_type == ''tap'' or dev_type ==
''tap2'':
>                    
self.info[''vbd_refs''].remove(dev_uuid)
>                 else:
>                     self.info[''%s_refs'' %
dev_type].remove(dev_uuid)
>@@ -1200,9 +1203,10 @@ class XendDomainInfo:
>         if self.domid is not None:
>             
>             #new blktap implementation may need a sysfs write after 
>everything is torn down.
>-            dev = self.getDeviceController(deviceClass).
>convertToDeviceNumber(devid)
>-            path = self.getDeviceController(deviceClass).readBackend(dev, 
>''params'')                
>-            if path and
path.startswith(''/dev/xen/blktap-2''):
>+
>+            if deviceClass == ''tap2'':
>+                dev = self.getDeviceController(deviceClass).
>convertToDeviceNumber(devid)
>+                path = self.getDeviceController(deviceClass).readBackend(
>dev, ''params'')
>                 frontpath = self.getDeviceController(deviceClass).
>frontendPath(dev)
>                 backpath = xstransact.Read(frontpath, "backend")
>                 thread.start_new_thread(self.getDeviceController(
>deviceClass).finishDeviceCleanup, (backpath, path))
>@@ -1238,7 +1242,7 @@ class XendDomainInfo:
>                     dev_info = self._getDeviceInfo_vif(mac)
>                 else:
>                     _, dev_info = sxprs[dev]
>-            else:  # ''vbd'' or ''tap''
>+            else:  # ''vbd'' or ''tap'' or
''tap2''
>                 dev_info = self._getDeviceInfo_vbd(dev)
>                 # To remove the UUID of the device from refs,
>                 # deviceClass must be always ''vbd''.
>@@ -1267,7 +1271,7 @@ class XendDomainInfo:
>             sxprs = []
>             dev_num = 0
>             for dev_type, dev_info in self.info.all_devices_sxpr():
>-                if (deviceClass == ''vbd'' and dev_type not
in [''vbd'', ''tap''
>]) or \
>+                if (deviceClass == ''vbd'' and dev_type not
in [''vbd'', ''tap''
>, ''tap2'']) or \
>                    (deviceClass != ''vbd'' and dev_type !=
deviceClass):
>                     continue
> 
>@@ -1295,12 +1299,27 @@ class XendDomainInfo:
>             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]
>+        # if the domain is running we can get the device class from 
>xenstore.
>+        # This is more accurate, as blktap1 devices show up as blktap2 
>devices
>+        # in the config.
>+        if self._stateGet() in (DOM_STATE_RUNNING, DOM_STATE_PAUSED, 
>DOM_STATE_CRASHED):
>+            # All block devices have a vbd frontend, so we know the 
>frontend path
>+            dev =
self.getDeviceController(''vbd'').convertToDeviceNumber(
>devid)
>+            frontendPath = "%s/device/vbd/%s" % (self.dompath,
dev)
>+            for devclass in XendDevices.valid_devices():
>+                for dev in xstransact.List("%s/device/%s" %
(self.vmpath,
>devclass)):
>+                    devFrontendPath =
xstransact.Read("%s/device/%s/%s/
>frontend" % (self.vmpath, devclass, dev))
>+                    if frontendPath == devFrontendPath:
>+                        return devclass
>+            
>+        else: # the domain is not active so we must get the device class
>+              # from the config
>+            # 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 _getDeviceInfo_vif(self, mac):
>         for dev_type, dev_info in self.info.all_devices_sxpr():
>@@ -1311,7 +1330,7 @@ class XendDomainInfo:
> 
>     def _getDeviceInfo_vbd(self, devid):
>         for dev_type, dev_info in self.info.all_devices_sxpr():
>-            if dev_type != ''vbd'' and dev_type !=
''tap'':
>+            if dev_type != ''vbd'' and dev_type !=
''tap'' and dev_type !=
>''tap2'':
>                 continue
>             dev = sxp.child_value(dev_info, ''dev'')
>             dev = dev.split('':'')[0]
>@@ -2256,26 +2275,19 @@ class XendDomainInfo:
>             log.debug("No device model")
> 
>         log.debug("Releasing devices")
>-        t = xstransact("%s/device" % self.dompath)
>+        t = xstransact("%s/device" % self.vmpath)
>         try:
>             for devclass in XendDevices.valid_devices():
>                 for dev in t.list(devclass):
>                     try:
>-                        true_devclass = devclass
>-                        if devclass == ''vbd'':
>-                            # In the case of "vbd", the true
device class
>-                            # may possibly be "tap". Just in
case, verify
>-                            # device class.
>-                            devid = dev.split(''/'')[-1]
>-                            true_devclass = self.getBlockDeviceClass(devid)
>                         log.debug("Removing %s", dev);
>-                        self.destroyDevice(true_devclass, dev, False);
>+                        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''],
>-                                      true_devclass, dev)
>+                                      devclass, dev)
>         finally:
>             t.abort()
> 
>@@ -2948,7 +2960,7 @@ class XendDomainInfo:
> 
>             fn = blkdev_uname_to_file(disk)
>             taptype = blkdev_uname_to_taptype(disk)
>-            mounted = devtype == ''tap'' and taptype !=
''aio'' and taptype !> ''sync'' and not
os.stat(fn).st_rdev
>+            mounted = devtype in [''tap'',
''tap2''] and taptype != ''aio'' and
>taptype != ''sync'' and not os.stat(fn).st_rdev
>             if mounted:
>                 # This is a file, not a device.  pygrub can cope with a
>                 # file if it''s raw, but if it''s QCOW or
other such formats
>@@ -3052,7 +3064,8 @@ class XendDomainInfo:
>             diff = time.time() - start
>             vbds =
self.getDeviceController(''vbd'').deviceIDs()
>             taps =
self.getDeviceController(''tap'').deviceIDs()
>-            for i in vbds + taps:
>+            tap2s =
self.getDeviceController(''tap2'').deviceIDs()
>+            for i in vbds + taps + tap2s:
>                 test = 1
>                 log.info("Dev %s still active, looping...", i)
>                 time.sleep(0.1)
>@@ -3635,7 +3648,7 @@ class XendDomainInfo:
>         """
>         xenapi_vbd[''image''] = vdi_image_path
>         if vdi_image_path.startswith(''tap''):
>-            dev_uuid = self.info.device_add(''tap'',
cfg_xenapi = xenapi_vbd)
>+            dev_uuid = self.info.device_add(''tap2'',
cfg_xenapi = xenapi_vbd)
>         else:
>             dev_uuid = self.info.device_add(''vbd'',
cfg_xenapi = xenapi_vbd)
>             
>@@ -3647,7 +3660,7 @@ class XendDomainInfo:
>             _, config = self.info[''devices''][dev_uuid]
>             
>             if vdi_image_path.startswith(''tap''):
>-                dev_control =
self.getDeviceController(''tap'')
>+                dev_control =
self.getDeviceController(''tap2'')
>             else:
>                 dev_control =
self.getDeviceController(''vbd'')
> 
>diff -r 7397608bce87 -r 6295171d4ccc tools/python/xen/xend/server/
>BlktapController.py
>--- a/tools/python/xen/xend/server/BlktapController.py	Mon Jun 29 15:50:32 
>2009 +0100
>+++ b/tools/python/xen/xend/server/BlktapController.py	Tue Jun 30 17:44:41 
>2009 -0700
>@@ -24,7 +24,7 @@ blktap_disk_types = [
>     ''ioemu'',
>     ''tapdisk'',
>     ]
>- 
>+
> def doexec(args, inputtext=None):
>     """Execute a subprocess, then return its return code,
stdout and
>stderr"""
>     proc = popen2.Popen3(args, True)
>@@ -49,8 +49,7 @@ def parseDeviceString(device):
> 
>     return minor, device, control
> 
>-
>-
>+# blktap1 device controller
> class BlktapController(BlkifController):
>     def __init__(self, vm):
>         BlkifController.__init__(self, vm)
>@@ -59,11 +58,6 @@ class BlktapController(BlkifController):
>         """@see DevController#frontendRoot"""
>         
>         return "%s/device/vbd" % self.vm.getDomainPath()
>-
>-    def devicePath(self, devid):
>-        """@see DevController#devicePath"""
>-        
>-        return "%s/device/vbd/%s" % (self.vm.vmpath, devid)
> 
>     def getDeviceDetails(self, config):
>         (devid, back, front) = BlkifController.getDeviceDetails(self, 
>config)
>@@ -123,6 +117,20 @@ class BlktapController(BlkifController):
> 
>         return (devid, back, front)
> 
>+class Blktap2Controller(BlktapController):
>+    def __init__(self, vm):
>+        BlktapController.__init__(self, vm)
>+
>+    def backendPath(self, backdom, devid):
>+        if self.deviceClass == ''tap2'':
>+            deviceClass = ''vbd''
>+        else:
>+            deviceClass = ''tap''
>+        return "%s/backend/%s/%s/%d" % (backdom.getDomainPath(),
>+                                        deviceClass,
>+                                        self.vm.getDomid(), devid)
>+
>+
>     def createDevice(self, config):
> 
>         uname = config.get(''required_uname'',
'''')
>@@ -140,12 +148,16 @@ class BlktapController(BlkifController):
>         stderr.close();
>         if( out.find("blktap2") >= 0 ):
>             blktap2_installed=1;
>-           
>+
>         if typ in (''tap''):
>-            if subtyp in (''tapdisk''):
>     
>+            if subtyp in (''tapdisk''):
>                 if params in (''ioemu'',
''qcow2'', ''vmdk'', ''sync'')
or not
>blktap2_installed:
>-                    log.warn(''WARNING: using deprecated blktap
module'');
>-                    return BlkifController.createDevice(self, config);
>+                    # pass this device off to BlktapController
>+                    log.warn(''WARNING: using deprecated blktap
module'')
>+                    self.deviceClass = ''tap''
>+                    devid = BlktapController.createDevice(self, config)
>+                    self.deviceClass = ''tap2''
>+                    return devid
> 
>         cmd = [ TAPDISK_BINARY, ''-n'',
''%s:%s'' % (params, file) ]
>         (rc,stdout,stderr) = doexec(cmd)
>@@ -165,7 +177,32 @@ class BlktapController(BlkifController):
>         #device is configured.  Then continue to create the device
>         config.update({''uname'' : ''phy:''
+ device.rstrip()})
> 
>-        self.deviceClass=''vbd''
>         devid = BlkifController.createDevice(self, config)
>-        self.deviceClass=''tap''
>         return devid
>+
>+    # The new blocktap implementation requires a sysfs signal to close
>+    # out disks.  This function is called from a thread when the
>+    # domain is detached from the disk.
>+    def finishDeviceCleanup(self, backpath, path):
>+        """Perform any device specific cleanup
>+
>+        @backpath backend xenstore path.
>+        @path frontend device path
>+
>+        """
>+
>+        #Figure out what we''re going to wait on.
>+        self.waitForBackend_destroy(backpath)
>+
>+        #Figure out the sysfs path.
>+        pattern =
re.compile(''/dev/xen/blktap-2/tapdev(\d+)$'')
>+        ctrlid = pattern.search(path)
>+        ctrl = ''/sys/class/blktap2/blktap'' +
ctrlid.group(1)
>+
>+        #Close out the disk
>+        f = open(ctrl + ''/remove'', ''w'')
>+        f.write(''remove'');
>+        f.close()
>+
>+        return
>+
>diff -r 7397608bce87 -r 6295171d4ccc tools/python/xen/xend/server/
>DevController.py
>--- a/tools/python/xen/xend/server/DevController.py	Mon Jun 29 15:50:32 
>2009 +0100
>+++ b/tools/python/xen/xend/server/DevController.py	Tue Jun 30 17:44:41 
>2009 -0700
>@@ -238,34 +238,6 @@ class DevController:
>             # xstransact.Remove(self.devicePath()) ?? Below is the same ?
>             self.vm._removeVm("device/%s/%d" % (self.deviceClass,
dev))
> 
>-    # The new blocktap implementation requires a sysfs signal to close
>-    # out disks.  This function is called from a thread when the
>-    # domain is detached from the disk.
>-    def finishDeviceCleanup(self, backpath, path):
>-        """Perform any device specific cleanup
>-
>-        @backpath backend xenstore path.
>-        @path frontend device path
>-
>-        """
>-        
>-        if path and path.startswith(''/dev/xen/blktap-2''):
>-            
>-            #Figure out what we''re going to wait on.
>-            self.waitForBackend_destroy(backpath)            
>-
>-            #Figure out the sysfs path.
>-            pattern =
re.compile(''/dev/xen/blktap-2/tapdev(\d+)$'')
>-            ctrlid = pattern.search(path)
>-            ctrl = ''/sys/class/blktap2/blktap'' +
ctrlid.group(1)
>-            
>-            #Close out the disk
>-            f = open(ctrl + ''/remove'',
''w'')
>-            f.write(''remove'');
>-            f.close()
>-
>-        return
>-
>     def configurations(self, transaction = None):
>         return map(lambda x: self.configuration(x, transaction), self.
>deviceIDs(transaction))
> 
>@@ -575,7 +547,6 @@ class DevController:
>             
>         backpath = self.readVm(devid, "backend")
> 
>-
>         if backpath:
>             statusPath = backpath + ''/'' +
HOTPLUG_STATUS_NODE
>             ev = Event()
>diff -r 7397608bce87 -r 6295171d4ccc tools/python/xen/xm/create.py
>--- a/tools/python/xen/xm/create.py	Mon Jun 29 15:50:32 2009 +0100
>+++ b/tools/python/xen/xm/create.py	Tue Jun 30 17:44:41 2009 -0700
>@@ -718,7 +718,7 @@ def configure_disks(config_devs, vals):
>     """
>     for (uname, dev, mode, backend, protocol) in vals.disk:
>         if uname.startswith(''tap:''):
>-            cls = ''tap''
>+            cls = ''tap2''
>         else:
>             cls = ''vbd''
> 
>diff -r 7397608bce87 -r 6295171d4ccc tools/python/xen/xm/main.py
>--- a/tools/python/xen/xm/main.py	Mon Jun 29 15:50:32 2009 +0100
>+++ b/tools/python/xen/xm/main.py	Tue Jun 30 17:44:41 2009 -0700
>@@ -2371,7 +2371,7 @@ def parse_block_configuration(args):
>     dom = args[0]
> 
>     if args[1].startswith(''tap:''):
>-        cls = ''tap''
>+        cls = ''tap2''
>     else:
>         cls = ''vbd''
> 
>@@ -2706,7 +2706,9 @@ def xm_block_detach(args):
>         dom = args[0]
>         dev = args[1]
>         dc = server.xend.domain.getBlockDeviceClass(dom, dev)
>-        if dc == "tap":
>+        if dc == "tap2":
>+            detach(args, ''tap2'')
>+        elif dc == "tap":
>             detach(args, ''tap'')
>         else:
>             detach(args, ''vbd'')
>
>_______________________________________________
>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

Hi Ryan,

I thought... 
Are blktap1 and blktap2 used as follows? 
And the head of the string of disk parameters for blktap devices is 
''tap:'' only, isn''t it?
Is my understanding right?

     |        blktap2        |
     +-----------+-----------+
     | Installed | Not       |
     |           | Installed |
-----+-----------+-----------+
aio  |blktap2    |blktap1    |
-----+-----------+-----------+
sync |blktap1    |blktap1    |
-----+-----------+-----------+
vmdk |blktap1    |blktap1    |
-----+-----------+-----------+
ram  |blktap2    |blktap1    |
-----+-----------+-----------+
qcow |blktap2    |blktap1    |
-----+-----------+-----------+
qcow2|blktap1    |blktap1    |
-----+-----------+-----------+
vhd  |blktap2    |blktap1    |
-----+-----------+-----------+
ioemu|blktap1    |blktap1    |
-----+-----------+-----------+

Best regards,
 Kan

Wed, 01 Jul 2009 13:33:35 +0900, Masaki Kanno wrote:
>Hi Ryan,
>
>>blktap2 devices must be handled differently than blktap2 devices.
blktap2
>                                                  ^^^^^^^
>                                                  blktap1?  typo?
>
>
>Basically questions:
>
>How should I define a disk parameter into VM configuration files to use 
>blktap1 devices?
>
>  e.g. disk = [ ''tap:aio:root-vm1.img,xvda,w'' ]
>
>
>How should I define a disk parameter into VM configuration files to use 
>blktap2 devices?
>
>  e.g. disk = [ ''tap2:aio:root-vm2.img,xvda,w'' ]
>
>
>If you distinguish the device class of blktap2 devices from the device 
>class of blktap1 devices, should you modify it as follows?
>I think that the device class of blktap1 devices should be
''tap'' if we
>use blktap1 devices as before.
>
>diff -r 7397608bce87 -r 6295171d4ccc tools/python/xen/xm/create.py
>--- a/tools/python/xen/xm/create.py	Mon Jun 29 15:50:32 2009 +0100
>+++ b/tools/python/xen/xm/create.py	Tue Jun 30 17:44:41 2009 -0700
>@@ -718,7 +718,7 @@ def configure_disks(config_devs, vals):
>     """
>     for (uname, dev, mode, backend, protocol) in vals.disk:
>         if uname.startswith(''tap:''):
>             cls = ''tap''
>+        elif uname.startswith(''tap2:''):
>+            cls = ''tap2''
>         else:
>             cls = ''vbd''
> 
>
>Best regards,
> Kan
>
>Tue, 30 Jun 2009 17:49:44 -0700, "Ryan O''Connor" wrote:
>
>># HG changeset patch
>># User Ryan O''Connor <rjo@cs.ubc.ca>
>># Date 1246409081 25200
>># Node ID 6295171d4ccc60f80f0411217d9b1c7d8543c804
>># Parent  7397608bce877b151ee507413a8e228092947ee4
>>blktap2: add blktap2 device class and device controller
>>
>>blktap2 devices must be handled differently than blktap2 devices.
blktap2
>>devices require a sysfs write to close the underlying device, as well as
>>extra
>>sysfs writes when the domU is paused/unpaused. The differences between 
>>blktap1
>>and blktap2 are great enough to warrant the creation of a new device
class,
>>''tap2'', and device controller for blktap2 devices.
>>
>>  * add a new device controller (Blktap2Controller) and device class
(tap2)
>> for
>>    blktap2 devices
>>  * move blktap2 specific code from DevController to Blktap2Controller
>>  * if possible, check xenstore to determine block device class
>>  * use vmpath (/vm/<uuid>/) when releasing devices
>>  * modify linux hotplug cleanup script to handle blktap2 device removal
>>
>>Signed-off-by: Ryan O''Connor <rjo@cs.ubc.ca>
>>
>>diff -r 7397608bce87 -r 6295171d4ccc
tools/hotplug/Linux/xen-hotplug-cleanup
>>--- a/tools/hotplug/Linux/xen-hotplug-cleanup	Mon Jun 29 15:50:32 2009
+0100
>>+++ b/tools/hotplug/Linux/xen-hotplug-cleanup	Tue Jun 30 17:44:41 2009
-0700
>>@@ -18,6 +18,13 @@ vm=$(xenstore_read_default "/local/domai
>> # construct /vm/UUID/device/DEVCLASS/DEVID
>> if [ "$vm" != "" ]; then
>>   vm_dev="$vm/device/${path_array[1]}/${path_array[3]}"
>>+
>>+  # if the vm path does not exist and the device class is
''vbd'' then we
>>may have
>>+  # a tap2 device
>>+  if [ ! $(xenstore-read "vm_dev" 2>/dev/null) ] \
>>+       && [ "${path_array[1]}" = "vbd" ];
then
>>+    vm_dev="$vm/device/tap2/${path_array[3]}"
>>+  fi
>> else
>>   vm_dev>> fi
>>diff -r 7397608bce87 -r 6295171d4ccc tools/python/xen/xend/XendConfig.py
>>--- a/tools/python/xen/xend/XendConfig.py	Mon Jun 29 15:50:32 2009 +0100
>>+++ b/tools/python/xen/xend/XendConfig.py	Tue Jun 30 17:44:41 2009 -0700
>>@@ -1114,7 +1114,7 @@ class XendConfig(dict):
>>                             controller =
domain.getDeviceController(cls)
>>                             configs = controller.configurations(txn)
>>                             for config in configs:
>>-                                if sxp.name(config) in
(''vbd'', ''tap''):
>>+                                if sxp.name(config) in
(''vbd'', ''tap'',
>>''tap2''):
>>                                     dev_uuid = sxp.child_value(config, 
>>''uuid'')
>>                                     dev_type, dev_cfg =
self[''devices''][
>>dev_uuid]
>>                                     if sxp.child_value(config,
''bootable'',
>> None) is None:
>>@@ -1179,7 +1179,7 @@ class XendConfig(dict):
>>     def device_duplicate_check(self, dev_type, dev_info,
defined_config,
>>config):
>>         defined_devices_sxpr = self.all_devices_sxpr(target = 
>>defined_config)
>>         
>>-        if dev_type == ''vbd'' or dev_type ==
''tap'':
>>+        if dev_type == ''vbd'' or dev_type ==
''tap'' or dev_type == ''tap2'':
>>             dev_uname = dev_info.get(''uname'')
>>             blkdev_name = dev_info.get(''dev'')
>>             devid = self._blkdev_name_to_number(blkdev_name)
>>@@ -1187,7 +1187,7 @@ class XendConfig(dict):
>>                 return
>>             
>>             for o_dev_type, o_dev_info in defined_devices_sxpr:
>>-                if o_dev_type == ''vbd'' or o_dev_type
== ''tap'':
>>+                if o_dev_type == ''vbd'' or o_dev_type
== ''tap'' or
>>o_dev_type == ''tap2'':
>>                     blkdev_file = blkdev_uname_to_file(dev_uname)
>>                     o_dev_uname = sxp.child_value(o_dev_info,
''uname'')
>>                     if o_dev_uname != None:
>>@@ -1369,7 +1369,7 @@ class XendConfig(dict):
>>                 else:
>>                     dev_info[''driver''] =
''paravirtualised''
>> 
>>-            if dev_type == ''tap'':
>>+            if dev_type == ''tap'' or dev_type ==
''tap2'':
>>                 if
dev_info[''uname''].split('':'')[1] not in
>> blktap_disk_types:
>>                     raise XendConfigError("tap:%s not a valid disk
type" %
>>                                    
dev_info[''uname''].split('':'')[1])
>>@@ -1406,7 +1406,7 @@ class XendConfig(dict):
>>                         # Compat hack -- mark first disk bootable
>>                         dev_info[''bootable''] =
int(not target[param])
>>                     target[param].append(dev_uuid)
>>-            elif dev_type == ''tap'':
>>+            elif dev_type == ''tap'' or dev_type ==
''tap2'':
>>                 if ''vbd_refs'' not in target:
>>                     target[''vbd_refs''] = []
>>                 if dev_uuid not in
target[''vbd_refs'']:
>>@@ -1504,7 +1504,7 @@ class XendConfig(dict):
>>                 target[''devices''][dev_uuid] =
(dev_type, dev_info)
>>                 target[''vif_refs''].append(dev_uuid)
>>             
>>-            elif dev_type in (''vbd'',
''tap''):
>>+            elif dev_type in (''vbd'',
''tap'', ''tap2''):
>>                 dev_info[''type''] =
cfg_xenapi.get(''type'', ''Disk'')
>>                 if dev_info[''type''] ==
''CD'':
>>                     old_vbd_type = ''cdrom''
>>diff -r 7397608bce87 -r 6295171d4ccc
tools/python/xen/xend/XendDevices.py
>>--- a/tools/python/xen/xend/XendDevices.py	Mon Jun 29 15:50:32 2009
+0100
>>+++ b/tools/python/xen/xend/XendDevices.py	Tue Jun 30 17:44:41 2009
-0700
>>@@ -20,7 +20,7 @@
>> #
>> 
>> from xen.xend.server import blkif, netif, tpmif, pciif, iopif, irqif, 
>>vfbif, vscsiif
>>-from xen.xend.server.BlktapController import BlktapController
>>+from xen.xend.server.BlktapController import BlktapController, 
>>Blktap2Controller
>> from xen.xend.server.ConsoleController import ConsoleController
>> 
>> 
>>@@ -42,6 +42,7 @@ class XendDevices:
>>         ''ioports'': iopif.IOPortsController,
>>         ''irq'': irqif.IRQController,
>>         ''tap'': BlktapController,
>>+        ''tap2'': Blktap2Controller,
>>         ''vfb'': vfbif.VfbifController,
>>         ''vkbd'': vfbif.VkbdifController,
>>         ''console'': ConsoleController,
>>diff -r 7397608bce87 -r 6295171d4ccc
tools/python/xen/xend/XendDomainInfo.py
>>--- a/tools/python/xen/xend/XendDomainInfo.py	Mon Jun 29 15:50:32 2009
+0100
>>+++ b/tools/python/xen/xend/XendDomainInfo.py	Tue Jun 30 17:44:41 2009
-0700
>>@@ -538,12 +538,11 @@ class XendDomainInfo:
>>         @raise XendError: Failed pausing a domain
>>         """
>>         try:
>>-            bepath="/local/domain/0/backend/"
>>             if(self.domid):
>>-                
>>-                dev =  xstransact.List(bepath + ''vbd''
+ "/%d" % (self.
>>domid,))
>>+                # get all blktap2 devices
>>+                dev =  xstransact.List(self.vmpath +
''device/tap2'')
>>                 for x in dev:
>>-                    path =
self.getDeviceController(''vbd'').readBackend(x,
>>''params'')
>>+                    path =
self.getDeviceController(''tap2'').readBackend(x,
>> ''params'')
>>                     if path and
path.startswith(''/dev/xen/blktap-2''):
>>                         #Figure out the sysfs path.
>>                         pattern =
re.compile(''/dev/xen/blktap-2/tapdev(\d+
>>)$'')
>>@@ -569,11 +568,10 @@ class XendDomainInfo:
>>         @raise XendError: Failed unpausing a domain
>>         """
>>         try:
>>-            bepath="/local/domain/0/backend/"
>>             if(self.domid):
>>-                dev =  xstransact.List(bepath + "vbd" +
"/%d" % (self.
>>domid,))
>>+                dev =  xstransact.List(self.vmpath +
''device/tap2'')
>>                 for x in dev:
>>-                    path =
self.getDeviceController(''vbd'').readBackend(x,
>>''params'')
>>+                    path =
self.getDeviceController(''tap2'').readBackend(x,
>> ''params'')
>>                     if path and
path.startswith(''/dev/xen/blktap-2''):
>>                         #Figure out the sysfs path.
>>                         pattern =
re.compile(''/dev/xen/blktap-2/tapdev(\d+
>>)$'')
>>@@ -812,6 +810,11 @@ class XendDomainInfo:
>>             try:
>>                 dev_config_dict[''devid''] = devid = \
>>                     self._createDevice(dev_type, dev_config_dict)
>>+                if dev_type == ''tap2'':
>>+                    # createDevice may create a blktap1 device if
blktap2
>>is not
>>+                    # installed or if the blktap driver is not
supported in
>>+                    # blktap1
>>+                    dev_type = self.getBlockDeviceClass(devid)
>>                 self._waitForDevice(dev_type, devid)
>>             except VmError, ex:
>>                 del self.info[''devices''][dev_uuid]
>>@@ -821,7 +824,7 @@ class XendDomainInfo:
>>                 elif dev_type == ''vscsi'':
>>                     for dev in
dev_config_dict[''devs'']:
>>                        
XendAPIStore.deregister(dev[''uuid''],
''DSCSI'')
>>-                elif dev_type == ''tap'':
>>+                elif dev_type == ''tap'' or dev_type ==
''tap2'':
>>                    
self.info[''vbd_refs''].remove(dev_uuid)
>>                 else:
>>                     self.info[''%s_refs'' %
dev_type].remove(dev_uuid)
>>@@ -1200,9 +1203,10 @@ class XendDomainInfo:
>>         if self.domid is not None:
>>             
>>             #new blktap implementation may need a sysfs write after 
>>everything is torn down.
>>-            dev = self.getDeviceController(deviceClass).
>>convertToDeviceNumber(devid)
>>-            path =
self.getDeviceController(deviceClass).readBackend(dev,
>>''params'')                
>>-            if path and
path.startswith(''/dev/xen/blktap-2''):
>>+
>>+            if deviceClass == ''tap2'':
>>+                dev = self.getDeviceController(deviceClass).
>>convertToDeviceNumber(devid)
>>+                path =
self.getDeviceController(deviceClass).readBackend(
>>dev, ''params'')
>>                 frontpath = self.getDeviceController(deviceClass).
>>frontendPath(dev)
>>                 backpath = xstransact.Read(frontpath,
"backend")
>>                 thread.start_new_thread(self.getDeviceController(
>>deviceClass).finishDeviceCleanup, (backpath, path))
>>@@ -1238,7 +1242,7 @@ class XendDomainInfo:
>>                     dev_info = self._getDeviceInfo_vif(mac)
>>                 else:
>>                     _, dev_info = sxprs[dev]
>>-            else:  # ''vbd'' or ''tap''
>>+            else:  # ''vbd'' or ''tap''
or ''tap2''
>>                 dev_info = self._getDeviceInfo_vbd(dev)
>>                 # To remove the UUID of the device from refs,
>>                 # deviceClass must be always ''vbd''.
>>@@ -1267,7 +1271,7 @@ class XendDomainInfo:
>>             sxprs = []
>>             dev_num = 0
>>             for dev_type, dev_info in self.info.all_devices_sxpr():
>>-                if (deviceClass == ''vbd'' and dev_type
not in [''vbd'', ''tap''
>>]) or \
>>+                if (deviceClass == ''vbd'' and dev_type
not in [''vbd'', ''tap''
>>, ''tap2'']) or \
>>                    (deviceClass != ''vbd'' and dev_type
!= deviceClass):
>>                     continue
>> 
>>@@ -1295,12 +1299,27 @@ class XendDomainInfo:
>>             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]
>>+        # if the domain is running we can get the device class from 
>>xenstore.
>>+        # This is more accurate, as blktap1 devices show up as blktap2 
>>devices
>>+        # in the config.
>>+        if self._stateGet() in (DOM_STATE_RUNNING, DOM_STATE_PAUSED, 
>>DOM_STATE_CRASHED):
>>+            # All block devices have a vbd frontend, so we know the 
>>frontend path
>>+            dev =
self.getDeviceController(''vbd'').convertToDeviceNumber(
>>devid)
>>+            frontendPath = "%s/device/vbd/%s" %
(self.dompath, dev)
>>+            for devclass in XendDevices.valid_devices():
>>+                for dev in xstransact.List("%s/device/%s" %
(self.vmpath,
>>devclass)):
>>+                    devFrontendPath =
xstransact.Read("%s/device/%s/%s/
>>frontend" % (self.vmpath, devclass, dev))
>>+                    if frontendPath == devFrontendPath:
>>+                        return devclass
>>+            
>>+        else: # the domain is not active so we must get the device
class
>>+              # from the config
>>+            # 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 _getDeviceInfo_vif(self, mac):
>>         for dev_type, dev_info in self.info.all_devices_sxpr():
>>@@ -1311,7 +1330,7 @@ class XendDomainInfo:
>> 
>>     def _getDeviceInfo_vbd(self, devid):
>>         for dev_type, dev_info in self.info.all_devices_sxpr():
>>-            if dev_type != ''vbd'' and dev_type !=
''tap'':
>>+            if dev_type != ''vbd'' and dev_type !=
''tap'' and dev_type !=
>>''tap2'':
>>                 continue
>>             dev = sxp.child_value(dev_info, ''dev'')
>>             dev = dev.split('':'')[0]
>>@@ -2256,26 +2275,19 @@ class XendDomainInfo:
>>             log.debug("No device model")
>> 
>>         log.debug("Releasing devices")
>>-        t = xstransact("%s/device" % self.dompath)
>>+        t = xstransact("%s/device" % self.vmpath)
>>         try:
>>             for devclass in XendDevices.valid_devices():
>>                 for dev in t.list(devclass):
>>                     try:
>>-                        true_devclass = devclass
>>-                        if devclass == ''vbd'':
>>-                            # In the case of "vbd", the true
device class
>>-                            # may possibly be "tap". Just in
case, verify
>>-                            # device class.
>>-                            devid =
dev.split(''/'')[-1]
>>-                            true_devclass =
self.getBlockDeviceClass(devid)
>>                         log.debug("Removing %s", dev);
>>-                        self.destroyDevice(true_devclass, dev, False);
>>+                        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''],
>>-                                      true_devclass, dev)
>>+                                      devclass, dev)
>>         finally:
>>             t.abort()
>> 
>>@@ -2948,7 +2960,7 @@ class XendDomainInfo:
>> 
>>             fn = blkdev_uname_to_file(disk)
>>             taptype = blkdev_uname_to_taptype(disk)
>>-            mounted = devtype == ''tap'' and taptype !=
''aio'' and taptype !>> ''sync'' and not
os.stat(fn).st_rdev
>>+            mounted = devtype in [''tap'',
''tap2''] and taptype != ''aio'' and
>>taptype != ''sync'' and not os.stat(fn).st_rdev
>>             if mounted:
>>                 # This is a file, not a device.  pygrub can cope with a
>>                 # file if it''s raw, but if it''s QCOW
or other such formats
>>@@ -3052,7 +3064,8 @@ class XendDomainInfo:
>>             diff = time.time() - start
>>             vbds =
self.getDeviceController(''vbd'').deviceIDs()
>>             taps =
self.getDeviceController(''tap'').deviceIDs()
>>-            for i in vbds + taps:
>>+            tap2s =
self.getDeviceController(''tap2'').deviceIDs()
>>+            for i in vbds + taps + tap2s:
>>                 test = 1
>>                 log.info("Dev %s still active, looping...",
i)
>>                 time.sleep(0.1)
>>@@ -3635,7 +3648,7 @@ class XendDomainInfo:
>>         """
>>         xenapi_vbd[''image''] = vdi_image_path
>>         if vdi_image_path.startswith(''tap''):
>>-            dev_uuid = self.info.device_add(''tap'',
cfg_xenapi = xenapi_vbd)
>>+            dev_uuid = self.info.device_add(''tap2'',
cfg_xenapi =
>>xenapi_vbd)
>>         else:
>>             dev_uuid = self.info.device_add(''vbd'',
cfg_xenapi = xenapi_vbd)
>>             
>>@@ -3647,7 +3660,7 @@ class XendDomainInfo:
>>             _, config =
self.info[''devices''][dev_uuid]
>>             
>>             if vdi_image_path.startswith(''tap''):
>>-                dev_control =
self.getDeviceController(''tap'')
>>+                dev_control =
self.getDeviceController(''tap2'')
>>             else:
>>                 dev_control =
self.getDeviceController(''vbd'')
>> 
>>diff -r 7397608bce87 -r 6295171d4ccc tools/python/xen/xend/server/
>>BlktapController.py
>>--- a/tools/python/xen/xend/server/BlktapController.py	Mon Jun 29 15:
50:32 
>>2009 +0100
>>+++ b/tools/python/xen/xend/server/BlktapController.py	Tue Jun 30 17:
44:41 
>>2009 -0700
>>@@ -24,7 +24,7 @@ blktap_disk_types = [
>>     ''ioemu'',
>>     ''tapdisk'',
>>     ]
>>- 
>>+
>> def doexec(args, inputtext=None):
>>     """Execute a subprocess, then return its return
code, stdout and
>>stderr"""
>>     proc = popen2.Popen3(args, True)
>>@@ -49,8 +49,7 @@ def parseDeviceString(device):
>> 
>>     return minor, device, control
>> 
>>-
>>-
>>+# blktap1 device controller
>> class BlktapController(BlkifController):
>>     def __init__(self, vm):
>>         BlkifController.__init__(self, vm)
>>@@ -59,11 +58,6 @@ class BlktapController(BlkifController):
>>         """@see
DevController#frontendRoot"""
>>         
>>         return "%s/device/vbd" % self.vm.getDomainPath()
>>-
>>-    def devicePath(self, devid):
>>-        """@see
DevController#devicePath"""
>>-        
>>-        return "%s/device/vbd/%s" % (self.vm.vmpath, devid)
>> 
>>     def getDeviceDetails(self, config):
>>         (devid, back, front) = BlkifController.getDeviceDetails(self, 
>>config)
>>@@ -123,6 +117,20 @@ class BlktapController(BlkifController):
>> 
>>         return (devid, back, front)
>> 
>>+class Blktap2Controller(BlktapController):
>>+    def __init__(self, vm):
>>+        BlktapController.__init__(self, vm)
>>+
>>+    def backendPath(self, backdom, devid):
>>+        if self.deviceClass == ''tap2'':
>>+            deviceClass = ''vbd''
>>+        else:
>>+            deviceClass = ''tap''
>>+        return "%s/backend/%s/%s/%d" %
(backdom.getDomainPath(),
>>+                                        deviceClass,
>>+                                        self.vm.getDomid(), devid)
>>+
>>+
>>     def createDevice(self, config):
>> 
>>         uname = config.get(''required_uname'',
'''')
>>@@ -140,12 +148,16 @@ class BlktapController(BlkifController):
>>         stderr.close();
>>         if( out.find("blktap2") >= 0 ):
>>             blktap2_installed=1;
>>-           
>>+
>>         if typ in (''tap''):
>>-            if subtyp in (''tapdisk''):
>>     
>>+            if subtyp in (''tapdisk''):
>>                 if params in (''ioemu'',
''qcow2'', ''vmdk'', ''sync'')
or not
>>blktap2_installed:
>>-                    log.warn(''WARNING: using deprecated blktap
module'');
>>-                    return BlkifController.createDevice(self, config);
>>+                    # pass this device off to BlktapController
>>+                    log.warn(''WARNING: using deprecated blktap
module'')
>>+                    self.deviceClass = ''tap''
>>+                    devid = BlktapController.createDevice(self, config)
>>+                    self.deviceClass = ''tap2''
>>+                    return devid
>> 
>>         cmd = [ TAPDISK_BINARY, ''-n'',
''%s:%s'' % (params, file) ]
>>         (rc,stdout,stderr) = doexec(cmd)
>>@@ -165,7 +177,32 @@ class BlktapController(BlkifController):
>>         #device is configured.  Then continue to create the device
>>         config.update({''uname'' :
''phy:'' + device.rstrip()})
>> 
>>-        self.deviceClass=''vbd''
>>         devid = BlkifController.createDevice(self, config)
>>-        self.deviceClass=''tap''
>>         return devid
>>+
>>+    # The new blocktap implementation requires a sysfs signal to close
>>+    # out disks.  This function is called from a thread when the
>>+    # domain is detached from the disk.
>>+    def finishDeviceCleanup(self, backpath, path):
>>+        """Perform any device specific cleanup
>>+
>>+        @backpath backend xenstore path.
>>+        @path frontend device path
>>+
>>+        """
>>+
>>+        #Figure out what we''re going to wait on.
>>+        self.waitForBackend_destroy(backpath)
>>+
>>+        #Figure out the sysfs path.
>>+        pattern =
re.compile(''/dev/xen/blktap-2/tapdev(\d+)$'')
>>+        ctrlid = pattern.search(path)
>>+        ctrl = ''/sys/class/blktap2/blktap'' +
ctrlid.group(1)
>>+
>>+        #Close out the disk
>>+        f = open(ctrl + ''/remove'',
''w'')
>>+        f.write(''remove'');
>>+        f.close()
>>+
>>+        return
>>+
>>diff -r 7397608bce87 -r 6295171d4ccc tools/python/xen/xend/server/
>>DevController.py
>>--- a/tools/python/xen/xend/server/DevController.py	Mon Jun 29 15:50:32 
>>2009 +0100
>>+++ b/tools/python/xen/xend/server/DevController.py	Tue Jun 30 17:44:41 
>>2009 -0700
>>@@ -238,34 +238,6 @@ class DevController:
>>             # xstransact.Remove(self.devicePath()) ?? Below is the same
?
>>             self.vm._removeVm("device/%s/%d" %
(self.deviceClass, dev))
>> 
>>-    # The new blocktap implementation requires a sysfs signal to close
>>-    # out disks.  This function is called from a thread when the
>>-    # domain is detached from the disk.
>>-    def finishDeviceCleanup(self, backpath, path):
>>-        """Perform any device specific cleanup
>>-
>>-        @backpath backend xenstore path.
>>-        @path frontend device path
>>-
>>-        """
>>-        
>>-        if path and
path.startswith(''/dev/xen/blktap-2''):
>>-            
>>-            #Figure out what we''re going to wait on.
>>-            self.waitForBackend_destroy(backpath)            
>>-
>>-            #Figure out the sysfs path.
>>-            pattern =
re.compile(''/dev/xen/blktap-2/tapdev(\d+)$'')
>>-            ctrlid = pattern.search(path)
>>-            ctrl = ''/sys/class/blktap2/blktap'' +
ctrlid.group(1)
>>-            
>>-            #Close out the disk
>>-            f = open(ctrl + ''/remove'',
''w'')
>>-            f.write(''remove'');
>>-            f.close()
>>-
>>-        return
>>-
>>     def configurations(self, transaction = None):
>>         return map(lambda x: self.configuration(x, transaction), self.
>>deviceIDs(transaction))
>> 
>>@@ -575,7 +547,6 @@ class DevController:
>>             
>>         backpath = self.readVm(devid, "backend")
>> 
>>-
>>         if backpath:
>>             statusPath = backpath + ''/'' +
HOTPLUG_STATUS_NODE
>>             ev = Event()
>>diff -r 7397608bce87 -r 6295171d4ccc tools/python/xen/xm/create.py
>>--- a/tools/python/xen/xm/create.py	Mon Jun 29 15:50:32 2009 +0100
>>+++ b/tools/python/xen/xm/create.py	Tue Jun 30 17:44:41 2009 -0700
>>@@ -718,7 +718,7 @@ def configure_disks(config_devs, vals):
>>     """
>>     for (uname, dev, mode, backend, protocol) in vals.disk:
>>         if uname.startswith(''tap:''):
>>-            cls = ''tap''
>>+            cls = ''tap2''
>>         else:
>>             cls = ''vbd''
>> 
>>diff -r 7397608bce87 -r 6295171d4ccc tools/python/xen/xm/main.py
>>--- a/tools/python/xen/xm/main.py	Mon Jun 29 15:50:32 2009 +0100
>>+++ b/tools/python/xen/xm/main.py	Tue Jun 30 17:44:41 2009 -0700
>>@@ -2371,7 +2371,7 @@ def parse_block_configuration(args):
>>     dom = args[0]
>> 
>>     if args[1].startswith(''tap:''):
>>-        cls = ''tap''
>>+        cls = ''tap2''
>>     else:
>>         cls = ''vbd''
>> 
>>@@ -2706,7 +2706,9 @@ def xm_block_detach(args):
>>         dom = args[0]
>>         dev = args[1]
>>         dc = server.xend.domain.getBlockDeviceClass(dom, dev)
>>-        if dc == "tap":
>>+        if dc == "tap2":
>>+            detach(args, ''tap2'')
>>+        elif dc == "tap":
>>             detach(args, ''tap'')
>>         else:
>>             detach(args, ''vbd'')
>>
>>_______________________________________________
>>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

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Hi Kan,

On Wed, Jul 01, 2009 at 02:44:55PM +0900, Masaki Kanno
wrote:
> Hi Ryan,
> 
> I thought... 
> Are blktap1 and blktap2 used as follows? 
> And the head of the string of disk parameters for blktap devices is 
> ''tap:'' only, isn''t it?
> Is my understanding right?
> 
>      |        blktap2        |
>      +-----------+-----------+
>      | Installed | Not       |
>      |           | Installed |
> -----+-----------+-----------+
> aio  |blktap2    |blktap1    |
> -----+-----------+-----------+
> sync |blktap1    |blktap1    |
> -----+-----------+-----------+
> vmdk |blktap1    |blktap1    |
> -----+-----------+-----------+
> ram  |blktap2    |blktap1    |
> -----+-----------+-----------+
> qcow |blktap2    |blktap1    |
> -----+-----------+-----------+
> qcow2|blktap1    |blktap1    |
> -----+-----------+-----------+
> vhd  |blktap2    |blktap1    |
> -----+-----------+-----------+
> ioemu|blktap1    |blktap1    |
> -----+-----------+-----------+
Yes, you are correct. Also, note that this is the same way
BlktapController created blktap1 and blktap2 devices before my patch.
> Wed, 01 Jul 2009 13:33:35 +0900, Masaki Kanno wrote:
> 
> >Hi Ryan,
> >
> >>blktap2 devices must be handled differently than blktap2 devices.
blktap2
> >                                                  ^^^^^^^
> >                                                  blktap1?  typo?
Thanks, this is a typo. It should say "blktap1".

Thanks again
Ryan
--
  Ryan O''Connor <rjo@cs.ubc.ca>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Hi Ryan,

I tested your patch by using a PV domain. 
The results are as follows.

Managed domains|Domain status |Commands        |tap:aio |tap:sync
---------------+--------------+----------------+--------+--------
Yes            |Active        |xm block-attach | OK     | OK
               |              +----------------+--------+--------
               |              |xm block-deatch | OK     |Failed-1
               |              +----------------+--------+--------
               |              |xm shutdown     | OK     | OK
               +--------------+----------------+--------+--------
               |Inactive      |xm start        | OK     | OK
               |              +----------------+--------+--------
               |              |xm block-attach | OK     | OK?-2
               |              +--------------- +--------+--------
               |              |xm block-detach | OK     | OK?-2
---------------+--------------+----------------+--------+--------
No             | -            |xm create       | OK     | OK
               |              +----------------+--------+--------
               |              |xm block-attach | OK     | OK
               |              +--------------- +--------+--------
               |              |xm block-detach | OK     | OK
               |              +----------------+--------+--------
               |              |xm shutdown     | OK     | OK
---------------+--------------+----------------+--------+--------

Failed-1:
xm block-detach command succeeded.
But I detected the following error message in xend.log.

[2009-07-01 16:12:30 3618] DEBUG (XendDomainInfo:1190)
XendDomainInfo.destroyDevice: deviceClass = tap, device = 833
[2009-07-01 16:12:30 3618] DEBUG (DevController:180) Waiting for 833 -
destroyDevice.
[2009-07-01 16:12:31 3618] DEBUG (DevController:649) deviceDestroyCallback
/local/domain/0/backend/tap/8/833/hotplug-status.
[2009-07-01 16:12:31 3618] DEBUG (DevController:649) deviceDestroyCallback
/local/domain/0/backend/tap/8/833/hotplug-status.
[2009-07-01 16:12:31 3618] DEBUG (DevController:658) deviceDestroyCallback 6.
[2009-07-01 16:12:31 3618] ERROR (XendConfig:1129) dumping sxp from device
controllers
Traceback (most recent call last):
  File "usr/lib/python2.4/site-packages/xen/xend/XendConfig.py", line
1115, in to_sxp
    configs = controller.configurations(txn)
  File
"usr/lib/python2.4/site-packages/xen/xend/server/DevController.py",
line 242, in configurations
    return map(lambda x: self.configuration(x, transaction),
self.deviceIDs(transaction))
  File
"usr/lib/python2.4/site-packages/xen/xend/server/DevController.py",
line 242, in <lambda>
    return map(lambda x: self.configuration(x, transaction),
self.deviceIDs(transaction))
  File
"usr/lib/python2.4/site-packages/xen/xend/server/DevController.py",
line 249, in configuration
    configDict = self.getDeviceConfiguration(devid, transaction)
  File "usr/lib/python2.4/site-packages/xen/xend/server/blkif.py",
line 160, in getDeviceConfiguration
    \047bootable\047)
  File
"usr/lib/python2.4/site-packages/xen/xend/server/DevController.py",
line 447, in readBackendTxn
    raise VmError("Device %s not connected" % devid)
VmError: Device 833 not connected


OK?-2:
The deivce class is ''tap2''.  Ideally, the device class should
be ''tap''.
Could you confirm information of xm list --long?


Best regards,
 Kan

Tue, 30 Jun 2009 23:04:18 -0700, Ryan O''Connor wrote:
>Hi Kan,
>
>On Wed, Jul 01, 2009 at 02:44:55PM +0900, Masaki Kanno wrote:
>> Hi Ryan,
>> 
>> I thought... 
>> Are blktap1 and blktap2 used as follows? 
>> And the head of the string of disk parameters for blktap devices is 
>> ''tap:'' only, isn''t it?
>> Is my understanding right?
>> 
>>      |        blktap2        |
>>      +-----------+-----------+
>>      | Installed | Not       |
>>      |           | Installed |
>> -----+-----------+-----------+
>> aio  |blktap2    |blktap1    |
>> -----+-----------+-----------+
>> sync |blktap1    |blktap1    |
>> -----+-----------+-----------+
>> vmdk |blktap1    |blktap1    |
>> -----+-----------+-----------+
>> ram  |blktap2    |blktap1    |
>> -----+-----------+-----------+
>> qcow |blktap2    |blktap1    |
>> -----+-----------+-----------+
>> qcow2|blktap1    |blktap1    |
>> -----+-----------+-----------+
>> vhd  |blktap2    |blktap1    |
>> -----+-----------+-----------+
>> ioemu|blktap1    |blktap1    |
>> -----+-----------+-----------+
>
>Yes, you are correct. Also, note that this is the same way
>BlktapController created blktap1 and blktap2 devices before my patch.
>
>> Wed, 01 Jul 2009 13:33:35 +0900, Masaki Kanno wrote:
>> 
>> >Hi Ryan,
>> >
>> >>blktap2 devices must be handled differently than blktap2
devices. blktap2
>> >                                                  ^^^^^^^
>> >                                                  blktap1?  typo?
>
>Thanks, this is a typo. It should say "blktap1".
>
>Thanks again
>Ryan
>--
>  Ryan O''Connor <rjo@cs.ubc.ca>
>
>_______________________________________________
>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

Hi Kan,

On Wed, Jul 01, 2009 at 04:41:10PM +0900, Masaki Kanno
wrote:
> Hi Ryan,
> 
> I tested your patch by using a PV domain. 
> The results are as follows.
> 
> Managed domains|Domain status |Commands        |tap:aio |tap:sync
> ---------------+--------------+----------------+--------+--------
> Yes            |Active        |xm block-attach | OK     | OK
>                |              +----------------+--------+--------
>                |              |xm block-deatch | OK     |Failed-1
>                |              +----------------+--------+--------
>                |              |xm shutdown     | OK     | OK
>                +--------------+----------------+--------+--------
>                |Inactive      |xm start        | OK     | OK
>                |              +----------------+--------+--------
>                |              |xm block-attach | OK     | OK?-2
>                |              +--------------- +--------+--------
>                |              |xm block-detach | OK     | OK?-2
> ---------------+--------------+----------------+--------+--------
> No             | -            |xm create       | OK     | OK
>                |              +----------------+--------+--------
>                |              |xm block-attach | OK     | OK
>                |              +--------------- +--------+--------
>                |              |xm block-detach | OK     | OK
>                |              +----------------+--------+--------
>                |              |xm shutdown     | OK     | OK
> ---------------+--------------+----------------+--------+--------
Thanks for testing my patch!
> 
> Failed-1:
> xm block-detach command succeeded.
> But I detected the following error message in xend.log.
> 
> [2009-07-01 16:12:30 3618] DEBUG (XendDomainInfo:1190)
XendDomainInfo.destroyDevice: deviceClass = tap, device = 833
> [2009-07-01 16:12:30 3618] DEBUG (DevController:180) Waiting for 833 -
destroyDevice.
> [2009-07-01 16:12:31 3618] DEBUG (DevController:649) deviceDestroyCallback
/local/domain/0/backend/tap/8/833/hotplug-status.
> [2009-07-01 16:12:31 3618] DEBUG (DevController:649) deviceDestroyCallback
/local/domain/0/backend/tap/8/833/hotplug-status.
> [2009-07-01 16:12:31 3618] DEBUG (DevController:658) deviceDestroyCallback
6.
> [2009-07-01 16:12:31 3618] ERROR (XendConfig:1129) dumping sxp from device
controllers
> Traceback (most recent call last):
>   File "usr/lib/python2.4/site-packages/xen/xend/XendConfig.py",
line 1115, in to_sxp
>     configs = controller.configurations(txn)
>   File
"usr/lib/python2.4/site-packages/xen/xend/server/DevController.py",
line 242, in configurations
>     return map(lambda x: self.configuration(x, transaction),
self.deviceIDs(transaction))
>   File
"usr/lib/python2.4/site-packages/xen/xend/server/DevController.py",
line 242, in <lambda>
>     return map(lambda x: self.configuration(x, transaction),
self.deviceIDs(transaction))
>   File
"usr/lib/python2.4/site-packages/xen/xend/server/DevController.py",
line 249, in configuration
>     configDict = self.getDeviceConfiguration(devid, transaction)
>   File
"usr/lib/python2.4/site-packages/xen/xend/server/blkif.py", line 160,
in getDeviceConfiguration
>     \047bootable\047)
>   File
"usr/lib/python2.4/site-packages/xen/xend/server/DevController.py",
line 447, in readBackendTxn
>     raise VmError("Device %s not connected" % devid)
> VmError: Device 833 not connected
I have been able to reproduce this, although sometimes the block-detach
works without error. Further, I also get a similar error without my
patch when detaching a vbd device from a running managed domain:

[2009-07-01 12:47:00 17306] DEBUG (XendDomainInfo:1187)
XendDomainInfo.destroyDevice: deviceClass = vbd, device = 51713
[2009-07-01 12:47:00 17306] DEBUG (DevController:180) Waiting for 51713 -
destroyDevice.
[2009-07-01 12:47:00 17306] DEBUG (DevController:678) deviceDestroyCallback
/local/domain/0/backend/vbd/13/51713/hotplug-status.
[2009-07-01 12:47:00 17306] DEBUG (DevController:678) deviceDestroyCallback
/local/domain/0/backend/vbd/13/51713/hotplug-status.
[2009-07-01 12:47:00 17306] DEBUG (DevController:687) deviceDestroyCallback 6.
[2009-07-01 12:47:00 17306] ERROR (XendConfig:1129) dumping sxp from device
controllers
Traceback (most recent call last):
  File "usr/lib/python2.5/site-packages/xen/xend/XendConfig.py", line
1119, in to_sxp
    dev_type, dev_cfg = self[''devices''][dev_uuid]
KeyError: None

This looks to be a race condition between the block device detach and
the managed_domain_save() code. The device is partially removed from
xenstore when to_sxp() builds the device sxprs, so to_sxp() tries to
collect information on a device that is being removed.
> OK?-2:
> The deivce class is ''tap2''.  Ideally, the device class
should be ''tap''.
> Could you confirm information of xm list --long?
Yes, you are correct that the device shows up as ''tap2'' before
the
domain is created. This is because it is not known if the device will be
a blktap1 or blktap2 device until we call Blktap2Controller.createDevice().
However, since the domain is inactive and the devices have not been
created we do not need to know the ''actual'' device class as
there are no
entries in xenstore to remove. All deviceRemove does at that point is
remove the entry from the domain''s config sxpr.

Thanks again,
Ryan
-- 
 Ryan O''Connor <rjo@cs.ubc.ca>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel