From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id 8B3031FF15C for ; Wed, 30 Oct 2024 09:42:56 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 56E5E12314; Wed, 30 Oct 2024 09:43:01 +0100 (CET) Message-ID: <6edc097f-fd3f-4916-a87a-865a44265446@proxmox.com> Date: Wed, 30 Oct 2024 09:42:27 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Beta To: Thomas Lamprecht , Proxmox VE development discussion References: <20240806122203.2266054-1-d.csapak@proxmox.com> <20240806122203.2266054-7-d.csapak@proxmox.com> Content-Language: en-US From: Dominik Csapak In-Reply-To: X-SPAM-LEVEL: Spam detection results: 0 AWL 0.016 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 manager 1/1] api/ui: improve mdev listing for pci mappings X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Proxmox VE development discussion Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Errors-To: pve-devel-bounces@lists.proxmox.com Sender: "pve-devel" On 10/24/24 18:53, Thomas Lamprecht wrote: > Am 06/08/2024 um 14:22 schrieb Dominik Csapak: >> currently when we have a pci resource mapping, we manually check only >> the available models for the first pci entry. This often works, but not >> always, since one could have completely different devices in one >> mapping, or with the new nvidia sysfs api we don't get the generally >> available models. >> >> To improve this, extend the 'pciid' regex to include pciids or mapping >> names, and for mappings, iterate over all local pci devices in it and >> extract the mdev types. >> >> This also vastly simplifies the ui code, since we only have to give the >> mapping to the selector instead of an (arbitrarily selected) pci id. >> >> Signed-off-by: Dominik Csapak >> --- >> PVE/API2/Hardware/PCI.pm | 45 +++++++++++++++++++++++++++++------- >> www/manager6/qemu/PCIEdit.js | 12 +--------- >> 2 files changed, 38 insertions(+), 19 deletions(-) >> >> diff --git a/PVE/API2/Hardware/PCI.pm b/PVE/API2/Hardware/PCI.pm >> index a3a689bf..7135a605 100644 >> --- a/PVE/API2/Hardware/PCI.pm >> +++ b/PVE/API2/Hardware/PCI.pm >> @@ -135,7 +135,7 @@ __PACKAGE__->register_method ({ >> >> __PACKAGE__->register_method ({ >> name => 'pciindex', >> - path => '{pciid}', >> + path => '{pciid-or-mapping}', >> method => 'GET', >> description => "Index of available pci methods", >> permissions => { >> @@ -145,9 +145,9 @@ __PACKAGE__->register_method ({ >> additionalProperties => 0, >> properties => { >> node => get_standard_option('pve-node'), >> - pciid => { >> + 'pciid-or-mapping' => { > > While in general this should be fine w.r.t. breaking changes for the API itself, > as the name of those template-variables from the API path can be basically seen > as internal detail, it would IMO be warranted to state that explicitly in the > commit message, as FWICT this is basically the only time where changing the > parameter name mid-release is legit without some backward compat handling. Yeah, you're right, i'll send a v2 of this with the commit message extended, but see below for an alternative. > > And just to be sure: you are certain that this cannot be passed anyhow else, > e.g. in some CLI (pvesh?) or other way? I did not find a way, and AFAICT this parameter is always part of the path, where we don't can give explicit names Of course, we can leave the name of the parameter the same if you'd prefer that (with a TODO notice for 9.x?), and just change the regex, then it's definitely backwards compatible, even if i overlooked some case where one can give the parameter name _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel