Re: [pve-devel] [PATCH qemu-server 1/2] fix #6680: avoid setting nonexistent 'write-cache' option for scsi-{block, generic}

["Fabian Grünbichler" <f.gruenbichler@proxmox.com>]

On August 13, 2025 10:58 am, Fiona Ebner wrote:
> Only scsi-cd and scsi-hd have a 'write-cache' option, scsi-block and
> scsi-generic do not.
> 
> Setting the 'cache' setting on such a drive in the VM configuration is
> still valid and should not be prohibited, because it affects the
> blockdev settings.
> 
> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
> ---
>  src/PVE/QemuServer.pm | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/src/PVE/QemuServer.pm b/src/PVE/QemuServer.pm
> index d40b14de..99d4da4f 100644
> --- a/src/PVE/QemuServer.pm
> +++ b/src/PVE/QemuServer.pm
> @@ -1203,6 +1203,7 @@ sub print_drivedevice_full {
>      my $maxdev = 0;
>  
>      my $machine_version = extract_version($machine_type, kvm_user_version());
> +    my $has_write_cache = 1; # whether the device has a 'write-cache' option
>  
>      my $drive_id = PVE::QemuServer::Drive::get_drive_id($drive);
>      if ($drive->{interface} eq 'virtio') {
> @@ -1244,7 +1245,8 @@ sub print_drivedevice_full {
>          }
>          $device .= ",wwn=$drive->{wwn}" if $drive->{wwn};
>  
> -        # only scsi-hd and scsi-cd support passing vendor and product information
> +        # only scsi-hd and scsi-cd support passing vendor and product information and have a
> +        # 'write-cache' option
>          if ($device_type eq 'hd' || $device_type eq 'cd') {
>              if (my $vendor = $drive->{vendor}) {
>                  $device .= ",vendor=$vendor";
> @@ -1252,6 +1254,10 @@ sub print_drivedevice_full {
>              if (my $product = $drive->{product}) {
>                  $device .= ",product=$product";
>              }
> +
> +            $has_write_cache = 1;

nit: this already is the default initial value above, and we don't loop
over devices here? so we could drop this and the comment above

> +        } else {

and instead add a comment here that -block and -generic don't support
this?

> +            $has_write_cache = 0;
>          }
>  
>      } elsif ($drive->{interface} eq 'ide' || $drive->{interface} eq 'sata') {
> @@ -1316,7 +1322,7 @@ sub print_drivedevice_full {
>      }
>  
>      if (min_version($machine_version, 10, 0)) { # for the switch to -blockdev
> -        if (!drive_is_cdrom($drive)) {
> +        if (!drive_is_cdrom($drive) && $has_write_cache) {
>              my $write_cache = 'on';
>              if (my $cache = $drive->{cache}) {
>                  $write_cache = 'off' if $cache eq 'writethrough' || $cache eq 'directsync';
> -- 
> 2.47.2
> 
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> 
> 


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel