all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: Fiona Ebner <f.ebner@proxmox.com>
To: Dominik Csapak <d.csapak@proxmox.com>,
	Proxmox VE development discussion <pve-devel@lists.proxmox.com>
Subject: Re: [pve-devel] [PATCH qemu-server 1/1] pci: call hookscript for each prepared pci device
Date: Thu, 22 Jan 2026 10:23:24 +0100	[thread overview]
Message-ID: <97b7090f-973e-44f6-a4e9-eff722076609@proxmox.com> (raw)
In-Reply-To: <59ce35fc-4557-465b-8897-2074818618ec@proxmox.com>

Am 22.01.26 um 10:05 AM schrieb Dominik Csapak:
> On 1/21/26 5:26 PM, Fiona Ebner wrote:
>> Am 14.01.26 um 4:50 PM schrieb Dominik Csapak:
>>> There are situations where a user might want to do extra things
>>> for a passed through PCI device after it has been prepared/created (e.g.
>>> in case of vGPU/mdev) but before the actual QEMU process is started.
>>>
>>> Two examples are (both are used with NVIDIA vGPUs):
>>> * setting 'vgpu_params' such as removing the frame-rate-limiter
>>> * setting the gpu_instance_id for MIG devices
>>>
>>> So instead of creating (nvidia-specific) interfaces for these, give a
>>> user the ability to do it themselves via the hookscript as a first step.
>>
>> How common are those use cases and how likely is it that such interfaces
>> will end up being implemented in the future? How likely is it that a
>> hookscript will be required for other stuff going forward? Just asking
>> for general evaluation :)
> 
> i'm still unsure if we should implement more vendor (in this case
> nvidia) specific apis... on one hand, it would improve the ux
> significantly for those who need it (though some use cases might be
> rather narrow for e.g. the frame rate limiter), but otoh this
> would introduce configs that we have to support "forever" (since
> we might want to restore a backup that includes these configs,
> even in future versions) and we basically have no control over
> how these things work and if they even continue to exist...
> 
> i'd personally would lean to implement as little vendor
> specific as possible, but maybe someone else has another argument...
> 
> as for how many things would require a hookscript in the future
> is unclear, since i'm not clairvoyant ;)
> 
> but currently there are two things that would require it, and
> one of those i'd lean to implement in our stack (MIGs, because
> i guess it'll be a relatively common use case), the other one
> (setting vgpu_params) is much more niche.
> 
> having a phase at that point in the hookscript with the
> pciids/uuid would make future additions much easier though
> without having us to do anything.
> 
> so, no straight forward answer here. sorry

That's perfectly fine. Thank you for elaborating!

>>>                   if ($d->{mdev} || $d->{nvidia}) {
>>>                       warn $@ if $@;
>>>                       $chosen_mdev = $info;
>>> -                    last if $chosen_mdev; # if successful, we're done
>>> +                    if (defined($chosen_mdev)) {
>>> +                        my $params = [$id, $chosen_mdev->{uuid} //
>>> $chosen_mdev->{name}];
>>
>> Having two semantically different arguments in the same place can be
>> rather confusing. Can we always put the name/pciid and just append the
>> uuid as an additional parameter if present to avoid this? Or
>> alternatively, have one param be the type, i.e. 'pciid' or 'uuid' and
>> the next param be the value. What do you think?
> 
> yes, good call. i'd simply always give the pciid (in case of mdevs of
> the underlying device) and the uuid if it's there.
> 
> question is if that is future-proof, since in case we'd need to give an
> additional parameter, but have no uuid, how could we call the hookscript
> int that case?
> 
> so maybe the type approach is better?
> 
> do you have a preference?

Thinking about this again, I don't like my 'type' suggestion after all
as it would become messy very quickly if more params pop up in the
future. For the backup hook scripts, we pass parameters via the
environment and maybe that's the best approach here too? It's
future-proof and we get nice key-value pairing for free.


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

  reply	other threads:[~2026-01-22  9:23 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-01-14 15:50 [pve-devel] [PATCH docs/guest-common/qemu-server 0/3] add new pci passthrough specific hookscript phase Dominik Csapak
2026-01-14 15:50 ` [pve-devel] [PATCH guest-common 1/1] helpers: exec hookscript: add optional parameters Dominik Csapak
2026-01-14 15:50 ` [pve-devel] [PATCH qemu-server 1/1] pci: call hookscript for each prepared pci device Dominik Csapak
2026-01-21 16:27   ` Fiona Ebner
2026-01-22  9:06     ` Dominik Csapak
2026-01-22  9:23       ` Fiona Ebner [this message]
2026-01-22  9:42         ` Dominik Csapak
2026-01-14 15:50 ` [pve-devel] [PATCH docs 1/1] examples: add new hookscript phase to example hookscript Dominik Csapak

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=97b7090f-973e-44f6-a4e9-eff722076609@proxmox.com \
    --to=f.ebner@proxmox.com \
    --cc=d.csapak@proxmox.com \
    --cc=pve-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal