Re: [pve-devel] [PATCH v5 qemu-server 9/9] migration: test targetnode min version for cloudinit section

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

Am 16.05.22 um 18:07 schrieb Alexandre Derumier:
> @@ -122,6 +123,13 @@ sub prepare {
>      # test if VM exists
>      my $conf = $self->{vmconf} = PVE::QemuConfig->load_config($vmid);
>  
> +    my $version = get_node_pvecfg_version($self->{node});
> +    my $cloudinit_config = $conf->{cloudinit};
> +
> +    if(defined($cloudinit_config) && keys %$cloudinit_config && !pvecfg_min_version($version, 7, 2, 4)) {

Just to note: version here is for pve-manger and needs to be adapted
upon applying. Also requires a dependency bump, so that having new
pve-manager ensures having new qemu-server.

> +	die "target node is too old and don't support new cloudinit format";

s/don't/doesn't/
s/format/section/
missing newline

> +    }
> +
>      my $repl_conf = PVE::ReplicationConfig->new();
>      $self->{replication_jobcfg} = $repl_conf->find_local_replication_job($vmid, $self->{node});
>      $self->{is_replicated} = $repl_conf->check_for_existing_jobs($vmid, 1);
> @@ -1284,4 +1292,30 @@ sub round_powerof2 {
>      return 2 << int(log($_[0]-1)/log(2));
>  }
>  

These two helpers are better added in PVE::QemuServer::Helpers (or could
even be moved to guest-common or pve-cluster together with version_cmp).

> +sub get_node_pvecfg_version {
> +    my ($node) = @_;
> +
> +    my $nodes_version_info = PVE::Cluster::get_node_kv('version-info');

Also passing the node to get_node_kv() avoids needlessly requesting the
info for all nodes.

> +    my $version = undef;
> +    if($nodes_version_info->{$node}) {
> +	my $version_info_json = $nodes_version_info->{$node};
> +	my $version_info = decode_json($version_info_json);
> +	$version = $version_info->{version} if $version_info->{version};
> +    }
> +    return $version;
> +}
> +
> +sub pvecfg_min_version {
> +    my ($verstr, $major, $minor, $release) = @_;
> +
> +    return 0 if !$verstr;
> +
> +    if ($verstr =~ m/^(\d+)\.(\d+)-(\d+)/) {
> +        return 1 if version_cmp($1, $major, $2, $minor, $3, $release) >= 0;
> +        return 0;

Style nit: should be tabs here.

> +    }
> +
> +    die "internal error: cannot check version of invalid string '$verstr'";
> +}
> +
>  1;