Bug 2041440

Summary:	Enable discard=unmap by default
Product:	Red Hat Enterprise Linux 9	Reporter:	Martin Pitt <mpitt>
Component:	virt-manager	Assignee:	Jonathon Jongsma <jjongsma>
virt-manager sub component:	Common	QA Contact:	Hongzhou Liu <hongzliu>
Status:	CLOSED CURRENTRELEASE	Docs Contact:
Severity:	low
Priority:	medium	CC:	briasmit, crobinso, hongzliu, jsuchane, juzhou, tyan, tzheng, virt-maint
Version:	9.0	Keywords:	TestOnly, Triaged
Target Milestone:	rc
Target Release:	---
Hardware:	Unspecified
OS:	Unspecified
Whiteboard:
Fixed In Version:	virt-manager-4.0.0-1.el9	Doc Type:	If docs needed, set a value
Doc Text:		Story Points:	---
Clone Of:		Environment:
Last Closed:	2022-11-17 09:50:35 UTC	Type:	Enhancement
Regression:	---	Mount Type:	---
Documentation:	---	CRM:
Verified Versions:		Category:	---
oVirt Team:	---	RHEL 7.3 requirements from Atomic Host:
Cloudforms Team:	---	Target Upstream Version:
Embargoed:
Bug Depends On:	2100525
Bug Blocks:

Description Martin Pitt 2022-01-17 11:09:50 UTC

The libvirt disks documentation [1] mentions the "discard". This seems to be a "unbreak my setup" option at first sight? fstrim on the host and LVM etc. was enabled by default years ago in Fedora [2] and [Debian/Ubuntu [3]. Is there reason at all to not just make this the default?

That would simplify the virt-manager GUI, would do the right thing for virt-install, and not require users to think about such internal implementation details in cockpit-machines and similar UIs.

Thanks for considering!

[1] https://libvirt.org/formatdomain.html#hard-drives-floppy-disks-cdroms
[2] https://fedoraproject.org/wiki/Changes/EnableFSTrimTimer
[3] https://blueprints.launchpad.net/ubuntu/+spec/core-1311-ssd-trimming

Comment 2 Cole Robinson 2022-08-21 20:02:27 UTC

This is in virt-manager 4.0.0:

commit 381aa4050c1ca4d3320fdabb579a128b5855d022
Author: Cole Robinson <crobinso>
Date:   Thu Feb 3 13:30:50 2022 -0500

    devices: disk: Set discard=unmap by default for some cases
    
    This recommendation came from an internal discussion. The cases are
    
    * For block storage. This means guest requests are passed through
      to the host device, which seems a more reasonable default than
      ignoring them
    
    * For sparse disk images we will create. discard=unmap helps preserve
      the sparseness of the disk image. If a user requests non-sparse, they
      are likely more concerned with performance than saving disk space,
      so we leave the default as is. We limit this to disk images we will
      create, since that's the easiest case to check, and it's less clear
      if we should change the behavior here for an arbitrary existing
      disk image.

Comment 3 John Ferlan 2022-08-22 10:49:08 UTC

Changing to ON_QA and adding TestOnly keyword - sorry this has been discovered late. I know this avoids the errata process for the bug, but can we just validate this please?

Comment 4 Hongzhou Liu 2022-08-23 05:14:34 UTC

Verify this feature on RHEL9.1

packages:

virt-manager-4.0.0-1.el9.noarch
virt-install-4.0.0-1.el9.noarch
kernel-5.14.0-138.el9.x86_64


Step1: Run virt-install with nothing set for discard.
# virt-install --name vm1 --memory 4096 --location http://download.eng.pek2.redhat.com/rhel-9/composes/RHEL-9/RHEL-9.2.0-20220822.2/compose/BaseOS/x86_64/os/ --disk /home/vm1.img,size=20

Step2: the vm can be installed successfully, check the xml related 
...
<devices>
    <emulator>/usr/libexec/qemu-kvm</emulator>
    <disk type='file' device='disk'>
      <driver name='qemu' type='qcow2' discard='unmap'/>
      <source file='/home/vm1.img' index='1'/>
...

Result: The value for discard will be set by default 'unmap'
 

Based on this result, I change the status to verified, Thanks

Comment 5 Thomas Huth 2022-11-17 09:50:35 UTC

RHEL 9.1 has been released, so I think we can close this now.