From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <pve-devel-bounces@lists.proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68])
	by lore.proxmox.com (Postfix) with ESMTPS id D8FCF1FF164
	for <inbox@lore.proxmox.com>; Fri, 20 Jun 2025 13:03:09 +0200 (CEST)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
	by firstgate.proxmox.com (Proxmox) with ESMTP id 2CE289ED5;
	Fri, 20 Jun 2025 13:03:39 +0200 (CEST)
Date: Fri, 20 Jun 2025 13:03:32 +0200
From: Fabian =?iso-8859-1?q?Gr=FCnbichler?= <f.gruenbichler@proxmox.com>
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>
References: <20250618130209.90649-1-f.ebner@proxmox.com>
 <20250618130209.90649-9-f.ebner@proxmox.com>
In-Reply-To: <20250618130209.90649-9-f.ebner@proxmox.com>
MIME-Version: 1.0
User-Agent: astroid/0.16.0 (https://github.com/astroidmail/astroid)
Message-Id: <1750417182.dphd8m5kmy.astroid@yuna.none>
X-SPAM-LEVEL: Spam detection results:  0
 AWL 0.045 Adjusted score from AWL reputation of From: address
 BAYES_00                 -1.9 Bayes spam probability is 0 to 1%
 DMARC_MISSING             0.1 Missing DMARC policy
 KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment
 SPF_HELO_NONE           0.001 SPF: HELO does not publish an SPF Record
 SPF_PASS               -0.001 SPF: sender matches SPF record
Subject: Re: [pve-devel] [PATCH qemu-server v2 08/32] drive: remove geometry
 options gone since QEMU 3.1
X-BeenThere: pve-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox VE development discussion <pve-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pve-devel>, 
 <mailto:pve-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pve-devel/>
List-Post: <mailto:pve-devel@lists.proxmox.com>
List-Help: <mailto:pve-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel>, 
 <mailto:pve-devel-request@lists.proxmox.com?subject=subscribe>
Reply-To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Errors-To: pve-devel-bounces@lists.proxmox.com
Sender: "pve-devel" <pve-devel-bounces@lists.proxmox.com>

On June 18, 2025 3:01 pm, Fiona Ebner wrote:
> It was not possible to start a QEMU instance with these options set
> since QEMU version 3.1, QEMU commit b24ec3c462 ("block: Remove
> deprecated -drive geometry options") and thus also not to take a
> backup. It is still possible to restore an old backup with these
> options set.

we could do this unconditionally on parsing a drive line, it doesn't
really matter whether the context is regular parsing or restore parsing,
it's better to just drop the offending properties rather than the whole
drive in both cases.

while it is not very likely such old configs are still around, it would
also allow us to drop the previous two patches and keep the interface
simpler..

unrelated to this series and thus not high priority - it might be nice
to think whether we have other properties that we could drop from the
schema(s) for 9.0 thanks to this new option..

> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
> ---
> 
> Changes in v2:
> * Different approach, skip dropped keys, rather than allowing all
>   additional properties.
> 
>  src/PVE/QemuServer.pm       |  2 +-
>  src/PVE/QemuServer/Drive.pm | 28 +++-------------------------
>  2 files changed, 4 insertions(+), 26 deletions(-)
> 
> diff --git a/src/PVE/QemuServer.pm b/src/PVE/QemuServer.pm
> index cfeb8949..95a84c56 100644
> --- a/src/PVE/QemuServer.pm
> +++ b/src/PVE/QemuServer.pm
> @@ -1543,7 +1543,7 @@ sub print_drive_commandline_full {
>      my $is_rbd = $path =~ m/^rbd:/;
>  
>      my $opts = '';
> -    my @qemu_drive_options = qw(heads secs cyls trans media cache rerror werror aio discard);
> +    my @qemu_drive_options = qw(media cache rerror werror aio discard);
>      foreach my $o (@qemu_drive_options) {
>          $opts .= ",$o=$drive->{$o}" if defined($drive->{$o});
>      }
> diff --git a/src/PVE/QemuServer/Drive.pm b/src/PVE/QemuServer/Drive.pm
> index 0e99257b..96bb12c4 100644
> --- a/src/PVE/QemuServer/Drive.pm
> +++ b/src/PVE/QemuServer/Drive.pm
> @@ -26,7 +26,7 @@ our @EXPORT_OK = qw(
>      print_drive
>  );
>  
> -my $DROPPED_PROPERTIES = [];
> +my $DROPPED_PROPERTIES = ['cyls', 'heads', 'secs', 'trans'];
>  
>  our $QEMU_FORMAT_RE = qr/raw|qcow|qcow2|qed|vmdk|cloop/;
>  
> @@ -176,27 +176,6 @@ my %drivedesc_base = (
>          default => 'disk',
>          optional => 1,
>      },
> -    cyls => {
> -        type => 'integer',
> -        description => "Force the drive's physical geometry to have a specific cylinder count.",
> -        optional => 1,
> -    },
> -    heads => {
> -        type => 'integer',
> -        description => "Force the drive's physical geometry to have a specific head count.",
> -        optional => 1,
> -    },
> -    secs => {
> -        type => 'integer',
> -        description => "Force the drive's physical geometry to have a specific sector count.",
> -        optional => 1,
> -    },
> -    trans => {
> -        type => 'string',
> -        enum => [qw(none lba auto)],
> -        description => "Force disk geometry bios translation mode.",
> -        optional => 1,
> -    },
>      snapshot => {
>          type => 'boolean',
>          description => "Controls qemu's snapshot mode feature."
> @@ -765,7 +744,7 @@ sub drive_is_read_only {
>      return $drive->{interface} ne 'sata' && $drive->{interface} ne 'ide';
>  }
>  
> -# ideX = [volume=]volume-id[,media=d][,cyls=c,heads=h,secs=s[,trans=t]]
> +# ideX = [volume=]volume-id[,media=d]
>  #        [,snapshot=on|off][,cache=on|off][,format=f][,backup=yes|no]
>  #        [,rerror=ignore|report|stop][,werror=enospc|ignore|report|stop]
>  #        [,aio=native|threads][,discard=ignore|on][,detect_zeroes=on|off]
> @@ -843,8 +822,7 @@ sub parse_drive {
>      return if $res->{iops_wr} && $res->{iops};
>  
>      if ($res->{media} && ($res->{media} eq 'cdrom')) {
> -        return if $res->{snapshot} || $res->{trans} || $res->{format};
> -        return if $res->{heads} || $res->{secs} || $res->{cyls};
> +        return if $res->{snapshot} || $res->{format};
>          return if $res->{interface} eq 'virtio';
>      }
>  
> -- 
> 2.39.5
> 
> 
> 
> _______________________________________________
> 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