Merge lp:~raharper/curtin/trunk.vmtest-multipath into lp:~curtin-dev/curtin/trunk

[Ryan Harper]

Add multipath testing

Add multipath testing via a test-case option, cls.multipath which will
if set to true, duplicate all disks in a test class. This has the effect
of looking exactly like multipath setups where we see the same device
with two paths.

To handle devices with wwn or serial numbers with spaces in them
curtin.block was updated to ensure it supports finding devices via there
serial or wwn attribute. Specifically for serial numbers with spaces, we
currently will escape the spaces to underscores; this is supported by
observing the names that show up in /dev/disk/by-id/* for those devices.

For wwn, /dev/disk/by-id/wwn-$wwn ; curtin resolves the symlink target to
obtain a kernel devname for use with sysfs.

For serial with spaces, curtin attempts to remove whitespace like udev to
find the disk under /dev/disk/by-id/ and determine a devname.

Xenial multipath fails without ensuring the multipath/bindings file is
written with whitespace removed; this is triggered on in-target mulitpath
versions >= 1.5.0 fixing LP:1551937.

Add multipath testing

Add multipath testing via a test-case option, cls.multipath which will
if set to true, duplicate all disks in a test class. This has the effect
of looking exactly like multipath setups where we see the same device
with two paths.

To handle devices with wwn or serial numbers with spaces in them
curtin.block was updated to ensure it supports finding devices via there
serial or wwn attribute. Specifically for serial numbers with spaces, we
currently will escape the spaces to underscores; this is supported by
observing the names that show up in /dev/disk/by-id/* for those devices.

For wwn, /dev/disk/by-id/wwn-$wwn ; curtin resolves the symlink target to
obtain a kernel devname for use with sysfs.

For serial with spaces, curtin attempts to remove whitespace like udev to
find the disk under /dev/disk/by-id/ and determine a devname.

Xenial multipath fails without ensuring the multipath/bindings file is
written with whitespace removed; this is triggered on in-target mulitpath
versions >= 1.5.0 fixing LP:1551937.

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:392
https://server-team-jenkins.canonical.com/job/curtin-ci/250/
Executed test runs:
    None: https://server-team-jenkins.canonical.com/job/generic-update-mp/247/console

Click here to trigger a rebuild:
https://server-team-jenkins.canonical.com/job/curtin-ci/250/rebuild

review: Approve (continuous-integration)

[Christian Ehrhardt  (paelzer)]

Hi Ryan,
nice work on multipath testing!

I have found nothing severe but a collection of things that could be more clear, more polished or just were unclear at least to me.

review: Needs Fixing

[Christian Ehrhardt  (paelzer)]

Probably also worth to mention that actually running the tests failed me.
I did an sync-images to ensure I'm not just on some older sh$§.

But still running the full testsuite on your branch won't fully work:
Note at the same environment my last branch works (just to ensure the system setup isn't borked)

rm -rf output/; CURTIN_VMTEST_KEEP_DATA_FAIL=all ./tools/jenkins-runner -vv --nologcapture --processes=5 --process-timeout=3600 tests/vmtests 2>&1 | tee ~/curtin-test-multipath.log; rm -rf output/
Yours: => http://paste.ubuntu.com/17167038/
Mine as reference: => http://paste.ubuntu.com/17167042/

You can give it a look yourself it is on ubuntu@horsea:/mnt/nvme/trunk.vmtest-multipath and the logfiles of my runs are in home.
I imported your LP id to ubuntu@

[Scott Moser (smoser)]

On Fri, 10 Jun 2016, ChristianEhrhardt wrote:

> Review: Needs Fixing
>
> Hi Ryan,
> nice work on multipath testing!
>
> I have found nothing severe but a collection of things that could be more clear, more polished or just were unclear at least to me.
>
> Do you think it would make sense to make this a parameter maybe from cls.multipath.num_parallel or so?
> Effectively what you did is already so great, I don't see why it has to be always 2.
> That way you could do some sort of limit testing by setting it to (48*2)*(96*2) which would be the system z max.
> Not really that number :-), but driving the test with 2,16,64 could be a great check!
>
> General question: Is there a limit on qemu (commandline) that we have to be scared of?

Probably not too much to worry about. I'm pretty sure that its total of
command line + environment < 128k.

I just tried:
  $ ./foo.sh
  128000
  129000
  130000
  131000
  /tmp/foo.sh: 8: /tmp/foo.sh: /bin/true: Argument list too long
  i=131072 2
  $ cat foo.sh
  #!/bin/sh
  b=""; i=0
  while i=$(($i+1)) && b="$b."; do
      [ $i -lt 128000 ] && continue
      [ $(($i%1000)) -eq 0 ] && echo $i
      /bin/true "$b" || { ret=$?; break; }
  done
  echo i=$i $ret

[Ryan Harper (raharper)]

> Hi Ryan,
> nice work on multipath testing!
>
> I have found nothing severe but a collection of things that could be more clear, more polished or just were unclear at least to me.
>

Thanks for the review Christian. Picking up most of your suggestions.
A few things will need to put off for now.

> > === modified file 'curtin/block/__init__.py'
> > --- curtin/block/__init__.py 2016-04-22 16:24:00 +0000
> > +++ curtin/block/__init__.py 2016-06-09 21:47:02 +0000
> > @@ -356,6 +356,7 @@
> > cmd.append('--replace-whitespace')
> > try:
> > (out, err) = util.subp(cmd, capture=True)
> > + LOG.debug("scsi_id output raw:\n%s", out)
>
> Since it is debug level anyway I'd think it would be nice to report the "err"
> as well. I mean there could be err output even without
> util.ProcessExecutionError in some cases - and especially those are the ones we
> will read the debug level info.
>

OK

> > scsi_wwid = out.rstrip('\n')
> > return scsi_wwid
> > except util.ProcessExecutionError as e:
> >
> > === modified file 'curtin/commands/block_meta.py'
> > --- curtin/commands/block_meta.py 2016-04-22 16:24:00 +0000
> > +++ curtin/commands/block_meta.py 2016-06-09 21:47:02 +0000
> > @@ -379,9 +379,14 @@
> > if vol.get('serial'):
> > volume_path = block.lookup_disk(vol.get('serial'))
> > elif vol.get('path'):
> > - volume_path = vol.get('path')
> > + # resolve any symlinks to the dev_kname so sys/class/block access
> > + # is valid. ie, there are no udev generated values in sysfs
> > + volume_path = os.path.realpath(vol.get('path'))
>
> I'm not sure so this is honestly just a question - could there be cases
> where os.path.realpath would throw some kind of OSError? Like on broken
> links or anything - and if so should we guard us against it?
> (same 4 lines below)

I'm sure os.path.realpath() throw and exception, however, I don't think there
is anything that we can do about that and still recover. That is, if
we fail to find the realpath to the device, we cannot fix that, and we'd exit.

Without catching the exception, we get the same semantics by allowing the
error to propigate upward.

> > === modified file 'curtin/commands/curthooks.py'
> > --- curtin/commands/curthooks.py 2016-05-17 21:14:36 +0000
> > +++ curtin/commands/curthooks.py 2016-06-09 21:47:02 +0000
> > @@ -638,6 +638,20 @@
> > LOG.info("Detected multipath devices. Installing support via %s", mppkgs)
> >
> > util.install_packages(mppkgs, target=target)
> > + replace_spaces = True
> > + try:
> > + # check in-target version
> > + pkg_ver = util.get_package_version('multipath-tools', target)
> > + upstream = pkg_ver.split('-')[0]
> > + major, minor, micro = upstream.split(".", 2)
> > + val = 1000 * int(major) + 100 * int(minor)
>
> Maybe moving the splitting into util as well (or an extra .py).
> This is very format depending, but I'd think there are only three or four kinds in Ubunutu.
> It would be nice for re-usability if you would define that as util for this format.
> Whoever needs another ...

[lp:~raharper/curtin/trunk.vmtest-multipath]

393. By Ryan Harper on 2016-06-10

Only use the filename of the output disk, not the whole path.

394. By Ryan Harper on 2016-06-10

Add error output along side scsi_id debug output.

395. By Ryan Harper on 2016-06-10

Not all vmtests use storage config, fail gracefully

396. By Ryan Harper on 2016-06-10

Move version parsing into utility method, return version dictionary for callers

397. By Ryan Harper on 2016-06-10

Use disk_wwns to match disk_serials

398. By Ryan Harper on 2016-06-10

Replace hardcoded number of paths for multipath to class variable, cls.multipath_num_paths; default to 2

399. By Ryan Harper on 2016-06-10

Replace single character temp variable in disk parameter construction; update formatting.

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:399
https://server-team-jenkins.canonical.com/job/curtin-ci/252/
Executed test runs:
    None: https://server-team-jenkins.canonical.com/job/generic-update-mp/249/console

Click here to trigger a rebuild:
https://server-team-jenkins.canonical.com/job/curtin-ci/252/rebuild

review: Approve (continuous-integration)

[Scott Moser (smoser)]

I approve of this branch.
We actually deployed a power8 system with multipath for xenial with it at revno 399.
I had conversation with Ryan in irc, and asked that we improve the get_package_version to do the multiplication of major/minor/micro centrally. something like this: http://paste.ubuntu.com/17806224/

i'd like that fixed, and then please merge. whoowho!

review: Approve

[lp:~raharper/curtin/trunk.vmtest-multipath]

400. By Ryan Harper on 2016-06-23

merge from trunk

401. By Ryan Harper on 2016-06-24

Update get_package_version to move sematic_version calculation internally.

402. By Ryan Harper on 2016-06-24

fixes from diglett run

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:402
https://server-team-jenkins.canonical.com/job/curtin-ci/277/
Executed test runs:
    None: https://server-team-jenkins.canonical.com/job/generic-update-mp/274/console

Click here to trigger a rebuild:
https://server-team-jenkins.canonical.com/job/curtin-ci/277/rebuild

review: Approve (continuous-integration)

=== modified file 'curtin/block/__init__.py'
--- curtin/block/__init__.py	2016-04-22 16:24:00 +0000
+++ curtin/block/__init__.py	2016-06-24 19:28:31 +0000
@@ -356,6 +356,7 @@
         cmd.append('--replace-whitespace')
     try:
         (out, err) = util.subp(cmd, capture=True)
+        LOG.debug("scsi_id output raw:\n%s\nerror:\n%s", out, err)
         scsi_wwid = out.rstrip('\n')
         return scsi_wwid
     except util.ProcessExecutionError as e:
@@ -474,9 +475,14 @@
     """
     # Get all volumes in /dev/disk/by-id/ containing the serial string. The
     # string specified can be either in the short or long serial format
-    disks = list(filter(lambda x: serial in x, os.listdir("/dev/disk/by-id/")))
+    # hack, some serials have spaces, udev usually converts ' ' -> '_'
+    serial_udev = serial.replace(' ', '_')
+    LOG.info('Processing serial %s via udev to %s', serial, serial_udev)
+
+    disks = list(filter(lambda x: serial_udev in x,
+                        os.listdir("/dev/disk/by-id/")))
     if not disks or len(disks) < 1:
-        raise ValueError("no disk with serial '%s' found" % serial)
+        raise ValueError("no disk with serial '%s' found" % serial_udev)
 
     # Sort by length and take the shortest path name, as the longer path names
     # will be the partitions on the disk. Then use os.path.realpath to
@@ -486,7 +492,7 @@
 
     if not os.path.exists(path):
         raise ValueError("path '%s' to block device for disk with serial '%s' \
-            does not exist" % (path, serial))
+            does not exist" % (path, serial_udev))
     return path
 
 

=== modified file 'curtin/commands/block_meta.py'
--- curtin/commands/block_meta.py	2016-04-22 16:24:00 +0000
+++ curtin/commands/block_meta.py	2016-06-24 19:28:31 +0000
@@ -379,9 +379,14 @@
         if vol.get('serial'):
             volume_path = block.lookup_disk(vol.get('serial'))
         elif vol.get('path'):
-            volume_path = vol.get('path')
+            # resolve any symlinks to the dev_kname so sys/class/block access
+            # is valid.  ie, there are no udev generated values in sysfs
+            volume_path = os.path.realpath(vol.get('path'))
+        elif vol.get('wwn'):
+            by_wwn = '/dev/disk/by-id/wwn-%s' % vol.get('wwn')
+            volume_path = os.path.realpath(by_wwn)
         else:
-            raise ValueError("serial number or path to block dev must be \
+            raise ValueError("serial, wwn or path to block dev must be \
                 specified to identify disk")
 
     elif vol.get('type') == "lvm_partition":

=== modified file 'curtin/commands/curthooks.py'
--- curtin/commands/curthooks.py	2016-06-14 20:57:58 +0000
+++ curtin/commands/curthooks.py	2016-06-24 19:28:31 +0000
@@ -638,6 +638,21 @@
     LOG.info("Detected multipath devices. Installing support via %s", mppkgs)
 
     util.install_packages(mppkgs, target=target)
+    replace_spaces = True
+    try:
+        # check in-target version
+        pkg_ver = util.get_package_version('multipath-tools', target=target)
+        LOG.debug("get_package_version:\n%s", pkg_ver)
+        LOG.debug("multipath version is %s (major=%s minor=%s micro=%s)",
+                  pkg_ver['semantic_version'], pkg_ver['major'],
+                  pkg_ver['minor'], pkg_ver['micro'])
+        # multipath-tools versions < 0.5.0 do _NOT_ want whitespace replaced
+        # i.e. 0.4.X in Trusty.
+        if pkg_ver['semantic_version'] < 500:
+            replace_spaces = False
+    except Exception as e:
+        LOG.warn("failed reading multipath-tools version, "
+                 "assuming it wants no spaces in wwids: %s", e)
 
     multipath_cfg_path = os.path.sep.join([target, '/etc/multipath.conf'])
     multipath_bind_path = os.path.sep.join([target, '/etc/multipath/bindings'])
@@ -658,7 +673,8 @@
     if mpbindings or not os.path.isfile(multipath_bind_path):
         # we do assume that get_devices_for_mp()[0] is /
         target_dev = block.get_devices_for_mp(target)[0]
-        wwid = block.get_scsi_wwid(target_dev)
+        wwid = block.get_scsi_wwid(target_dev,
+                                   replace_whitespace=replace_spaces)
         blockdev, partno = block.get_blockdev_for_partition(target_dev)
 
         mpname = "mpath0"

=== modified file 'curtin/util.py'
--- curtin/util.py	2016-05-19 18:43:05 +0000
+++ curtin/util.py	2016-06-24 19:28:31 +0000
@@ -499,6 +499,62 @@
         return False
 
 
+def parse_dpkg_version(raw, name=None, semx=None):
+    """Parse a dpkg version string into various parts and calcualate a
+       numerical value of the version for use in comparing package versions
+
+       returns a dictionary with the results
+    """
+    if semx is None:
+        semx = (10000, 100, 1)
+
+    upstream = raw.split('-')[0]
+    toks = upstream.split(".", 2)
+    if len(toks) == 3:
+        major, minor, micro = toks
+    elif len(toks) == 2:
+        major, minor, micro = (toks[0], toks[1], 0)
+    elif len(toks) == 1:
+        major, minor, micro = (toks[0], 0, 0)
+
+    version = {
+        'major': major,
+        'minor': minor,
+        'micro': micro,
+        'raw': raw,
+        'upstream': upstream,
+    }
+    if name:
+        version['name'] = name
+
+    if semx:
+        try:
+            version['semantic_version'] = int(
+                int(major) * semx[0] + int(minor) * semx[1] +
+                int(micro) * semx[2])
+        except (ValueError, IndexError):
+            version['semantic_version'] = None
+
+    return version
+
+
+def get_package_version(pkg, target=None, semx=None):
+    """Use dpkg-query to extract package pkg's version string
+       and parse the version string into a dictionary
+    """
+    chroot = []
+    if target is not None:
+        chroot = ['chroot', target]
+    try:
+        out, _ = subp(chroot + ['dpkg-query', '--show', '--showformat',
+                                '${Version}', pkg],
+                      capture=True)
+        raw = out.rstrip()
+        return parse_dpkg_version(raw, name=pkg, semx=semx)
+    except ProcessExecutionError:
+        return None
+
+
 def find_newer(src, files):
     mtime = os.stat(src).st_mtime
     return [f for f in files if

=== added file 'examples/tests/basic_scsi.yaml'
--- examples/tests/basic_scsi.yaml	1970-01-01 00:00:00 +0000
+++ examples/tests/basic_scsi.yaml	2016-06-24 19:28:31 +0000
@@ -0,0 +1,72 @@
+showtrace: true
+storage:
+    version: 1
+    config:
+      - id: sda
+        type: disk
+        ptable: msdos
+        wwn: '0x39cc071e72c64cc4'
+        name: main_disk
+        wipe: superblock
+        grub_device: true
+      - id: sda1
+        type: partition
+        number: 1
+        size: 3GB
+        device: sda
+        flag: boot
+      - id: sda2
+        type: partition
+        number: 2
+        size: 1GB
+        device: sda
+      - id: sda1_root
+        type: format
+        fstype: ext4
+        volume: sda1
+      - id: sda2_home
+        type: format
+        fstype: ext4
+        volume: sda2
+      - id: sda1_mount
+        type: mount
+        path: /
+        device: sda1_root
+      - id: sda2_mount
+        type: mount
+        path: /home
+        device: sda2_home
+      - id: sparedisk_id
+        type: disk
+        wwn: '0x080258d13ea95ae5'
+        name: sparedisk
+        wipe: superblock
+      - id: btrfs_disk_id
+        type: disk
+        wwn: '0x22dc58dc023c7008'
+        name: btrfs_volume
+        wipe: superblock
+      - id: btrfs_disk_fmt_id
+        type: format
+        fstype: btrfs
+        volume: btrfs_disk_id
+      - id: btrfs_disk_mnt_id
+        type: mount
+        path: /btrfs
+        device: btrfs_disk_fmt_id
+      - id: pnum_disk
+        type: disk
+        wwn: '0x550a270c3a5811c5'
+        name: pnum_disk
+        wipe: superblock
+        ptable: gpt
+      - id: pnum_disk_p1
+        type: partition
+        number: 1
+        size: 1GB
+        device: pnum_disk
+      - id: pnum_disk_p2
+        type: partition
+        number: 10
+        size: 1GB
+        device: pnum_disk

=== added file 'examples/tests/multipath.yaml'
--- examples/tests/multipath.yaml	1970-01-01 00:00:00 +0000
+++ examples/tests/multipath.yaml	2016-06-24 19:28:31 +0000
@@ -0,0 +1,38 @@
+showtrace: true
+storage:
+    version: 1
+    config:
+      - id: sda
+        type: disk
+        ptable: msdos
+        serial: 'IPR-0 1234567890'
+        name: mpath_a
+        wipe: superblock
+        grub_device: true
+      - id: sda1
+        type: partition
+        number: 1
+        size: 3GB
+        device: sda
+        flag: boot
+      - id: sda2
+        type: partition
+        number: 2
+        size: 1GB
+        device: sda
+      - id: sda1_root
+        type: format
+        fstype: ext4
+        volume: sda1
+      - id: sda2_home
+        type: format
+        fstype: ext4
+        volume: sda2
+      - id: sda1_mount
+        type: mount
+        path: /
+        device: sda1_root
+      - id: sda2_mount
+        type: mount
+        path: /home
+        device: sda2_home

=== modified file 'tests/vmtests/__init__.py'
--- tests/vmtests/__init__.py	2016-06-20 18:11:20 +0000
+++ tests/vmtests/__init__.py	2016-06-24 19:28:31 +0000
@@ -48,6 +48,7 @@
 
 
 DEFAULT_BRIDGE = os.environ.get("CURTIN_VMTEST_BRIDGE", "user")
+OUTPUT_DISK_NAME = 'output_disk.img'
 
 _TOPDIR = None
 
@@ -333,7 +334,7 @@
 
         # create output disk, mount ro
         logger.debug('Creating output disk')
-        self.output_disk = os.path.join(self.boot, "output_disk.img")
+        self.output_disk = os.path.join(self.boot, OUTPUT_DISK_NAME)
         subprocess.check_call(["qemu-img", "create", "-f", TARGET_IMAGE_FORMAT,
                               self.output_disk, "10M"],
                               stdout=DEVNULL, stderr=subprocess.STDOUT)
@@ -350,20 +351,23 @@
 class VMBaseClass(TestCase):
     __test__ = False
     arch_skip = []
+    boot_timeout = 300
+    collect_scripts = []
+    conf_file = "examples/tests/basic.yaml"
     disk_block_size = 512
+    disk_driver = 'virtio-blk'
     disk_to_check = {}
+    extra_disks = []
+    extra_kern_args = None
     fstab_expected = {}
-    extra_kern_args = None
-    recorded_errors = 0
-    recorded_failures = 0
     image_store_class = ImageStore
-    collect_scripts = []
+    install_timeout = 3000
     interactive = False
-    conf_file = "examples/tests/basic.yaml"
-    extra_disks = []
+    multipath = False
+    multipath_num_paths = 2
     nvme_disks = []
-    boot_timeout = 300
-    install_timeout = 3000
+    recorded_errors = 0
+    recorded_failures = 0
     uefi = False
 
     # these get set from base_vm_classes
@@ -446,21 +450,51 @@
             netdevs.extend(["--netdev=" + DEFAULT_BRIDGE])
 
         # build disk arguments
-        # --disk source:size:driver:block_size
-        extra_disks = []
+        disks = []
+        sc = util.load_file(cls.conf_file)
+        storage_config = yaml.load(sc).get('storage', {}).get('config', {})
+        cls.disk_wwns = ["wwn=%s" % x.get('wwn') for x in storage_config
+                         if 'wwn' in x]
+        cls.disk_serials = ["serial=%s" % x.get('serial')
+                            for x in storage_config if 'serial' in x]
+
+        target_disk = "{}:{}:{}:{}:".format(cls.td.target_disk,
+                                            "",
+                                            cls.disk_driver,
+                                            cls.disk_block_size)
+        if len(cls.disk_wwns):
+            target_disk += cls.disk_wwns[0]
+
+        if len(cls.disk_serials):
+            target_disk += cls.disk_serials[0]
+
+        disks.extend(['--disk', target_disk])
+
+        # --disk source:size:driver:block_size:devopts
         for (disk_no, disk_sz) in enumerate(cls.extra_disks):
             dpath = os.path.join(cls.td.disks, 'extra_disk_%d.img' % disk_no)
-            extra_disks.extend(
-                ['--disk', '{}:{}:{}:{}'.format(dpath, disk_sz, "",
-                                                cls.disk_block_size)])
+            extra_disk = '{}:{}:{}:{}:'.format(dpath, disk_sz,
+                                               cls.disk_driver,
+                                               cls.disk_block_size)
+            if len(cls.disk_wwns):
+                w_index = disk_no + 1
+                if w_index < len(cls.disk_wwns):
+                    extra_disk += cls.disk_wwns[w_index]
+
+            if len(cls.disk_serials):
+                w_index = disk_no + 1
+                if w_index < len(cls.disk_serials):
+                    extra_disk += cls.disk_serials[w_index]
+
+            disks.extend(['--disk', extra_disk])
 
         # build nvme disk args if needed
-        nvme_disks = []
         for (disk_no, disk_sz) in enumerate(cls.nvme_disks):
             dpath = os.path.join(cls.td.disks, 'nvme_disk_%d.img' % disk_no)
-            nvme_disks.extend(
-                ['--disk', '{}:{}:nvme:{}'.format(dpath, disk_sz,
-                                                  cls.disk_block_size)])
+            nvme_disk = '{}:{}:nvme:{}:{}'.format(dpath, disk_sz,
+                                                  cls.disk_block_size,
+                                                  "serial=nvme-%d" % disk_no)
+            disks.extend(['--disk', nvme_disk])
 
         # proxy config
         configs = [cls.conf_file]
@@ -485,11 +519,10 @@
             shutil.copy(OVMF_VARS, nvram)
             cmd.extend(["--uefi", nvram])
 
-        # --disk source:size:driver:block_size
-        target_disk = "{}:{}:{}:{}".format(cls.td.target_disk, "", "",
-                                           cls.disk_block_size)
-        cmd.extend(netdevs + ["--disk", target_disk] +
-                   extra_disks + nvme_disks +
+        if cls.multipath:
+            disks = disks * cls.multipath_num_paths
+
+        cmd.extend(netdevs + disks +
                    [boot_img, "--kernel=%s" % boot_kernel, "--initrd=%s" %
                     boot_initrd, "--", "curtin", "-vv", "install"] +
                    ["--config=%s" % f for f in configs] +
@@ -535,39 +568,58 @@
             cls.tearDownClass()
             raise
 
-        # drop the size parameter if present in extra_disks
-        extra_disks = [x if ":" not in x else x.split(':')[0]
-                       for x in extra_disks]
         # create --disk params for nvme disks
         bsize_args = "logical_block_size={}".format(cls.disk_block_size)
         bsize_args += ",physical_block_size={}".format(cls.disk_block_size)
         bsize_args += ",min_io_size={}".format(cls.disk_block_size)
-        disk_driver = "virtio-blk"
 
         target_disks = []
-        for (disk_no, disk) in enumerate([cls.td.target_disk,
-                                          cls.td.output_disk]):
-            d = '--disk={},driver={},format={},{}'.format(disk, disk_driver,
-                                                          TARGET_IMAGE_FORMAT,
-                                                          bsize_args)
-            target_disks.extend([d])
+        for (disk_no, disk) in enumerate([cls.td.target_disk]):
+            disk = '--disk={},driver={},format={},{}'.format(
+                disk, cls.disk_driver, TARGET_IMAGE_FORMAT, bsize_args)
+            if len(cls.disk_wwns):
+                disk += ",%s" % cls.disk_wwns[0]
+            if len(cls.disk_serials):
+                disk += ",%s" % cls.disk_serials[0]
+
+            target_disks.extend([disk])
 
         extra_disks = []
         for (disk_no, disk_sz) in enumerate(cls.extra_disks):
             dpath = os.path.join(cls.td.disks, 'extra_disk_%d.img' % disk_no)
-            d = '--disk={},driver={},format={},{}'.format(dpath, disk_driver,
-                                                          TARGET_IMAGE_FORMAT,
-                                                          bsize_args)
-            extra_disks.extend([d])
+            disk = '--disk={},driver={},format={},{}'.format(
+                dpath, cls.disk_driver, TARGET_IMAGE_FORMAT, bsize_args)
+            if len(cls.disk_wwns):
+                w_index = disk_no + 1
+                if w_index < len(cls.disk_wwns):
+                    disk += ",%s" % cls.disk_wwns[w_index]
+
+            if len(cls.disk_serials):
+                w_index = disk_no + 1
+                if w_index < len(cls.disk_serials):
+                    disk += ",%s" % cls.disk_serials[w_index]
+
+            extra_disks.extend([disk])
 
         nvme_disks = []
         disk_driver = 'nvme'
         for (disk_no, disk_sz) in enumerate(cls.nvme_disks):
             dpath = os.path.join(cls.td.disks, 'nvme_disk_%d.img' % disk_no)
-            d = '--disk={},driver={},format={},{}'.format(dpath, disk_driver,
-                                                          TARGET_IMAGE_FORMAT,
-                                                          bsize_args)
-            nvme_disks.extend([d])
+            disk = '--disk={},driver={},format={},{}'.format(
+                dpath, disk_driver, TARGET_IMAGE_FORMAT, bsize_args)
+            nvme_disks.extend([disk])
+
+        if cls.multipath:
+            target_disks = target_disks * cls.multipath_num_paths
+            extra_disks = extra_disks * cls.multipath_num_paths
+            nvme_disks = nvme_disks * cls.multipath_num_paths
+
+        # output disk is always virtio-blk, with serial of output_disk.img
+        output_disk = '--disk={},driver={},format={},{},{}'.format(
+            cls.td.output_disk, 'virtio-blk',
+            TARGET_IMAGE_FORMAT, bsize_args,
+            'serial=%s' % os.path.basename(cls.td.output_disk))
+        target_disks.extend([output_disk])
 
         # create xkvm cmd
         cmd = (["tools/xkvm", "-v", dowait] + netdevs +
@@ -951,9 +1003,9 @@
               'content': yaml.dump(base_cloudconfig, indent=1)},
              {'type': 'text/cloud-config', 'content': ssh_keys}]
 
+    output_dir = '/mnt/output'
     output_dir_macro = 'OUTPUT_COLLECT_D'
-    output_dir = '/mnt/output'
-    output_device = '/dev/vdb'
+    output_device = '/dev/disk/by-id/virtio-%s' % OUTPUT_DISK_NAME
 
     collect_prep = textwrap.dedent("mkdir -p " + output_dir)
     collect_post = textwrap.dedent(
@@ -961,7 +1013,7 @@
 
     # failsafe poweroff runs on precise only, where power_state does
     # not exist.
-    precise_poweroff = textwrap.dedent("""#!/bin/sh
+    precise_poweroff = textwrap.dedent("""#!/bin/sh -x
         [ "$(lsb_release -sc)" = "precise" ] || exit 0;
         shutdown -P now "Shutting down on precise"
         """)
@@ -971,7 +1023,7 @@
 
     for part in scripts:
         if not part.startswith("#!"):
-            part = "#!/bin/sh\n" + part
+            part = "#!/bin/sh -x\n" + part
         part = part.replace(output_dir_macro, output_dir)
         logger.debug('Cloud config archive content (pre-json):' + part)
         parts.append({'content': part, 'type': 'text/x-shellscript'})

=== modified file 'tests/vmtests/test_basic.py'
--- tests/vmtests/test_basic.py	2016-06-13 20:49:15 +0000
+++ tests/vmtests/test_basic.py	2016-06-24 19:28:31 +0000
@@ -233,3 +233,120 @@
 
 class YakketyTestBasic(relbase.yakkety, TestBasicAbs):
     __test__ = True
+
+
+class TestBasicScsiAbs(TestBasicAbs):
+    conf_file = "examples/tests/basic_scsi.yaml"
+    disk_driver = 'scsi-hd'
+    extra_disks = ['128G', '128G', '4G']
+    nvme_disks = ['4G']
+    collect_scripts = [textwrap.dedent("""
+        cd OUTPUT_COLLECT_D
+        blkid -o export /dev/sda > blkid_output_sda
+        blkid -o export /dev/sda1 > blkid_output_sda1
+        blkid -o export /dev/sda2 > blkid_output_sda2
+        btrfs-show-super /dev/sdc > btrfs_show_super_sdc
+        cat /proc/partitions > proc_partitions
+        ls -al /dev/disk/by-uuid/ > ls_uuid
+        ls -al /dev/disk/by-id/ > ls_disk_id
+        cat /etc/fstab > fstab
+        mkdir -p /dev/disk/by-dname
+        ls /dev/disk/by-dname/ > ls_dname
+        find /etc/network/interfaces.d > find_interfacesd
+
+        v=""
+        out=$(apt-config shell v Acquire::HTTP::Proxy)
+        eval "$out"
+        echo "$v" > apt-proxy
+        """)]
+
+    def test_output_files_exist(self):
+        self.output_files_exist(
+            ["blkid_output_sda", "blkid_output_sda1", "blkid_output_sda2",
+             "btrfs_show_super_sdc", "fstab", "ls_dname", "ls_uuid",
+             "ls_disk_id", "proc_partitions"])
+
+    def test_ptable(self):
+        blkid_info = self.get_blkid_data("blkid_output_sda")
+        self.assertEquals(blkid_info["PTTYPE"], "dos")
+
+    def test_partition_numbers(self):
+        # vde should have partitions 1 and 10
+        disk = "sdd"
+        proc_partitions_path = os.path.join(self.td.collect,
+                                            'proc_partitions')
+        self.assertTrue(os.path.exists(proc_partitions_path))
+        found = []
+        with open(proc_partitions_path, 'r') as fp:
+            for line in fp.readlines():
+                if disk in line:
+                    found.append(line.split()[3])
+        # /proc/partitions should have 3 lines with 'vde' in them.
+        expected = [disk + s for s in ["", "1", "10"]]
+        self.assertEqual(found, expected)
+
+    def test_partitions(self):
+        with open(os.path.join(self.td.collect, "fstab")) as fp:
+            fstab_lines = fp.readlines()
+        print("\n".join(fstab_lines))
+        # Test that vda1 is on /
+        blkid_info = self.get_blkid_data("blkid_output_sda1")
+        fstab_entry = None
+        for line in fstab_lines:
+            if blkid_info['UUID'] in line:
+                fstab_entry = line
+                break
+        self.assertIsNotNone(fstab_entry)
+        self.assertEqual(fstab_entry.split(' ')[1], "/")
+
+        # Test that vda2 is on /home
+        blkid_info = self.get_blkid_data("blkid_output_sda2")
+        fstab_entry = None
+        for line in fstab_lines:
+            if blkid_info['UUID'] in line:
+                fstab_entry = line
+                break
+        self.assertIsNotNone(fstab_entry)
+        self.assertEqual(fstab_entry.split(' ')[1], "/home")
+
+        # Test whole disk sdc is mounted at /btrfs
+        fstab_entry = None
+        for line in fstab_lines:
+            if "/dev/sdc" in line:
+                fstab_entry = line
+                break
+        self.assertIsNotNone(fstab_entry)
+        self.assertEqual(fstab_entry.split(' ')[1], "/btrfs")
+
+    def test_whole_disk_format(self):
+        # confirm the whole disk format is the expected device
+        with open(os.path.join(self.td.collect,
+                  "btrfs_show_super_sdc"), "r") as fp:
+            btrfs_show_super = fp.read()
+
+        with open(os.path.join(self.td.collect, "ls_uuid"), "r") as fp:
+            ls_uuid = fp.read()
+
+        # extract uuid from btrfs superblock
+        btrfs_fsid = [line for line in btrfs_show_super.split('\n')
+                      if line.startswith('fsid\t\t')]
+        self.assertEqual(len(btrfs_fsid), 1)
+        btrfs_uuid = btrfs_fsid[0].split()[1]
+        self.assertTrue(btrfs_uuid is not None)
+
+        # extract uuid from /dev/disk/by-uuid on /dev/sdc
+        # parsing ls -al output on /dev/disk/by-uuid:
+        # lrwxrwxrwx 1 root root   9 Dec  4 20:02
+        #  d591e9e9-825a-4f0a-b280-3bfaf470b83c -> ../../vdg
+        uuid = [line.split()[8] for line in ls_uuid.split('\n')
+                if 'sdc' in line]
+        self.assertEqual(len(uuid), 1)
+        uuid = uuid.pop()
+        self.assertTrue(uuid is not None)
+
+        # compare them
+        self.assertEqual(uuid, btrfs_uuid)
+
+
+class XenialTestScsiBasic(relbase.xenial, TestBasicScsiAbs):
+    __test__ = True

=== added file 'tests/vmtests/test_multipath.py'
--- tests/vmtests/test_multipath.py	1970-01-01 00:00:00 +0000
+++ tests/vmtests/test_multipath.py	2016-06-24 19:28:31 +0000
@@ -0,0 +1,63 @@
+from . import VMBaseClass
+from .releases import base_vm_classes as relbase
+
+import os
+import textwrap
+
+
+class TestMultipathBasicAbs(VMBaseClass):
+    conf_file = "examples/tests/multipath.yaml"
+    multipath = True
+    disk_driver = 'scsi-hd'
+    extra_disks = []
+    nvme_disks = []
+    collect_scripts = [textwrap.dedent("""
+        cd OUTPUT_COLLECT_D
+        blkid -o export /dev/sda > blkid_output_sda
+        blkid -o export /dev/sda1 > blkid_output_sda1
+        blkid -o export /dev/sda2 > blkid_output_sda2
+        blkid -o export /dev/sdb > blkid_output_sdb
+        blkid -o export /dev/sdb1 > blkid_output_sdb1
+        blkid -o export /dev/sdb2 > blkid_output_sdb2
+        dmsetup ls > dmsetup_ls
+        dmsetup info > dmsetup_info
+        cat /proc/partitions > proc_partitions
+        multipath -ll > multipath_ll
+        multipath -v3 -ll > multipath_v3_ll
+        multipath -r > multipath_r
+        cp -a /etc/multipath* .
+        ls -al /dev/disk/by-uuid/ > ls_uuid
+        ls -al /dev/disk/by-id/ > ls_disk_id
+        readlink -f /sys/class/block/sda/holders/dm-0 > holders_sda
+        readlink /sys/class/block/sdb/holders/dm-0 > holders_sdb
+        cat /etc/fstab > fstab
+        mkdir -p /dev/disk/by-dname
+        ls /dev/disk/by-dname/ > ls_dname
+        find /etc/network/interfaces.d > find_interfacesd
+        """)]
+
+    def test_multipath_disks_match(self):
+        sda = os.path.join(self.td.collect, 'holders_sda')
+        sdb = os.path.join(self.td.collect, 'holders_sdb')
+        self.assertTrue(os.path.exists(sda))
+        self.assertTrue(os.path.exists(sdb))
+        with open(sda, 'r') as fp:
+            sda_data = fp.read()
+            print('sda holders:\n%s' % sda_data)
+        with open(sda, 'r') as fp:
+            sdb_data = fp.read()
+            print('sdb holders:\n%s' % sda_data)
+
+        self.assertEqual(sda_data, sdb_data)
+
+
+class TrustyTestMultipathBasic(relbase.trusty, TestMultipathBasicAbs):
+    __test__ = True
+
+
+class XenialTestMultipathBasic(relbase.xenial, TestMultipathBasicAbs):
+    __test__ = True
+
+
+class YakketyTestMultipathBasic(relbase.yakkety, TestMultipathBasicAbs):
+    __test__ = True

=== modified file 'tools/launch'
--- tools/launch	2016-04-08 03:14:21 +0000
+++ tools/launch	2016-06-24 19:28:31 +0000
@@ -1,4 +1,4 @@
-#!/bin/bash
+#!/bin/bash -x
 
 VERBOSITY=0
 TEMP_D=""
@@ -23,11 +23,16 @@
            --add  F[:T] add file 'F' to the curtin archive at T
       -a | --append     append args to kernel cmdline (--kernel)
       -A | --arch   A   assume guest kernel architecture A
-      -d | --disk   D   add a disk 'D' format (path[:size][:driver][:bsize])
+      -d | --disk   D   add a disk 'D' format
+                        (path[:size][:driver][:bsize][:devopts])
                         driver can be specified without size using path::driver
                         driver defaults to virtio-blk
                         bsize <logical>[,<physical>][,<min_io_size>]
                         bsize defaults to 512b sector size
+                        opts is a comma delimitted list of property=value
+                        elements. Examine qemu-kvm -device scsi-hd,? for 
+                        details.
+           --vnc D      use -vnc D (mutually exclusive with --silent)
            --uefi   N   enable uefi boot method, store nvram at N
       -h | --help       show this message
       -i | --initrd F   use initramfs F
@@ -280,7 +285,7 @@
         case "$cur" in
                --add) addfiles[${#addfiles[@]}]="$next"; shift;;
             -a|--append) uappend="$next"; shift;;
-            -A|--arch) arch_hint="$next"; shift;;	       
+            -A|--arch) arch_hint="$next"; shift;;       
             -d|--disk) disks[${#disks[@]}]="$next"; shift;;
             -h|--help) Usage ; exit 0;;
             -i|--initrd) initrd="$next"; shift;;
@@ -388,20 +393,21 @@
         { error "failed to get dir for $0"; return 1; }
 
     local disk="" src="" size="" fmt="" out="" id="" driver="" if=""
-    local split_input=""
+    local split_input="" serial=""
     disk_args=( )
     id=1
     for disk in "${disks[@]}"; do
         ((id++))
-
         # 1=src
         # 2=src:size
         # 3=src:size:driver
         # 4=src:size:driver:bsize
+        # 5=src:size:driver:bsize:devopts
         src=$(echo $disk | awk -F: '{print $1}')
         size=$(echo $disk | awk -F: '{print $2}')
         driver=$(echo $disk | awk -F: '{print $3}')
         bsize=$(echo $disk | awk -F: '{print $4}')
+        devopts=$(echo $disk | awk -F: '{print $5}')
         
         if [ -z "${src}" ]; then
             error "Failed to provide disk source"
@@ -430,6 +436,11 @@
                 { error "failed to determine format of $src"; return 1; }
         fi
 
+        # prepend comma if passing devopts
+        if [ -n "${devopts}" ]; then
+            devopts=",${devopts}"
+        fi
+
         # set logical/physical size blocksz is logical:phys
         local logbs=$(round_up ${bsize%%:*})
         local phybs=$(round_up ${bsize##*:})
@@ -441,7 +452,7 @@
             "file=${src},if=none,cache=unsafe,format=$fmt,id=drv${id},index=$id" )
 
         disk_args=( "${disk_args[@]}" "-device"
-            "${driver},drive=drv${id},serial=dev${id},${bs_args}" )
+            "${driver},drive=drv${id},${bs_args}${devopts}" )
         
     done
 
@@ -557,7 +568,9 @@
         # if this is a partition image, root=/dev/vda. else root=/dev/vda1
         # this hack is necessary because LABEL even UUID  might be the same
         # in the boot image and the target (if re-using target)
-        if tmp=$(blkid "$bootimg_dist" -ovalue -s UUID) && [ -n "$tmp" ]; then
+        local bottom=$(qemu-img info --backing-chain "$bootimg_dist" |
+                       grep ^image | tail -n1 | awk '{print $2}')
+        if tmp=$(blkid "$bottom" -ovalue -s UUID) && [ -n "$tmp" ]; then
             root="/dev/vda"
         else
             root="/dev/vda1"
@@ -586,17 +599,19 @@
     local cmd serial_args="" chardev_arg=""
     [ "${serial_log}" = "none" ] && serial_log=""
     if [ -n "${serial_log}" ]; then
-	if [ "${arch_hint}" = "s390x" ]; then
-	    if [ "${serial_log}" = "stdio" ]; then
-		chardev_arg="stdio"
-	    else
-		chardev_arg="file,path=${serial_log}"
-	    fi
-	    serial_args="-nodefaults -chardev ${chardev_arg},id=charconsole0 -device sclpconsole,chardev=charconsole0,id=console0"
-	else
-	    serial_args="-serial file:${serial_log}"
-	fi
+        if [ "${arch_hint}" = "s390x" ]; then
+            if [ "${serial_log}" = "stdio" ]; then
+                chardev_arg="stdio"
+            else
+                chardev_arg="file,path=${serial_log}"
+            fi
+                serial_args="-nodefaults -chardev ${chardev_arg},id=charconsole0 -device sclpconsole,chardev=charconsole0,id=console0"
+        else
+            serial_args="-serial file:${serial_log}"
+            #debug mode serial_args="-serial ${serial_log} -monitor stdio"
+        fi
     fi
+    # -monitor stdio 
     cmd=( 
         xkvm "${pt[@]}" "${netargs[@]}" --
         "${bios_opts[@]}"
@@ -625,6 +640,11 @@
     return $ret
 }
 
+random_wwn() {
+    # wwn must be a int64, less than (1 << 63) - 1
+    # we achieve this by combining 4 (1 << 15) ints
+    printf "0x%04x%04x%04x%04x" $RANDOM $RANDOM $RANDOM $RANDOM
+}
 
 round_up() {
     local size="${1}"