Re: [PATCH cluster v4 5/5] cluster files: support registering UTF-8 configuration file

[Fiona Ebner <f.ebner@proxmox.com>]

Am 10.03.26 um 11:39 AM schrieb Hannes Laimer:
> On 2026-03-10 11:16, Fiona Ebner wrote:
>> A configuration file registered as UTF-8 will be automatically decoded
>> from UTF-8 to Perl's internal string format after reading and encoded
>> in the other direction before writing.
>>
>> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
>> ---
>>
>> Versioned dependency bump pve-cluster -> pve-common needed!
>>
>> Changes in v4:
>> * tell the parser() directly whether the file was registered as UTF-8
>>
>>  src/PVE/Cluster.pm | 18 ++++++++++++++----
>>  1 file changed, 14 insertions(+), 4 deletions(-)
>>
>> diff --git a/src/PVE/Cluster.pm b/src/PVE/Cluster.pm
>> index cd5d6b5..c1e7d4c 100644
>> --- a/src/PVE/Cluster.pm
>> +++ b/src/PVE/Cluster.pm
>> @@ -519,7 +519,7 @@ sub verify_token {
>>  my $file_info = {};
>>  
>>  sub cfs_register_file {
>> -    my ($filename, $parser, $writer) = @_;
>> +    my ($filename, $parser, $writer, $options) = @_;
>>  
>>      $observed->{$filename} || die "unknown file '$filename'";
>>  
>> @@ -529,12 +529,13 @@ sub cfs_register_file {
>>          parser => $parser,
>>          writer => $writer,
>>      };
>> +    $file_info->{$filename}->{utf8} = 1 if $options && $options->{utf8};
>>  
>>      return;
>>  }
>>  
>>  my $ccache_read = sub {
>> -    my ($filename, $parser, $version) = @_;
>> +    my ($filename, $parser, $version, $utf8) = @_;
>>  
>>      $ccache->{$filename} = {} if !$ccache->{$filename};
>>  
>> @@ -544,7 +545,14 @@ my $ccache_read = sub {
>>          # we always call the parser, even when the file does not exist
>>          # (in that case $data is undef)
>>          my $data = get_config($filename);
>> -        $ci->{data} = &$parser("/etc/pve/$filename", $data);
>> +        my $options = {};
>> +
>> +        if ($utf8) {
>> +            $data = decode('UTF-8', $data);
> 
> The docs[1] mention calling decode with undef is harmless, but will
> produce a warning, we should probably check for undef before decode

Good catch! get_config() returns undef if the file does not exist. I
cannot see a warning though:

[I] root@pve9a1 ~# cat asdf.pl
#!/usr/bin/perl
use warnings;
use strict;
use Data::Dumper;
use Encode;
my $res = decode('UTF-8', undef);
print Dumper($res);
[I] root@pve9a1 ~# perl asdf.pl
$VAR1 = undef;

I can add a check in v5, but will wait for further reviews first.

> not sure if we need one, but are we missing a
> ```
> +use Encode qw(decode);
> ```
> in Cluster.pm?

There already is
use Encode;
which auto-imports 'decode' and 'encode' (among others)
If we change it, we also need to import 'encode' explicitly (and check
if any others are used) to not break the existing calls to 'encode'.

> 
> 
> [1]
> https://perldoc.perl.org/5.8.2/Encode#$string-=-decode(ENCODING,-$octets-%5B,-CHECK%5D)