Re: [pve-devel] [PATCH qemu-server] don't default to O_DIRECT on btrfs without nocow

Re: [pve-devel] [PATCH qemu-server] don't default to O_DIRECT on btrfs without nocow

[Wolfgang Bumiller]

> On 06/24/2021 4:58 PM Wolfgang Bumiller <w.bumiller@proxmox.com> wrote:
> 
>  
> otherwise it'll produce a whole lot of checksum errors

Just a quick note that this can be more refined.

For one: it would be nice to have a `volume_has_feature` for this, but
for instance the directory storage plugin might not be able to determine
this properly at all, so I'm not sure how to really deal with this.
OTOH, for btrfs this really depends on whether the NOCOW flag was active
during the *creation* of the disk, so in the case of btrfs, the storage
can make a *better* decision there...

Thoughts?

[pve-devel] [PATCH qemu-server] don't default to O_DIRECT on btrfs without nocow

[Wolfgang Bumiller]

otherwise it'll produce a whole lot of checksum errors

and while this would be nice as a storage feature check,
it's hard to be 100% accurate there anyway since a directory
storage can point anywhere, like for instance a btrfs
directory, causing the same issue...

Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
---
 PVE/QemuServer.pm | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 25282b7..8af4176 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -1530,14 +1530,15 @@ sub print_drive_commandline_full {
     my $format = $drive->{format};
     my $drive_id = get_drive_id($drive);
 
+    my ($storeid, $volname) = PVE::Storage::parse_volume_id($volid, 1);
+    my $scfg = $storeid ? PVE::Storage::storage_config($storecfg, $storeid) : undef;
+
     if (drive_is_cdrom($drive)) {
 	$path = get_iso_path($storecfg, $vmid, $volid);
         die "$drive_id: cannot back cdrom drive with PBS snapshot\n" if $pbs_name;
     } else {
-	my ($storeid, $volname) = PVE::Storage::parse_volume_id($volid, 1);
 	if ($storeid) {
 	    $path = PVE::Storage::path($storecfg, $volid);
-	    my $scfg = PVE::Storage::storage_config($storecfg, $storeid);
 	    $format //= qemu_img_format($scfg, $volname);
 	} else {
 	    $path = $volid;
@@ -1594,7 +1595,7 @@ sub print_drive_commandline_full {
 
     if (my $cache = $drive->{cache}) {
 	$cache_direct = $cache =~ /^(?:off|none|directsync)$/;
-    } elsif (!drive_is_cdrom($drive)) {
+    } elsif (!drive_is_cdrom($drive) && !($scfg && $scfg->{type} eq 'btrfs' && !$scfg->{nocow})) {
 	$opts .= ",cache=none";
 	$cache_direct = 1;
     }
-- 
2.30.2