cockpit-project / cockpit-machines

Cockpit UI for virtual machines
GNU Lesser General Public License v2.1
289 stars 74 forks source link

qcow2 volumes are created with qcow2 v2 (compat 0.10) instead of v3 (compat 1.1) #1450

Closed JeffWDH closed 7 months ago

JeffWDH commented 7 months ago

OS: Oracle Linux 9 cockpit-machines Version: 298

Hello, I am wondering if there is a way to have cockpit-machines create qcow2 v3 by default? There are no options to select the version/compat level. The only options are qcow2 and raw.

To reproduce you can create a volume from the VM configuration or from the storage pool.

Example:

$ qemu-img info Test.qcow2
image: Test.qcow2
file format: qcow2
virtual size: 100 GiB (107374182400 bytes)
disk size: 16.5 KiB
cluster_size: 65536
Format specific information:
    compat: 0.10
    compression type: zlib
    refcount bits: 16
Child node '/file':
    filename: Test.qcow2
    protocol type: file
    file length: 194 KiB (198656 bytes)
    disk size: 16.5 KiB
martinpitt commented 7 months ago

cockpit-machines does not create raw/qcow images directly, this happens through virt-install. Can you please report this against the "virt-manager" component in https://bugzilla.redhat.com/enter_bug.cgi?product=Fedora .

JeffWDH commented 7 months ago

@martinpitt I took some time to quickly review the code for this project and it seems like volume creation is done directly through libvirt.

Quote from the libvirt docs (https://github.com/libvirt/libvirt/blob/7cb4a68500b9b1e4aea40ac49f5d383e7b87ddec/docs/formatstorage.rst#storage-volume-target-elements):

compat Specify compatibility level. So far, this is only used for type='qcow2' volumes. Valid values are 0.10 and 1.1 so far, specifying QEMU version the images should be compatible with. If the feature element is present, 1.1 is used. Since 1.1.0 If omitted, 0.10 is used. Since 1.1.2

This means that unless using a specific feature, or defining compat, the default will be 0.10. This isn't ideal as some things require 1.1 to function (ie. live backups using virtnbdbackup).

Storage volume creation is found in this file: https://github.com/cockpit-project/cockpit-machines/blob/main/src/libvirtApi/storageVolume.js

import { getVolumeXML } from '../libvirt-xml-create.js';

...

export async function storageVolumeCreate({ connectionName, poolName, volName, size, format }) {
    const volXmlDesc = getVolumeXML(volName, size, format);

    const [path] = await call(connectionName, '/org/libvirt/QEMU', 'org.libvirt.Connect', 'StoragePoolLookupByName',
                              [poolName], { timeout, type: 's' });
    await call(connectionName, path, 'org.libvirt.StoragePool', 'StorageVolCreateXML', [volXmlDesc, 0], { timeout, type: 'su' });
    await storagePoolRefresh({ connectionName, objPath: path });

The XML is generated in libvirt-xml-create.js. I've created a patch that will set the compat option to 1.1 and this works as expected:

diff -u libvirt-xml-create.js.orig libvirt-xml-create.js
--- libvirt-xml-create.js.orig  2024-02-20 08:57:50.314394851 -0500
+++ libvirt-xml-create.js       2024-02-20 08:55:06.484537915 -0500
@@ -144,6 +144,11 @@
         const formatElem = doc.createElement('format');
         formatElem.setAttribute('type', format);
         targetElem.appendChild(formatElem);
+        if (format == 'qcow2') {
+          const compatElem = doc.createElement('compat');
+          compatElem.appendChild(doc.createTextNode('1.1'));
+          targetElem.appendChild(compatElem);
+        }
     }

     volElem.appendChild(targetElem);
qemu-img info Test
image: Test
file format: qcow2
virtual size: 2 GiB (2147483648 bytes)
disk size: 16.5 KiB
cluster_size: 65536
Format specific information:
    compat: 1.1
    compression type: zlib
    lazy refcounts: false
    refcount bits: 16
    corrupt: false
    extended l2: false
Child node '/file':
    filename: Test
    protocol type: file
    file length: 192 KiB (197120 bytes)
    disk size: 16.5 KiB

This patch was just a proof of concept. I think ideally this would be available for the user to pick from the web interface during volume creation.

According to the QEMU page about qcow2 (v3/1.1 - https://wiki.qemu.org/Features/Qcow3), it is the default qcow version since QEMU 1.7. I'd say this project should follow QEMU and use it by default as well, or at least provide and easy option.

Thoughts?

martinpitt commented 7 months ago

@JeffWDH : Yes, that's what I meant. The "primary" disk of a newly created VM gets created through virt-install, and even the additional disks get created through libvirt's StorageVolCreate(). We don't want to second-guess libvirt's defaults here -- if the new format can be used, it should default to it, or otherwise decide when it's appropriate. Hardcoding version numbers in callers is a no-go -- at least that needs to be detected dynamically. (But I'd still reject such a patch for upstream -- this needs to go into libvirt directly). Thanks!