[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH] blktap2: add blktap2 device class and devicecontroller
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@xxxxxxxxx> ># 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@xxxxxxxxx> > >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@xxxxxxxxxxxxxxxxxxx >http://lists.xensource.com/xen-devel _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxx http://lists.xensource.com/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |