[pve-devel] [PATCH cluster 1/4] add migration format to datacenter config

[Fabian Grünbichler]

On Thu, Oct 27, 2016 at 05:00:10PM +0200, Thomas Lamprecht wrote:
> This adds a new format for configuring cluster wide migration
> settings.
> Those settings include the migration transfer method, secure ssh or
> unsecure tcp, this deprecates the migration_unsecure parameter which
> we only keep for backward compatibility and map it to the new
> property.
> Further the migration network can be set, this denotes the network
> used for sending the migration traffic.
> 
> Signed-off-by: Thomas Lamprecht <t.lamprecht at proxmox.com>
> ---
>  data/PVE/Cluster.pm | 42 ++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 40 insertions(+), 2 deletions(-)
> 
> diff --git a/data/PVE/Cluster.pm b/data/PVE/Cluster.pm
> index fe741cf..893dbe0 100644
> --- a/data/PVE/Cluster.pm
> +++ b/data/PVE/Cluster.pm
> @@ -1291,6 +1291,25 @@ sub ssh_merge_known_hosts {
>  
>  }
>  
> +my $migration_format = {
> +    type => {
> +	default_key => 1,
> +	type => 'string',
> +	enum => ['ssh', 'insecure'],
> +	description => "Migration is secure using SSH tunnel by default. For " .
> +	  "secure private networks you can disable it to speed up migration.",
> +	default => 'ssh',
> +	format_description => 'migration type',
> +    },
> +    network => {
> +	optional => 1,
> +	type => 'string', format => 'CIDR',
> +	format_description => 'CIDR',
> +	description => "Set the network CIDR for the network you want to use " .
> +	  "for transfering migration data.",
> +    },
> +};
> +

it might make sense to make the descriptions easier to understand, maybe
something like:

"Type of migration tunnel. Migration traffic is encrypted using an SSH
tunnel by default. On secure, completely private networks this can be
disabled to increase performance."

"CIDR of the (sub) network that is used for migration."

>  my $datacenter_schema = {
>      type => "object",
>      additionalProperties => 0,
> @@ -1316,7 +1335,14 @@ my $datacenter_schema = {
>  	migration_unsecure => {
>  	    optional => 1,
>  	    type => 'boolean',
> -	    description => "Migration is secure using SSH tunnel by default. For secure private networks you can disable it to speed up migration.",
> +	    description => "Migration is secure using SSH tunnel by default. " .
> +	      "For secure private networks you can disable it to speed up " .
> +	      "migration. Deprecated, use the 'migration' property instead!",
> +	},
> +	migration => {
> +	    optional => 1,
> +	    type => 'string', format => $migration_format,
> +	    description => "For cluster wide migration settings.",
>  	},
>  	console => {
>  	    optional => 1,
> @@ -1362,7 +1388,19 @@ sub get_datacenter_schema { return $datacenter_schema };
>  sub parse_datacenter_config {
>      my ($filename, $raw) = @_;
>  
> -    return PVE::JSONSchema::parse_config($datacenter_schema, $filename, $raw // '');
> +    my $res = PVE::JSONSchema::parse_config($datacenter_schema, $filename, $raw // '');
> +
> +    if (my $migration = $res->{migration}) {
> +	$res->{migration} = PVE::JSONSchema::parse_property_string($migration_format, $migration);
> +    }
> +
> +    # for backwards compatibility only, new migration property has precedence
> +    if (my $migration_unsecure = $res->{migration_unsecure} &&
> +        !defined($res->{migration}->{type})) {
> +      $res->{migration}->{type} = ($migration_unsecure) ? 'insecure' : 'ssh';
> +    }
> +
> +    return $res;

wouldn't it make more sense to not allow both options (and error out or
at least warn)?

>  }
>  
>  sub write_datacenter_config {
> -- 
> 2.1.4
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel at pve.proxmox.com
> http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel