fix(backend): improved listen ip for display

issue #1261

lib/Ravada.pm

@@ -1332,7 +1332,7 @@ sub _upgrade_tables {
 
     $self->_upgrade_table('domains','needs_restart','int not null default 0');
 
-    if ($self->_upgrade_table('domains','screenshot','BLOB')) {
+    if ($self->_upgrade_table('domains','screenshot','MEDIUMBLOB')) {
 
     $self->_upgrade_screenshots();
 
@@ -1407,7 +1407,14 @@ Returns the default display IP read from the config file
 
 =cut
 
-sub display_ip {
+sub display_ip($self=undef, $new_ip=undef) {
+    if (defined $new_ip) {
+        if (!length $new_ip) {
+            delete $CONFIG->{display_ip};
+        } else {
+            $CONFIG->{display_ip} = $new_ip;
+        }
+    }
     my $ip = $CONFIG->{display_ip};
     return $ip if $ip;
 }

lib/Ravada/Domain.pm

@@ -152,8 +152,9 @@ around 'prepare_base' => \&_around_prepare_base;
 #before 'prepare_base' => \&_pre_prepare_base;
 # after 'prepare_base' => \&_post_prepare_base;
 
-before 'start' => \&_start_preconditions;
- after 'start' => \&_post_start;
+#before 'start' => \&_start_preconditions;
+# after 'start' => \&_post_start;
+around 'start' => \&_around_start;
 
 before 'pause' => \&_allow_shutdown;
  after 'pause' => \&_post_pause;
@@ -291,6 +292,38 @@ sub _vm_disconnect {
     $self->_vm->disconnect();
 }
 
+sub _around_start($orig, $self, @arg) {
+    $self->_start_preconditions(@arg);
+
+    my %arg;
+    if (!(scalar(@arg) % 2) ) {
+        %arg = @arg;
+    } else {
+        $arg{user} = $arg[0];
+    }
+
+    my $listen_ip = delete $arg{listen_ip};
+    my $remote_ip = $arg{remote_ip};
+
+    if (!defined $listen_ip) {
+        my $display_ip;
+        if ($remote_ip) {
+            my $set_password = 0;
+            my $network = Ravada::Network->new(address => $remote_ip);
+            $set_password = 1 if $network->requires_password();
+            $display_ip = $self->_listen_ip($remote_ip);
+            $arg{set_password} = $set_password;
+        } else {
+            $display_ip = $self->_listen_ip();
+        }
+        $arg{listen_ip} = $display_ip;
+    }
+    my $ret = $self->$orig(%arg);
+
+    $self->_post_start(%arg);
+
+}
+
 sub _start_preconditions{
     my ($self) = @_;
 
@@ -876,7 +909,7 @@ sub _around_display_info($orig,$self,$user ) {
 
     if (!$self->readonly) {
         $self->_set_display_ip($display);
-        $self->_data(display => encode_json($display));
+        $self->_data(display => encode_json($display)) if $self->is_active;
     }
     return $display;
 }

lib/Ravada/Domain/KVM.pm

@@ -592,15 +592,16 @@ sub display_info($self, $user) {
     warn "ERROR: Machine ".$self->name." is not active in node ".$self->_vm->name."\n"
         if !$port && !$self->is_active;
 
-    my $display = $type."://$address:$port";
 
     my %display = (
                 type => $type
                ,port => $port
                  ,ip => $address
-            ,display => $display
           ,tls_port => $tls_port
     );
+    $port = '' if !defined $port;
+    my $display = $type."://$address:$port";
+    $display{display} = $display;
     lock_hash(%display);
     return \%display;
 }
@@ -644,18 +645,12 @@ sub start {
         %arg = @_;
     }
 
-    my $set_password=0;
     my $remote_ip = delete $arg{remote_ip};
     my $request = delete $arg{request};
+    my $listen_ip = ( delete $arg{listen_ip} or $self->_listen_ip);
+    my $set_password = delete $arg{set_password};
 
-    my $display_ip = $self->_listen_ip();
-    if ($remote_ip) {
-        $set_password = 0;
-        my $network = Ravada::Network->new(address => $remote_ip);
-        $set_password = 1 if $network->requires_password();
-        $display_ip = $self->_listen_ip($remote_ip);
-    }
-    $self->_set_spice_ip($set_password, $display_ip);
+    $self->_set_spice_ip($set_password, $listen_ip);
     $self->status('starting');
 
     my $error;

lib/Ravada/Domain/Void.pm

@@ -64,6 +64,10 @@ sub _set_display($self, $listen_ip=$self->_vm->listen_ip) {
     return $display_data;
 }
 
+sub _set_spice_ip($self, $password=undef, $listen_ip=$self->_vm->listen_ip) {
+    return $self->_set_display($listen_ip);
+}
+
 sub is_active {
     my $self = shift;
     return ($self->_value('is_active') or 0);
@@ -223,6 +227,7 @@ sub start($self, @args) {
     %args = @args if scalar(@args) % 2 == 0;
     my $listen_ip = delete $args{listen_ip};
     my $remote_ip = delete $args{remote_ip};
+    my $set_password = delete $args{set_password}; # unused
     my $user = delete $args{user};
     delete $args{'id_vm'};
     confess "Error: unknown args ".Dumper(\%args) if keys %args;

lib/Ravada/VM.pm

@@ -647,7 +647,7 @@ sub _interface_ip($self, $remote_ip=undef) {
 }
 
 sub listen_ip($self, $remote_ip=undef) {
-    return Ravada::display_ip() if Ravada::display_ip();
+    return Ravada::display_ip() if $self->is_local && Ravada::display_ip();
     return $self->public_ip     if $self->public_ip;
 
     return $self->_interface_ip($remote_ip) if $remote_ip;
@@ -1486,7 +1486,8 @@ sub _shared_storage_cache($self, $node, $dir, $value=undef) {
         "INSERT INTO storage_nodes (id_node1, id_node2, dir, is_shared) "
         ." VALUES (?,?,?,?)"
     );
-    $sth->execute($self->id, $node->id, $dir, $value);
+    eval { $sth->execute($self->id, $node->id, $dir, $value) };
+    confess $@ if $@ && $@ !~ /Duplicate entry/i;
     return $value;
 }
 

t/kvm/n10_nodes.t

@@ -176,7 +176,6 @@ sub test_domain {
     is($clone->is_active,1) or return;
 
     my $local_ip = $node->ip;
-
     like($clone->display(user_admin),qr($local_ip));
 
     my ($local_port) = $clone->display(user_admin) =~ m{:(\d+)};
@@ -392,6 +391,9 @@ sub test_already_started_twice($vm_name, $node) {
 
     my $clone_local = $vm->search_domain($clone->name);
     is($clone_local->_vm->is_local, 1);
+    my $ip_local = $vm->ip;
+    $clone_local->_set_spice_ip(1,$vm->ip);# if $clone_local->type eq 'KVM';
+    like($clone_local->display(user_admin),qr($ip_local)) or exit;
 
     start_domain_internal($clone);
     start_domain_internal($clone_local);
@@ -430,7 +432,9 @@ sub test_already_started_hibernated($vm_name, $node) {
     my $clone_local = $vm->search_domain($clone->name);
     is($clone_local->_vm->is_local, 1);
 
+    $clone->_set_spice_ip(1,$node->ip) if $clone_local->type eq 'KVM';
     start_domain_internal($clone);
+    $clone_local->_set_spice_ip(1,$vm->ip) if $clone_local->type eq 'KVM';
     hibernate_domain_internal($clone_local);
 
     is($clone->is_active, 1,"expecting clone active on remote");
@@ -1169,6 +1173,7 @@ SKIP: {
     is($node->is_local,0,"Expecting ".$node->name." ".$node->ip." is remote" ) or BAIL_OUT();
 
     is($vm->shared_storage($node,'/var/tmp/'),0) or exit;
+    test_already_started_twice($vm_name, $node);
 
     test_domain($vm_name, $node);
 

t/kvm/p10_password.t

@@ -130,7 +130,7 @@ sub test_domain_password1 {
     my $vm2 = rvd_back->search_vm($vm_name);
     my $domain2 = $vm2->search_domain($domain->name);
     my $password = $domain2->spice_password();
-    like($password,qr/./,"Expecting a password, got '".($password or '')."'");
+    like($password,qr/./,"Expecting a password, got '".($password or '')."'") or die $domain2->name;
 
     $password = $domain->spice_password();
     like($password,qr/./,"Expecting a password, got '".($password or '')."'");

t/lib/Test/Ravada.pm

@@ -1372,6 +1372,7 @@ sub shutdown_domain_internal($domain) {
 
 sub start_domain_internal($domain) {
     if ($domain->type eq 'KVM') {
+        $domain->_set_spice_ip(1,$domain->_vm->ip);
         $domain->domain->create();
     } elsif ($domain->type eq 'Void') {
         $domain->_store(is_active => 1 );

t/nodes/10_basic.t

@@ -67,7 +67,7 @@ sub test_remove_req($vm, $node) {
              ,name => $clone1->name
     );
 
-    rvd_back->_process_requests_dont_fork(1);
+    rvd_back->_process_requests_dont_fork();
 
     is($req->status, 'done');
     is($req->error, '');
@@ -254,7 +254,7 @@ sub test_set_vm($vm, $node) {
         , value => 1
         , uid => user_admin->id
     );
-    rvd_back->_process_requests_dont_fork(1);
+    rvd_back->_process_requests_dont_fork();
     is($req->status, 'done');
     is($req->error, '');
 
@@ -277,13 +277,17 @@ sub test_set_vm($vm, $node) {
 
     is($base_f->list_instances,2) or exit;
 
+    test_bind_ip($node, $base,'1.2.3.4',1);
     test_bind_ip($node, $base,'1.2.3.4');
     test_bind_ip($node, $base);
     $base->remove(user_admin);
     is(scalar($base->list_instances),undef);
 }
 
-sub test_bind_ip($node, $base, $remote_ip=undef) {
+sub test_bind_ip($node, $base, $remote_ip=undef, $config=undef) {
+    if ($config) {
+        rvd_back->display_ip("127.0.0.1");
+    }
     my @clone;
     my $clone_2;
     my @remote_ip;
@@ -299,6 +303,19 @@ sub test_bind_ip($node, $base, $remote_ip=undef) {
             ,@remote_ip
         );
         wait_request();
+        is($req->error, '');
+        my $clone_v = Ravada::Domain->open($clone->id);
+        if ($clone_v->is_local) {
+            if (!$config) {
+                my $vm_ip = $clone_v->_vm->ip;
+                like($clone_v->display(user_admin),qr($vm_ip)) or confess $clone_v->name;
+            } else {
+                like($clone_v->display(user_admin),qr(127.0.0.1)) or die $clone_v->name;
+            }
+        } else {
+            my $node_ip = $node->ip;
+            like($clone_v->display(user_admin), qr($node_ip));
+        }
         is($req->status,'done');
         is($req->error, '');
         push @clone,($clone);
@@ -311,12 +328,7 @@ sub test_bind_ip($node, $base, $remote_ip=undef) {
     for (@clone) {
         $_->remove(user_admin);
     }
-}
-
-sub test_instances($clone, $expected) {
-    confess;
-    my @instances = $clone->list_instances();
-    is(@instances,$expected,Dumper(\@instances)) or exit;
+    rvd_back->display_ip("") if $config;
 }
 
 sub test_volatile($vm, $node) {