virt: fix VM creating with disk volume

Since volumes in a virtual storage pool of type 'disk' are partitions,
then need to be named after the disk name with sequential numbers rather
than using the VM and disk names.

Also, the format passed by the user is the one used when creating the volume.
However in the VM definition for logical and disk volumes, the format should
be set to raw.

salt/modules/virt.py

@@ -732,6 +732,9 @@ def _gen_xml(
                         "usage": usage,
                     }
             else:
+                if pool_type in ["disk", "logical"]:
+                    # The volume format for these types doesn't match the driver format in the VM
+                    disk_context["format"] = "raw"
                 disk_context["type"] = "volume"
                 disk_context["pool"] = disk["pool"]
 
@@ -1296,6 +1299,18 @@ def _fill_disk_filename(conn, vm_name, disk, hypervisor, pool_caps):
                 else:
                     disk["format"] = volume_options.get("default_format", None)
 
+            # Disk pools volume names are partition names, they need to be named based on the device name
+            if pool_type == "disk":
+                device = pool_xml.find("./source/device").get("path")
+                indexes = [
+                    int(re.sub("[a-z]+", "", vol_name))
+                    for vol_name in pool_obj.listVolumes()
+                ] or [0]
+                index = min(
+                    [idx for idx in range(1, max(indexes) + 2) if idx not in indexes]
+                )
+                disk["filename"] = "{}{}".format(os.path.basename(device), index)
+
     elif hypervisor == "bhyve" and vm_name:
         disk["filename"] = "{0}.{1}".format(vm_name, disk["name"])
         disk["source_file"] = os.path.join(

tests/unit/modules/test_virt.py

@@ -860,6 +860,64 @@ class VirtTestCase(TestCase, LoaderModuleMockMixin):
             self.assertEqual(len(diskp), 1)
             self.assertEqual(diskp[0]["source_file"], ("/path/to/my/image.qcow2"))
 
+    def test_disk_profile_pool_disk_type(self):
+        """
+        Test virt._disk_profile(), with a disk pool of disk type
+        """
+        self.mock_conn.listStoragePools.return_value = ["test-vdb"]
+        self.mock_conn.storagePoolLookupByName.return_value.XMLDesc.return_value = """
+            <pool type="disk">
+              <name>test-vdb</name>
+              <source>
+                <device path='/dev/vdb'/>
+              </source>
+              <target>
+                <path>/dev</path>
+              </target>
+            </pool>
+        """
+
+        # No existing disk case
+        self.mock_conn.storagePoolLookupByName.return_value.listVolumes.return_value = (
+            []
+        )
+        diskp = virt._disk_profile(
+            self.mock_conn,
+            None,
+            "kvm",
+            [{"name": "mydisk", "pool": "test-vdb"}],
+            "hello",
+        )
+        self.assertEqual(diskp[0]["filename"], ("vdb1"))
+
+        # Append to the end case
+        self.mock_conn.storagePoolLookupByName.return_value.listVolumes.return_value = [
+            "vdb1",
+            "vdb2",
+        ]
+        diskp = virt._disk_profile(
+            self.mock_conn,
+            None,
+            "kvm",
+            [{"name": "mydisk", "pool": "test-vdb"}],
+            "hello",
+        )
+        self.assertEqual(diskp[0]["filename"], ("vdb3"))
+
+        # Hole in the middle case
+        self.mock_conn.storagePoolLookupByName.return_value.listVolumes.return_value = [
+            "vdb1",
+            "vdb3",
+        ]
+        diskp = virt._disk_profile(
+            self.mock_conn,
+            None,
+            "kvm",
+            [{"name": "mydisk", "pool": "test-vdb"}],
+            "hello",
+        )
+        self.assertEqual(diskp[0]["filename"], ("vdb2"))
+
     def test_gen_xml_volume(self):
         """
         Test virt._gen_xml(), generating a disk of volume type
@@ -1007,6 +1065,52 @@ class VirtTestCase(TestCase, LoaderModuleMockMixin):
         )
         self.mock_conn.secretLookupByUUIDString.assert_called_once_with("some-uuid")
 
+        # Disk volume test case
+        self.mock_conn.getStoragePoolCapabilities.return_value = """
+            <storagepoolCapabilities>
+              <pool type='disk' supported='yes'>
+                <volOptions>
+                  <defaultFormat type='none'/>
+                  <enum name='targetFormatType'>
+                    <value>none</value>
+                    <value>linux</value>
+                    <value>fat16</value>
+                  </enum>
+                </volOptions>
+              </pool>
+            </storagepoolCapabilities>
+        """
+        self.mock_conn.storagePoolLookupByName.return_value.XMLDesc.return_value = """
+            <pool type='disk'>
+              <name>test-vdb</name>
+              <source>
+                <device path='/dev/vdb'/>
+                <format type='gpt'/>
+              </source>
+            </pool>
+        """
+        self.mock_conn.listStoragePools.return_value = ["test-vdb"]
+        self.mock_conn.storagePoolLookupByName.return_value.listVolumes.return_value = [
+            "vdb1",
+        ]
+        diskp = virt._disk_profile(
+            self.mock_conn,
+            None,
+            "kvm",
+            [{"name": "system", "pool": "test-vdb"}],
+            "test-vm",
+        )
+        xml_data = virt._gen_xml(
+            self.mock_conn, "hello", 1, 512, diskp, nicp, "kvm", "hvm", "x86_64",
+        )
+        root = ET.fromstring(xml_data)
+        disk = root.findall(".//disk")[0]
+        self.assertEqual(disk.attrib["type"], "volume")
+        source = disk.find("source")
+        self.assertEqual("test-vdb", source.attrib["pool"])
+        self.assertEqual("vdb2", source.attrib["volume"])
+        self.assertEqual("raw", disk.find("driver").get("type"))
+
     def test_gen_xml_cdrom(self):
         """
         Test virt._gen_xml(), generating a cdrom device (different disk type, no source)