Fix #1025 hardware (#1031)

fix(backend): move to master and remove clones on change hard

* test(nodes): check removed remote clones if change hardware
* refactor(KVM): remove excess amount below zero
* test(nodes): check removed remote clones if change hardware
* refactor(test): reload daemon user grants for testing
* refactor(grants): new private method to reload grants

issue #1025

lib/Ravada/Auth/SQL.pm

@@ -670,6 +670,11 @@ sub _load_grants($self) {
 
 }
 
+sub _reload_grants($self) {
+    delete $self->{_grant};
+    return $self->_load_grants();
+}
+
 sub _grant_alias($self, $name) {
     my $alias = $name;
     return $self->{_grant_alias}->{$name} if exists $self->{_grant_alias}->{$name};
@@ -749,7 +754,6 @@ sub grant_admin_permissions($self,$user) {
         $self->grant($user,$name);
     }
     $sth->finish;
-
 }
 
 =head2 revoke_all_permissions

lib/Ravada/Domain.pm

@@ -199,6 +199,10 @@ around 'is_hibernated' => \&_around_is_hibernated;
 
 around 'autostart' => \&_around_autostart;
 
+before 'set_controller' => \&_pre_change_hardware;
+before 'remove_controller' => \&_pre_change_hardware;
+before 'change_hardware' => \&_pre_change_hardware;
+
 after 'set_controller' => \&_post_change_hardware;
 after 'remove_controller' => \&_post_change_hardware;
 after 'change_hardware' => \&_post_change_hardware;
