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

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

Am 20.06.22 um 12:44 schrieb Alexandre Derumier:
> Signed-off-by: Alexandre Derumier <aderumier@odiso.com>
> ---
>  PVE/QemuMigrate.pm        | 10 +++++++++-
>  PVE/QemuServer/Helpers.pm | 26 ++++++++++++++++++++++++++
>  2 files changed, 35 insertions(+), 1 deletion(-)
> 
> diff --git a/PVE/QemuMigrate.pm b/PVE/QemuMigrate.pm
> index d52dc8d..e594564 100644
> --- a/PVE/QemuMigrate.pm
> +++ b/PVE/QemuMigrate.pm
> @@ -5,6 +5,7 @@ use warnings;
>  
>  use IO::File;
>  use IPC::Open2;
> +use JSON;

Not used anymore in v6.

>  use POSIX qw( WNOHANG );
>  use Time::HiRes qw( usleep );
>  
> @@ -23,7 +24,7 @@ use PVE::Tunnel;
>  use PVE::QemuConfig;
>  use PVE::QemuServer::CPUConfig;
>  use PVE::QemuServer::Drive;
> -use PVE::QemuServer::Helpers qw(min_version);
> +use PVE::QemuServer::Helpers qw(min_version version_cmp);

Same.

>  use PVE::QemuServer::Machine;
>  use PVE::QemuServer::Monitor qw(mon_cmd);
>  use PVE::QemuServer;
> @@ -122,6 +123,13 @@ sub prepare {
>      # test if VM exists
>      my $conf = $self->{vmconf} = PVE::QemuConfig->load_config($vmid);
>  
> +    my $version = PVE::QemuServer::Helpers::get_node_pvecfg_version($self->{node});
> +    my $cloudinit_config = $conf->{cloudinit};
> +
> +    if(defined($cloudinit_config) && keys %$cloudinit_config && !PVE::QemuServer::Helpers::pvecfg_min_version($version, 7, 2, 4)) {

Version should be 7, 2, 5 (since currently released pve-manager 7.2-4
would already pass this).

Style nit: missing space after if, line too long

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

missing newline in error.

> +    }
> +
>      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);
> diff --git a/PVE/QemuServer/Helpers.pm b/PVE/QemuServer/Helpers.pm
> index c10d842..c9d6b64 100644
> --- a/PVE/QemuServer/Helpers.pm
> +++ b/PVE/QemuServer/Helpers.pm
> @@ -4,6 +4,7 @@ use strict;
>  use warnings;
>  
>  use File::stat;
> +use JSON;
>  
>  use PVE::INotify;
>  use PVE::ProcFSTools;
> @@ -12,6 +13,7 @@ use base 'Exporter';
>  our @EXPORT_OK = qw(
>  min_version
>  config_aware_timeout
> +version_cmp

No need for the new export in v6.

>  );
>  
>  my $nodename = PVE::INotify::nodename();
> @@ -161,4 +163,28 @@ sub config_aware_timeout {
>      return $timeout;
>  }
>  
> +sub get_node_pvecfg_version {
> +    my ($node) = @_;
> +
> +    my $nodes_version_info = PVE::Cluster::get_node_kv('version-info', $node);
> +    return if !$nodes_version_info->{$node};
> +
> +    my $version_info_json = $nodes_version_info->{$node};

Style nit: no need for this temporary variable.

> +    my $version_info = decode_json($version_info_json);
> +    return $version_info->{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;
> +    }
> +
> +    die "internal error: cannot check version of invalid string '$verstr'";
> +}
> +
>  1;