From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [IPv6:2a0f:8001:1:32::40]) by lore.proxmox.com (Postfix) with ESMTPS id A19AA1FF0A5 for ; Fri, 04 Sep 2026 13:51:51 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id A10B7215B2; Fri, 04 Sep 2026 13:51:48 +0200 (CEST) Message-ID: Date: Fri, 4 Sep 2026 13:51:43 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Beta Subject: Re: [PATCH docs v4 7/7] examples: add new hookscript phase to example hookscript To: Elias Huhsovitz , pve-devel@lists.proxmox.com References: <20260825135502.3971930-1-d.csapak@proxmox.com> <20260825135502.3971930-8-d.csapak@proxmox.com> Content-Language: en-US From: Dominik Csapak In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1788522699726 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.542 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: WJ4WSK6SP3LAHMA6BML7YRQ7NJO4L7UL X-Message-ID-Hash: WJ4WSK6SP3LAHMA6BML7YRQ7NJO4L7UL X-MailFrom: d.csapak@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: On 9/3/26 10:41 AM, Elias Huhsovitz wrote: > Comments inline. > > On Tue Aug 25, 2026 at 3:54 PM CEST, Dominik Csapak wrote: >> qemu-server has a new phase 'post-pci-prepare' that is called for vms >> with pci passthrough for each device prepared. >> >> add that to the example hookscript and explain when it's called and its >> parameters with a comment. >> >> Signed-off-by: Dominik Csapak >> --- >> examples/guest-example-hookscript.pl | 29 ++++++++++++++++++++++++++++ >> 1 file changed, 29 insertions(+) >> >> diff --git a/examples/guest-example-hookscript.pl b/examples/guest-example-hookscript.pl >> index 1cce2e3..02e7e7d 100755 >> --- a/examples/guest-example-hookscript.pl >> +++ b/examples/guest-example-hookscript.pl >> @@ -31,6 +31,35 @@ if ($phase eq 'pre-start') { >> # print "preparations failed, aborting." >> # exit(1); >> >> +} elsif ($phase eq 'post-pci-prepare') { > > The example hookscript ends with > > else { > die "got unknown phase '$phase'\n"; > } > > Adding this new phase would then result in a breaking change. > Current hookscripts do not check for 'post-pci-prepare', falling back to > the else branch causing --> die "got unknown phase '$phase'\n"; > > So all vms using a passed-thorugh GPU would have to update their hookscript to respect > the new phase in order to avoid dying. > > I suggest 3 ways of handling this: > > A: > > Warn users in advance, so they are ready for the switch. > > B: > > Warn users in advance and change the > > do not die at the end of a hookscript, e.g. > > else { > warn "got unknown phase '$phase'\n"; > } > > (altough this might defeat the purpose of using $stop_on_error=1 when > executing the hookscript) > > C: > > seperate hookscripts for each phase. You would have > * pre-start.pl > * post-pci-prepare.pl > * post-start.pl > * pre-stop.pl > * post-stop.pl > > When no hookscript for a phase (e.g. post-pci-prepare) exists, the > call to > > PVE::GuestHelpers::exec_hookscript($conf, $vmid, 'post-pci-prepare', 1, $params); > > is never made, treating them optional for each phase. > > Although this would introduce significant overhead for simple uses, it > would resolve this issue in "cleaner" way IMHO. > > > All 3 Options that came to mind are not very elegant. Is there some > obvious solution that I am missing? I'd personally argue that this is fine as is, even if it breaks some vm starts (the admin instantly notices that they have to adapt the hookscript) There shouldn't be too many users with hookscripts and the number that simply verbatim copied the example are (hopefully) also the minority... Separate hookscripts seem overkill, especially we'd have to handle the legacy case of a single hookscript anyway. disarming the example (downgrade die to warn) makes sense IMO but maybe @fiona has some more input on this. > > >> + >> + # Only called for virtual machines, not containers. >> + # >> + # This phase will be called for each pci device that is passed through, >> + # after it was prepared by the Proxmox VE stack. In other words when either >> + # * the mdev/vGPU was created >> + # * the driver was changed to vfio-pci and the device was reset >> + # >> + # This phase can be useful to do additional preparation that the Proxmox VE >> + # stack does not do, like setting vgpu_params for NVIDIA vGPU. >> + # >> + # This phase has 3 additional parameters (one optional) given via the environment: >> + >> + # the id from the config, e.g. 'hostpci0' >> + my $hostpci_id = $ENV{PVE_HOOK_ID}; >> + >> + # the pciid of the passed through device or the underlying device in case of an mdev/vGPU >> + # e.g. '0000:01:00.0' >> + my $pciid = $ENV{PVE_HOOK_PCIID}; >> + >> + # the uuid of the mediated device if it was one, >> + # e.g. '00000001-0000-0000-0000-000000008006' >> + # This is not set if the pci device is not an mdev. >> + my $mdev_uuid = $ENV{PVE_HOOK_MDEV_UUID}; >> + >> + print "Prepared PCI device for $hostpci_id with pciid: $pciid.\n"; >> + print "It is a mediated device with UUID: $mdev_uuid\n" if $mdev_uuid; >> + >> } elsif ($phase eq 'post-start') { >> >> # Second phase 'post-start' will be executed after the guest >