From 7c91803d73cb76bef329f0b8031f4d05465d93e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Bosdonnat?= Date: Mon, 29 Oct 2018 16:52:32 +0100 Subject: [PATCH 1/2] virt: expose source_file disk property to user User may want to update or create a VM with a disk image created or provided by a third party tool. In order to do this, let the user set a source_file property to the disk. Thus a {'name': 'myimage', 'source_file': '/path/to/image'} disk will just use the provided disk image. --- salt/modules/virt.py | 91 +++++++++++++++++++++++++-------- tests/unit/modules/test_virt.py | 13 +++++ 2 files changed, 82 insertions(+), 22 deletions(-) diff --git a/salt/modules/virt.py b/salt/modules/virt.py index 07ce53567f..03f06e681e 100644 --- a/salt/modules/virt.py +++ b/salt/modules/virt.py @@ -900,31 +900,42 @@ def _disk_profile(profile, hypervisor, disks=None, vm_name=None, image=None, poo 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']) + # We may have an already computed source_file (i.e. image not created by our module) + if 'source_file' in disk: + disk['filename'] = os.path.basename(disk['source_file']) + else: + _fill_disk_filename(vm_name, disk, hypervisor, **kwargs) return disklist +def _fill_disk_filename(vm_name, disk, hypervisor, **kwargs): + ''' + Compute the disk file name and update it in the disk value. + ''' + 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']) + + def _complete_nics(interfaces, hypervisor, dmac=None): ''' Complete missing data for network interfaces. @@ -1294,6 +1305,39 @@ def init(name, ``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``) +<<<<<<< HEAD +======= + hostname_property + When using ZFS volumes, setting this value to a ZFS property ID will make Salt store the name of the + virtual machine inside this property. (Default: ``None``) + + sparse_volume + Boolean to specify whether to use a thin provisioned ZFS volume. + + Example profile for a bhyve VM with two ZFS disks. The first is + cloned from the specified image. The second disk is a thin + provisioned volume. + + .. code-block:: yaml + + virt: + disk: + two_zvols: + - system: + image: zroot/bhyve/CentOS-7-x86_64-v1@v1.0.5 + hostname_property: virt:hostname + pool: zroot/bhyve/guests + - data: + pool: tank/disks + size: 20G + hostname_property: virt:hostname + sparse_volume: True + + source_file + Absolute path to the disk image to use. Not to be confused with ``image`` parameter. This + parameter is useful to use disk images that are created outside of this module. + +>>>>>>> 148e60488e... virt: expose source_file disk property to user .. _init-graphics-def: .. rubric:: Graphics Definition @@ -1433,7 +1477,10 @@ def init(name, else: create_overlay = _disk.get('overlay_image', False) - img_dest = _qemu_image_create(_disk, create_overlay, saltenv) + if os.path.exists(_disk['source_file']): + img_dest = _disk['source_file'] + else: + img_dest = _qemu_image_create(_disk, create_overlay, saltenv) # Seed only if there is an image specified if seed and _disk.get('image', None): diff --git a/tests/unit/modules/test_virt.py b/tests/unit/modules/test_virt.py index d6fc071438..cee5855a96 100644 --- a/tests/unit/modules/test_virt.py +++ b/tests/unit/modules/test_virt.py @@ -661,6 +661,19 @@ class VirtTestCase(TestCase, LoaderModuleMockMixin): self.assertTrue(diskp[0]['source_file'].startswith(pools_path)) self.assertTrue(diskp[1]['source_file'].startswith(default_path)) + def test_disk_profile_kvm_disk_external_image(self): + ''' + Test virt._gen_xml(), KVM case with an external image. + ''' + diskp = virt._disk_profile(None, 'kvm', [ + { + 'name': 'mydisk', + 'source_file': '/path/to/my/image.qcow2' + }], 'hello') + + self.assertEqual(len(diskp), 1) + self.assertEqual(diskp[0]['source_file'], ('/path/to/my/image.qcow2')) + @patch('salt.modules.virt.pool_info', return_value={}) def test_disk_profile_kvm_disk_pool_notfound(self, mock_poolinfo): ''' From ce1288e4377affba6c092c8f191b6e15fb6d30d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Bosdonnat?= Date: Mon, 29 Oct 2018 17:41:59 +0100 Subject: [PATCH 2/2] virt: changing disk bus should update it in the VM So far two disks are considered equal if they have the same source file. When a user only wants to change the disk bus, then the disk comparison algorithm will consider it unchanged. Now disks are compared on source and target bus. The other disk variables can be left out side the target device is computed. --- salt/modules/virt.py | 7 ++++++- tests/unit/modules/test_virt.py | 14 ++++++++++++-- 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/salt/modules/virt.py b/salt/modules/virt.py index 03f06e681e..c7b447c7bf 100644 --- a/salt/modules/virt.py +++ b/salt/modules/virt.py @@ -1539,8 +1539,13 @@ def _disks_equal(disk1, disk2): ''' Test if two disk elements should be considered like the same device ''' + target1 = disk1.find('target') + target2 = disk2.find('target') + return ElementTree.tostring(disk1.find('source')) == \ - ElementTree.tostring(disk2.find('source')) + ElementTree.tostring(disk2.find('source')) and \ + target1 is not None and target2 is not None and \ + target1.get('bus') == target2.get('bus') def _nics_equal(nic1, nic2): diff --git a/tests/unit/modules/test_virt.py b/tests/unit/modules/test_virt.py index cee5855a96..9b6b63a928 100644 --- a/tests/unit/modules/test_virt.py +++ b/tests/unit/modules/test_virt.py @@ -767,6 +767,10 @@ class VirtTestCase(TestCase, LoaderModuleMockMixin): + + + + ''').findall('disk') @@ -780,16 +784,22 @@ class VirtTestCase(TestCase, LoaderModuleMockMixin): + + + + ''').findall('disk') ret = virt._diff_disk_lists(old_disks, new_disks) self.assertEqual([disk.find('source').get('file') for disk in ret['deleted']], - ['/path/to/img1.qcow2', '/path/to/img2.qcow2']) + ['/path/to/img1.qcow2', '/path/to/img2.qcow2', '/path/to/img4.qcow2']) self.assertEqual([disk.find('source').get('file') for disk in ret['unchanged']], ['/path/to/img0.qcow2']) self.assertEqual([disk.find('source').get('file') for disk in ret['new']], - ['/path/to/img3.qcow2']) + ['/path/to/img3.qcow2', '/path/to/img4.qcow2']) self.assertEqual(ret['new'][0].find('target').get('dev'), 'vdb') + self.assertEqual(ret['new'][1].find('target').get('dev'), 'vdc') + self.assertEqual(ret['new'][1].find('target').get('bus'), 'virtio') def test_diff_nics(self): '''