From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Daniel P. Berrange" Subject: Re: Re: [Xen-devel] Re: Writing a tool for Shared Persistent Windows Boot Image Date: Fri, 29 Jun 2007 21:16:22 +0100 Message-ID: <20070629201622.GA7204@redhat.com> References: <467ABF50.50209@codemonkey.ws> <13A934B9-F615-4838-8D26-4E33F0BCFF2E@gmail.com> <467AE21F.1020700@codemonkey.ws> <37B43CC2-BED7-4336-9CC4-0CE1C7894458@gmail.com> <467AF0C6.5010101@codemonkey.ws> <20070628182736.GB12711@redhat.com> <20070629144251.GC25518@redhat.com> Reply-To: "Daniel P. Berrange" Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="UugvWAfsgieZRqgk" Return-path: Content-Disposition: inline In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-users-bounces@lists.xensource.com Errors-To: xen-users-bounces@lists.xensource.com To: Andrew Warfield Cc: Anthony Liguori , xen-devel@lists.xensource.com, xen-users@lists.xensource.com, Jim Burnes List-Id: xen-devel@lists.xenproject.org --UugvWAfsgieZRqgk Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Fri, Jun 29, 2007 at 09:18:50AM -0700, Andrew Warfield wrote: > If you can send some more details on the crash we should be able to > sort this out -- it's certainly something that has worked in the past. Ok, so here's the scenario. Traditionally with HVM I have a disk file:/xen/rhel4i386.img,hda,w Having seen the changeset 13827:6524e02edbeb I tried tap:aio:/xen/rhel4i386.img,hda,w And tap:aio:/xen/rhel4i386.img,xvda,w The latter is the preferred, since paravirt drivers should not be hijacking the IDE devices inside the guest. However, the changeset 13827 doesn't seem to support xvd* since QEMU filters out any devices with such a name. With vanilla Xen 3.1.0 qemu goes defunct when starting the guest logging qemu: could not open hard disk image 'aio:/xen/rhel4i386.img' After a little investigation I found that in BlktapController try: imagetype = self.vm.info['image']['type'] except: imagetype = "" Has long ago been broken and should instead be try: imagetype = self.vm.info.image_type() except: imagetype = "" Once I made that change I can see a phantom device being created, but QEMU still crashes & burns with qemu-dm-XXXX.log showing qemu: could not open hard disk image '/dev/xvdc1' I started to debug this, but looking at changeset 13827:6524e02edbeb I rapidly came to the conclusion that the whole idea of phantom devices is complete overkill & the entire problem could be addressed with a couple of lines in ioemu/xenstore.c. QEMU already knows how to handle all the different types of file format blktap does, so there's no need to setup extra phantom devices. All thats needed is for QEMU to a) strip the aio: (or equivalent) prefix and b) convert xvdN -> hdN if required. So I'm attaching two patches. The first reverts 13827:6524e02edbeb and the second tweaks ioemu/xenstore.c so it can handle blktap devices directly. With these applied ontop of Xen 3.1.0 I can successfully start HVM guests using the two example tap:aio lines I show earlier in this mail. The patch also adds a couple of logging lines which end up in qemu-dm-XXX.log as they'll be useful if ever debugging QEMU boot issues - it is far too silent when things go wrong which makes diagnosis hard. Signed-off-by: Daniel Berrange The patch being reverted: $ diffstat xen-revert-phantom.patch ioemu/xenstore.c | 46 --------------------- python/xen/xend/XendConfig.py | 41 ------------------- python/xen/xend/XendDomainInfo.py | 58 --------------------------- python/xen/xend/server/BlktapController.py | 62 ----------------------------- python/xen/xend/server/DevController.py | 13 ------ 5 files changed, 3 insertions(+), 217 deletions(-) The new patch: $ diffstat xen-qemu-blktap.patch xenstore.c | 28 ++++++++++++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-) Regards, Dan. -- |=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=| |=- Perl modules: http://search.cpan.org/~danberr/ -=| |=- Projects: http://freshmeat.net/~danielpb/ -=| |=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=| --UugvWAfsgieZRqgk Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename="xen-revert-phantom.patch" diff -r c0b0974fb055 tools/ioemu/xenstore.c --- a/tools/ioemu/xenstore.c Fri May 18 16:59:32 2007 +0100 +++ b/tools/ioemu/xenstore.c Fri Jun 29 15:48:53 2007 -0400 @@ -10,7 +10,6 @@ #include "vl.h" #include "block_int.h" -#include #include #include #include @@ -61,28 +60,11 @@ void xenstore_check_new_media_present(in qemu_mod_timer(insert_timer, qemu_get_clock(rt_clock) + timeout); } -static void waitForDevice(char *fn) -{ - struct stat sbuf; - int status; - int uwait = UWAIT_MAX; - - do { - status = stat(fn, &sbuf); - if (!status) break; - usleep(UWAIT); - uwait -= UWAIT; - } while (uwait > 0); - - return; -} - void xenstore_parse_domain_config(int domid) { char **e = NULL; char *buf = NULL, *path; - char *fpath = NULL, *bpath = NULL, - *dev = NULL, *params = NULL, *type = NULL; + char *bpath = NULL, *dev = NULL, *params = NULL, *type = NULL; int i, is_scsi; unsigned int len, num, hd_index; @@ -140,36 +122,12 @@ void xenstore_parse_domain_config(int do params = xs_read(xsh, XBT_NULL, buf, &len); if (params == NULL) continue; - /* - * check if device has a phantom vbd; the phantom is hooked - * to the frontend device (for ease of cleanup), so lookup - * the frontend device, and see if there is a phantom_vbd - * if there is, we will use resolution as the filename - */ - if (pasprintf(&buf, "%s/device/vbd/%s/phantom_vbd", path, e[i]) == -1) - continue; - free(fpath); - fpath = xs_read(xsh, XBT_NULL, buf, &len); - if (fpath) { - if (pasprintf(&buf, "%s/dev", fpath) == -1) - continue; - free(params); - params = xs_read(xsh, XBT_NULL, buf , &len); - if (params) { - /* - * wait for device, on timeout silently fail because we will - * fail to open below - */ - waitForDevice(params); - } - } bs_table[hd_index + (is_scsi ? MAX_DISKS : 0)] = bdrv_new(dev); /* check if it is a cdrom */ if (type && !strcmp(type, "cdrom")) { bdrv_set_type_hint(bs_table[hd_index], BDRV_TYPE_CDROM); - if (pasprintf(&buf, "%s/params", bpath) != -1) - xs_watch(xsh, buf, dev); + xs_watch(xsh, buf, dev); } /* open device now if media present */ if (params[0]) { diff -r c0b0974fb055 tools/python/xen/xend/XendConfig.py --- a/tools/python/xen/xend/XendConfig.py Fri May 18 16:59:32 2007 +0100 +++ b/tools/python/xen/xend/XendConfig.py Fri Jun 29 15:37:43 2007 -0400 @@ -1126,47 +1126,6 @@ class XendConfig(dict): # no valid device to add return '' - def phantom_device_add(self, dev_type, cfg_xenapi = None, - target = None): - """Add a phantom tap device configuration in XenAPI struct format. - """ - - if target == None: - target = self - - if dev_type not in XendDevices.valid_devices() and \ - dev_type not in XendDevices.pseudo_devices(): - raise XendConfigError("XendConfig: %s not a valid device type" % - dev_type) - - if cfg_xenapi == None: - raise XendConfigError("XendConfig: device_add requires some " - "config.") - - if cfg_xenapi: - log.debug("XendConfig.phantom_device_add: %s" % str(cfg_xenapi)) - - if cfg_xenapi: - dev_info = {} - if dev_type in ('vbd', 'tap'): - if dev_type == 'vbd': - dev_info['uname'] = cfg_xenapi.get('image', '') - dev_info['dev'] = '%s:disk' % cfg_xenapi.get('device') - elif dev_type == 'tap': - if cfg_xenapi.get('image').find('tap:') == -1: - dev_info['uname'] = 'tap:qcow:%s' % cfg_xenapi.get('image') - dev_info['dev'] = '/dev/%s' % cfg_xenapi.get('device') - dev_info['uname'] = cfg_xenapi.get('image') - dev_info['mode'] = cfg_xenapi.get('mode') - dev_info['backend'] = '0' - dev_uuid = cfg_xenapi.get('uuid', uuid.createString()) - dev_info['uuid'] = dev_uuid - self['devices'][dev_uuid] = (dev_type, dev_info) - self['vbd_refs'].append(dev_uuid) - return dev_uuid - - return '' - def console_add(self, protocol, location, other_config = {}): dev_uuid = uuid.createString() if protocol == 'vt100': diff -r c0b0974fb055 tools/python/xen/xend/XendDomainInfo.py --- a/tools/python/xen/xend/XendDomainInfo.py Fri May 18 16:59:32 2007 +0100 +++ b/tools/python/xen/xend/XendDomainInfo.py Fri Jun 29 15:37:43 2007 -0400 @@ -1623,49 +1623,14 @@ class XendDomainInfo: # VM Destroy # - def _prepare_phantom_paths(self): - # get associated devices to destroy - # build list of phantom devices to be removed after normal devices - plist = [] - if self.domid is not None: - from xen.xend.xenstore.xstransact import xstransact - t = xstransact("%s/device/vbd" % GetDomainPath(self.domid)) - for dev in t.list(): - backend_phantom_vbd = xstransact.Read("%s/device/vbd/%s/phantom_vbd" \ - % (self.dompath, dev)) - if backend_phantom_vbd is not None: - frontend_phantom_vbd = xstransact.Read("%s/frontend" \ - % backend_phantom_vbd) - plist.append(backend_phantom_vbd) - plist.append(frontend_phantom_vbd) - return plist - - def _cleanup_phantom_devs(self, plist): - # remove phantom devices - if not plist == []: - time.sleep(2) - for paths in plist: - if paths.find('backend') != -1: - from xen.xend.server import DevController - # Modify online status /before/ updating state (latter is watched by - # drivers, so this ordering avoids a race). - xstransact.Write(paths, 'online', "0") - xstransact.Write(paths, 'state', str(DevController.xenbusState['Closing'])) - # force - xstransact.Remove(paths) - def destroy(self): """Cleanup VM and destroy domain. Nothrow guarantee.""" log.debug("XendDomainInfo.destroy: domid=%s", str(self.domid)) - - paths = self._prepare_phantom_paths() self._cleanupVm() if self.dompath is not None: self.destroyDomain() - - self._cleanup_phantom_devs(paths) if "transient" in self.info["other_config"] \ and bool(self.info["other_config"]["transient"]): @@ -1675,8 +1640,6 @@ class XendDomainInfo: def destroyDomain(self): log.debug("XendDomainInfo.destroyDomain(%s)", str(self.domid)) - - paths = self._prepare_phantom_paths() try: if self.domid is not None: @@ -1692,7 +1655,7 @@ class XendDomainInfo: XendDomain.instance().remove_domain(self) self.cleanupDomain() - self._cleanup_phantom_devs(paths) + def resumeDomain(self): @@ -2403,25 +2366,6 @@ class XendDomainInfo: return dev_uuid - def create_phantom_vbd_with_vdi(self, xenapi_vbd, vdi_image_path): - """Create a VBD using a VDI from XendStorageRepository. - - @param xenapi_vbd: vbd struct from the Xen API - @param vdi_image_path: VDI UUID - @rtype: string - @return: uuid of the device - """ - xenapi_vbd['image'] = vdi_image_path - dev_uuid = self.info.phantom_device_add('tap', cfg_xenapi = xenapi_vbd) - if not dev_uuid: - raise XendError('Failed to create device') - - if self._stateGet() == XEN_API_VM_POWER_STATE_RUNNING: - _, config = self.info['devices'][dev_uuid] - config['devid'] = self.getDeviceController('tap').createDevice(config) - - return config['devid'] - def create_vif(self, xenapi_vif): """Create VIF device from the passed struct in Xen API format. diff -r c0b0974fb055 tools/python/xen/xend/server/BlktapController.py --- a/tools/python/xen/xend/server/BlktapController.py Fri May 18 16:59:32 2007 +0100 +++ b/tools/python/xen/xend/server/BlktapController.py Fri Jun 29 15:37:43 2007 -0400 @@ -2,10 +2,7 @@ from xen.xend.server.blkif import BlkifController -from xen.xend.XendLogging import log -phantomDev = 0; -phantomId = 0; class BlktapController(BlkifController): def __init__(self, vm): @@ -15,62 +12,3 @@ class BlktapController(BlkifController): """@see DevController#frontendRoot""" return "%s/device/vbd" % self.vm.getDomainPath() - - def getDeviceDetails(self, config): - (devid, back, front) = BlkifController.getDeviceDetails(self, config) - - phantomDevid = 0 - wrapped = False - - try: - imagetype = self.vm.info['image']['type'] - except: - imagetype = "" - - if imagetype == 'hvm': - tdevname = back['dev'] - index = ['c', 'd', 'e', 'f', 'g', 'h', 'i', \ - 'j', 'l', 'm', 'n', 'o', 'p'] - while True: - global phantomDev - global phantomId - import os, stat - - phantomId = phantomId + 1 - if phantomId == 16: - if index[phantomDev] == index[-1]: - if wrapped: - raise VmError(" No loopback block \ - devices are available. ") - wrapped = True - phantomDev = 0 - else: - phantomDev = phantomDev + 1 - phantomId = 1 - devname = 'xvd%s%d' % (index[phantomDev], phantomId) - try: - info = os.stat('/dev/%s' % devname) - except: - break - - vbd = { 'mode': 'w', 'device': devname } - fn = 'tap:%s' % back['params'] - - # recurse ... by creating the vbd, then fallthrough - # and finish creating the original device - - from xen.xend import XendDomain - dom0 = XendDomain.instance().privilegedDomain() - phantomDevid = dom0.create_phantom_vbd_with_vdi(vbd, fn) - # we need to wait for this device at a higher level - # the vbd that gets created will have a link to us - # and will let them do it there - - # add a hook to point to the phantom device, - # root path is always the same (dom0 tap) - if phantomDevid != 0: - front['phantom_vbd'] = '/local/domain/0/backend/tap/0/%s' \ - % str(phantomDevid) - - return (devid, back, front) - diff -r c0b0974fb055 tools/python/xen/xend/server/DevController.py --- a/tools/python/xen/xend/server/DevController.py Fri May 18 16:59:32 2007 +0100 +++ b/tools/python/xen/xend/server/DevController.py Fri Jun 29 15:37:43 2007 -0400 @@ -474,19 +474,6 @@ class DevController: def waitForBackend(self, devid): frontpath = self.frontendPath(devid) - # lookup a phantom - phantomPath = xstransact.Read(frontpath, 'phantom_vbd') - if phantomPath is not None: - log.debug("Waiting for %s's phantom %s.", devid, phantomPath) - statusPath = phantomPath + '/' + HOTPLUG_STATUS_NODE - ev = Event() - result = { 'status': Timeout } - xswatch(statusPath, hotplugStatusCallback, ev, result) - ev.wait(DEVICE_CREATE_TIMEOUT) - err = xstransact.Read(statusPath, HOTPLUG_ERROR_NODE) - if result['status'] != 'Connected': - return (result['status'], err) - backpath = xstransact.Read(frontpath, "backend") --UugvWAfsgieZRqgk Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename="xen-qemu-blktap.patch" diff -r ff1a3395024c tools/ioemu/xenstore.c --- a/tools/ioemu/xenstore.c Fri Jun 29 15:37:48 2007 -0400 +++ b/tools/ioemu/xenstore.c Fri Jun 29 16:06:13 2007 -0400 @@ -64,7 +64,7 @@ void xenstore_parse_domain_config(int do { char **e = NULL; char *buf = NULL, *path; - char *bpath = NULL, *dev = NULL, *params = NULL, *type = NULL; + char *bpath = NULL, *dev = NULL, *params = NULL, *type = NULL, *drv = NULL; int i, is_scsi; unsigned int len, num, hd_index; @@ -98,6 +98,13 @@ void xenstore_parse_domain_config(int do bpath = xs_read(xsh, XBT_NULL, buf, &len); if (bpath == NULL) continue; + /* read the driver type of the device */ + if (pasprintf(&buf, "%s/type", bpath) == -1) + continue; + free(drv); + drv = xs_read(xsh, XBT_NULL, buf, &len); + if (drv == NULL) + continue; /* read the name of the device */ if (pasprintf(&buf, "%s/dev", bpath) == -1) continue; @@ -105,6 +112,13 @@ void xenstore_parse_domain_config(int do dev = xs_read(xsh, XBT_NULL, buf, &len); if (dev == NULL) continue; + /* Force xvdN to look like hdN */ + if (!strncmp(dev, "xvd", 3)) { + fprintf(logfile, "Converting device type '%s'\n", dev); + memmove(dev, dev+1, strlen(dev)); + dev[0] = 'h'; + dev[1] = 'd'; + } is_scsi = !strncmp(dev, "sd", 2); if ((strncmp(dev, "hd", 2) && !is_scsi) || strlen(dev) != 3 ) continue; @@ -122,7 +136,15 @@ void xenstore_parse_domain_config(int do params = xs_read(xsh, XBT_NULL, buf, &len); if (params == NULL) continue; - + /* Strip off blktap sub-type prefix aio: - QEMU can autodetect this */ + if (!strcmp(drv, "tap") && params[0]) { + char *offset = strchr(params, ':'); + if (!offset) + continue; + fprintf(logfile, "Stripping blktap sub-type prefix from %s\n", params); + memmove(params, offset+1, strlen(offset+1)+1); + } + fprintf(logfile, "Creating disk '%s' with driver '%s'\n", dev, drv); bs_table[hd_index + (is_scsi ? MAX_DISKS : 0)] = bdrv_new(dev); /* check if it is a cdrom */ if (type && !strcmp(type, "cdrom")) { @@ -131,6 +153,7 @@ void xenstore_parse_domain_config(int do } /* open device now if media present */ if (params[0]) { + fprintf(logfile, "Initializing disk '%s' with media '%s'\n", dev, params); if (bdrv_open(bs_table[hd_index + (is_scsi ? MAX_DISKS : 0)], params, 0 /* snapshot */) < 0) fprintf(stderr, "qemu: could not open hard disk image '%s'\n", @@ -146,6 +169,7 @@ void xenstore_parse_domain_config(int do out: + free(drv); free(type); free(params); free(dev); --UugvWAfsgieZRqgk Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Xen-users mailing list Xen-users@lists.xensource.com http://lists.xensource.com/xen-users --UugvWAfsgieZRqgk--