@@ -524,6 +528,8 @@ sub _around_remove_volume {
 
     my $ok = $self->$orig(@_);
 
+    return $ok if !$self->is_local;
+
     my $sth = $$CONNECTOR->dbh->prepare(
         "DELETE FROM volumes "
         ." WHERE id_domain=? AND file=?"
@@ -1281,6 +1287,7 @@ sub _pre_remove_domain($self, $user, @) {
     }
     $self->pre_remove();
     $self->_remove_iptables()   if $self->is_known();
+
 }
 
 sub _check_active_node($self) {
@@ -1308,11 +1315,11 @@ sub _after_remove_domain {
         $self->_do_remove_base($user);
         $self->_remove_files_base();
     }
-    return if !$self->{_data};
     $self->_remove_all_volumes();
+    return if !$self->{_data};
     return if $cascade;
-    $self->_remove_base_db();
     $self->_finish_requests_db();
+    $self->_remove_base_db();
     $self->_remove_access_attributes_db();
     $self->_remove_volumes_db();
     $self->_remove_bases_vm_db();
@@ -3317,9 +3324,17 @@ sub needs_restart($self, $value=undef) {
     return $self->_data('needs_restart',$value);
 }
 
+sub _pre_change_hardware($self, @) {
+    if (!$self->_vm->is_local) {
+        my $vm_local = $self->_vm->new( host => 'localhost' );
+        $self->_set_vm($vm_local, 1);
+    }
+}
+
 sub _post_change_hardware {
     my $self = shift;
     $self->info(Ravada::Utils::user_daemon) if $self->is_known();
+    $self->_remove_domain_cascade(Ravada::Utils::user_daemon,1);
     $self->needs_restart(1) if $self->is_known && $self->_data('status') eq 'active';
 }
 

lib/Ravada/Domain/Void.pm

@@ -72,8 +72,11 @@ sub remove {
     my $self = shift;
 
     $self->remove_disks();
-    $self->_vm->run_command("/bin/rm",$self->_config_file());
-    $self->_vm->run_command("/bin/rm",$self->_config_file().".lock");
+
+    my $config_file = $self->_config_file;
+    my ($out, $err) = $self->_vm->run_command("/bin/rm",$config_file);
+    warn $err if $err;
+    $self->_vm->run_command("/bin/rm",$config_file.".lock");
 }
 
 sub can_hibernate { return 1; }
@@ -235,6 +238,7 @@ sub _vol_remove {
         unlink $file or die "$! $file"
             if -e $file;
     } else {
+        return if !$self->_vm->file_exists($file);
         my ($out, $err) = $self->_vm->run_command('ls',$file,'&&','rm',$file);
         warn $err if $err;
     }
@@ -328,9 +332,9 @@ sub add_volume {
 
 sub remove_volume($self, $file) {
     confess "Missing file" if ! defined $file || !length($file);
-    return if ! -e $file;
-    unlink $file or die "$! $file";
 
+    $self->_vol_remove($file);
+    return if ! $self->_vm->file_exists($self->_config_file);
     my $data = $self->_load();
     my $hardware = $data->{hardware};
 

lib/Ravada/Utils.pm

@@ -65,6 +65,7 @@ sub user_daemon {
         );
         $USER_DAEMON = Ravada::Auth::SQL->new(name => $USER_DAEMON_NAME);
     }
+    $USER_DAEMON->_reload_grants();
     return $USER_DAEMON;
 }
 

lib/Ravada/VM/KVM.pm

@@ -2224,6 +2224,7 @@ sub _free_memory_available($self) {
         $used += $memory;
     }
     my $free_mem = $info->{total} - $used;
+    $free_mem = 0 if $free_mem < 0;
     my $free_real = $self->_free_memory_overcommit;
 
     $free_mem = $free_real if $free_real < $free_mem;

t/lib/Test/Ravada.pm

@@ -222,6 +222,7 @@ sub rvd_back($config=undef, $init=1) {
     $RVD_BACK = $rvd;
     $ARG_CREATE_DOM{KVM} = [ id_iso => search_id_iso('Alpine') ];
 
+    Ravada::Utils::user_daemon->_reload_grants();
     return $rvd;
 }
 
@@ -323,12 +324,11 @@ sub remote_config_nodes {
     return $conf;
 }
 
-sub _remove_old_domains_vm {
-    my $vm_name = shift;
+sub _remove_old_domains_vm($vm_name) {
 
+    confess "Undefined vm_name" if !defined $vm_name;
 
     my $domain;
-
     my $vm;
 
     if (ref($vm_name)) {
@@ -875,10 +875,8 @@ sub _domain_node($node) {
 }
 
 sub hibernate_node($node) {
-    diag("hibernate node ".$node->type." ".$node->name);
     if ($node->is_active) {
         for my $domain ($node->list_domains()) {
-            diag("Shutting down ".$domain->name." on node ".$node->name);
             $domain->shutdown_now(user_admin);
         }
     }
@@ -900,7 +898,6 @@ sub hibernate_node($node) {
 
 sub shutdown_node($node) {
 
-    diag("shutdown node ".$node->type." ".$node->name);
     if ($node->is_active) {
         $node->run_command("service lightdm stop");
         $node->run_command("service gdm stop");
@@ -929,7 +926,6 @@ sub shutdown_node($node) {
 sub start_node($node) {
 
     confess "Undefined node"    if !defined $node;
-    diag("start node ".$node->type." ".$node->name);
     confess "Undefined node " if!$node;
 
     $node->disconnect;
@@ -1141,11 +1137,11 @@ sub _do_remote_node($vm_name, $remote_config) {
 
     eval { $node = $vm->new(%{$remote_config}) };
     ok(!$@,"Expecting no error connecting to $vm_name at ".Dumper($remote_config
-).", got :'"
+        ).", got :'"
         .($@ or '')."'") or return;
+    push @NODES,($node) if !grep { $_->name eq $node->name } @NODES;
     ok($node) or return;
 
-    push @NODES,($node) if !grep { $_->name eq $node->name } @NODES;
 
     is($node->type,$vm->type) or return;
 

t/nodes/10_basic.t

@@ -246,7 +246,7 @@ sub test_volatile($vm, $node) {
     $base->volatile_clones(1);
 
     my @clones;
-    for ( 1 .. 4 ) {
+    for ( 1 .. 10 ) {
         my $clone = $base->clone(user => user_admin, name => new_domain_name);
         is($clone->_vm->is_active,1);
         is($clone->is_active(),1,"Expecting clone ".$clone->name." active on ".$clone->_vm->name);

t/nodes/50_three_nodes.t

@@ -92,16 +92,18 @@ for my $vm_name ( 'Void', 'KVM') {
         diag("Testing remote node in $vm_name");
         my ($node1, $node2) = remote_node_2($vm_name);
 
-        clean_remote_node($node1);
-        clean_remote_node($node2);
+        if ( !$node1 || !$node2 ) {
+            diag("Skipped: No remote nodes configured in $Test::Ravada::FILE_CONFIG_REMOTE_2");
+            goto NEXT;
+        }
 
         test_remove($vm, $node1, $node2);
 
         NEXT:
-        clean_remote_node($node1);
-        clean_remote_node($node2);
-        remove_node($node1);
-        remove_node($node2);
+        clean_remote_node($node1)   if $node1;
+        clean_remote_node($node2)   if $node2;
+        remove_node($node1)         if $node1;
+        remove_node($node2)         if $node2;
     }
 
 }

t/nodes/60_hardware.t

+use warnings;
+use strict;
+
+use Carp qw(confess);
+use Data::Dumper;
+use Digest::MD5;
+use Test::More;
+
+use lib 't/lib';
+use Test::Ravada;
+
+no warnings "experimental::signatures";
+use feature qw(signatures);
+
+##################################################################################
+
+sub test_change_hardware($vm, @nodes) {
+    diag("[".$vm->type."] testing remove with ".scalar(@nodes)." node ".join(",",map { $_->name } @nodes));
+    my $domain = create_domain($vm);
+    my $clone = $domain->clone(name => new_domain_name, user => user_admin);
+    my @volumes = $clone->list_volumes();
+
+    for my $node (@nodes) {
+        $domain->set_base_vm( vm => $node, user => user_admin);
+        my $clone2 = $node->search_domain($clone->name);
+        ok(!$clone2);
+        $clone->migrate($node);
+        $clone2 = $node->search_domain($clone->name);
+        ok($clone2);
+    }
+
+    my $info = $domain->info(user_admin);
+    my ($hardware) = grep { !/disk|volume/ } keys %{$info->{hardware}};
+    $clone->remove_controller($hardware,0);
+
+    for my $node (@nodes) {
+        my $clone2 = $node->search_domain($clone->name);
+        ok(!$clone2,"Expecting no clone ".$clone->name." in remote node ".$node->name) or exit;
+    }
+
+    is($clone->_vm->is_local,1) or exit;
+    for (@volumes) {
+        ok(-e $_,$_) or exit;
+    }
+    $clone->remove(user_admin);
+    $domain->remove(user_admin);
+}
+
+##################################################################################
+
+clean();
+
+$Ravada::Domain::MIN_FREE_MEMORY = 256 * 1024;
+
+my @nodes;
+
+for my $vm_name ( 'Void', 'KVM') {
+    my $vm;
+    eval { $vm = rvd_back->search_vm($vm_name) };
+
+    SKIP: {
+
+        my $msg = "SKIPPED: $vm_name virtual manager not found ".($@ or '');
+
+        if ($vm && $>) {
+            $msg = "SKIPPED: Test must run as root";
+            $vm = undef;
+        }
+
+        diag($msg)      if !$vm;
+        skip($msg,10)   if !$vm;
+
+        diag("Testing remote node in $vm_name");
+        my ($node1,$node2) = remote_node_2($vm_name);
+
+        clean_remote_node($node1);
+        clean_remote_node($node2);
+
+        test_change_hardware($vm);
+        test_change_hardware($vm, $node1);
+        test_change_hardware($vm, $node2);
+        test_change_hardware($vm, $node1, $node2);
+
+        NEXT:
+        clean_remote_node($node1);
+        remove_node($node1);
+        clean_remote_node($node2);
+        remove_node($node2);
+    }
+
+}
+
+END: {
+    clean();
+    done_testing();
+}
+

templates/main/settings_machine_tabs_body.html.ep

@@ -57,11 +57,7 @@
 % }
 % if ($USER->can_change_settings($domain->id)) {
     <div class="tab-pane fade" id="nav-hardware" role="tabpanel" aria-labelledby="nav-hardware-tab" >
-% if ($domain->is_base) {
-        %= include 'main/vm_locked_is_base'
-% } else {
         %= include 'main/vm_hardware'
-% }
     </div>
 % }
     <div class="tab-pane fade" id="nav-copy" role="tabpanel" aria-labelledby="nav-copy-tab">

templates/main/vm_hardware.html.ep

-<div class="panel-body">
+<div class="panel-body" ng-show="showmachine.is_active || showmachine.is_base">
+    <div ng-show="showmachine.is_active" class="alert alert-danger" role="alert">
+        This machine is running and can't be modified.
+    </div>
+    <div ng-show="showmachine.is_base" class="alert alert-danger" role="alert">
+        This machine is base and can't be modified.
+    </div>
+</div>
+<div class="panel-body" ng-show="showmachine.is_active == 0 && showmachine.is_base == 0">
 %=   include "main/needs_restart"
     <div ng-repeat="(name,value) in showmachine.hardware" ng-hide="show_new_disk">
         <h2>{{name}}</h2>