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 <d.csa...@proxmox.com>
---
  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

Reply via email to