Merge branch 'develop' into unicoded-wildcard-route53

.ci/docs

@@ -22,7 +22,7 @@ pipeline {
         stage('build') {
             steps {
                 sh 'eval "$(pyenv init -)"; make -C doc clean html'
-                archiveArtifacts artifacts: 'doc/_build/html'
+                archiveArtifacts artifacts: 'doc/_build/html/'
             }
         }
     }

doc/conf.py

@@ -129,55 +129,56 @@ MOCK_MODULES = [
     # modules, renderers, states, returners, et al
     'ClusterShell',
     'ClusterShell.NodeSet',
-    'django',
-    'libvirt',
     'MySQLdb',
     'MySQLdb.cursors',
-    'nagios_json',
-    'psutil',
-    'pycassa',
-    'pymongo',
-    'rabbitmq_server',
-    'redis',
-    #'requests',
-    #'requests.exceptions',
-    'rpm',
-    'rpmUtils',
-    'rpmUtils.arch',
-    'yum',
     'OpenSSL',
-    'zfs',
-    'salt.ext.six.moves.winreg',
-    'win32security',
-    'ntsecuritycon',
-    'napalm',
+    'avahi',
+    'boto.regioninfo',
+    'concurrent',
+    'dbus',
+    'django',
+    'dns',
+    'dns.resolver',
     'dson',
     'hjson',
     'jnpr',
-    'json',
-    'lxml',
-    'lxml.etree',
     'jnpr.junos',
     'jnpr.junos.utils',
     'jnpr.junos.utils.config',
     'jnpr.junos.utils.sw',
-    'dns',
-    'dns.resolver',
+    'json',
     'keyring',
+    'libvirt',
+    'lxml',
+    'lxml.etree',
+    'msgpack',
+    'nagios_json',
+    'napalm',
     'netaddr',
     'netaddr.IPAddress',
     'netaddr.core',
     'netaddr.core.AddrFormatError',
+    'ntsecuritycon',
+    'psutil',
+    'pycassa',
+    'pyconnman',
+    'pyiface',
+    'pymongo',
     'pyroute2',
     'pyroute2.ipdb',
-    'avahi',
-    'dbus',
+    'rabbitmq_server',
+    'redis',
+    'rpm',
+    'rpmUtils',
+    'rpmUtils.arch',
+    'salt.ext.six.moves.winreg',
     'twisted',
     'twisted.internet',
     'twisted.internet.protocol',
     'twisted.internet.protocol.DatagramProtocol',
-    'msgpack',
-    'boto.regioninfo',
+    'win32security',
+    'yum',
+    'zfs',
 ]
 
 for mod_name in MOCK_MODULES:
@@ -255,7 +256,7 @@ on_saltstack = 'SALT_ON_SALTSTACK' in os.environ
 project = 'Salt'
 
 version = salt.version.__version__
-latest_release = '2018.3.1'  # latest release
+latest_release = '2018.3.2'  # latest release
 previous_release = '2017.7.6'  # latest release from previous branch
 previous_release_dir = '2017.7'  # path on web server for previous branch
 next_release = ''  # next release

doc/ref/proxy/all/salt.proxy.netmiko_px.rst

@@ -1,6 +1,6 @@
-==================
-salt.proxy.netmiko
-==================
+=====================
+salt.proxy.netmiko_px
+=====================
 
 .. automodule:: salt.proxy.netmiko_px
     :members:

doc/ref/sdb/all/index.rst

@@ -18,6 +18,7 @@ sdb modules
     etcd_db
     keyring_db
     memcached
+    redis_sdb
     rest
     sqlite3
     tism

doc/ref/sdb/all/salt.sdb.redis_sdb.rst

@@ -0,0 +1,5 @@
+salt.sdb.redis_sdb module
+=========================
+
+.. automodule:: salt.sdb.redis_sdb
+    :members:

doc/topics/cloud/aws.rst

@@ -472,6 +472,19 @@ EC2 API or AWS Console.
       spot_config:
         spot_price: 0.10
 
+You can optionally specify tags to apply to the EC2 spot instance request.
+A spot instance request itself is an object in AWS. The following example
+will set two tags on the spot instance request.
+
+.. code-block:: yaml
+
+    my-ec2-config:
+      spot_config:
+        spot_price: 0.10
+        tag:
+          tag0: value
+          tag1: value
+
 By default, the spot instance type is set to 'one-time', meaning it will
 be launched and, if it's ever terminated for whatever reason, it will not
 be recreated. If you would like your spot instances to be relaunched after

doc/topics/releases/2018.3.2.rst

@@ -1,9 +1,8 @@
-========================================
-In Progress: Salt 2018.3.2 Release Notes
-========================================
+===========================
+Salt 2018.3.2 Release Notes
+===========================
 
-Version 2018.3.2 is an **unreleased** bugfix release for :ref:`2018.3.0 <release-2018-3-0>`.
-This release is still in progress and has not been released yet.
+Version 2018.3.2 is a bugfix release for :ref:`2018.3.0 <release-2018-3-0>`.
 
 The ``2018.3.2`` release contains only a small number of fixes, detailed below.
 

doc/topics/tutorials/cloud_controller.rst

@@ -246,7 +246,7 @@ the new virtual machine on. Using ``salt://`` assumes that the CentOS virtual
 machine image is located in the root of the :ref:`file-server` on the master.
 When images are cloned (i.e. copied locatlly after retrieval from the file server)
 the destination directory on the hypervisor minion is determined by the ``virt:images``
-config option; by default this is ``/srv/salt/salt-images/``.
+config option; by default this is ``/srv/salt-images/``.
 
 When a VM is initialized using ``virt.init`` the image is copied to the hypervisor
 using ``cp.cache_file`` and will be mounted and seeded with a minion. Seeding includes

salt/cloud/clouds/ec2.py

@@ -2059,6 +2059,8 @@ def request_instance(vm_=None, call=None):
     if spot_config:
         sir_id = data[0]['spotInstanceRequestId']
 
+        vm_['spotRequestId'] = sir_id
+
         def __query_spot_instance_request(sir_id, location):
             params = {'Action': 'DescribeSpotInstanceRequests',
                       'SpotInstanceRequestId.1': sir_id}
@@ -2681,6 +2683,51 @@ def create(vm_=None, call=None):
         location=location
     )
 
