From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [45.144.208.40]) by lore.proxmox.com (Postfix) with ESMTPS id 30F261FF0AA for ; Tue, 22 Sep 2026 15:52:21 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id E8C86214C1; Tue, 22 Sep 2026 15:52:20 +0200 (CEST) Message-ID: Date: Tue, 22 Sep 2026 15:52:05 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH docs v4 7/7] examples: add new hookscript phase to example hookscript To: Dominik Csapak , 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: Fiona Ebner In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1790085129791 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.538 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: S3VO5UKZRETXKR2MLWASJKZSLCN4H3EP X-Message-ID-Hash: S3VO5UKZRETXKR2MLWASJKZSLCN4H3EP X-MailFrom: f.ebner@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: Am 04.09.26 um 1:51 PM schrieb Dominik Csapak: > 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. > I too prefer simply disarming the example and using 'warn' instead of 'die'. In almost all cases, existing hook scripts will just continue working if we add a new phase, so it is much more natural as an example. People can still adapt it to 'die' if they really prefer.