Re: [PATCH guest-common v4 1/7] helpers: exec hookscript: add optional parameters

["Jakob Klocker" <j.klocker@proxmox.com>]

style-nit inline.
On Tue Aug 25, 2026 at 3:54 PM CEST, Dominik Csapak wrote:
> sometimes we may want to call the hookscript with additional parameters
> in some phases, e.g. we want to call it for each pci device that was
> prepared before starting with the correct uuid or pci id.
>
> Add these new parameters to the environment instead of the positional
> parameters of the hookscript, since that is more future proof and we get
> a key/value pair instead of just the position.
>
> Use the prefix 'PVE_HOOK_' so they're always in a separate namespace.
>
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
>  src/PVE/GuestHelpers.pm | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/src/PVE/GuestHelpers.pm b/src/PVE/GuestHelpers.pm
> index f8d112b..6899b05 100644
> --- a/src/PVE/GuestHelpers.pm
> +++ b/src/PVE/GuestHelpers.pm
> @@ -115,14 +115,24 @@ sub check_hookscript {
>  }
>  
>  sub exec_hookscript {
> -    my ($conf, $vmid, $phase, $stop_on_error) = @_;
> +    my ($conf, $vmid, $phase, $stop_on_error, $params) = @_;
>  
>      return if !$conf->{hookscript};
>  
> +    $params //= {};

According to the Perl style guide [0], 
`$params = {} if !defined($params);` is prefered over the `//=`
syntax.

Since I keep seeing this "old" syntax in new patches, I'm not sure if
this is strictly lived though.

[0] https://pve.proxmox.com/wiki/Perl_Style_Guide#Perl_syntax_choices

> +
>      eval {
>          my $hookscript = check_hookscript($conf->{hookscript});
>          die $@ if $@;
>  
> +        # copy EVN before adding to it

misspelled `ENV` in the comment. Might also be good adding why
we're making a copy, instead of commenting what we're doing

> +        local %ENV = (%ENV);
> +
> +        for my $key (keys $params->%*) {
> +            my $new_key = "PVE_HOOK_" . uc($key);
> +            $ENV{$new_key} = $params->{$key};
> +        }
> +
>          PVE::Tools::run_command([$hookscript, $vmid, $phase]);
>      };
>      if (my $err = $@) {