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@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
--
Ryan O'Connor <rjo@xxxxxxxxx>
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home