On Thu Mar 12, 2026 at 2:06 PM CET, Wolfgang Bumiller wrote:
> On Wed, Mar 11, 2026 at 01:26:18PM +0100, Daniel Kral wrote:
>> diff --git a/src/PVE/SectionConfig.pm b/src/PVE/SectionConfig.pm
>> index 84ff81a..d0332f9 100644
>> --- a/src/PVE/SectionConfig.pm
>> +++ b/src/PVE/SectionConfig.pm
>> @@ -1572,11 +1572,12 @@ sub write_config {
>> next if $opts->{$k}->{optional};
>> $done_hash->{$k} = 1;
>> my $v = $scfg->{$k};
>> - die "section '$sectionId' - missing value for required option
>> '$k'\n"
>> - if !defined($v);
>> $v = $class->encode_value($type, $k, $v);
>
> Note that this also changes the public section config API by requiring
> `encode_value()` to deal with the `$value` parameter being `undef` -
> previously this would not happen here.
Right, good catch! I had a bad gut feeling about moving it farther here
and only did so that the die is coupled to whether
format_config_line(...) does add a line to the config $data string.
I'd go for having both the definedness check above and the 'is the data
actually written to the config file?' check below with a more telling
error message in a v2, if there are no objections.
>
> Its description would need to be updated, and its example as well, since
> it only checks the `$key` before then dereferencing `$value` as a hash.
>
> A quick grep shows a bunch of unprotected uses:
> - PVE::Storage::Plugin for `nodes` or `content{,-dirs}`
> - PVE::SDN::Zones::Plugin
> - PVE::ACME::Challenge
> - PVE::HA::Rules
> - PVE::HA::Groups
> - plugins for PVE::Job::Registry
>
>> my $prop = $class->get_property_schema($type, $k);
>> - $data .= format_config_line($prop, $k, $v);
>> + my $cfg_line = format_config_line($prop, $k, $v);
>> + die "section '$sectionId' - missing value for required option
>> '$k'\n"
>> + if !$cfg_line;
>> + $data .= $cfg_line;
>> }
>>
>> for my $k (@option_keys) {
>> --
>> 2.47.3