+    # Once instance tags are set, tag the spot request if configured
+    if 'spot_config' in vm_ and 'tag' in vm_['spot_config']:
+
+        if not isinstance(vm_['spot_config']['tag'], dict):
+            raise SaltCloudConfigError(
+                '\'tag\' should be a dict.'
+            )
+
+        for value in six.itervalues(vm_['spot_config']['tag']):
+            if not isinstance(value, str):
+                raise SaltCloudConfigError(
+                    '\'tag\' values must be strings. Try quoting the values. '
+                    'e.g. "2013-09-19T20:09:46Z".'
+                )
+
+        spot_request_tags = {}
+
+        if 'spotRequestId' not in vm_:
+            raise SaltCloudConfigError('Failed to find spotRequestId')
+
+        sir_id = vm_['spotRequestId']
+
+        spot_request_tags['Name'] = vm_['name']
+
+        for k, v in six.iteritems(vm_['spot_config']['tag']):
+            spot_request_tags[k] = v
+
+        __utils__['cloud.fire_event'](
+            'event',
+            'setting tags',
+            'salt/cloud/spot_request_{0}/tagging'.format(sir_id),
+            args={'tags': spot_request_tags},
+            sock_dir=__opts__['sock_dir'],
+            transport=__opts__['transport']
+        )
+        salt.utils.cloud.wait_for_fun(
+            set_tags,
+            timeout=30,
+            name=vm_['name'],
+            tags=spot_request_tags,
+            instance_id=sir_id,
+            call='action',
+            location=location
+        )
+
     network_interfaces = config.get_cloud_config_value(
         'network_interfaces',
         vm_,

salt/modules/netmiko_mod.py

@@ -36,8 +36,7 @@ in a configuration block under the ``netmiko`` key in the configuration opts
 simultaneously. These fields are the exact same supported by the ``netmiko``
 Proxy Module:
 
-device_type
-    Class selection based on device type. Supported options:
+- ``device_type`` - Class selection based on device type. Supported options:
 
   - ``a10``: A10 Networks
   - ``accedian``: Accedian Networks
@@ -110,70 +109,54 @@ device_type
   - ``ruckus_fastiron_telnet``: Ruckus Fastiron over Telnet
   - ``cisco_ios_serial``: Cisco IOS over serial port
 
-ip
-  IP address of target device. Not required if ``host`` is provided.
+- ``ip`` - IP address of target device (not required if ``host`` is provided)
 
-host
-  Hostname of target device. Not required if ``ip`` is provided.
+- ``host`` - Hostname of target device (not required if ``ip`` is provided)
 
-username
-  Username to authenticate against target device if required.
+- ``username`` - Username to authenticate against target device, if required
 
-password
-  Password to authenticate against target device if required.
+- ``password`` - Password to authenticate against target device, if required
 
-secret
-  The enable password if target device requires one.
+- ``secret`` - The enable password if target device requires one
 
-port
-  The destination port used to connect to the target device.
+- ``port`` - The destination port used to connect to the target device
 
-global_delay_factor: ``1``
-  Multiplication factor affecting Netmiko delays (default: ``1``).
+- ``global_delay_factor`` - Multiplication factor affecting Netmiko delays
+  (default: ``1``)
 
-use_keys: ``False``
-  Connect to target device using SSH keys.
+- ``use_keys`` - Connect to target device using SSH keys (default: ``False``)
 
-key_file
-  Filename path of the SSH key file to use.
+- ``key_file`` - Filename path of the SSH key file to use
 
-allow_agent
-  Enable use of SSH key-agent.
+- ``allow_agent`` - Enable use of SSH key-agent
 
-ssh_strict: ``False``
-  Automatically reject unknown SSH host keys (default: ``False``, which means
-  unknown SSH host keys will be accepted).
+- ``ssh_strict`` - Automatically reject unknown SSH host keys (default:
+  ``False``, which means unknown SSH host keys will be accepted)
 
-system_host_keys: ``False``
-  Load host keys from the user's 'known_hosts' file.
+- ``system_host_keys`` - Load host keys from the user's "known_hosts" file
+  (default: ``False``)
 
-alt_host_keys: ``False``
-  If ``True`` host keys will be loaded from the file specified in
-  ``alt_key_file``.
+- ``alt_host_keys`` - If ``True``,  host keys will be loaded from the file
+  specified in ``alt_key_file`` (default: ``False``)
 
-alt_key_file
-  SSH host key file to use (if ``alt_host_keys=True``).
+- ``alt_key_file`` - SSH host key file to use (if ``alt_host_keys=True``)
 
-ssh_config_file
-  File name of OpenSSH configuration file.
+- ``ssh_config_file`` - File name of OpenSSH configuration file
 
-timeout: ``90``
-  Connection timeout (in seconds).
+- ``timeout`` - Connection timeout, in seconds (default: ``90``)
 
-session_timeout: ``60``
-  Set a timeout for parallel requests (in seconds).
+- ``session_timeout`` - Set a timeout for parallel requests, in seconds
+  (default: ``60``)
 
-keepalive: ``0``
-  Send SSH keepalive packets at a specific interval, in seconds. Currently
-  defaults to ``0``, for backwards compatibility (it will not attempt to keep
-  the connection alive using the KEEPALIVE packets).
+- ``keepalive`` - Send SSH keepalive packets at a specific interval, in
+  seconds. Currently defaults to ``0``, for backwards compatibility (it will
+  not attempt to keep the connection alive using the KEEPALIVE packets).
 
-default_enter: ``\n``
-  Character(s) to send to correspond to enter key (default: ``\n``).
+- ``default_enter`` - Character(s) to send to correspond to enter key (default:
+  ``\\n``)
 
-response_return: ``\n``
-    Character(s) to use in normalized return data to represent enter key
-    (default: ``\n``)
+- ``response_return`` - Character(s) to use in normalized return data to
+  represent enter key (default: ``\\n``)
 
 Example (when not running in a ``netmiko`` Proxy Minion):
 

salt/modules/nilrt_ip.py

@@ -8,7 +8,6 @@ The networking module for NI Linux Real-Time distro
 from __future__ import absolute_import, print_function, unicode_literals
 import logging
 import time
-import configparser
 import os
 
 # Import salt libs
@@ -18,6 +17,7 @@ import salt.exceptions
 
 # Import 3rd-party libs
 from salt.ext import six
+from salt.ext.six.moves import configparser
 try:
     import pyconnman
     HAS_PYCONNMAN = True

salt/modules/smtp.py

@@ -91,14 +91,9 @@ def send_msg(recipient,
 
     .. code-block:: bash
 
-        smtp.send_msg 'admin@example.com' 'This is a salt module test' \
-            profile='my-smtp-account'
-        smtp.send_msg 'admin@example.com' 'This is a salt module test' \
-            username='myuser' password='verybadpass' sender="admin@example.com' \
-            server='smtp.domain.com'
-        smtp.send_msg 'admin@example.com' 'This is a salt module test' \
-            username='myuser' password='verybadpass' sender="admin@example.com' \
-            server='smtp.domain.com' attachments="['/var/log/messages']"
+        salt '*' smtp.send_msg 'admin@example.com' 'This is a salt module test' profile='my-smtp-account'
+        salt '*' smtp.send_msg 'admin@example.com' 'This is a salt module test' username='myuser' password='verybadpass' sender='admin@example.com' server='smtp.domain.com'
+        salt '*' smtp.send_msg 'admin@example.com' 'This is a salt module test' username='myuser' password='verybadpass' sender='admin@example.com' server='smtp.domain.com' attachments="['/var/log/messages']"
     '''
     if profile:
         creds = __salt__['config.option'](profile)

salt/modules/virt.py

@@ -57,7 +57,6 @@ whatever the ``virt:connection`` is.
 
 The calls not using the libvirt connection setup are:
 
-- ``get_profiles``
 - ``seed_non_shared_migrate``
 - ``virt_type``
 - ``is_*hyper``
@@ -102,6 +101,7 @@ except ImportError:
 
 # Import salt libs
 import salt.utils.files
+import salt.utils.json
 import salt.utils.network
 import salt.utils.path
 import salt.utils.stringutils
@@ -130,8 +130,6 @@ VIRT_STATE_NAME_MAP = {0: 'running',
                        5: 'shutdown',
                        6: 'crashed'}
 
-VIRT_DEFAULT_HYPER = 'kvm'
-
 
 def __virtual__():
     if not HAS_LIBVIRT:
@@ -247,6 +245,14 @@ def __get_conn(**kwargs):
     return conn
 
 
+def _get_domain_types(**kwargs):
+    '''
+    Return the list of possible values for the <domain> type attribute.
+    '''
+    caps = capabilities(**kwargs)
+    return sorted(set([x for y in [guest['arch']['domains'].keys() for guest in caps['guests']] for x in y]))
+
+
 def _get_domain(conn, *vms, **kwargs):
     '''
     Return a domain object for the named VM or return domain object for all VMs.
@@ -287,48 +293,43 @@ def _get_domain(conn, *vms, **kwargs):
 
 def _parse_qemu_img_info(info):
     '''
-    Parse qemu-img info output into disk infos YAML
+    Parse qemu-img info JSON output into disk infos dictionary
     '''
-    output = []
-    snapshots = False
-    columns = None
-    lines = info.strip().split('\n')
-    for line in lines:
-        if line.startswith('Snapshot list:'):
-            snapshots = True
-            continue
+    raw_infos = salt.utils.json.loads(info)
+    disks = []
+    for disk_infos in raw_infos:
+        disk = {
+                   'file': disk_infos['filename'],
+                   'file format': disk_infos['format'],
+                   'disk size': disk_infos['actual-size'],
+                   'virtual size': disk_infos['virtual-size'],
+                   'cluster size': disk_infos['cluster-size']
+               }
 
-        # If this is a copy-on-write image, then the backing file
-        # represents the base image
-        #
-        # backing file: base.qcow2 (actual path: /var/shared/base.qcow2)
-        elif line.startswith('backing file'):
-            matches = re.match(r'.*\(actual path: (.*?)\)', line)
-            if matches:
-                output.append('backing file: {0}'.format(matches.group(1)))
-            continue
+        if 'full-backing-filename' in disk_infos.keys():
+            disk['backing file'] = format(disk_infos['full-backing-filename'])
 
-        elif snapshots:
-            if line.startswith('ID'):  # Do not parse table headers
-                line = line.replace('VM SIZE', 'VMSIZE')
-                line = line.replace('VM CLOCK', 'TIME VMCLOCK')
-                columns = re.split(r'\s+', line)
-                columns = [c.lower() for c in columns]
-                output.append('snapshots:')
-                continue
-            fields = re.split(r'\s+', line)
-            for i, field in enumerate(fields):
-                sep = ' '
-                if i == 0:
-                    sep = '-'
-                output.append(
-                    '{0} {1}: "{2}"'.format(
-                        sep, columns[i], field
-                    )
-                )
-            continue
-        output.append(line)
-    return '\n'.join(output)
+        if 'snapshots' in disk_infos.keys():
+            disk['snapshots'] = [
+                    {
+                        'id': snapshot['id'],
+                        'tag': snapshot['name'],
+                        'vmsize': snapshot['vm-state-size'],
+                        'date': datetime.datetime.fromtimestamp(
+                            float('{}.{}'.format(snapshot['date-sec'], snapshot['date-nsec']))).isoformat(),
+                        'vmclock': datetime.datetime.utcfromtimestamp(
+                            float('{}.{}'.format(snapshot['vm-clock-sec'],
+                                                 snapshot['vm-clock-nsec']))).time().isoformat()
+                    } for snapshot in disk_infos['snapshots']]
+        disks.append(disk)
+
+    for disk in disks:
+        if 'backing file' in disk.keys():
+            candidates = [info for info in disks if 'file' in info.keys() and info['file'] == disk['backing file']]
+            if candidates:
+                disk['backing file'] = candidates[0]
+
+    return disks[0]
 
 
 def _get_uuid(dom):
@@ -485,25 +486,26 @@ def _get_disks(dom):
                 qemu_target = '{0}:{1}'.format(
                         source.getAttribute('protocol'),
                         source.getAttribute('name'))
-            if qemu_target:
-                disks[target.getAttribute('dev')] = {
-                    'file': qemu_target,
-                    'type': elem.getAttribute('device')}
-    for dev in disks:
-        try:
-            hypervisor = __salt__['config.get']('libvirt:hypervisor', 'kvm')
-            if hypervisor not in ['qemu', 'kvm']:
-                break
+            if not qemu_target:
+                continue
 
+            disk = {'file': qemu_target,
+                    'type': elem.getAttribute('device')}
+
+            driver = _get_xml_first_element_by_tag_name(elem, 'driver')
+            if driver and driver.getAttribute('type') == 'qcow2':
+                try:
                     stdout = subprocess.Popen(
-                        ['qemu-img', 'info', disks[dev]['file']],
+                                ['qemu-img', 'info', '--output', 'json', '--backing-chain', disk['file']],
                                 shell=False,
                                 stdout=subprocess.PIPE).communicate()[0]
                     qemu_output = salt.utils.stringutils.to_str(stdout)
                     output = _parse_qemu_img_info(qemu_output)
-            disks[dev].update(salt.utils.yaml.safe_load(output))
+                    disk.update(output)
                 except TypeError:
-            disks[dev].update({'image': 'Does not exist'})
+                    disk.update({'file': 'Does not exist'})
+
+            disks[target.getAttribute('dev')] = disk
     return disks
 
 
@@ -565,11 +567,11 @@ def _gen_xml(name,
              diskp,
              nicp,
              hypervisor,
+             graphics=None,
              **kwargs):
     '''
     Generate the XML string to define a libvirt VM
     '''
-    hypervisor = 'vmware' if hypervisor == 'esxi' else hypervisor
     mem = int(mem) * 1024  # MB
     context = {
         'hypervisor': hypervisor,
@@ -579,11 +581,17 @@ def _gen_xml(name,
     }
     if hypervisor in ['qemu', 'kvm']:
         context['controller_model'] = False
-    elif hypervisor in ['esxi', 'vmware']:
+    elif hypervisor == 'vmware':
         # TODO: make bus and model parameterized, this works for 64-bit Linux
         context['controller_model'] = 'lsilogic'
 
-    context['enable_vnc'] = bool(kwargs.get('enable_vnc', True))
+    # By default, set the graphics to listen to all addresses
+    if graphics:
+        if 'listen' not in graphics:
+            graphics['listen'] = {'type': 'address', 'address': '0.0.0.0'}
+        elif 'address' not in graphics['listen'] and graphics['listen']['type'] == 'address':
+            graphics['listen']['address'] = '0.0.0.0'
+    context['graphics'] = graphics
 
     if 'boot_dev' in kwargs:
         context['boot_dev'] = []
@@ -607,24 +615,20 @@ def _gen_xml(name,
 
     context['disks'] = {}
     for i, disk in enumerate(diskp):
-        for disk_name, args in six.iteritems(disk):
-            context['disks'][disk_name] = {}
-            fn_ = '{0}.{1}'.format(disk_name, args['format'])
-            context['disks'][disk_name]['file_name'] = fn_
-            context['disks'][disk_name]['source_file'] = os.path.join(args['pool'],
-                                                                      name,
-                                                                      fn_)
+        context['disks'][disk['name']] = {}
+        context['disks'][disk['name']]['file_name'] = disk['filename']
+        context['disks'][disk['name']]['source_file'] = disk['source_file']
         if hypervisor in ['qemu', 'kvm']:
-                context['disks'][disk_name]['target_dev'] = 'vd{0}'.format(string.ascii_lowercase[i])
-                context['disks'][disk_name]['address'] = False
-                context['disks'][disk_name]['driver'] = True
+            context['disks'][disk['name']]['target_dev'] = 'vd{0}'.format(string.ascii_lowercase[i])
+            context['disks'][disk['name']]['address'] = False
+            context['disks'][disk['name']]['driver'] = True
         elif hypervisor in ['esxi', 'vmware']:
-                context['disks'][disk_name]['target_dev'] = 'sd{0}'.format(string.ascii_lowercase[i])
-                context['disks'][disk_name]['address'] = True
-                context['disks'][disk_name]['driver'] = False
-            context['disks'][disk_name]['disk_bus'] = args['model']
-            context['disks'][disk_name]['type'] = args['format']
-            context['disks'][disk_name]['index'] = six.text_type(i)
+            context['disks'][disk['name']]['target_dev'] = 'sd{0}'.format(string.ascii_lowercase[i])
+            context['disks'][disk['name']]['address'] = True
+            context['disks'][disk['name']]['driver'] = False
+        context['disks'][disk['name']]['disk_bus'] = disk['model']
+        context['disks'][disk['name']]['type'] = disk['format']
+        context['disks'][disk['name']]['index'] = six.text_type(i)
 
     context['nics'] = nicp
 
@@ -730,32 +734,23 @@ def _get_images_dir():
     return img_dir
 
 
-def _qemu_image_create(vm_name,
-                       disk_file_name,
-                       disk_image=None,
-                       disk_size=None,
-                       disk_type='qcow2',
-                       enable_qcow=False,
-                       saltenv='base'):
+def _qemu_image_create(disk, create_overlay=False, saltenv='base'):
     '''
     Create the image file using specified disk_size or/and disk_image
 
     Return path to the created image file
     '''
+    disk_size = disk.get('size', None)
+    disk_image = disk.get('image', None)
+
     if not disk_size and not disk_image:
         raise CommandExecutionError(
             'Unable to create new disk {0}, please specify'
             ' disk size and/or disk image argument'
-            .format(disk_file_name)
+            .format(disk['filename'])
         )
 
-    img_dir = _get_images_dir()
-
-    img_dest = os.path.join(
-        img_dir,
-        vm_name,
-        disk_file_name
-    )
+    img_dest = disk['source_file']
     log.debug('Image destination will be %s', img_dest)
     img_dir = os.path.dirname(img_dest)
     log.debug('Image destination directory is %s', img_dir)
@@ -774,7 +769,7 @@ def _qemu_image_create(vm_name,
             imageinfo = salt.utils.yaml.safe_load(res)
             qcow2 = imageinfo['file format'] == 'qcow2'
         try:
-            if enable_qcow and qcow2:
+            if create_overlay and qcow2:
                 log.info('Cloning qcow2 image %s using copy on write', sfn)
                 __salt__['cmd.run'](
                     'qemu-img create -f qcow2 -o backing_file={0} {1}'
@@ -811,7 +806,7 @@ def _qemu_image_create(vm_name,
                 log.debug('Create empty image with size %sM', disk_size)
                 __salt__['cmd.run'](
                     'qemu-img create -f {0} {1} {2}M'
-                    .format(disk_type, img_dest, disk_size)
+                    .format(disk.get('format', 'qcow2'), img_dest, disk_size)
                 )
             else:
                 raise CommandExecutionError(
@@ -833,7 +828,7 @@ def _qemu_image_create(vm_name,
     return img_dest
 
 
-def _disk_profile(profile, hypervisor, **kwargs):
+def _disk_profile(profile, hypervisor, disks=None, vm_name=None, image=None, pool=None, **kwargs):
     '''
     Gather the disk profile from the config or apply the default based
     on the active hypervisor
@@ -873,43 +868,152 @@ def _disk_profile(profile, hypervisor, **kwargs):
     default to whatever is best suitable for the active hypervisor.
     '''
     default = [{'system':
-                {'size': '8192'}}]
-    if hypervisor in ['esxi', 'vmware']:
+                {'size': 8192}}]
+    if hypervisor == 'vmware':
         overlay = {'format': 'vmdk',
                    'model': 'scsi',
-                   'pool': '[{0}] '.format(kwargs.get('pool', '0'))}
+                   'pool': '[{0}] '.format(pool if pool else '0')}
     elif hypervisor in ['qemu', 'kvm']:
         overlay = {'format': 'qcow2',
-                   'model': 'virtio',
-                   'pool': _get_images_dir()}
+                   'model': 'virtio'}
     else:
         overlay = {}
 
+    # Get the disks from the profile
     disklist = copy.deepcopy(
         __salt__['config.get']('virt:disk', {}).get(profile, default))
+
+    # Transform the list to remove one level of dictionnary and add the name as a property
+    disklist = [dict(d, name=name) for disk in disklist for name, d in disk.items()]
+
+    # Add the image to the first disk if there is one
+    if image:
+        # If image is specified in module arguments, then it will be used
+        # for the first disk instead of the image from the disk profile
+        log.debug('%s image from module arguments will be used for disk "%s"'
+                  ' instead of %s', image, disklist[0]['name'], disklist[0].get('image', ""))
+        disklist[0]['image'] = image
+
+    # Merge with the user-provided disks definitions
+    if disks:
+        for udisk in disks:
+            if 'name' in udisk:
+                found = [disk for disk in disklist if udisk['name'] == disk['name']]
+                if found:
+                    found[0].update(udisk)
+                else:
+                    disklist.append(udisk)
+
+    for disk in disklist:
+        # Add the missing properties that have defaults
         for key, val in six.iteritems(overlay):
-        for i, disks in enumerate(disklist):
-            for disk in disks:
-                if key not in disks[disk]:
-                    disklist[i][disk][key] = val
+            if key not in disk:
+                disk[key] = val
+
+        base_dir = disk.get('pool', None)
+        if hypervisor in ['qemu', 'kvm']:
+            # Compute the base directory from the pool property. We may have either a path
+            # or a libvirt pool name there.
+            # If the pool is a known libvirt one with a target path, use it as target path
+            if not base_dir:
+                base_dir = _get_images_dir()
+            else:
+                if not base_dir.startswith('/'):
+                    # The pool seems not to be a path, lookup for pool infos
+                    pool = pool_info(base_dir, **kwargs)
+                    if not pool or not pool['target_path'] or pool['target_path'].startswith('/dev'):
+                        raise CommandExecutionError(
+                                    'Unable to create new disk {0}, specified pool {1} does not exist '
+                                    'or is unsupported'.format(disk['name'], base_dir))
+                    base_dir = pool['target_path']
+
+        # Compute the filename and source file properties if possible
+        if vm_name:
+            disk['filename'] = '{0}_{1}.{2}'.format(vm_name, disk['name'], disk['format'])
+            disk['source_file'] = os.path.join(base_dir, disk['filename'])
+
     return disklist
 
 
-def _nic_profile(profile_name, hypervisor, **kwargs):
+def _complete_nics(interfaces, hypervisor, dmac=None):
     '''
-    Compute NIC data based on profile
+    Complete missing data for network interfaces.
     '''
 
-    default = [{'eth0': {}}]
     vmware_overlay = {'type': 'bridge', 'source': 'DEFAULT', 'model': 'e1000'}
     kvm_overlay = {'type': 'bridge', 'source': 'br0', 'model': 'virtio'}
     overlays = {
             'kvm': kvm_overlay,
             'qemu': kvm_overlay,
-            'esxi': vmware_overlay,
             'vmware': vmware_overlay,
             }
 
+    def _normalize_net_types(attributes):
+        '''
+        Guess which style of definition:
+
+            bridge: br0
+
+             or
+
+            network: net0
+
+             or
+
+            type: network
+            source: net0
+        '''
+        for type_ in ['bridge', 'network']:
+            if type_ in attributes:
+                attributes['type'] = type_
+                # we want to discard the original key
+                attributes['source'] = attributes.pop(type_)
+
+        attributes['type'] = attributes.get('type', None)
+        attributes['source'] = attributes.get('source', None)
+
+    def _apply_default_overlay(attributes):
+        '''
+        Apply the default overlay to attributes
+        '''
+        for key, value in six.iteritems(overlays[hypervisor]):
+            if key not in attributes or not attributes[key]:
+                attributes[key] = value
+
+    def _assign_mac(attributes, hypervisor):
+        '''
+        Compute mac address for NIC depending on hypervisor
+        '''
+        if dmac is not None:
+            log.debug('Default MAC address is %s', dmac)
+            if salt.utils.validate.net.mac(dmac):
+                attributes['mac'] = dmac
+            else:
+                msg = 'Malformed MAC address: {0}'.format(dmac)
+                raise CommandExecutionError(msg)
+        else:
+            if hypervisor in ['qemu', 'kvm']:
+                attributes['mac'] = salt.utils.network.gen_mac(
+                    prefix='52:54:00')
+            else:
+                attributes['mac'] = salt.utils.network.gen_mac()
+
+    for interface in interfaces:
+        _normalize_net_types(interface)
+        _assign_mac(interface, hypervisor)
+        if hypervisor in overlays:
+            _apply_default_overlay(interface)
+
+    return interfaces
+
+
+def _nic_profile(profile_name, hypervisor, dmac=None):
+    '''
+    Compute NIC data based on profile
+    '''
+
+    default = [{'eth0': {}}]
+
     # support old location
     config_data = __salt__['config.option']('virt.nic', {}).get(
         profile_name, None
@@ -971,64 +1075,8 @@ def _nic_profile(profile_name, hypervisor, **kwargs):
                 else:
                     interfaces.append(interface)
 
-    def _normalize_net_types(attributes):
-        '''
-        Guess which style of definition:
-
-            bridge: br0
-
-             or
-
-            network: net0
-
-             or
-
-            type: network
-            source: net0
-        '''
-        for type_ in ['bridge', 'network']:
-            if type_ in attributes:
-                attributes['type'] = type_
-                # we want to discard the original key
-                attributes['source'] = attributes.pop(type_)
-
-        attributes['type'] = attributes.get('type', None)
-        attributes['source'] = attributes.get('source', None)
-
-    def _apply_default_overlay(attributes):
-        '''
-        Apply the default overlay to attributes
-        '''
-        for key, value in six.iteritems(overlays[hypervisor]):
-            if key not in attributes or not attributes[key]:
-                attributes[key] = value
-
-    def _assign_mac(attributes, hypervisor):
-        '''
-        Compute mac address for NIC depending on hypervisor
-        '''
-        dmac = kwargs.get('dmac', None)
-        if dmac is not None:
-            log.debug('DMAC address is %s', dmac)
-            if salt.utils.validate.net.mac(dmac):
-                attributes['mac'] = dmac
-            else:
-                msg = 'Malformed MAC address: {0}'.format(dmac)
-                raise CommandExecutionError(msg)
-        else:
-            if hypervisor in ['qemu', 'kvm']:
-                attributes['mac'] = salt.utils.network.gen_mac(
-                    prefix='52:54:00')
-            else:
-                attributes['mac'] = salt.utils.network.gen_mac()
-
-    for interface in interfaces:
-        _normalize_net_types(interface)
-        _assign_mac(interface, hypervisor)
-        if hypervisor in overlays:
-            _apply_default_overlay(interface)
-
-    return interfaces
+    # dmac can only be used from init()
+    return _complete_nics(interfaces, hypervisor, dmac=dmac)
 
 
 def init(name,
@@ -1036,9 +1084,11 @@ def init(name,
          mem,
          image=None,
          nic='default',
-         hypervisor=VIRT_DEFAULT_HYPER,
+         interfaces=None,
+         hypervisor=None,
          start=True,  # pylint: disable=redefined-outer-name
          disk='default',
+         disks=None,
          saltenv='base',
          seed=True,
          install=True,
@@ -1047,6 +1097,7 @@ def init(name,
          seed_cmd='seed.apply',
          enable_vnc=False,
          enable_qcow=False,
+         graphics=None,
          **kwargs):
     '''
     Initialize a new vm
@@ -1055,11 +1106,33 @@ def init(name,
     :param cpu: Number of virtual CPUs to assign to the virtual machine
     :param mem: Amount of memory to allocate to the virtual machine in MiB.
     :param image: Path to a disk image to use as the first disk (Default: ``None``).
+                  Deprecated in favor of the ``disks`` parameter. To set (or change) the image of a
+                  disk, add the following to the disks definitions:
+
+                  .. code-block:: python
+
+                      {
+                          'name': 'name_of_disk_to_change',
+                          'image': '/path/to/the/image'
+                      }
+
     :param nic: NIC profile to use (Default: ``'default'``).
+                The profile interfaces can be customized / extended with the interfaces parameter.
+    :param interfaces:
+        List of dictionaries providing details on the network interfaces to create.
+        These data are merged with the ones from the nic profile. The structure of
+        each dictionary is documented in :ref:`init-nic-def`.
+
+        .. versionadded:: Fluorine
     :param hypervisor: the virtual machine type. By default the value will be computed according
                        to the virtual host capabilities.
     :param start: ``True`` to start the virtual machine after having defined it (Default: ``True``)
     :param disk: Disk profile to use (Default: ``'default'``).
+    :param disks: List of dictionaries providing details on the disk devices to create.
+                  These data are merged with the ones from the disk profile. The structure of
+                  each dictionary is documented in :ref:`init-disk-def`.
+
+                  .. versionadded:: Fluorine
     :param saltenv: Fileserver environment (Default: ``'base'``).
                     See :mod:`cp module for more details <salt.modules.cp>`
     :param seed: ``True`` to seed the disk image. Only used when the ``image`` parameter is provided.
@@ -1068,14 +1141,67 @@ def init(name,
     :param pub_key: public key to seed with (Default: ``None``)
     :param priv_key: public key to seed with (Default: ``None``)
     :param seed_cmd: Salt command to execute to seed the image. (Default: ``'seed.apply'``)
-    :param enable_vnc: ``True`` to setup a vnc display for the VM (Default: ``False``)
+    :param enable_vnc:
+        ``True`` to setup a vnc display for the VM (Default: ``False``)
+
+        Deprecated in favor of the ``graphics`` parameter. Could be replaced with
+        the following:
+
+        .. code-block:: python
+
+            graphics={'type': 'vnc'}
+
+        .. deprecated:: Fluorine
+    :param graphics:
+        Dictionary providing details on the graphics device to create. (Default: ``None``)
+        See :ref:`init-graphics-def` for more details on the possible values.
+
+        .. versionadded:: Fluorine
     :param enable_qcow:
         ``True`` to create a QCOW2 overlay image, rather than copying the image
         (Default: ``False``).
-    :param pool: Path of the folder where the image files are located for vmware/esx hypervisors.
+
+        Deprecated in favor of ``disks`` parameter. Add the following to the disks
+        definitions to create an overlay image of a template disk image with an
+        image set:
+
+        .. code-block:: python
+
+            {
+                'name': 'name_of_disk_to_change',
+                'overlay_image': True
+            }
+
+        .. deprecated:: Fluorine
+    :param pool:
+        Path of the folder where the image files are located for vmware/esx hypervisors.
+
+        Deprecated in favor of ``disks`` parameter. Add the following to the disks
+        definitions to set the vmware datastore of a disk image:
+
+        .. code-block:: python
+
+            {
+                'name': 'name_of_disk_to_change',
+                'pool': 'mydatastore'
+            }
+
+        .. deprecated:: Flurorine
     :param dmac:
         Default MAC address to use for the network interfaces. By default MAC addresses are
         automatically generated.
+
+        Deprecated in favor of ``interfaces`` parameter. Add the following to the interfaces
+        definitions to force the mac address of a NIC:
+
+        .. code-block:: python
+
+            {
+                'name': 'name_of_nic_to_change',
+                'mac': 'MY:MA:CC:ADD:RE:SS'
+            }
+
+        .. deprecated:: Fluorine
     :param config: minion configuration to use when seeding.
                    See :mod:`seed module for more details <salt.modules.seed>`
     :param boot_dev: String of space-separated devices to boot from (Default: ``'hd'``)
@@ -1092,7 +1218,90 @@ def init(name,
 
                      .. versionadded:: Fluorine
 
-    CLI Example:
+    .. _init-nic-def:
+
+    .. rubric:: Network Interfaces Definitions
+
+    Network interfaces dictionaries can contain the following properties:
+
+    name
+        Name of the network interface. This is only used as a key to merge with the profile data
+
+    type
+        Network type. One of ``'bridge'``, ``'network'``
+
+    source
+        The network source, typically the bridge or network name
+
+    mac
+        The desired mac address, computed if ``None`` (Default: ``None``).
+
+    model
+        The network card model (Default: depends on the hypervisor)
+
+    .. _init-disk-def:
+
+    .. rubric:: Disks Definitions
+
+    Disk dictionaries can contain the following properties:
+
+    disk_name
+        Name of the disk. This is mostly used in the name of the disk image and as a key to merge
+        with the profile data.
+
+    format
+        Format of the disk image, like ``'qcow2'``, ``'raw'``, ``'vmdk'``.
+        (Default: depends on the hypervisor)
+
+    size
+        Disk size in MiB
+
+    pool
+        Path to the folder or name of the pool where disks should be created.
+        (Default: depends on hypervisor)
+
+    model
+        One of the disk busses allowed by libvirt (Default: depends on hypervisor)
+
+        See `libvirt documentation <https://libvirt.org/formatdomain.html#elementsDisks>`_
+        for the allowed bus types.
+
+    image
+        Path to the image to use for the disk. If no image is provided, an empty disk will be created
+        (Default: ``None``)
+
+    overlay_image
+        ``True`` to create a QCOW2 disk image with ``image`` as backing file. If ``False``
+        the file pointed to by the ``image`` property will simply be copied. (Default: ``False``)
+
+    .. _init-graphics-def:
+
+    .. rubric:: Graphics Definition
+
+    The graphics dictionnary can have the following properties:
+
+    type
+        Graphics type. The possible values are ``'spice'``, ``'vnc'`` and other values
+        allowed as a libvirt graphics type (Default: ``None``)
+
+        See `the libvirt documentation <https://libvirt.org/formatdomain.html#elementsGraphics>`_
+        for more details on the possible types
+
+    port
+        Port to export the graphics on for ``vnc``, ``spice`` and ``rdp`` types.
+
+    tls_port
+        Port to export the graphics over a secured connection for ``spice`` type.
+
+    listen
+        Dictionary defining on what address to listen on for ``vnc``, ``spice`` and ``rdp``.
+        It has a ``type`` property with ``address`` and ``None`` as possible values, and an
+        ``address`` property holding the IP or hostname to listen on.
+
+        By default, not setting the ``listen`` part of the dictionary will default to
+        listen on all addresses.
+
+    .. rubric:: CLI Example
 
     .. code-block:: bash
 
@@ -1102,37 +1311,83 @@ def init(name,
 
     The disk images will be created in an image folder within the directory
     defined by the ``virt:images`` option. Its default value is
-    ``/srv/salt/salt-images/`` but this can changed with such a configuration:
+    ``/srv/salt-images/`` but this can changed with such a configuration:
 
     .. code-block:: yaml
 
         virt:
             images: /data/my/vm/images/
     '''
+    hypervisors = _get_domain_types(**kwargs)
     hypervisor = __salt__['config.get']('libvirt:hypervisor', hypervisor)
+    if hypervisor is not None:
+        salt.utils.versions.warn_until(
+            'Sodium',
+            '\'libvirt:hypervisor\' configuration property has been deprecated. '
+            'Rather use the \'virt:connection:uri\' to properly define the libvirt '
+            'URI or alias of the host to connect to. \'libvirt:hypervisor\' will '
+            'stop being used in {version}.'
+        )
+    else:
+        # Use the machine types as possible values
+        # Prefer 'kvm' over the others if available
+        hypervisor = 'kvm' if 'kvm' in hypervisors else hypervisors[0]
+
+    # esxi used to be a possible value for the hypervisor: map it to vmware since it's the same
+    hypervisor = 'vmware' if hypervisor == 'esxi' else hypervisor
+
     log.debug('Using hyperisor %s', hypervisor)
 
-    nicp = _nic_profile(nic, hypervisor, **kwargs)
+    # the NICs are computed as follows:
+    # 1 - get the default NICs from the profile
+    # 2 - Complete the users NICS
+    # 3 - Update the default NICS list to the users one, matching key is the name
+    dmac = kwargs.get('dmac', None)
+    if dmac:
+        salt.utils.versions.warn_until(
+            'Sodium',
+            '\'dmac\' parameter has been deprecated. Rather use the \'interfaces\' parameter '
+            'to properly define the desired MAC address. \'dmac\' will be removed in {version}.'
+        )
+    nicp = _nic_profile(nic, hypervisor, dmac=dmac)
     log.debug('NIC profile is %s', nicp)
+    if interfaces:
+        users_nics = _complete_nics(interfaces, hypervisor)
+        for unic in users_nics:
+            found = [nic for nic in nicp if nic['name'] == unic['name']]
+            if found:
+                found[0].update(unic)
+            else:
+                nicp.append(unic)
+        log.debug('Merged NICs: %s', nicp)
 
-    diskp = _disk_profile(disk, hypervisor, **kwargs)
+    # the disks are computed as follows:
+    # 1 - get the disks defined in the profile
+    # 2 - set the image on the first disk (will be removed later)
+    # 3 - update the disks from the profile with the ones from the user. The matching key is the name.
+    pool = kwargs.get('pool', None)
+    if pool:
+        salt.utils.versions.warn_until(
+            'Sodium',
+            '\'pool\' parameter has been deprecated. Rather use the \'disks\' parameter '
+            'to properly define the vmware datastore of disks. \'pool\' will be removed in {version}.'
+        )
 
     if image:
-        # If image is specified in module arguments, then it will be used
-        # for the first disk instead of the image from the disk profile
-        disk_name = next(six.iterkeys(diskp[0]))
-        log.debug('%s image from module arguments will be used for disk "%s"'
-                  ' instead of %s', image, disk_name, diskp[0][disk_name].get('image'))
-        diskp[0][disk_name]['image'] = image
+        salt.utils.versions.warn_until(
+            'Sodium',
+            '\'image\' parameter has been deprecated. Rather use the \'disks\' parameter '
+            'to override or define the image. \'image\' will be removed in {version}.'
+        )
+
+    diskp = _disk_profile(disk, hypervisor, disks, name, image=image, pool=pool, **kwargs)
 
     # Create multiple disks, empty or from specified images.
     for _disk in diskp:
         log.debug("Creating disk for VM [ %s ]: %s", name, _disk)
 
-        for disk_name, args in six.iteritems(_disk):
-
-            if hypervisor in ['esxi', 'vmware']:
-                if 'image' in args:
+        if hypervisor == 'vmware':
+            if 'image' in _disk:
                 # TODO: we should be copying the image file onto the ESX host
                 raise SaltInvocationError(
                     'virt.init does not support image '
@@ -1143,32 +1398,30 @@ def init(name,
                 log.debug('Generating libvirt XML for %s', _disk)
                 vol_xml = _gen_vol_xml(
                     name,
-                        disk_name,
-                        args['format'],
-                        args['size'],
-                        args['pool']
+                    _disk['name'],
+                    _disk['format'],
+                    _disk['size'],
+                    _disk['pool']
                 )
                 define_vol_xml_str(vol_xml)
 
         elif hypervisor in ['qemu', 'kvm']:
 
-                disk_type = args.get('format', 'qcow2')
-                disk_image = args.get('image', None)
-                disk_size = args.get('size', None)
-                disk_file_name = '{0}.{1}'.format(disk_name, disk_type)
-
-                img_dest = _qemu_image_create(
-                    vm_name=name,
-                    disk_file_name=disk_file_name,
-                    disk_image=disk_image,
-                    disk_size=disk_size,
-                    disk_type=disk_type,
-                    enable_qcow=enable_qcow,
-                    saltenv=saltenv,
+            create_overlay = enable_qcow
+            if create_overlay:
+                salt.utils.versions.warn_until(
+                    'Sodium',
+                    '\'enable_qcow\' parameter has been deprecated. Rather use the \'disks\' '
+                    'parameter to override or define the image. \'enable_qcow\' will be removed '
+                    'in {version}.'
                 )
+            else:
+                create_overlay = _disk.get('overlay_image', False)
+
+            img_dest = _qemu_image_create(_disk, create_overlay, saltenv)
 
             # Seed only if there is an image specified
-                if seed and disk_image:
+            if seed and _disk.get('image', None):
                 log.debug('Seed command is %s', seed_cmd)
                 __salt__[seed_cmd](
                     img_dest,
@@ -1187,8 +1440,16 @@ def init(name,
             )
 
     log.debug('Generating VM XML')
-    kwargs['enable_vnc'] = enable_vnc
-    vm_xml = _gen_xml(name, cpu, mem, diskp, nicp, hypervisor, **kwargs)
+
+    if enable_vnc:
+        salt.utils.versions.warn_until(
+            'Sodium',
+            '\'enable_vnc\' parameter has been deprecated in favor of '
+            '\'graphics\'. Use graphics={\'type\': \'vnc\'} for the same behavior. '
+            '\'enable_vnc\' will be removed in {version}. ')
+        graphics = {'type': 'vnc'}
+
+    vm_xml = _gen_xml(name, cpu, mem, diskp, nicp, hypervisor, graphics, **kwargs)
     conn = __get_conn(**kwargs)
     try:
         conn.defineXML(vm_xml)
@@ -1783,7 +2044,7 @@ def get_xml(vm_, **kwargs):
     return xml_desc
 
 
-def get_profiles(hypervisor=None):
+def get_profiles(hypervisor=None, **kwargs):
     '''
     Return the virt profiles for hypervisor.
 
@@ -1811,10 +2072,24 @@ def get_profiles(hypervisor=None):
         salt '*' virt.get_profiles hypervisor=esxi
     '''
     ret = {}
-    if hypervisor:
-        hypervisor = hypervisor
+
+    hypervisors = _get_domain_types(**kwargs)
+    default_hypervisor = 'kvm' if 'kvm' in hypervisors else hypervisors[0]
+
+    if not hypervisor:
+        hypervisor = __salt__['config.get']('libvirt:hypervisor')
+        if hypervisor is not None:
+            salt.utils.versions.warn_until(
+                'Sodium',
+                '\'libvirt:hypervisor\' configuration property has been deprecated. '
+                'Rather use the \'virt:connection:uri\' to properly define the libvirt '
+                'URI or alias of the host to connect to. \'libvirt:hypervisor\' will '
+                'stop being used in {version}.'
+            )
         else:
-        hypervisor = __salt__['config.get']('libvirt:hypervisor', VIRT_DEFAULT_HYPER)
+            # Use the machine types as possible values
+            # Prefer 'kvm' over the others if available
+            hypervisor = default_hypervisor
     virtconf = __salt__['config.get']('virt', {})
     for typ in ['disk', 'nic']:
         _func = getattr(sys.modules[__name__], '_{0}_profile'.format(typ))
@@ -4000,6 +4275,9 @@ def pool_info(name, **kwargs):
         infos = pool.info()
         states = ['inactive', 'building', 'running', 'degraded', 'inaccessible']
         state = states[infos[0]] if infos[0] < len(states) else 'unknown'
+        desc = minidom.parseString(pool.XMLDesc())
+        pool_node = _get_xml_first_element_by_tag_name(desc, 'pool')
+        path_node = _get_xml_first_element_by_tag_name(desc, 'path')
         result = {
             'uuid': pool.UUIDString(),
             'state': state,
@@ -4007,7 +4285,9 @@ def pool_info(name, **kwargs):
             'allocation': infos[2],
             'free': infos[3],
             'autostart': pool.autostart(),
-            'persistent': pool.isPersistent()
+            'persistent': pool.isPersistent(),
+            'target_path': _get_xml_element_text(path_node) if path_node else None,
+            'type': pool_node.getAttribute('type')
         }
     except libvirt.libvirtError as err:
         log.debug('Silenced libvirt error: %s', str(err))

salt/pillar/consul_pillar.py

@@ -134,10 +134,10 @@ import salt.utils.yaml
 # Import third party libs
 try:
     import consul
-    HAS_CONSUL = True
-    CONSUL_VERSION = consul.__version__
+    if not hasattr(consul, '__version__'):
+        consul.__version__ = '0.1'  # Some packages has no version, and so this pillar crashes on access to it.
 except ImportError:
-    HAS_CONSUL = False
+    consul = None
 
 __virtualname__ = 'consul'
 
@@ -149,7 +149,7 @@ def __virtual__():
     '''
     Only return if python-consul is installed
     '''
-    return __virtualname__ if HAS_CONSUL else False
+    return __virtualname__ if consul is not None else False
 
 
 def ext_pillar(minion_id,
@@ -290,9 +290,9 @@ def get_conn(opts, profile):
         pillarenv = opts_merged.get('pillarenv') or 'base'
         params['dc'] = _resolve_datacenter(params['dc'], pillarenv)
 
-    if HAS_CONSUL:
+    if consul:
         # Sanity check. ACL Tokens are supported on python-consul 0.4.7 onwards only.
-        if CONSUL_VERSION < '0.4.7' and params.get('target'):
+        if consul.__version__ < '0.4.7' and params.get('target'):
             params.pop('target')
         return consul.Consul(**params)
     else:

salt/proxy/netmiko_px.py

@@ -24,8 +24,7 @@ Pillar
 The ``netmiko`` proxy configuration requires the following parameters in order
 to connect to the network device:
 
-device_type
-    Class selection based on device type. Supported options:
+- ``device_type`` - Class selection based on device type. Supported options:
 
   - ``a10``: A10 Networks
   - ``accedian``: Accedian Networks
@@ -98,81 +97,64 @@ device_type
   - ``ruckus_fastiron_telnet``: Ruckus Fastiron over Telnet
   - ``cisco_ios_serial``: Cisco IOS over serial port
 
-ip
-  IP address of target device. Not required if ``host`` is provided.
+- ``ip`` - IP address of target device (not required if ``host`` is provided)
 
-host
-  Hostname of target device. Not required if ``ip`` is provided.
+- ``host`` - Hostname of target device (not required if ``ip`` is provided)
 
-username
-  Username to authenticate against target device if required.
+- ``username`` - Username to authenticate against target device, if required
 
-password
-  Password to authenticate against target device if required.
+- ``password`` - Password to authenticate against target device, if required
 
-secret
-  The enable password if target device requires one.
+- ``secret`` - The enable password if target device requires one
 
-port
-  The destination port used to connect to the target device.
+- ``port`` - The destination port used to connect to the target device
 
-global_delay_factor: ``1``
-  Multiplication factor affecting Netmiko delays (default: ``1``).
+- ``global_delay_factor`` - Multiplication factor affecting Netmiko delays
+  (default: ``1``)
 
-use_keys: ``False``
-  Connect to target device using SSH keys.
+- ``use_keys`` - Connect to target device using SSH keys (default: ``False``)
 
-key_file
-  Filename path of the SSH key file to use.
+- ``key_file`` - Filename path of the SSH key file to use
 
-allow_agent
-  Enable use of SSH key-agent.
+- ``allow_agent`` - Enable use of SSH key-agent
 
-ssh_strict: ``False``
-  Automatically reject unknown SSH host keys (default: ``False``, which means
-  unknown SSH host keys will be accepted).
+- ``ssh_strict`` - Automatically reject unknown SSH host keys (default:
+  ``False``, which means unknown SSH host keys will be accepted)
 
-system_host_keys: ``False``
-  Load host keys from the user's 'known_hosts' file.
+- ``system_host_keys`` - Load host keys from the user's "known_hosts" file
+  (default: ``False``)
 
-alt_host_keys: ``False``
-  If ``True`` host keys will be loaded from the file specified in
-  ``alt_key_file``.
+- ``alt_host_keys`` - If ``True``,  host keys will be loaded from the file
+  specified in ``alt_key_file`` (default: ``False``)
 
-alt_key_file
-  SSH host key file to use (if ``alt_host_keys=True``).
+- ``alt_key_file`` - SSH host key file to use (if ``alt_host_keys=True``)
 
-ssh_config_file
-  File name of OpenSSH configuration file.
+- ``ssh_config_file`` - File name of OpenSSH configuration file
 
-timeout: ``90``
-  Connection timeout (in seconds).
+- ``timeout`` - Connection timeout, in seconds (default: ``90``)
 
-session_timeout: ``60``
-  Set a timeout for parallel requests (in seconds).
+- ``session_timeout`` - Set a timeout for parallel requests, in seconds
+  (default: ``60``)
 
-keepalive: ``0``
-  Send SSH keepalive packets at a specific interval, in seconds. Currently
-  defaults to ``0``, for backwards compatibility (it will not attempt to keep
-  the connection alive using the KEEPALIVE packets).
+- ``keepalive`` - Send SSH keepalive packets at a specific interval, in
+  seconds. Currently defaults to ``0``, for backwards compatibility (it will
+  not attempt to keep the connection alive using the KEEPALIVE packets).
 
-default_enter: ``\n``
-  Character(s) to send to correspond to enter key (default: ``\n``).
+- ``default_enter`` - Character(s) to send to correspond to enter key (default:
+  ``\\n``)
 
-response_return: ``\n``
-    Character(s) to use in normalized return data to represent enter key
-    (default: ``\n``)
+- ``response_return`` - Character(s) to use in normalized return data to
+  represent enter key (default: ``\\n``)
 
-always_alive: ``True``
-    In certain less dynamic environments, maintaining the remote connection
-    permanently open with the network device is not always beneficial. In that
-    case, the user can select to initialize the connection only when needed, by
-    specifying this field to ``false``.
-    Default: ``true`` (maintains the connection with the remote network device).
+- ``always_alive`` - In certain less dynamic environments, maintaining the
+  remote connection permanently open with the network device is not always
+  beneficial. In that case, the user can select to initialize the connection
+  only when needed, by setting this option to ``False``. By default this option
+  is set to ``True`` (maintains the connection with the remote network device)
 
-multiprocessing: ``False``
-    Overrides the :conf_minion:`multiprocessing` option, per proxy minion, as
-    the Netmiko communication channel is mainly SSH.
+- ``multiprocessing`` - Overrides the :conf_minion:`multiprocessing` option,
+  per proxy minion, as the Netmiko communication channel is mainly SSH
+  (default: ``False``)
 
 Proxy Pillar Example
 --------------------

salt/sdb/redis_sdb.py

@@ -0,0 +1,84 @@
+# -*- coding: utf-8 -*-
+'''
+Redis SDB module
+================
+
+ .. versionadded:: Fluorine
+
+This module allows access to Redis  using an ``sdb://`` URI.
+
+Like all SDB modules, the Redis module requires a configuration profile to
+be configured in either the minion or master configuration file. This profile
+requires very little. For example:
+
+.. code-block:: yaml
+
+    sdb_redis:
+      driver: redis
+      host: 127.0.0.1
+      port: 6379
+      password: pass
+      db: 1
+
+The ``driver`` refers to the Redis module, all other options are optional.
+For option details see: https://redis-py.readthedocs.io/en/latest/.
+
+'''
+from __future__ import absolute_import, print_function, unicode_literals
+
+try:
+    import redis
+    HAS_REDIS = True
+except ImportError:
+    HAS_REDIS = False
+
+
+__func_alias__ = {
+    'set_': 'set'
+}
+__virtualname__ = 'redis'
+
+
+def __virtual__():
+    '''
+    Module virtual name.
+    '''
+    if not HAS_REDIS:
+        return (False, 'Please install python-redis to use this SDB module.')
+    return __virtualname__
+
+
+def set_(key, value, profile=None):
+    '''
+    Set a value into the Redis SDB.
+    '''
+    if not profile:
+        return False
+    redis_kwargs = profile.copy()
+    redis_kwargs.pop('driver')
+    redis_conn = redis.StrictRedis(**redis_kwargs)
+    return redis_conn.set(key, value)
+
+
+def get(key, profile=None):
+    '''
+    Get a value from the Redis SDB.
+    '''
+    if not profile:
+        return False
+    redis_kwargs = profile.copy()
+    redis_kwargs.pop('driver')
+    redis_conn = redis.StrictRedis(**redis_kwargs)
+    return redis_conn.get(key)
+
+
+def delete(key, profile=None):
+    '''
+    Delete a key from the Redis SDB.
+    '''
+    if not profile:
+        return False
+    redis_kwargs = profile.copy()
+    redis_kwargs.pop('driver')
+    redis_conn = redis.StrictRedis(**redis_kwargs)
+    return redis_conn.get(key)

salt/templates/virt/libvirt_domain.jinja

@@ -33,8 +33,23 @@
                     <model type='{{ nic.model }}'/>
                 </interface>
                 {% endfor %}
-                {% if enable_vnc %}
-                <graphics type='vnc' listen='0.0.0.0' autoport='yes' />
+                {% if graphics %}
+                <graphics type='{{ graphics.type }}'
+                          {% if graphics.listen.address %}
+                          listen='{{ graphics.listen.address }}'
+                          {% endif %}
+                          {% if graphics.port %}
+                          port='{{ graphics.port }}'
+                          {% endif %}
+                          {% if graphics.type == 'spice' and graphics.tls_port %}
+                          tlsPort='{{ graphics.tls_port }}'
+                          {% endif %}
+                          autoport='{{ 'no' if graphics.port or graphics.tls_port else 'yes' }}'>
+                    <listen type='{{ graphics.listen.type }}'
+                            {% if graphics.listen.address %}
+                            address='{{ graphics.listen.address }}'
+                            {% endif %}/>
+                </graphics>
                 {% endif %}
                 {% if serial_type == 'pty' %}
                 <serial type='pty'>

salt/utils/napalm.py

@@ -17,6 +17,7 @@ Utils for the NAPALM modules and proxy.
 
 # Import Python libs
 from __future__ import absolute_import, unicode_literals, print_function
+import copy
 import traceback
 import logging
 import importlib
@@ -26,6 +27,7 @@ from functools import wraps
 from salt.ext import six as six
 import salt.output
 import salt.utils.platform
+import salt.utils.args
 
 # Import third party libs
 try:
@@ -93,14 +95,14 @@ def virtual(opts, virtualname, filename):
     '''
     Returns the __virtual__.
     '''
-    if ((HAS_NAPALM and NAPALM_MAJOR >= 2) or HAS_NAPALM_BASE) and (is_proxy(opts) or is_minion(opts)):
+    if (HAS_NAPALM and NAPALM_MAJOR >= 2) or HAS_NAPALM_BASE:
         return virtualname
     else:
         return (
             False,
             (
                 '"{vname}"" {filename} cannot be loaded: '
-                'NAPALM is not installed or not running in a (proxy) minion'
+                'NAPALM is not installed: ``pip install napalm``'
             ).format(
                 vname=virtualname,
                 filename='({filename})'.format(filename=filename)
@@ -255,15 +257,12 @@ def get_device_opts(opts, salt_obj=None):
     '''
     network_device = {}
     # by default, look in the proxy config details
-    device_dict = opts.get('proxy', {}) or opts.get('napalm', {})
+    device_dict = opts.get('proxy', {}) if is_proxy(opts) else opts.get('napalm', {})
     if opts.get('proxy') or opts.get('napalm'):
         opts['multiprocessing'] = device_dict.get('multiprocessing', False)
         # Most NAPALM drivers are SSH-based, so multiprocessing should default to False.
         # But the user can be allows one to have a different value for the multiprocessing, which will
         #   override the opts.
-    if salt_obj and not device_dict:
-        # get the connection details from the opts
-        device_dict = salt_obj['config.merge']('napalm')
     if not device_dict:
         # still not able to setup
         log.error('Incorrect minion config. Please specify at least the napalm driver name!')
@@ -367,11 +366,12 @@ def proxy_napalm_wrap(func):
         # the execution modules will make use of this variable from now on
         # previously they were accessing the device properties through the __proxy__ object
         always_alive = opts.get('proxy', {}).get('always_alive', True)
-        if salt.utils.platform.is_proxy() and always_alive:
-            # if it is running in a proxy and it's using the default always alive behaviour,
-            # will get the cached copy of the network device
+        if is_proxy(opts) and always_alive:
+            # if it is running in a NAPALM Proxy and it's using the default
+            # always alive behaviour, will get the cached copy of the network
+            # device object which should preserve the connection.
             wrapped_global_namespace['napalm_device'] = proxy['napalm.get_device']()
-        elif salt.utils.platform.is_proxy() and not always_alive:
+        elif is_proxy(opts) and not always_alive:
             # if still proxy, but the user does not want the SSH session always alive
             # get a new device instance
             # which establishes a new connection
@@ -398,10 +398,36 @@ def proxy_napalm_wrap(func):
                 # otherwise we risk to open multiple sessions
                 wrapped_global_namespace['napalm_device'] = kwargs['inherit_napalm_device']
         else:
-            # if no proxy
+            # if not a NAPLAM proxy
             # thus it is running on a regular minion, directly on the network device
+            # or another flavour of Minion from where we can invoke arbitrary
+            # NAPALM commands
             # get __salt__ from func_globals
+            log.debug('Not running in a NAPALM Proxy Minion')
             _salt_obj = wrapped_global_namespace.get('__salt__')
+            napalm_opts = _salt_obj['config.get']('napalm', {})
+            napalm_inventory = _salt_obj['config.get']('napalm_inventory', {})
+            log.debug('NAPALM opts found in the Minion config')
+            log.debug(napalm_opts)
+            clean_kwargs = salt.utils.args.clean_kwargs(**kwargs)
+            napalm_opts.update(clean_kwargs)  # no need for deeper merge
+            log.debug('Merging the found opts with the CLI args')
+            log.debug(napalm_opts)
+            host = napalm_opts.get('host') or napalm_opts.get('hostname') or\
+                   napalm_opts.get('fqdn') or napalm_opts.get('ip')
+            if host and napalm_inventory and isinstance(napalm_inventory, dict) and\
+               host in napalm_inventory:
+                inventory_opts = napalm_inventory[host]
+                log.debug('Found %s in the NAPALM inventory:', host)
+                log.debug(inventory_opts)
+                napalm_opts.update(inventory_opts)
+                log.debug('Merging the config for %s with the details found in the napalm inventory:', host)
+                log.debug(napalm_opts)
+            opts = copy.deepcopy(opts)  # make sure we don't override the original
+            # opts, but just inject the CLI args from the kwargs to into the
+            # object manipulated by ``get_device_opts`` to extract the
+            # connection details, then use then to establish the connection.
+            opts['napalm'] = napalm_opts
             if 'inherit_napalm_device' not in kwargs or ('inherit_napalm_device' in kwargs and
                                                          not kwargs['inherit_napalm_device']):
                 # try to open a new connection

salt/utils/thin.py

@@ -78,6 +78,12 @@ import salt.utils.stringutils
 import salt.exceptions
 import salt.version
 
+if _six.PY2:
+    import concurrent
+else:
+    concurrent = None
+
+
 log = logging.getLogger(__name__)
 
 
@@ -259,7 +265,7 @@ def get_tops(extra_mods='', so_mods=''):
     :return:
     '''
     tops = []
-    for mod in [salt, jinja2, yaml, tornado, msgpack, certifi, singledispatch,
+    for mod in [salt, jinja2, yaml, tornado, msgpack, certifi, singledispatch, concurrent,
                 singledispatch_helpers, ssl_match_hostname, markupsafe, backports_abc]:
         if mod:
             log.debug('Adding module to the tops: "%s"', mod.__name__)

salt/utils/vmware.py

@@ -89,7 +89,6 @@ import salt.utils.path
 import salt.utils.platform
 import salt.utils.stringutils
 
-
 # Import Third Party Libs
 from salt.ext import six
 from salt.ext.six.moves.http_client import BadStatusLine  # pylint: disable=E0611
@@ -119,7 +118,7 @@ def __virtual__():
     '''
     if HAS_PYVMOMI:
         return True
-    else:
+
     return False, 'Missing dependency: The salt.utils.vmware module requires pyVmomi.'
 
 
@@ -227,8 +226,8 @@ def _get_service_instance(host, username, password, protocol,
             log.error('This may mean that a version of PyVmomi EARLIER than 6.0.0.2016.6 is installed.')
             log.error('We recommend updating to that version or later.')
             raise
-    except Exception as exc:
-
+    except Exception as exc:  # pylint: disable=broad-except
+        # pyVmomi's SmartConnect() actually raises Exception in some cases.
         default_msg = 'Could not connect to host \'{0}\'. ' \
                       'Please check the debug log for more information.'.format(host)
 
@@ -236,8 +235,6 @@ def _get_service_instance(host, username, password, protocol,
             if (isinstance(exc, vim.fault.HostConnectFault) and
                 '[SSL: CERTIFICATE_VERIFY_FAILED]' in exc.msg) or \
                '[SSL: CERTIFICATE_VERIFY_FAILED]' in six.text_type(exc):
-
-                import ssl
                 service_instance = SmartConnect(
                     host=host,
                     user=username,
@@ -251,9 +248,9 @@ def _get_service_instance(host, username, password, protocol,
                 log.exception(exc)
                 err_msg = exc.msg if hasattr(exc, 'msg') else default_msg
                 raise salt.exceptions.VMwareConnectionError(err_msg)
-        except Exception as exc:
+        except Exception as exc:  # pylint: disable=broad-except
+            # pyVmomi's SmartConnect() actually raises Exception in some cases.
             if 'certificate verify failed' in six.text_type(exc):
-                import ssl
                 context = ssl.SSLContext(ssl.PROTOCOL_TLSv1)
                 context.verify_mode = ssl.CERT_NONE
                 try:
@@ -362,7 +359,9 @@ def get_service_instance(host, username=None, password=None, protocol=None,
     service_instance = GetSi()
     if service_instance:
         stub = GetStub()
-        if salt.utils.platform.is_proxy() or (hasattr(stub, 'host') and stub.host != ':'.join([host, six.text_type(port)])):
+        if (salt.utils.platform.is_proxy() or
+            (hasattr(stub, 'host') and
+             stub.host != ':'.join([host, six.text_type(port)]))):
             # Proxies will fork and mess up the cached service instance.
             # If this is a proxy or we are connecting to a different host
             # invalidate the service instance to avoid a potential memory leak
@@ -432,9 +431,9 @@ def get_new_service_instance_stub(service_instance, path, ns=None,
         Version of the new stub.
         Default value is None.
     '''
-    #For python 2.7.9 and later, the defaul SSL conext has more strict
-    #connection handshaking rule. We may need turn of the hostname checking
-    #and client side cert verification
+    # For python 2.7.9 and later, the default SSL context has more strict
+    # connection handshaking rule. We may need turn off the hostname checking
+    # and the client side cert verification.
     context = None
     if sys.version_info[:3] > (2, 7, 8):
         context = ssl.create_default_context()
@@ -669,9 +668,7 @@ def get_hardware_grains(service_instance):
     if get_inventory(service_instance).about.apiType == 'HostAgent':
         view = service_instance.content.viewManager.CreateContainerView(service_instance.RetrieveContent().rootFolder,
                                                                         [vim.HostSystem], True)
-        if view:
-            if view.view:
-                if len(view.view) > 0:
+        if view and view.view:
             hw_grain_data['manufacturer'] = view.view[0].hardware.systemInfo.vendor
             hw_grain_data['productname'] = view.view[0].hardware.systemInfo.model
 
@@ -947,9 +944,9 @@ def get_mors_with_properties(service_instance, object_type, property_list=None,
         content = get_content(*content_args, **content_kwargs)
     except BadStatusLine:
         content = get_content(*content_args, **content_kwargs)
-    except IOError as e:
-        if e.errno != errno.EPIPE:
-            raise e
+    except IOError as exc:
+        if exc.errno != errno.EPIPE:
+            raise exc
         content = get_content(*content_args, **content_kwargs)
 
     object_list = []
@@ -1029,6 +1026,8 @@ def get_network_adapter_type(adapter_type):
     elif adapter_type == 'e1000e':
         return vim.vm.device.VirtualE1000e()
 
+    raise ValueError('An unknown network adapter object type name.')
+
 
 def get_network_adapter_object_type(adapter_object):
     '''
@@ -1048,6 +1047,8 @@ def get_network_adapter_object_type(adapter_object):
     if isinstance(adapter_object, vim.vm.device.VirtualE1000):
         return 'e1000'
 
+    raise ValueError('An unknown network adapter object type.')
+
 
 def get_dvss(dc_ref, dvs_names=None, get_all_dvss=False):
     '''
@@ -1223,8 +1224,8 @@ def get_dvportgroups(parent_ref, portgroup_names=None,
     get_all_portgroups
         Return all portgroups in the parent. Default is False.
     '''
-    if not (isinstance(parent_ref, vim.Datacenter) or
-            isinstance(parent_ref, vim.DistributedVirtualSwitch)):
+    if not (isinstance(parent_ref,
+                       (vim.Datacenter, vim.DistributedVirtualSwitch))):
         raise salt.exceptions.ArgumentValueError(
             'Parent has to be either a datacenter, '
             'or a distributed virtual switch')
@@ -1554,7 +1555,7 @@ def add_license(service_instance, key, description, license_manager=None):
     label.value = description
     log.debug('Adding license \'%s\'', description)
     try:
-        license = license_manager.AddLicense(key, [label])
+        vmware_license = license_manager.AddLicense(key, [label])
     except vim.fault.NoPermission as exc:
         log.exception(exc)
         raise salt.exceptions.VMwareApiError(
@@ -1566,7 +1567,7 @@ def add_license(service_instance, key, description, license_manager=None):
     except vmodl.RuntimeFault as exc:
         log.exception(exc)
         raise salt.exceptions.VMwareRuntimeError(exc.msg)
-    return license
+    return vmware_license
 
 
 def get_assigned_licenses(service_instance, entity_ref=None, entity_name=None,
@@ -1709,9 +1710,10 @@ def assign_license(service_instance, license_key, license_name,
 
     log.trace('Assigning license to \'%s\'', entity_name)
     try:
-        license = license_assignment_manager.UpdateAssignedLicense(
+        vmware_license = license_assignment_manager.UpdateAssignedLicense(
             entity_id,
-            license_key)
+            license_key,
+            license_name)
     except vim.fault.NoPermission as exc:
         log.exception(exc)
         raise salt.exceptions.VMwareApiError(
@@ -1723,7 +1725,7 @@ def assign_license(service_instance, license_key, license_name,
     except vmodl.RuntimeFault as exc:
         log.exception(exc)
         raise salt.exceptions.VMwareRuntimeError(exc.msg)
-    return license
+    return vmware_license
 
 
 def list_datacenters(service_instance):
@@ -2189,7 +2191,8 @@ def _get_partition_info(storage_system, device_path):
     return partition_infos[0]
 
 
-def _get_new_computed_partition_spec(hostname, storage_system, device_path,
+def _get_new_computed_partition_spec(storage_system,
+                                     device_path,
                                      partition_info):
     '''
     Computes the new disk partition info when adding a new vmfs partition that
@@ -2198,7 +2201,7 @@ def _get_new_computed_partition_spec(hostname, storage_system, device_path,
     '''
     log.trace('Adding a partition at the end of the disk and getting the new '
               'computed partition spec')
-    #TODO implement support for multiple partitions
+    # TODO implement support for multiple partitions
     # We support adding a partition add the end of the disk with partitions
     free_partitions = [p for p in partition_info.layout.partition
                        if p.type == 'none']
@@ -2282,7 +2285,10 @@ def create_vmfs_datastore(host_ref, datastore_name, disk_ref,
                                          target_disk.devicePath)
     log.trace('partition_info = %s', partition_info)
     new_partition_number, partition_spec = _get_new_computed_partition_spec(
-        hostname, storage_system, target_disk.devicePath, partition_info)
+        storage_system,
+        target_disk.devicePath,
+        partition_info
+    )
     spec = vim.VmfsDatastoreCreateSpec(
         vmfs=vim.HostVmfsSpec(
             majorVersion=vmfs_major_version,
@@ -2355,7 +2361,6 @@ def remove_datastore(service_instance, datastore_ref):
         datastore_ref, ['host', 'info', 'name'])
     ds_name = ds_props['name']
     log.debug('Removing datastore \'%s\'', ds_name)
-    ds_info = ds_props['info']
     ds_hosts = ds_props.get('host')
     if not ds_hosts:
         raise salt.exceptions.VMwareApiError(
@@ -2417,7 +2422,6 @@ def get_hosts(service_instance, datacenter_name=None, host_names=None,
         if cluster_name:
             # Retrieval to test if cluster exists. Cluster existence only makes
             # sense if the datacenter has been specified
-            cluster = get_cluster(start_point, cluster_name)
             properties.append('parent')
 
     # Search for the objects
@@ -2469,7 +2473,6 @@ def _get_scsi_address_to_lun_key_map(service_instance,
     hostname
         Name of the host. Default is None.
     '''
-    map = {}
     if not hostname:
         hostname = get_managed_object_name(host_ref)
     if not storage_system:
@@ -2834,19 +2837,18 @@ def _check_disks_in_diskgroup(disk_group, cache_disk_id, capacity_disk_ids):
         raise salt.exceptions.ArgumentValueError(
             'Incorrect diskgroup cache disk; got id: \'{0}\'; expected id: '
             '\'{1}\''.format(disk_group.ssd.canonicalName, cache_disk_id))
-    if sorted([d.canonicalName for d in disk_group.nonSsd]) != \
-        sorted(capacity_disk_ids):
-
+    non_ssd_disks = [d.canonicalName for d in disk_group.nonSsd]
+    if sorted(non_ssd_disks) != sorted(capacity_disk_ids):
         raise salt.exceptions.ArgumentValueError(
             'Incorrect capacity disks; got ids: \'{0}\'; expected ids: \'{1}\''
-            ''.format(sorted([d.canonicalName for d in disk_group.nonSsd]),
+            ''.format(sorted(non_ssd_disks),
                       sorted(capacity_disk_ids)))
     log.trace('Checked disks in diskgroup with cache disk id \'%s\'',
               cache_disk_id)
     return True
 
 
-#TODO Support host caches on multiple datastores
+# TODO Support host caches on multiple datastores
 def get_host_cache(host_ref, host_cache_manager=None):
     '''
     Returns a vim.HostScsiDisk if the host cache is configured on the specified
@@ -2887,7 +2889,7 @@ def get_host_cache(host_ref, host_cache_manager=None):
         return results['cacheConfigurationInfo'][0]
 
 
-#TODO Support host caches on multiple datastores
+# TODO Support host caches on multiple datastores
 def configure_host_cache(host_ref, datastore_ref, swap_size_MiB,
                          host_cache_manager=None):
     '''
@@ -3222,8 +3224,9 @@ def get_vm_by_property(service_instance, name, datacenter=None, vm_properties=No
     if not vm_formatted:
         raise salt.exceptions.VMwareObjectRetrievalError('The virtual machine was not found.')
     elif len(vm_formatted) > 1:
-        raise salt.exceptions.VMwareMultipleObjectsError('Multiple virtual machines were found with the '
-                                                  'same name, please specify a container.')
+        raise salt.exceptions.VMwareMultipleObjectsError(' '.join([
+            'Multiple virtual machines were found with the'
+            'same name, please specify a container.']))
     return vm_formatted[0]
 
 
@@ -3250,13 +3253,15 @@ def get_folder(service_instance, datacenter, placement, base_vm_name=None):
         if 'parent' in vm_props:
             folder_object = vm_props['parent']
         else:
-            raise salt.exceptions.VMwareObjectRetrievalError('The virtual machine parent '
-                                                      'object is not defined')
+            raise salt.exceptions.VMwareObjectRetrievalError(' '.join([
+                'The virtual machine parent',
+                'object is not defined']))
     elif 'folder' in placement:
         folder_objects = salt.utils.vmware.get_folders(service_instance, [placement['folder']], datacenter)
         if len(folder_objects) > 1:
-            raise salt.exceptions.VMwareMultipleObjectsError('Multiple instances are available of the '
-                                                      'specified folder {0}'.format(placement['folder']))
+            raise salt.exceptions.VMwareMultipleObjectsError(' '.join([
+                'Multiple instances are available of the',
+                'specified folder {0}'.format(placement['folder'])]))
         folder_object = folder_objects[0]
     elif datacenter:
         datacenter_object = salt.utils.vmware.get_datacenter(service_instance, datacenter)
@@ -3286,9 +3291,12 @@ def get_placement(service_instance, datacenter, placement=None):
     if 'host' in placement:
         host_objects = get_hosts(service_instance, datacenter_name=datacenter, host_names=[placement['host']])
         if not host_objects:
-            raise salt.exceptions.VMwareObjectRetrievalError('The specified host {0} cannot be found.'.format(placement['host']))
+            raise salt.exceptions.VMwareObjectRetrievalError(' '.join([
+                'The specified host',
+                '{0} cannot be found.'.format(placement['host'])]))
         try:
-            host_props = get_properties_of_managed_object(host_objects[0],
+            host_props = \
+                get_properties_of_managed_object(host_objects[0],
                                                  properties=['resourcePool'])
             resourcepool_object = host_props['resourcePool']
         except vmodl.query.InvalidProperty:
@@ -3316,16 +3324,18 @@ def get_placement(service_instance, datacenter, placement=None):
                                                   [placement['resourcepool']],
                                                   datacenter_name=datacenter)
         if len(resourcepool_objects) > 1:
-            raise salt.exceptions.VMwareMultipleObjectsError('Multiple instances are available of the '
-                                                      'specified host {}.'.format(placement['host']))
+            raise salt.exceptions.VMwareMultipleObjectsError(' '.join([
+                'Multiple instances are available of the',
+                'specified host {}.'.format(placement['host'])]))
         resourcepool_object = resourcepool_objects[0]
         res_props = get_properties_of_managed_object(resourcepool_object,
                                                      properties=['parent'])
         if 'parent' in res_props:
             placement_object = res_props['parent']
         else:
-            raise salt.exceptions.VMwareObjectRetrievalError('The resource pool\'s parent '
-                                                      'object is not defined')
+            raise salt.exceptions.VMwareObjectRetrievalError(' '.join([
+                'The resource pool\'s parent',
+                'object is not defined']))
     elif 'cluster' in placement:
         datacenter_object = get_datacenter(service_instance, datacenter)
         cluster_object = get_cluster(datacenter_object, placement['cluster'])
@@ -3334,12 +3344,14 @@ def get_placement(service_instance, datacenter, placement=None):
         if 'resourcePool' in clus_props:
             resourcepool_object = clus_props['resourcePool']
         else:
-            raise salt.exceptions.VMwareObjectRetrievalError('The cluster\'s resource pool '
-                                                      'object is not defined')
+            raise salt.exceptions.VMwareObjectRetrievalError(' '.join([
+                'The cluster\'s resource pool',
+                'object is not defined']))
         placement_object = cluster_object
     else:
         # We are checking the schema for this object, this exception should never be raised
-        raise salt.exceptions.VMwareObjectRetrievalError('Placement is not defined.')
+        raise salt.exceptions.VMwareObjectRetrievalError(' '.join([
+            'Placement is not defined.']))
     return (resourcepool_object, placement_object)
 
 
@@ -3409,8 +3421,9 @@ def power_cycle_vm(virtual_machine, action='on'):
     try:
         wait_for_task(task, get_managed_object_name(virtual_machine), task_name)
     except salt.exceptions.VMwareFileNotFoundError as exc:
-        raise salt.exceptions.VMwarePowerOnError('An error occurred during power '
-                                      'operation, a file was not found: {0}'.format(exc))
+        raise salt.exceptions.VMwarePowerOnError(' '.join([
+            'An error occurred during power',
+            'operation, a file was not found: {0}'.format(exc)]))
     return virtual_machine
 
 

tests/unit/modules/test_virt.py

@@ -8,7 +8,7 @@ virt execution module unit tests
 # Import python libs
 from __future__ import absolute_import, print_function, unicode_literals
 import re
-import os
+import datetime
 
 # Import Salt Testing libs
 from tests.support.mixins import LoaderModuleMockMixin
@@ -21,6 +21,7 @@ import salt.modules.virt as virt
 import salt.modules.config as config
 from salt._compat import ElementTree as ET
 import salt.config
+from salt.exceptions import CommandExecutionError
 
 # Import third party libs
 from salt.ext import six
@@ -50,14 +51,19 @@ class VirtTestCase(TestCase, LoaderModuleMockMixin):
         self.mock_libvirt = LibvirtMock()
         self.mock_conn = MagicMock()
         self.mock_libvirt.openAuth.return_value = self.mock_conn
+        self.mock_popen = MagicMock()
         self.addCleanup(delattr, self, 'mock_libvirt')
         self.addCleanup(delattr, self, 'mock_conn')
+        self.addCleanup(delattr, self, 'mock_popen')
+        mock_subprocess = MagicMock()
+        mock_subprocess.Popen.return_value = self.mock_popen  # pylint: disable=no-member
         loader_globals = {
             '__salt__': {
                 'config.get': config.get,
                 'config.option': config.option,
             },
-            'libvirt': self.mock_libvirt
+            'libvirt': self.mock_libvirt,
+            'subprocess': mock_subprocess
         }
         return {virt: loader_globals, config: loader_globals}
 
@@ -73,11 +79,35 @@ class VirtTestCase(TestCase, LoaderModuleMockMixin):
         # Return state as shutdown
         mock_domain.info.return_value = [4, 0, 0, 0]  # pylint: disable=no-member
 
+    def test_disk_profile_merge(self):
+        '''
+        Test virt._disk_profile() when merging with user-defined disks
+        '''
+        userdisks = [{'name': 'data', 'size': 16384, 'format': 'raw'}]
+
+        disks = virt._disk_profile('default', 'kvm', userdisks, 'myvm', image='/path/to/image')
+        self.assertEqual(
+            [{'name': 'system',
+              'size': 8192,
+              'format': 'qcow2',
+              'model': 'virtio',
+              'filename': 'myvm_system.qcow2',
+              'image': '/path/to/image',
+              'source_file': '/srv/salt-images/myvm_system.qcow2'},
+             {'name': 'data',
+              'size': 16384,
+              'format': 'raw',
+              'model': 'virtio',
+              'filename': 'myvm_data.raw',
+              'source_file': '/srv/salt-images/myvm_data.raw'}],
+            disks
+        )
+
     def test_boot_default_dev(self):
         '''
         Test virt._gen_xml() default boot device
         '''
-        diskp = virt._disk_profile('default', 'kvm')
+        diskp = virt._disk_profile('default', 'kvm', [], 'hello')
         nicp = virt._nic_profile('default', 'kvm')
         xml_data = virt._gen_xml(
             'hello',
@@ -94,7 +124,7 @@ class VirtTestCase(TestCase, LoaderModuleMockMixin):
         '''
         Test virt._gen_xml() custom boot device
         '''
-        diskp = virt._disk_profile('default', 'kvm')
+        diskp = virt._disk_profile('default', 'kvm', [], 'hello')
         nicp = virt._nic_profile('default', 'kvm')
         xml_data = virt._gen_xml(
             'hello',
@@ -112,7 +142,7 @@ class VirtTestCase(TestCase, LoaderModuleMockMixin):
         '''
         Test virt._gen_xml() multiple boot devices
         '''
-        diskp = virt._disk_profile('default', 'kvm')
+        diskp = virt._disk_profile('default', 'kvm', [], 'hello')
         nicp = virt._nic_profile('default', 'kvm')
         xml_data = virt._gen_xml(
             'hello',
@@ -131,7 +161,7 @@ class VirtTestCase(TestCase, LoaderModuleMockMixin):
         '''
         Test virt._gen_xml() serial console
         '''
-        diskp = virt._disk_profile('default', 'kvm')
+        diskp = virt._disk_profile('default', 'kvm', [], 'hello')
         nicp = virt._nic_profile('default', 'kvm')
         xml_data = virt._gen_xml(
             'hello',
@@ -151,7 +181,7 @@ class VirtTestCase(TestCase, LoaderModuleMockMixin):
         '''
         Test virt._gen_xml() telnet console
         '''
-        diskp = virt._disk_profile('default', 'kvm')
+        diskp = virt._disk_profile('default', 'kvm', [], 'hello')
         nicp = virt._nic_profile('default', 'kvm')
         xml_data = virt._gen_xml(
             'hello',
@@ -173,7 +203,7 @@ class VirtTestCase(TestCase, LoaderModuleMockMixin):
         '''
         Test virt._gen_xml() telnet console without any specified port
         '''
-        diskp = virt._disk_profile('default', 'kvm')
+        diskp = virt._disk_profile('default', 'kvm', [], 'hello')
         nicp = virt._nic_profile('default', 'kvm')
         xml_data = virt._gen_xml(
             'hello',
@@ -194,7 +224,7 @@ class VirtTestCase(TestCase, LoaderModuleMockMixin):
         '''
         Test virt._gen_xml() with no serial console
         '''
-        diskp = virt._disk_profile('default', 'kvm')
+        diskp = virt._disk_profile('default', 'kvm', [], 'hello')
         nicp = virt._nic_profile('default', 'kvm')
         xml_data = virt._gen_xml(
             'hello',
@@ -214,7 +244,7 @@ class VirtTestCase(TestCase, LoaderModuleMockMixin):
         '''
         Test virt._gen_xml() with no telnet console
         '''
-        diskp = virt._disk_profile('default', 'kvm')
+        diskp = virt._disk_profile('default', 'kvm', [], 'hello')
         nicp = virt._nic_profile('default', 'kvm')
         xml_data = virt._gen_xml(
             'hello',
@@ -230,16 +260,105 @@ class VirtTestCase(TestCase, LoaderModuleMockMixin):
         self.assertEqual(root.find('devices/serial').attrib['type'], 'tcp')
         self.assertEqual(root.find('devices/console'), None)
 
+    def test_gen_xml_nographics_default(self):
+        '''
+        Test virt._gen_xml() with default no graphics device
+        '''
+        diskp = virt._disk_profile('default', 'kvm', [], 'hello')
+        nicp = virt._nic_profile('default', 'kvm')
+        xml_data = virt._gen_xml(
+            'hello',
+            1,
+            512,
+            diskp,
+            nicp,
+            'kvm'
+            )
+        root = ET.fromstring(xml_data)
+        self.assertIsNone(root.find('devices/graphics'))
+
+    def test_gen_xml_vnc_default(self):
+        '''
+        Test virt._gen_xml() with default vnc graphics device
+        '''
+        diskp = virt._disk_profile('default', 'kvm', [], 'hello')
+        nicp = virt._nic_profile('default', 'kvm')
+        xml_data = virt._gen_xml(
+            'hello',
+            1,
+            512,
+            diskp,
+            nicp,
+            'kvm',
+            graphics={'type': 'vnc', 'port': 1234, 'tlsPort': 5678,
+                      'listen': {'type': 'address', 'address': 'myhost'}},
+            )
+        root = ET.fromstring(xml_data)
+        self.assertEqual(root.find('devices/graphics').attrib['type'], 'vnc')
+        self.assertEqual(root.find('devices/graphics').attrib['autoport'], 'no')
+        self.assertEqual(root.find('devices/graphics').attrib['port'], '1234')
+        self.assertFalse('tlsPort' in root.find('devices/graphics').attrib)
+        self.assertEqual(root.find('devices/graphics').attrib['listen'], 'myhost')
+        self.assertEqual(root.find('devices/graphics/listen').attrib['type'], 'address')
+        self.assertEqual(root.find('devices/graphics/listen').attrib['address'], 'myhost')
+
+    def test_gen_xml_spice_default(self):
+        '''
+        Test virt._gen_xml() with default spice graphics device
+        '''
+        diskp = virt._disk_profile('default', 'kvm', [], 'hello')
+        nicp = virt._nic_profile('default', 'kvm')
+        xml_data = virt._gen_xml(
+            'hello',
+            1,
+            512,
+            diskp,
+            nicp,
+            'kvm',
+            graphics={'type': 'spice'},
+            )
+        root = ET.fromstring(xml_data)
+        self.assertEqual(root.find('devices/graphics').attrib['type'], 'spice')
+        self.assertEqual(root.find('devices/graphics').attrib['autoport'], 'yes')
+        self.assertEqual(root.find('devices/graphics').attrib['listen'], '0.0.0.0')
+        self.assertEqual(root.find('devices/graphics/listen').attrib['type'], 'address')
+        self.assertEqual(root.find('devices/graphics/listen').attrib['address'], '0.0.0.0')
+
+    def test_gen_xml_spice(self):
+        '''
+        Test virt._gen_xml() with spice graphics device
+        '''
+        diskp = virt._disk_profile('default', 'kvm', [], 'hello')
+        nicp = virt._nic_profile('default', 'kvm')
+        xml_data = virt._gen_xml(
+            'hello',
+            1,
+            512,
+            diskp,
+            nicp,
+            'kvm',
+            graphics={'type': 'spice', 'port': 1234, 'tls_port': 5678, 'listen': {'type': 'none'}},
+            )
+        root = ET.fromstring(xml_data)
+        self.assertEqual(root.find('devices/graphics').attrib['type'], 'spice')
+        self.assertEqual(root.find('devices/graphics').attrib['autoport'], 'no')
+        self.assertEqual(root.find('devices/graphics').attrib['port'], '1234')
+        self.assertEqual(root.find('devices/graphics').attrib['tlsPort'], '5678')
+        self.assertFalse('listen' in root.find('devices/graphics').attrib)
+        self.assertEqual(root.find('devices/graphics/listen').attrib['type'], 'none')
+        self.assertFalse('address' in root.find('devices/graphics/listen').attrib)
+
     def test_default_disk_profile_hypervisor_esxi(self):
         '''
         Test virt._disk_profile() default ESXi profile
         '''
         mock = MagicMock(return_value={})
         with patch.dict(virt.__salt__, {'config.get': mock}):  # pylint: disable=no-member
-            ret = virt._disk_profile('nonexistent', 'esxi')
+            ret = virt._disk_profile('nonexistent', 'vmware')
             self.assertTrue(len(ret) == 1)
-            self.assertIn('system', ret[0])
-            system = ret[0]['system']
+            found = [disk for disk in ret if disk['name'] == 'system']
+            self.assertTrue(bool(found))
+            system = found[0]
             self.assertEqual(system['format'], 'vmdk')
             self.assertEqual(system['model'], 'scsi')
             self.assertTrue(int(system['size']) >= 1)
@@ -252,8 +371,9 @@ class VirtTestCase(TestCase, LoaderModuleMockMixin):
         with patch.dict(virt.__salt__, {'config.get': mock}):  # pylint: disable=no-member
             ret = virt._disk_profile('nonexistent', 'kvm')
             self.assertTrue(len(ret) == 1)
-            self.assertIn('system', ret[0])
-            system = ret[0]['system']
+            found = [disk for disk in ret if disk['name'] == 'system']
+            self.assertTrue(bool(found))
+            system = found[0]
             self.assertEqual(system['format'], 'qcow2')
             self.assertEqual(system['model'], 'virtio')
             self.assertTrue(int(system['size']) >= 1)
@@ -264,7 +384,7 @@ class VirtTestCase(TestCase, LoaderModuleMockMixin):
         '''
         mock = MagicMock(return_value={})
         with patch.dict(virt.__salt__, {'config.get': mock}):  # pylint: disable=no-member
-            ret = virt._nic_profile('nonexistent', 'esxi')
+            ret = virt._nic_profile('nonexistent', 'vmware')
             self.assertTrue(len(ret) == 1)
             eth0 = ret[0]
             self.assertEqual(eth0['name'], 'eth0')
@@ -301,7 +421,7 @@ class VirtTestCase(TestCase, LoaderModuleMockMixin):
         '''
         Test virt._gen_xml(), KVM default profile case
         '''
-        diskp = virt._disk_profile('default', 'kvm')
+        diskp = virt._disk_profile('default', 'kvm', [], 'hello')
         nicp = virt._nic_profile('default', 'kvm')
         xml_data = virt._gen_xml(
             'hello',
@@ -322,7 +442,7 @@ class VirtTestCase(TestCase, LoaderModuleMockMixin):
         disk = disks[0]
         root_dir = salt.config.DEFAULT_MINION_OPTS.get('root_dir')
         self.assertTrue(disk.find('source').attrib['file'].startswith(root_dir))
-        self.assertTrue(os.path.join('hello', 'system') in disk.find('source').attrib['file'])
+        self.assertTrue('hello_system' in disk.find('source').attrib['file'])
         self.assertEqual(disk.find('target').attrib['dev'], 'vda')
         self.assertEqual(disk.find('target').attrib['bus'], 'virtio')
         self.assertEqual(disk.find('driver').attrib['name'], 'qemu')
@@ -341,17 +461,17 @@ class VirtTestCase(TestCase, LoaderModuleMockMixin):
 
     def test_gen_xml_for_esxi_default_profile(self):
         '''
-        Test virt._gen_xml(), ESXi default profile case
+        Test virt._gen_xml(), ESXi/vmware default profile case
         '''
-        diskp = virt._disk_profile('default', 'esxi')
-        nicp = virt._nic_profile('default', 'esxi')
+        diskp = virt._disk_profile('default', 'vmware', [], 'hello')
+        nicp = virt._nic_profile('default', 'vmware')
         xml_data = virt._gen_xml(
             'hello',
             1,
             512,
             diskp,
             nicp,
-            'esxi',
+            'vmware',
             )
         root = ET.fromstring(xml_data)
         self.assertEqual(root.attrib['type'], 'vmware')
@@ -363,7 +483,7 @@ class VirtTestCase(TestCase, LoaderModuleMockMixin):
         self.assertEqual(len(disks), 1)
         disk = disks[0]
         self.assertTrue('[0]' in disk.find('source').attrib['file'])
-        self.assertTrue(os.path.join('hello', 'system') in disk.find('source').attrib['file'])
+        self.assertTrue('hello_system' in disk.find('source').attrib['file'])
         self.assertEqual(disk.find('target').attrib['dev'], 'sda')
         self.assertEqual(disk.find('target').attrib['bus'], 'scsi')
         self.assertEqual(disk.find('address').attrib['unit'], '0')
@@ -381,45 +501,31 @@ class VirtTestCase(TestCase, LoaderModuleMockMixin):
 
     def test_gen_xml_for_esxi_custom_profile(self):
         '''
-        Test virt._gen_xml(), ESXi custom profile case
+        Test virt._gen_xml(), ESXi/vmware custom profile case
         '''
-        diskp_yaml = '''
-- first:
-    size: 8192
-    format: vmdk
-    model: scsi
-    pool: datastore1
-- second:
-    size: 4096
-    format: vmdk
-    model: scsi
-    pool: datastore2
-'''
-        nicp_yaml = '''
-- type: bridge
-  name: eth1
-  source: ONENET
-  model: e1000
-  mac: '00:00:00:00:00:00'
-- name: eth2
-  type: bridge
-  source: TWONET
-  model: e1000
-  mac: '00:00:00:00:00:00'
-'''
-        with patch('salt.modules.virt._nic_profile') as nic_profile, \
-                patch('salt.modules.virt._disk_profile') as disk_profile:
-            disk_profile.return_value = salt.utils.yaml.safe_load(diskp_yaml)
-            nic_profile.return_value = salt.utils.yaml.safe_load(nicp_yaml)
-            diskp = virt._disk_profile('noeffect', 'esxi')
-            nicp = virt._nic_profile('noeffect', 'esxi')
+        disks = {
+            'noeffect': [
+                {'first': {'size': 8192, 'pool': 'datastore1'}},
+                {'second': {'size': 4096, 'pool': 'datastore2'}}
+            ]
+        }
+        nics = {
+            'noeffect': [
+                {'name': 'eth1', 'source': 'ONENET'},
+                {'name': 'eth2', 'source': 'TWONET'}
+            ]
+        }
+        with patch.dict(virt.__salt__,  # pylint: disable=no-member
+                        {'config.get': MagicMock(side_effect=[disks, nics])}):
+            diskp = virt._disk_profile('noeffect', 'vmware', [], 'hello')
+            nicp = virt._nic_profile('noeffect', 'vmware')
             xml_data = virt._gen_xml(
                 'hello',
                 1,
                 512,
                 diskp,
                 nicp,
-                'esxi',
+                'vmware',
                 )
             root = ET.fromstring(xml_data)
             self.assertEqual(root.attrib['type'], 'vmware')
@@ -433,35 +539,21 @@ class VirtTestCase(TestCase, LoaderModuleMockMixin):
         '''
         Test virt._gen_xml(), KVM custom profile case
         '''
-        diskp_yaml = '''
-- first:
-    size: 8192
-    format: qcow2
-    model: virtio
-    pool: /var/lib/images
-- second:
-    size: 4096
-    format: qcow2
-    model: virtio
-    pool: /var/lib/images
-'''
-        nicp_yaml = '''
-- type: bridge
-  name: eth1
-  source: b2
-  model: virtio
-  mac: '00:00:00:00:00:00'
-- name: eth2
-  type: bridge
-  source: b2
-  model: virtio
-  mac: '00:00:00:00:00:00'
-'''
-        with patch('salt.modules.virt._nic_profile') as nic_profile, \
-                patch('salt.modules.virt._disk_profile') as disk_profile:
-            disk_profile.return_value = salt.utils.yaml.safe_load(diskp_yaml)
-            nic_profile.return_value = salt.utils.yaml.safe_load(nicp_yaml)
-            diskp = virt._disk_profile('noeffect', 'kvm')
+        disks = {
+            'noeffect': [
+                {'first': {'size': 8192, 'pool': '/var/lib/images'}},
+                {'second': {'size': 4096, 'pool': '/var/lib/images'}}
+            ]
+        }
+        nics = {
+            'noeffect': [
+                {'name': 'eth1', 'source': 'b2'},
+                {'name': 'eth2', 'source': 'b2'}
+            ]
+        }
+        with patch.dict(virt.__salt__, {'config.get': MagicMock(side_effect=[  # pylint: disable=no-member
+                disks, nics])}):
+            diskp = virt._disk_profile('noeffect', 'kvm', [], 'hello')
             nicp = virt._nic_profile('noeffect', 'kvm')
             xml_data = virt._gen_xml(
                 'hello',
@@ -479,19 +571,68 @@ class VirtTestCase(TestCase, LoaderModuleMockMixin):
             self.assertTrue(len(root.findall('.//disk')) == 2)
             self.assertTrue(len(root.findall('.//interface')) == 2)
 
+    @patch('salt.modules.virt.pool_info', return_value={'target_path': '/pools/default'})
+    def test_disk_profile_kvm_disk_pool(self, mock_poolinfo):
+        '''
+        Test virt._gen_xml(), KVM case with pools defined.
+        '''
+        disks = {
+            'noeffect': [
+                {'first': {'size': 8192, 'pool': 'default'}},
+                {'second': {'size': 4096}}
+            ]
+        }
+        with patch.dict(virt.__salt__, {'config.get': MagicMock(side_effect=[  # pylint: disable=no-member
+                disks, "/default/path/"])}):
+            diskp = virt._disk_profile('noeffect', 'kvm', [], 'hello')
+
+            self.assertEqual(len(diskp), 2)
+            self.assertTrue(diskp[0]['source_file'].startswith('/pools/default/'))
+            self.assertTrue(diskp[1]['source_file'].startswith('/default/path/'))
+
+    @patch('salt.modules.virt.pool_info', return_value={})
+    def test_disk_profile_kvm_disk_pool_notfound(self, mock_poolinfo):
+        '''
+        Test virt._gen_xml(), KVM case with pools defined.
+        '''
+        disks = {
+            'noeffect': [
+                {'first': {'size': 8192, 'pool': 'default'}},
+            ]
+        }
+        with patch.dict(virt.__salt__, {'config.get': MagicMock(side_effect=[  # pylint: disable=no-member
+                disks, "/default/path/"])}):
+            with self.assertRaises(CommandExecutionError):
+                virt._disk_profile('noeffect', 'kvm', [], 'hello')
+
+    @patch('salt.modules.virt.pool_info', return_value={'target_path': '/dev/disk/by-path'})
+    def test_disk_profile_kvm_disk_pool_invalid(self, mock_poolinfo):
+        '''
+        Test virt._gen_xml(), KVM case with pools defined.
+        '''
+        disks = {
+            'noeffect': [
+                {'first': {'size': 8192, 'pool': 'default'}},
+            ]
+        }
+        with patch.dict(virt.__salt__, {'config.get': MagicMock(side_effect=[  # pylint: disable=no-member
+                disks, "/default/path/"])}):
+            with self.assertRaises(CommandExecutionError):
+                virt._disk_profile('noeffect', 'kvm', [], 'hello')
+
     def test_controller_for_esxi(self):
         '''
-        Test virt._gen_xml() generated device controller for ESXi
+        Test virt._gen_xml() generated device controller for ESXi/vmware
         '''
-        diskp = virt._disk_profile('default', 'esxi')
-        nicp = virt._nic_profile('default', 'esxi')
+        diskp = virt._disk_profile('default', 'vmware', [], 'hello')
+        nicp = virt._nic_profile('default', 'vmware')
         xml_data = virt._gen_xml(
             'hello',
             1,
             512,
             diskp,
             nicp,
-            'esxi'
+            'vmware'
             )
         root = ET.fromstring(xml_data)
         controllers = root.findall('.//devices/controller')
@@ -503,7 +644,7 @@ class VirtTestCase(TestCase, LoaderModuleMockMixin):
         '''
         Test virt._gen_xml() generated device controller for KVM
         '''
-        diskp = virt._disk_profile('default', 'kvm')
+        diskp = virt._disk_profile('default', 'kvm', [], 'hello')
         nicp = virt._nic_profile('default', 'kvm')
         xml_data = virt._gen_xml(
             'hello',
@@ -608,11 +749,130 @@ class VirtTestCase(TestCase, LoaderModuleMockMixin):
         self.assertEqual('bridge', nic['type'])
         self.assertEqual('ac:de:48:b6:8b:59', nic['mac'])
 
-    @patch('subprocess.Popen')
-    @patch('subprocess.Popen.communicate', return_value="")
-    def test_get_disks(self, mock_communicate, mock_popen):
+    def test_parse_qemu_img_info(self):
         '''
-        Test virt.get_discs()
+        Make sure that qemu-img info output is properly parsed
+        '''
+        qemu_infos = '''[{
+            "snapshots": [
+                {
+                    "vm-clock-nsec": 0,
+                    "name": "first-snap",
+                    "date-sec": 1528877587,
+                    "date-nsec": 380589000,
+                    "vm-clock-sec": 0,
+                    "id": "1",
+                    "vm-state-size": 1234
+                },
+                {
+                    "vm-clock-nsec": 0,
+                    "name": "second snap",
+                    "date-sec": 1528877592,
+                    "date-nsec": 933509000,
+                    "vm-clock-sec": 0,
+                    "id": "2",
+                    "vm-state-size": 4567
+                }
+            ],
+            "virtual-size": 25769803776,
+            "filename": "/disks/test.qcow2",
+            "cluster-size": 65536,
+            "format": "qcow2",
+            "actual-size": 217088,
+            "format-specific": {
+                "type": "qcow2",
+                "data": {
+                    "compat": "1.1",
+                    "lazy-refcounts": false,
+                    "refcount-bits": 16,
+                    "corrupt": false
+                }
+            },
+            "full-backing-filename": "/disks/mybacking.qcow2",
+            "backing-filename": "mybacking.qcow2",
+            "dirty-flag": false
+        },
+        {
+            "virtual-size": 25769803776,
+            "filename": "/disks/mybacking.qcow2",
+            "cluster-size": 65536,
+            "format": "qcow2",
+            "actual-size": 393744384,
+            "format-specific": {
+                "type": "qcow2",
+                "data": {
+                    "compat": "1.1",
+                    "lazy-refcounts": false,
+                    "refcount-bits": 16,
+                    "corrupt": false
+                }
+            },
+            "full-backing-filename": "/disks/root.qcow2",
+            "backing-filename": "root.qcow2",
+            "dirty-flag": false
+        },
+        {
+            "virtual-size": 25769803776,
+            "filename": "/disks/root.qcow2",
+            "cluster-size": 65536,
+            "format": "qcow2",
+            "actual-size": 196872192,
+            "format-specific": {
+                "type": "qcow2",
+                "data": {
+                    "compat": "1.1",
+                    "lazy-refcounts": false,
+                    "refcount-bits": 16,
+                    "corrupt": false
+                }
+            },
+            "dirty-flag": false
+        }]'''
+
+        self.assertEqual(
+            {
+                'file': '/disks/test.qcow2',
+                'file format': 'qcow2',
+                'backing file': {
+                    'file': '/disks/mybacking.qcow2',
+                    'file format': 'qcow2',
+                    'disk size': 393744384,
+                    'virtual size': 25769803776,
+                    'cluster size': 65536,
+                    'backing file': {
+                        'file': '/disks/root.qcow2',
+                        'file format': 'qcow2',
+                        'disk size': 196872192,
+                        'virtual size': 25769803776,
+                        'cluster size': 65536,
+                    }
+                },
+                'disk size': 217088,
+                'virtual size': 25769803776,
+                'cluster size': 65536,
+                'snapshots': [
+                    {
+                        'id': '1',
+                        'tag': 'first-snap',
+                        'vmsize': 1234,
+                        'date': datetime.datetime.fromtimestamp(
+                            float("{}.{}".format(1528877587, 380589000))).isoformat(),
+                        'vmclock': '00:00:00'
+                    },
+                    {
+                        'id': '2',
+                        'tag': 'second snap',
+                        'vmsize': 4567,
+                        'date': datetime.datetime.fromtimestamp(
+                            float("{}.{}".format(1528877592, 933509000))).isoformat(),
+                        'vmclock': '00:00:00'
+                    }
+                ],
+            }, virt._parse_qemu_img_info(qemu_infos))
+
+    def test_get_disks(self):
+        '''
+        Test virt.get_disks()
         '''
         xml = '''<domain type='kvm' id='7'>
               <name>test-vm</name>
@@ -633,20 +893,58 @@ class VirtTestCase(TestCase, LoaderModuleMockMixin):
         '''
         self.set_mock_vm("test-vm", xml)
 
+        qemu_infos = '''[{
+            "virtual-size": 25769803776,
+            "filename": "/disks/test.qcow2",
+            "cluster-size": 65536,
+            "format": "qcow2",
+            "actual-size": 217088,
+            "format-specific": {
+                "type": "qcow2",
+                "data": {
+                    "compat": "1.1",
+                    "lazy-refcounts": false,
+                    "refcount-bits": 16,
+                    "corrupt": false
+                }
+            },
+            "full-backing-filename": "/disks/mybacking.qcow2",
+            "backing-filename": "mybacking.qcow2",
+            "dirty-flag": false
+        },
+        {
+            "virtual-size": 25769803776,
+            "filename": "/disks/mybacking.qcow2",
+            "cluster-size": 65536,
+            "format": "qcow2",
+            "actual-size": 393744384,
+            "format-specific": {
+                "type": "qcow2",
+                "data": {
+                    "compat": "1.1",
+                    "lazy-refcounts": false,
+                    "refcount-bits": 16,
+                    "corrupt": false
+                }
+            },
+            "dirty-flag": false
+        }]'''
+
+        self.mock_popen.communicate.return_value = [qemu_infos]  # pylint: disable=no-member
         disks = virt.get_disks('test-vm')
         disk = disks.get('vda')
         self.assertEqual('/disks/test.qcow2', disk['file'])
         self.assertEqual('disk', disk['type'])
+        self.assertEqual('/disks/mybacking.qcow2', disk['backing file']['file'])
         cdrom = disks.get('hda')
         self.assertEqual('/disks/test-cdrom.iso', cdrom['file'])
         self.assertEqual('cdrom', cdrom['type'])
+        self.assertFalse('backing file' in cdrom.keys())
 
-    @patch('subprocess.Popen')
-    @patch('subprocess.Popen.communicate', return_value="")
     @patch('salt.modules.virt.stop', return_value=True)
     @patch('salt.modules.virt.undefine')
     @patch('os.remove')
-    def test_purge_default(self, mock_remove, mock_undefine, mock_stop, mock_communicate, mock_popen):
+    def test_purge_default(self, mock_remove, mock_undefine, mock_stop):
         '''
         Test virt.purge() with default parameters
         '''
@@ -669,17 +967,35 @@ class VirtTestCase(TestCase, LoaderModuleMockMixin):
         '''
         self.set_mock_vm("test-vm", xml)
 
+        qemu_infos = '''[{
+            "virtual-size": 25769803776,
+            "filename": "/disks/test.qcow2",
+            "cluster-size": 65536,
+            "format": "qcow2",
+            "actual-size": 217088,
+            "format-specific": {
+                "type": "qcow2",
+                "data": {
+                    "compat": "1.1",
+                    "lazy-refcounts": false,
+                    "refcount-bits": 16,
+                    "corrupt": false
+                }
+            },
+            "dirty-flag": false
+        }]'''
+
+        self.mock_popen.communicate.return_value = [qemu_infos]  # pylint: disable=no-member
+
         res = virt.purge('test-vm')
         self.assertTrue(res)
         mock_remove.assert_any_call('/disks/test.qcow2')
         mock_remove.assert_any_call('/disks/test-cdrom.iso')
 
-    @patch('subprocess.Popen')
-    @patch('subprocess.Popen.communicate', return_value="")
     @patch('salt.modules.virt.stop', return_value=True)
     @patch('salt.modules.virt.undefine')
     @patch('os.remove')
-    def test_purge_noremovable(self, mock_remove, mock_undefine, mock_stop, mock_communicate, mock_popen):
+    def test_purge_noremovable(self, mock_remove, mock_undefine, mock_stop):
         '''
         Test virt.purge(removables=False)
         '''
@@ -708,6 +1024,26 @@ class VirtTestCase(TestCase, LoaderModuleMockMixin):
         '''
         self.set_mock_vm("test-vm", xml)
 
+        qemu_infos = '''[{
+            "virtual-size": 25769803776,
+            "filename": "/disks/test.qcow2",
+            "cluster-size": 65536,
+            "format": "qcow2",
+            "actual-size": 217088,
+            "format-specific": {
+                "type": "qcow2",
+                "data": {
+                    "compat": "1.1",
+                    "lazy-refcounts": false,
+                    "refcount-bits": 16,
+                    "corrupt": false
+                }
+            },
+            "dirty-flag": false
+        }]'''
+
+        self.mock_popen.communicate.return_value = [qemu_infos]  # pylint: disable=no-member
+
         res = virt.purge('test-vm', removables=False)
         self.assertTrue(res)
         mock_remove.assert_called_once()
@@ -1285,6 +1621,23 @@ class VirtTestCase(TestCase, LoaderModuleMockMixin):
         pool_mock.info.return_value = [0, 1234, 5678, 123]
         pool_mock.autostart.return_value = True
         pool_mock.isPersistent.return_value = True
+        pool_mock.XMLDesc.return_value = '''<pool type='dir'>
+  <name>default</name>
+  <uuid>d92682d0-33cf-4e10-9837-a216c463e158</uuid>
+  <capacity unit='bytes'>854374301696</capacity>
+  <allocation unit='bytes'>596275986432</allocation>
+  <available unit='bytes'>258098315264</available>
+  <source>
+  </source>
+  <target>
+    <path>/srv/vms</path>
+    <permissions>
+      <mode>0755</mode>
+      <owner>0</owner>
+      <group>0</group>
+    </permissions>
+  </target>
+</pool>'''
         self.mock_conn.storagePoolLookupByName.return_value = pool_mock
         # pylint: enable=no-member
 
@@ -1296,7 +1649,9 @@ class VirtTestCase(TestCase, LoaderModuleMockMixin):
             'allocation': 5678,
             'free': 123,
             'autostart': True,
-            'persistent': True}, pool)
+            'persistent': True,
+            'type': 'dir',
+            'target_path': '/srv/vms'}, pool)
 
     def test_pool_info_notfound(self):
         '''

tests/unit/pillar/test_consul.py

@@ -38,7 +38,7 @@ SIMPLE_DICT = {'key1': {'key2': 'val1'}}
 
 
 @skipIf(NO_MOCK, NO_MOCK_REASON)
-@skipIf(not consul_pillar.HAS_CONSUL, 'python-consul module not installed')
+@skipIf(not consul_pillar.consul, 'python-consul module not installed')
 class ConsulPillarTestCase(TestCase, LoaderModuleMockMixin):
     '''
     Test cases for salt.pillar.consul_pillar

tests/unit/utils/test_thin.py

@@ -280,6 +280,7 @@ class SSHThinTestCase(TestCase):
     @patch('salt.utils.thin.ssl_match_hostname', type(str('ssl_match_hostname'), (), {'__file__': '/site-packages/ssl_mh'}))
     @patch('salt.utils.thin.markupsafe', type(str('markupsafe'), (), {'__file__': '/site-packages/markupsafe'}))
     @patch('salt.utils.thin.backports_abc', type(str('backports_abc'), (), {'__file__': '/site-packages/backports_abc'}))
+    @patch('salt.utils.thin.concurrent', type(str('concurrent'), (), {'__file__': '/site-packages/concurrent'}))
     @patch('salt.utils.thin.log', MagicMock())
     def test_get_tops(self):
         '''
@@ -289,7 +290,7 @@ class SSHThinTestCase(TestCase):
         base_tops = ['/site-packages/salt', '/site-packages/jinja2', '/site-packages/yaml',
                      '/site-packages/tornado', '/site-packages/msgpack', '/site-packages/certifi',
                      '/site-packages/sdp', '/site-packages/sdp_hlp', '/site-packages/ssl_mh',
-                     '/site-packages/markupsafe', '/site-packages/backports_abc']
+                     '/site-packages/markupsafe', '/site-packages/backports_abc', '/site-packages/concurrent']
 
         tops = thin.get_tops()
         assert len(tops) == len(base_tops)
@@ -306,6 +307,7 @@ class SSHThinTestCase(TestCase):
     @patch('salt.utils.thin.ssl_match_hostname', type(str('ssl_match_hostname'), (), {'__file__': '/site-packages/ssl_mh'}))
     @patch('salt.utils.thin.markupsafe', type(str('markupsafe'), (), {'__file__': '/site-packages/markupsafe'}))
     @patch('salt.utils.thin.backports_abc', type(str('backports_abc'), (), {'__file__': '/site-packages/backports_abc'}))
+    @patch('salt.utils.thin.concurrent', type(str('concurrent'), (), {'__file__': '/site-packages/concurrent'}))
     @patch('salt.utils.thin.log', MagicMock())
     def test_get_tops_extra_mods(self):
         '''
@@ -314,7 +316,7 @@ class SSHThinTestCase(TestCase):
         '''
         base_tops = ['/site-packages/salt', '/site-packages/jinja2', '/site-packages/yaml',
                      '/site-packages/tornado', '/site-packages/msgpack', '/site-packages/certifi',
-                     '/site-packages/sdp', '/site-packages/sdp_hlp', '/site-packages/ssl_mh',
+                     '/site-packages/sdp', '/site-packages/sdp_hlp', '/site-packages/ssl_mh', '/site-packages/concurrent',
                      '/site-packages/markupsafe', '/site-packages/backports_abc', '/custom/foo', '/custom/bar.py']
         builtins = sys.version_info.major == 3 and 'builtins' or '__builtin__'
         with patch('{}.__import__'.format(builtins),
@@ -335,6 +337,7 @@ class SSHThinTestCase(TestCase):
     @patch('salt.utils.thin.ssl_match_hostname', type(str('ssl_match_hostname'), (), {'__file__': '/site-packages/ssl_mh'}))
     @patch('salt.utils.thin.markupsafe', type(str('markupsafe'), (), {'__file__': '/site-packages/markupsafe'}))
     @patch('salt.utils.thin.backports_abc', type(str('backports_abc'), (), {'__file__': '/site-packages/backports_abc'}))
+    @patch('salt.utils.thin.concurrent', type(str('concurrent'), (), {'__file__': '/site-packages/concurrent'}))
     @patch('salt.utils.thin.log', MagicMock())
     def test_get_tops_so_mods(self):
         '''
@@ -343,7 +346,7 @@ class SSHThinTestCase(TestCase):
         '''
         base_tops = ['/site-packages/salt', '/site-packages/jinja2', '/site-packages/yaml',
                      '/site-packages/tornado', '/site-packages/msgpack', '/site-packages/certifi',
-                     '/site-packages/sdp', '/site-packages/sdp_hlp', '/site-packages/ssl_mh',
+                     '/site-packages/sdp', '/site-packages/sdp_hlp', '/site-packages/ssl_mh', '/site-packages/concurrent',
                      '/site-packages/markupsafe', '/site-packages/backports_abc', '/custom/foo.so', '/custom/bar.so']
         builtins = sys.version_info.major == 3 and 'builtins' or '__builtin__'
         with patch('{}.__import__'.format(builtins),

tests/unit/utils/vmware/test_host.py

@@ -105,7 +105,6 @@ class GetHostsTestCase(TestCase):
                         self.mock_si, datacenter_name='fake_datacenter',
                         cluster_name='fake_cluster')
         mock_get_dc.assert_called_once_with(self.mock_si, 'fake_datacenter')
-        mock_get_cl.assert_called_once_with(mock_dc, 'fake_cluster')
         mock_get_mors.assert_called_once_with(self.mock_si,
                                               vim.HostSystem,
                                               container_ref=mock_dc,

tests/unit/utils/vmware/test_license.py

@@ -614,7 +614,7 @@ class AssignLicenseTestCase(TestCase):
                                          'fake_license_name',
                                          entity_name='fake_entity_name')
         self.mock_update_assigned_license.assert_called_once_with(
-            self.mock_ent_id, self.mock_lic_key)
+            self.mock_ent_id, self.mock_lic_key, 'fake_license_name')
 
     def test_update_assigned_licenses_call_with_entity(self):
         salt.utils.vmware.assign_license(self.mock_si,
@@ -623,7 +623,7 @@ class AssignLicenseTestCase(TestCase):
                                          self.mock_entity_ref,
                                          'fake_entity_name')
         self.mock_update_assigned_license.assert_called_once_with(
-            self.mock_moid, self.mock_lic_key)
+            self.mock_moid, self.mock_lic_key, 'fake_license_name')
 
     def test_update_assigned_licenses_raises_no_permission(self):
         exc = vim.fault.NoPermission()