[pve-devel] [PATCH cluster 2/4] add get_local_migration_ip method

[Thomas Lamprecht]

On 10/28/2016 12:13 PM, Fabian Grünbichler wrote:
> On Thu, Oct 27, 2016 at 05:00:11PM +0200, Thomas Lamprecht wrote:
>> used to get the local IP from the datacenter migration->network
>> CIDR, if any.
>>
>> Signed-off-by: Thomas Lamprecht <t.lamprecht at proxmox.com>
>> ---
>>   data/PVE/Cluster.pm | 29 +++++++++++++++++++++++++++++
>>   1 file changed, 29 insertions(+)
>>
>> diff --git a/data/PVE/Cluster.pm b/data/PVE/Cluster.pm
>> index 893dbe0..c69f339 100644
>> --- a/data/PVE/Cluster.pm
>> +++ b/data/PVE/Cluster.pm
>> @@ -16,6 +16,7 @@ use PVE::INotify;
>>   use PVE::IPCC;
>>   use PVE::SafeSyslog;
>>   use PVE::JSONSchema;
>> +use PVE::Network;
>>   use JSON;
>>   use RRDs;
>>   use Encode;
>> @@ -1048,6 +1049,34 @@ sub remote_node_ip {
>>       return wantarray ? ($ip, $family) : $ip;
>>   }
>>   
>> +sub get_local_migration_ip {
>> +    my ($migration_network, $noerr) = @_;
>> +
>> +    my $ip;
>> +    my $cidr = $migration_network;
>> +
>> +    if (!defined($cidr)) {
>> +	my $dc_conf = cfs_read_file('datacenter.cfg');
>> +	$cidr = $dc_conf->{migration}->{network}
>> +	  if defined($dc_conf->{migration}->{network});
>> +    }
> the last defined check is redundant, or am I missing something? $cidr
> was undef when entering the if, and the next if checks defined-ness
> again anyway.

I had in the mind that perl warns if I try to make a hash ref on an 
undefined object (like
$dc_conf->{migration} could be possible here), but I was wrong, so yes 
this check is unnecessary

>
>> +
>> +    if (defined($cidr)) {
>> +	my $ips = PVE::Network::get_local_ip_from_cidr($cidr);
>> +
>> +	die "no IP address configured on local node for the '$cidr' " .
>> +	  " sub network\n" if !$noerr && (scalar(@$ips) == 0);
> "no IP address configured for local node in sub network '$cidr'"?
>
>> +
>> +	warn "multiple IP address configured for the '$cidr' sub network," .
>> +	  "selecting the first found!\n" if !$noerr && (scalar(@$ips) > 1);
> see above, and maybe log the first one?

You are both times correct, they were a bit ugly, Dietmar already 
cleaned them up :)

>
>> +
>> +	$ip = @$ips[0];
>> +	$ip = "[$ip]" if $ip && Net::IP::ip_is_ipv6($ip);
>> +    }
>> +
>> +    return $ip;
>> +};
>> +
>>   # ssh related utility functions
>>   
>>   sub ssh_merge_keys {
>> -- 
>> 2.1.4
>>
>>
>> _______________________________________________
>> pve-devel mailing list
>> pve-devel at pve.proxmox.com
>> http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> _______________________________________________
> pve-devel mailing list
> pve-devel at pve.proxmox.com
> http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel