Fix spice password (#1460)

fix(backend): set spice password

* refactor(test): skip tun devices
* refactor(backend): allow set ip to null

lib/Ravada.pm

@@ -1431,6 +1431,11 @@ sub _sql_insert_defaults($self){
                 ,name => 'delay_migrate_back'
                 ,value => 600
             }
+            ,{
+                id_parent => $id_backend
+                ,name => 'display_password'
+                ,value => 1
+            }
         ]
     );
     my %field = ( settings => 'name' );
@@ -4465,7 +4470,7 @@ Returns the value of a configuration setting
 
 =cut
 
-sub setting($self, $name) {
+sub setting($self, $name, $new_value=undef) {
     my $sth = $CONNECTOR->dbh->prepare(
         "SELECT id,value "
         ." FROM settings "
@@ -4482,6 +4487,13 @@ sub setting($self, $name) {
 
         $id_parent = $id;
     }
+    if (defined $new_value && $new_value ne $value ) {
+        $sth = $CONNECTOR->dbh->prepare(
+            "UPDATE settings set value=? WHERE id=?"
+        );
+        $sth->execute($new_value , $id);
+        return $new_value;
+    }
     return $value;
 }
 

lib/Ravada/Domain.pm

@@ -339,11 +339,14 @@ sub _around_start($orig, $self, @arg) {
         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;
+                if ( Ravada::setting(undef,"/backend/display_password") ) {
+                    # We'll see if we set it from the network, defaults to 0 meanwhile
+                    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();
             }
@@ -3992,7 +3995,7 @@ sub rsync($self, @args) {
         my $msg = $self->_msg_log_rsync($file, $node, "rsync", $request);
 
         $request->status("syncing") if $request;
-        $request->error("Syncing $file");
+        $request->error("Syncing $file")    if $request;
         $request->error($msg)       if $request && $DEBUG_RSYNC;
         warn "$msg\n" if $DEBUG_RSYNC;
 
@@ -4009,7 +4012,7 @@ sub rsync($self, @args) {
             .Dumper($files)."\n"
             .join(' ',@{$rsync->err});
     }
-    $request->error("rsync done ".(time - $time_rsync)." seconds");
+    $request->error("rsync done ".(time - $time_rsync)." seconds")  if $request;
     $node->refresh_storage_pools();
 }
 

lib/Ravada/Domain/KVM.pm

@@ -712,15 +712,15 @@ sub start {
     my $listen_ip = ( delete $arg{listen_ip} or $self->_listen_ip);
     my $set_password = delete $arg{set_password};
 
-    $self->_set_spice_ip($set_password, $listen_ip);
 
     my $is_active = 0;
     eval { $is_active = $self->domain->is_active };
     warn $@ if $@;
-    if (!$is_active) {
+    if (!$is_active && !$self->is_hibernated) {
         $self->_check_qcow_format($request);
         $self->_set_volumes_backing_store();
         $self->_detect_disks_driver();
+        $self->_set_spice_ip($set_password, $listen_ip);
     }
 
     $self->status('starting');
@@ -1617,6 +1617,8 @@ sub rename_volumes {
 
 sub _set_spice_ip($self, $set_password, $ip=undef) {
 
+    return if $self->is_hibernated() || $self->domain->is_active;
+
     my $doc = XML::LibXML->load_xml(string
                             => $self->domain->get_xml_description);
     my @graphics = $doc->findnodes('/domain/devices/graphics');
@@ -1642,7 +1644,11 @@ sub _set_spice_ip($self, $set_password, $ip=undef) {
         # we should consider in the future add a new listen if it ain't one
         next if !$listen;
         $listen->setAttribute('address' => ($ip or $self->_vm->listen_ip));
-        $self->domain->update_device($graphics);
+        $self->domain->update_device($graphics, Sys::Virt::Domain::DEVICE_MODIFY_CONFIG);
+
+        $self->domain->update_device($graphics, Sys::Virt::Domain::DEVICE_MODIFY_LIVE)
+        if $self->domain->is_active;
+
     }
 }
 

lib/Ravada/VM.pm

@@ -676,11 +676,13 @@ sub display_ip($self, $value=undef) {
 }
 
 sub _set_display_ip($self, $value) {
-    my %ip_address = $self->_list_ip_address();
+    if (defined $value && length $value ) {
+        my %ip_address = $self->_list_ip_address();
 
-    confess "Error: $value is not in any interface in node ".$self->name
-    .". Found ".Dumper(\%ip_address)
-    if !exists $ip_address{$value};
+        confess "Error: $value is not in any interface in node ".$self->name
+        .". Found ".Dumper(\%ip_address)
+        if !exists $ip_address{$value};
+    }
 
     $self->_data( display_ip => $value );
 }

t/kvm/n10_nodes.t

@@ -725,7 +725,7 @@ sub test_clone_not_in_node {
     $domain->prepare_base(user_admin);
     is($domain->base_in_vm($vm->id), 1);
     $domain->set_base_vm(vm => $node, user => user_admin);
-    wait_request(debug => 1);
+    wait_request(debug => 0);
 
     is($domain->base_in_vm($node->id), 1);
 
@@ -979,6 +979,7 @@ sub test_migrate_back($node) {
     eval { $clone->migrate($vm) };
     is(''.$@, '');
 
+    wait_request(debug => 0);
     for my $file ($clone->list_volumes) {
         my $md5 = _md5($file, $vm);
         my $md5_remote = _md5($file, $node);
@@ -1023,7 +1024,7 @@ sub test_shutdown($node) {
     my $req = Ravada::Request->refresh_vms();
     wait_request(debug => 0);
     is($req->status,'done');
-    is($req->error,'');
+    like($req->error,qr{^($|checked \d+)});
 
     my $clone2 = Ravada::Domain->open($clone->id); #open will clean internal shutdown
     is($clone2->is_active,0) or exit;

t/kvm/p10_password.t

@@ -9,7 +9,8 @@ use Test::More;
 use lib 't/lib';
 use Test::Ravada;
 
-use_ok('Ravada');
+no warnings "experimental::signatures";
+use feature qw(signatures);
 
 init();
 my @VMS = vm_names();
@@ -113,14 +114,17 @@ sub test_domain_password2 {
     return $domain;
 }
 
-sub test_domain_password1 {
-    my $vm_name = shift;
+sub test_domain_password1($vm_name, $requires_password=1) {
     my $vm = rvd_back->search_vm($vm_name);
 
     my $net2 = Ravada::Network->new(address => '10.0.0.1/32');
 
     ok($net2->requires_password,"Expecting net requires password ")
         or return;
+
+    if (!$requires_password) {
+        rvd_back->setting("/backend/display_password" => 0);
+    }
     my $domain = $vm->create_domain( name => new_domain_name
                 , disk => 1024 * 1024
                 , id_iso => search_id_iso('Alpine') , id_owner => $USER->id);
@@ -130,10 +134,15 @@ 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 '')."'") or die $domain2->name;
+    if ($requires_password) {
+        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 '')."'");
+    } else {
+        is($password, undef);
+    }
 
-    $password = $domain->spice_password();
-    like($password,qr/./,"Expecting a password, got '".($password or '')."'");
 
     my $domain_f = rvd_front()->search_domain($domain->name);
     my $password_f;
@@ -142,10 +151,28 @@ sub test_domain_password1 {
     is($password_f , $password,"Expecting password : '".($password or '')."'"
                                 ." got : '".($password_f or '')."'");
 
+    my $domain3 = Ravada::Domain->open($domain->id);
+    test_password_xml($domain3,$password);
+
     $domain->shutdown_now($USER);
+
+    # default is display password = 1
+    rvd_back->setting("/backend/display_password" => 1);
     return $domain;
 }
 
+sub test_password_xml($domain, $exp_password) {
+    my $xml = XML::LibXML->load_xml(string => $domain->domain->get_xml_description(Sys::Virt::Domain::XML_SECURE));
+    my $found = 0;
+    for my $graphics ( $xml->findnodes("/domain/devices/graphics") ) {
+        next if $graphics->getAttribute('type') ne 'spice';
+        $found++;
+        is($graphics->getAttribute('passwd'),$exp_password,$domain->name) or exit;
+    }
+    ok($found,"Expecting a graphics type='spice' found in ".$domain->name);
+}
+
+
 sub test_any_network_password {
     my $vm_name = shift;
     my $vm = rvd_back->search_vm($vm_name);
@@ -373,7 +400,7 @@ SKIP: {
     skip($msg,10)   if !$vm;
 
     add_network_10();
-    my $domain1 = test_domain_password1($vm_name);
+    my $domain1 = test_domain_password1($vm_name, 0);
     my $domain2 = test_domain_password2($vm_name);
     remove_network_10();
 

t/lib/Test/Ravada.pm

@@ -866,7 +866,11 @@ sub wait_request {
                         like($req->error,qr(^$|libvirt error code));
                     } else {
                         my $error = ($req->error or '');
-                        is($error,'') or confess $req->command;
+                        if ($req->command =~ m{rsync_back|set_base_vm|start}) {
+                            like($error,qr{^($|rsync done)});
+                        } else {
+                            is($error,'') or confess $req->command;
+                        }
                     }
                 }
             }

t/nodes/10_basic.t

@@ -494,7 +494,7 @@ sub test_set_vm($vm, $node) {
     );
     rvd_back->_process_requests_dont_fork();
     is($req->status, 'done');
-    is($req->error, '');
+    like($req->error, qr{^($|rsync done)});
 
     is($base->_vm->id, $vm->id);
 
@@ -541,7 +541,7 @@ sub test_bind_ip($node, $base, $remote_ip=undef, $config=undef) {
             ,@remote_ip
         );
         wait_request();
-        is($req->error, '');
+        like($req->error, qr{^($|rsync done)});
         my $clone_v = Ravada::Domain->open($clone->id);
         if ($clone_v->is_local) {
             if (!$config) {
@@ -555,7 +555,7 @@ sub test_bind_ip($node, $base, $remote_ip=undef, $config=undef) {
             like($clone_v->display(user_admin), qr($node_ip));
         }
         is($req->status,'done');
-        is($req->error, '');
+        like($req->error, qr{^($|rsync done)});
         push @clone,($clone);
         $clone_2 = Ravada::Domain->open($clone->id);
         last if $clone_2->_vm->id == $node->id;
@@ -1155,14 +1155,14 @@ sub test_migrate_req($vm, $node) {
         , retry => 10
     );
     for ( 1 .. 30 ) {
-        wait_request( debug => 1, check_error => 0);
+        wait_request( debug => 0, check_error => 0);
         is($req->status,'done');
-        last if !$req->error;
+        last if !$req->error || $req->error =~ /rsync done/;
         diag($req->status);
         diag($req->error." try : ".$req->retry);
         sleep 1;
     }
-    is($req->error,'') or exit;
+    like($req->error,qr{^($|rsync done)}) or exit;
 
     my $domain3 = Ravada::Domain->open($domain->id);
     is($domain3->is_active,1);
@@ -1216,6 +1216,7 @@ sub test_display_ip($vm, $node, $set_localhost_dp=0) {
 
     $node->_data(display_ip => '');
     rvd_back->display_ip('') if $set_localhost_dp;
+    $vm->display_ip('')      if $set_localhost_dp;
 }
 
 sub test_nat($vm, $node, $set_localhost_natip=0) {

t/nodes/20_dead_node.t

@@ -38,7 +38,7 @@ sub test_down_node($vm, $node) {
     my $req = Ravada::Request->refresh_vms();
     rvd_back->_process_requests_dont_fork();
     is($req->status, 'done');
-    is($req->error, '',"Expecting no error after refresh vms");
+    like($req->error, qr{^($|checked)},"Expecting no error after refresh vms");
 
     is($clone[0]->is_active, 0, "Expecting clone not active after node shutdown");
 
@@ -90,7 +90,7 @@ sub test_disabled_node($vm, $node) {
     my $req = Ravada::Request->refresh_vms( timeout_shutdown => $timeout );
     rvd_back->_process_requests_dont_fork();
     is($req->status, 'done');
-    is($req->error, '',"Expecting no error after refresh vms");
+    like($req->error, qr{^($|checked)},"Expecting no error after refresh vms");
 
     my @reqs = $clone->list_requests();
     delete $clone->{_data};

t/vm/n10_nat.t

@@ -42,6 +42,7 @@ sub test_route($vm) {
     my %route = ( '127.0.0.0/24', '127.0.0.1');
     my $routes = `ip route`;
     for my $line ( split /\n/, $routes ) {
+        next if $line =~ / dev tun\d+ /;
         my ($network,$ip) = $line =~ /(^[\d+\.\/]+).*src ([\d+\.]+)/;
         next if !$network || !$ip;
         $route{$network} = $ip;

templates/main/admin_settings.html.ep

@@ -114,6 +114,19 @@
                 <%= l 'Debug' %>
             </div>
         </div>
+		<div class="row">
+            <div class="col-md-1"></div>
+            <div class="col-md-2">
+            <label for="debug">Display Password</label>
+            </div>
+            <div class="col-md-6">
+            <input name="debug" ng-model="settings.backend.display_password.value"
+                ng-true-value="1" ng-false-value="0"
+                type="checkbox">
+                <%= l 'Enables display password when available' %>
+            </div>
+        </div>
+
 
         <hr>