tteck / Proxmox

Proxmox VE Helper-Scripts
https://Helper-Scripts.com
MIT License
13.23k stars 2.05k forks source link

Add btrfs COW and all disk cache options. #3168

Closed prudy closed 3 months ago

prudy commented 3 months ago

I wanted to make you aware that I am meticulous when it comes to merging code into the main branch, so please don't take it personally if I reject your request.

Description

The VM image storing on the btrfs has some restrictions vs. the VM disk cache mode used. The cache modes issuing O_DIRECT will cause errors: pve.proxmox.com/wiki/Storage:_BTRFS That can be still handled by disabling the COW for a particular image file.

This PR adds the COW disabling dialog functionality. It also fixes the meaning of the cache selection, as the 'None' resulted ni cache 'default' instead of 'none'.

Here is the comparison table:

Installation Type Selections cache (main branch) cache Dialog COW disable (only on btrfs storage) msg_ok (only on btrfs storage)
Standard + Write Through (default) writethrough writethrough - ✓ Btrfs COW: default
Advanced + None default none Yes preselected + info ✓ Btrfs COW: default/disabled
Advanced + Write Through (default) writethrough writethrough No preselected ✓ Btrfs COW: default/disabled
Advanced + Write Back - writeback No preselected ✓ Btrfs COW: default/disabled
Advanced + Direct Sync - directsync Yes preselected + info ✓ Btrfs COW: default/disabled
Advanced + Unsafe - unsafe No preselected ✓ Btrfs COW: default/disabled
Advanced + Default - default No preselected ✓ Btrfs COW: default/disabled

Type of change

Please delete options that are not relevant.

tteck commented 3 months ago

I'm having a hard time understanding why this is needed.

prudy commented 3 months ago

Please see those two links below, not going into details/bugs down there. The first one mentions that proxmox replaces 'none' with 'writeback' but I don't know the details and it does not follow the note in the btrfs storage wiki. https://forum.proxmox.com/threads/important-information-on-btrfs-getting-lost-in-wiki-wrong-vm-disk-defaults-with-btrfs-storage.143413/#post-644402 https://forum.proxmox.com/threads/disk-cache-settings-for-vms-on-btrfs.93091/

I guess that your script was meant to make things easiest and dding all possible cache options might not be necesary. I added them only because chnging this menu anyway for fixing the meaning of none/default. I'm fine if you do not mege it for keeping things simple, but at least would be good to change the none/default and maybe thrown any info about carefull cache/cow usage when dealing with btrfs (I was not aware of this either but found out when started digging for what was the sense of using cow for disk image file). BTW. Even if I do not use 'none' I still turn off the COW in btrfs for the VM image file, so having this possibility on the advanced menu might be helpful.