[BUG] 3006: virt: Externally managed LVM volumes no longer work (libvirt 8+, qemu 6.2+)

[ewenmcneill]

Description

For manually maintained virt VM disks (ie, not in a pool), if source_file is given to define the disk storage, then salt/modules/virt.py _gen_xml forces the libvirt disk type to be file.

Before libvirt 8 / qemu 6.2 (eg, as present on Ubuntu 20.04 LTS and earlier), having a LVM logical volume that was referred to as type file was supported, and worked with saltstack before 3002, and since saltstack 3004 (saltstack 3002 introduced an attempt to turn the source_file into a file if it was not a file, which was fixed in 3003 -- backstory in #53081 (comment) and an example in #60296).

Since libvirt 8 / qemu 6.2 (eg as present on Ubuntu 22.04 LTS and later), referring to a LVM logical volume disk as type file is rejected:

'file' driver requires '/dev/ssd/monitor_root' to be a regular file

which means the VM fails to define/start.

That libvirt 8 / qemu 6.2 issue can be avoided by changing the type to block (and the source file= to source dev=).

Salt 3006 has the necessary XML templating code to support this generating the right XML when type = 'block' is set:

salt/salt/templates/virt/libvirt_domain.jinja

Lines 244 to 263 in 21e99f6

	{%- for disk in disks %}
	<disk type='{{ disk.type }}' device='{{ disk.device }}'>
	{%- if disk.type == 'file' and 'source_file' in disk -%}
	<source file='{{ disk.source_file }}' />
	{%- endif %}
	{%- if disk.type == 'block' -%}
	<source dev='{{ disk.source_file }}' />
	{%- endif %}
	{%- if disk.type == 'volume' and 'pool' in disk -%}
	<source pool='{{ disk.pool }}' volume='{{ disk.volume }}' />
	{%- endif %}
	{%- if disk.type == 'network' %}{{ libvirt_disks.network_source(disk) }}{%- endif %}
	<target dev='{{ disk.target_dev }}' bus='{{ disk.disk_bus }}' />
	{%- if disk.address -%}
	<address type='drive' controller='0' bus='0' target='0' unit='{{ disk.index }}' />
	{%- endif %}
	{%- if disk.driver -%}
	<driver name='qemu' type='{{ disk.format}}' cache='none' io='{{ disk.io }}'{{ opt_attribute(disk, "iothread") }}/>
	{%- endif %}
	</disk>

but salt/modules/virt.py _gen_xml() forces type = 'file' any time source_file is defined, no matter what the filename is:

salt/salt/modules/virt.py

Lines 1063 to 1067 in 21e99f6

	if disk.get("source_file"):
	url = urllib.parse.urlparse(disk["source_file"])
	if not url.scheme or not url.hostname:
	disk_context["source_file"] = disk["source_file"]
	disk_context["type"] = "file"

which means there is no way to pass in the required type = 'block' from the disk defintion.

And even if the disk type is manually changed in the libvirt definition (eg, via virsh the salt virt.py is overriding it back to type = 'file', which rewrites the libvirl XML and breaks the VM definition.

Setup

• on-prem machine
• VM (Virtualbox, KVM, etc. please specify)
• VM running on a cloud service, please be explicit and add details
• container (Kubernetes, Docker, containerd, etc. please specify)
• or a combination, please be explicit
• jails if it is FreeBSD
• classic packaging
• onedir packaging
• used bootstrap to install

The salt master is a VM. The relevant salt miniion is an Ubuntu 22.04 LTS VM (upgraded from Ubuntu 20.04 LTS earlier this month, due to the end of support of Ubuntu 20.04 LTS approaching very soon). The salt minion is a VM host, with libvirt / qemu, managed by salt.

Steps to Reproduce the behavior

Define a LVM volume group (perhaps backed by a temporarily created file as the physical volume). Define a logical volume on that volume group. Make sure libvirt / qemu are installed on the target. (See, eg, #60296 for a simple example of that.)

Define a hardcoded disk type (to avoid the deafult qcow2 disk being injected into the VM with manually managed storage (eg in a pillar):

virt:
  disk:
    hardcoded: []

Then use virt.running to define a VM disk a disk that refers to the LVM logical volume created above, eg:

testvm:
  virt.running:
    - cpu: 1
    - mem: 8
    - disk_profile: hardcoded
    - disks::
       - name: vda
         source_file: /dev/ssd/monitor_root
         format: raw
         model: virtio
         device: disk
         type: block

Attempt to get salt to define that VM (eg, highstate), and observe that the type will be set to file, which will not work.

Expected behavior

It should be possible to override the default type = 'file' by specifying the type: block in the salt disk definition, so the user can override the default type: file that is being set (when source_file is set, or by default). Eg,

...
   - disks:
       - name: vda
         source_file: /dev/ssd/monitor_root
         format: raw
         model: virtio
         device: disk
         type: block

and that should override the default salt/modules/virt.py type = 'file' setting.

This can be achieved with a trivial code change in salt/modules/virt.py in _gen_xml() to look for a user supplied type override:

        if disk.get("source_file"):
            url = urllib.parse.urlparse(disk["source_file"])
            if not url.scheme or not url.hostname:
                disk_context["source_file"] = disk["source_file"]
                # 2025-04-08 - permit having type=block if user specified
                #disk_context["type"] = "file"
                disk_context["type"] = disk.get("type", "file")

Alternatively if the source_file starts with /dev then it could be automatically treated as a block type.

(For now I'm not attempting to make this a PR, as I'm unsure if y'all want it to be a documented disk parameter, just an "if you know, it works" parameter, or magically autodetect based on disk['source_file'].startswith('/dev').

Screenshots

N/A.

Versions Report

salt --versions-report

salt-master:

ewen@salt:~$ salt --versions-report
Salt Version:
          Salt: 3006.10
 
Python Version:
        Python: 3.10.16 (main, Mar  6 2025, 02:23:15) [GCC 11.2.0]
 
Dependency Versions:
          cffi: 1.14.6
      cherrypy: unknown
  cryptography: 42.0.5
      dateutil: 2.8.1
     docker-py: Not Installed
         gitdb: Not Installed
     gitpython: Not Installed
        Jinja2: 3.1.6
       libgit2: Not Installed
  looseversion: 1.0.2
      M2Crypto: Not Installed
          Mako: Not Installed
       msgpack: 1.0.2
  msgpack-pure: Not Installed
  mysql-python: Not Installed
     packaging: 22.0
     pycparser: 2.21
      pycrypto: Not Installed
  pycryptodome: 3.19.1
        pygit2: Not Installed
  python-gnupg: 0.4.8
        PyYAML: 6.0.1
         PyZMQ: 23.2.0
        relenv: 0.18.1
         smmap: Not Installed
       timelib: 0.2.4
       Tornado: 4.5.3
           ZMQ: 4.3.4
 
System Versions:
          dist: debian 12 bookworm
        locale: utf-8
       machine: x86_64
       release: 6.1.0-32-amd64
        system: Linux
       version: Debian GNU/Linux 12 bookworm
 
ewen@salt:~$ 

The relevant minion is the same, but on Ubuntu 22.04 LTS.

Additional context

N/A.