[PATCH] Allow blktap to be able to be booted as system volume for PV-on-HVM

[PATCH] Allow blktap to be able to be booted as system volume for PV-on-HVM

[Kasai Takanori]

[-- Attachment #1: Type: text/plain, Size: 1259 bytes --]

Hi All,

We were testing the PV driver on the HVM domain.
When blktap was booting system volume, PV-on-HVM domain was not able to be 
started.

The configuration file is specified as follows.
disk = [ 'tap:aio:/xen/test/rhel5ga_full.img,hda,w' ]

The error occurred by the initialization of system volume in qemu-dm.

qemu: could not open hard disk image 'aio:/xen/test/rhel5ga_full.img'

It is because "aio:" is added to the head of params in xenstore.
However, qemu-dm open device by params.

This patch corrected the problem of params on the qemu-dm.

Signed-off-by: Takanori Kasai <kasai.takanori@jp.fujitsu.com>
Signed-off-by: Tomonari Horikoshi <t.horikoshi@jp.fujitsu.com>
Signed-off-by: Tsunehisa Doi <Doi.Tsunehisa@jp.fujitsu.com>

The execution result is as follows.

# xm create -f rhel5ga_vti.conf
Using config file "./rhel5ga_vti.conf".
Started domain RHEL5GA-test2
# xm block-list RHEL5GA-test2
Vdev  BE handle state evt-ch ring-ref BE-path
768    0    0     1      -1     -1    /local/domain/0/backend/tap/75/768
...      <<= /* Waiting for booting process */
# xm block-list RHEL5GA-test2
Vdev  BE handle state evt-ch ring-ref BE-path
768    0    0     4      7      8     /local/domain/0/backend/tap/75/768

Best Regards,

--
Takanori Kasai 

[-- Attachment #2: allow-blktap-system-volume.patch --]
[-- Type: application/octet-stream, Size: 1327 bytes --]

# HG changeset patch
# User kasai.takanori@jp.fujitsu.com
# Date 1183713661 -32400
# Node ID 9d9d64fae3cd39cc7dbd398dd94ead6e413980bd
# Parent  d54d47fc8c6cdea23437476407bec05d85742760
Allow blktap to be able to be booted as system volume for PV-on-HVM

Signed-off-by: Takanori Kasai <kasai.takanori@jp.fujitsu.com>
Signed-off-by: Tomonari Horikoshi <t.horikoshi@jp.fujitsu.com>
Signed-off-by: Tsunehisa Doi <Doi.Tsunehisa@jp.fujitsu.com>

diff -r d54d47fc8c6c -r 9d9d64fae3cd tools/ioemu/xenstore.c
--- a/tools/ioemu/xenstore.c	Thu Jul 05 10:19:25 2007 +0100
+++ b/tools/ioemu/xenstore.c	Fri Jul 06 18:21:01 2007 +0900
@@ -173,10 +173,17 @@ void xenstore_parse_domain_config(int do
         }
         /* open device now if media present */
         if (params[0]) {
+            char *p = NULL;
+            if ((p = strchr(params, ':'))) {
+                p++;
+            }
+            else {
+                p = params;
+            }
             if (bdrv_open(bs_table[hd_index + (is_scsi ? MAX_DISKS : 0)],
-                          params, 0 /* snapshot */) < 0)
+                          p, 0 /* snapshot */) < 0)
                 fprintf(stderr, "qemu: could not open hard disk image '%s'\n",
-                        params);
+                        p);
         }
     }
 

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

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

Re: [PATCH] Allow blktap to be able to be booted as system volume for PV-on-HVM

[Daniel P. Berrange]

[-- Attachment #1: Type: text/plain, Size: 1355 bytes --]

On Fri, Jul 06, 2007 at 07:50:06PM +0900, Kasai Takanori wrote:
> Hi All,
> 
> We were testing the PV driver on the HVM domain.
> When blktap was booting system volume, PV-on-HVM domain was not able to be 
> started.
> 
> The configuration file is specified as follows.
> disk = [ 'tap:aio:/xen/test/rhel5ga_full.img,hda,w' ]
> 
> The error occurred by the initialization of system volume in qemu-dm.
> 
> qemu: could not open hard disk image 'aio:/xen/test/rhel5ga_full.img'
> 
> It is because "aio:" is added to the head of params in xenstore.
> However, qemu-dm open device by params.
> 
> This patch corrected the problem of params on the qemu-dm.

That's not safe enough since it always looks for a ':' regardless of
whether the driver is 'tap' or not. It also doesn't take account of
fact that the device may be named 'xvda' rather than 'hda'. 

I posted patches to address this more completely here:

http://lists.xensource.com/archives/html/xen-devel/2007-06/msg01021.html

I'm attaching them again for convenience

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  -=| 

[-- Attachment #2: xen-revert-phantom.patch --]
[-- Type: text/plain, Size: 12359 bytes --]

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 <unistd.h>
 #include <sys/ipc.h>
 #include <sys/shm.h>
 #include <sys/types.h>
@@ -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")
 
 

[-- Attachment #3: qemu-blktap.patch --]
[-- Type: text/plain, Size: 1557 bytes --]

--- xen-3.1.0-src.new/tools/ioemu/xenstore.c	2007-05-18 10:45:21.000000000 -0400
+++ xen-3.1.0-src/tools/ioemu/xenstore.c	2007-06-22 13:48:55.000000000 -0400
@@ -83,7 +83,7 @@ void xenstore_parse_domain_config(int do
     char *buf = NULL, *path;
     char *fpath = NULL, *bpath = NULL,
         *dev = NULL, *params = NULL, *type = NULL;
-    int i, is_scsi;
+    int i, is_scsi, is_tap = 0;
     unsigned int len, num, hd_index;
 
     for(i = 0; i < MAX_DISKS + MAX_SCSI_DISKS; i++)
@@ -123,6 +123,14 @@ 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)) {
+	  memmove(dev, dev+1, strlen(dev));
+	  dev[0] = 'h';
+	  dev[1] = 'd';
+	  is_tap = 1;
+	  fprintf(stderr, "Got blktap '%s'\n", dev);
+	}
         is_scsi = !strncmp(dev, "sd", 2);
         if ((strncmp(dev, "hd", 2) && !is_scsi) || strlen(dev) != 3 )
             continue;
@@ -140,6 +148,14 @@ 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: etc */
+	if (is_tap) {
+	  char *offset = strchr(params, ':');
+	  if (!offset)
+	    continue;
+	  memmove(params, offset+1, strlen(offset+1)+1);
+	  fprintf(stderr, "Got params '%s'\n", params);
+	}
         /* 
          * check if device has a phantom vbd; the phantom is hooked
          * to the frontend device (for ease of cleanup), so lookup 

[-- Attachment #4: Type: text/plain, Size: 138 bytes --]

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

Re: [PATCH] Allow blktap to be able to be booted assystem volume for PV-on-HVM

[Kasai Takanori]

Hi Daniel,

Thank you for the reply.

> That's not safe enough since it always looks for a ':' regardless of
> whether the driver is 'tap' or not.

Does it mean to have to check the assumed keyword(aio,sync,qcow)?
If it is so, I will correct the patch.

> It also doesn't take account of
> fact that the device may be named 'xvda' rather than 'hda'.

I saw the presented patches. The xvdN was forcedly looked like the hdN.
For example, the hdc is a special device named CDROM in qemu-dm.
Therefore, I think that it should not forcedly do it because it cannot take the 
consistency.

I think the HVM domain to be should boot on the hda as it is a specification of 
qemu-dm.

Best Regards,

--
Takanori Kasai

Re: [PATCH] Allow blktap to be able to be booted assystem volume for PV-on-HVM

[Daniel P. Berrange]

On Mon, Jul 09, 2007 at 02:09:47PM +0900, Kasai Takanori wrote:
> >It also doesn't take account of
> >fact that the device may be named 'xvda' rather than 'hda'.
> 
> I saw the presented patches. The xvdN was forcedly looked like the hdN.
> For example, the hdc is a special device named CDROM in qemu-dm.

No that is wrong. Any hdX device can be a CDROM - whether it is a CDROM
or not is determined by the :cdrom postfix, not the device name.

eg You can happily make hdc a harddisk:

   tap:aio:/var/lib/xen/images/dom.img,hdc,w

Or make hda a CDROM

   tap:aio:/var/lib/xen/images/dom.img,hda:cdrom,w


> Therefore, I think that it should not forcedly do it because it cannot take 
> the consistency.

Being able to use 'xvd*', as well as 'hd*' or 'sd*' as device names is 
important for consistency with paravirt guests, where xvd* is the 
recommended default.

> I think the HVM domain to be should boot on the hda as it is a 
> specification of qemu-dm.

It can do that. My patch made it possible to use either

   tap:aio:/var/lib/xen/images/dom.img,hda,w

Or

   tap:aio:/var/lib/xen/images/dom.img,xvda,w

At the user's choice. 

n/
-- 
|=- 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  -=| 

Re: [PATCH] Allow blktap to be able to be booted assystemvolume for PV-on-HVM

[Kasai Takanori]

Hi Daniel,

Thank you for the reply.

We think about the specification of the device as follows. 

The hda is initialized of emulator first.
It works by the emulator if there is no PV driver in the HVM domain. 
It is initialized as VBD if there is PV driver in the HVM domain. 
And, the hda changes to VBD and works as VBD. 

On the other hand, the xvda doesn't initialize it as an emulator.
It doesn't work if there is no PV driver in the HVM domain. 
It works as VBD if there is PV driver in the HVM domain. 

The hda is used as a device from which the emulator is switched to VBD. 
And, the xvd* is used as a device only for VBD. 

Therefore, the system disk is specified for hda as well as a usual HVM domain. 
Other data disks are specified for xvd*. 
We boot the HVM domain specifying both hda and xvda. 

What scenario are you assuming?
Do you initialize all 'xvd*' as an emulator?

Best Regards,

--
Takanori Kasai