fix(nodes): check different storage pools

Two storage pools with the same name may point to
different mount points.

lib/Ravada.pm

@@ -2151,6 +2151,7 @@ Disconnect all the Virtual Managers connections.
 sub disconnect_vm {
     my $self = shift;
     $self->_disconnect_vm();
+    Ravada::VM::_clean_cache();
 }
 
 sub _disconnect_vm{
@@ -3108,6 +3109,7 @@ sub _execute {
     my $pid = fork();
     die "I can't fork" if !defined $pid;
 
+    $self->disconnect_vm();
     if ( $pid == 0 ) {
         srand();
         $self->_do_execute_command($sub, $request);

lib/Ravada/Domain.pm

@@ -284,24 +284,7 @@ sub _set_vm($self, $vm, $force=0) {
 }
 
 sub _check_equal_storage_pools($self, $vm2) {
-    my $vm1 = $self->_vm;
-    my @sp;
-    push @sp,($vm1->default_storage_pool_name)  if $vm1->default_storage_pool_name;
-    push @sp,($vm1->base_storage_pool)  if $vm1->base_storage_pool;
-    push @sp,($vm1->clone_storage_pool) if $vm1->clone_storage_pool;
-
-    my %sp1 = map { $_ => 1 } @sp;
-
-    my @sp1 = grep /./,keys %sp1;
-
-    my %sp2 = map { $_ => 1 } $vm2->list_storage_pools();
-
-    for my $pool ( @sp1 ) {
-        next if $sp2{ $pool };
-        die "Error: Storage pool '$pool' not found on node ".$vm2->name."\n"
-            .Dumper([keys %sp2]);
-    }
-    return 1;
+    return $self->_vm->_check_equal_storage_pools($vm2);
 }
 
 sub _vm_connect {

lib/Ravada/VM.pm

@@ -306,6 +306,7 @@ sub _post_disconnect($self) {
         $self->clear_netssh();
         delete $SSH{$self->host};
     }
+    delete $VM{$self->id};
 }
 
 sub _pre_create_domain {
@@ -2126,6 +2127,32 @@ sub _list_bridges($self) {
     return @networks;
 }
 
+sub _check_equal_storage_pools($vm1, $vm2) {
+    my @sp;
+    push @sp,($vm1->default_storage_pool_name)  if $vm1->default_storage_pool_name;
+    push @sp,($vm1->base_storage_pool)  if $vm1->base_storage_pool;
+    push @sp,($vm1->clone_storage_pool) if $vm1->clone_storage_pool;
+
+    my %sp1 = map { $_ => 1 } @sp;
+
+    my @sp1 = grep /./,keys %sp1;
+
+    my %sp2 = map { $_ => 1 } $vm2->list_storage_pools();
+
+    for my $pool ( @sp1 ) {
+        die "Error: Storage pool '$pool' not found on node ".$vm2->name."\n"
+            .Dumper([keys %sp2])
+        if !$sp2{ $pool };
+
+        my ($path1, $path2) = ($vm1->_storage_path($pool), $vm2->_storage_path($pool));
+
+        die "Error: Storage pool '$pool' different. In ".$vm1->name." $path1 , "
+            ." in ".$vm2->name." $path2" if $path1 ne $path2;
+    }
+    return 1;
+}
+
+
 1;
 
 

lib/Ravada/VM/KVM.pm

@@ -128,6 +128,7 @@ sub _connect {
 }
 
 sub _list_storage_pools($vm) {
+    confess if !ref($vm);
    for ( ;; ) {
        my @pools;
        eval { @pools = $vm->list_storage_pools };

t/kvm/n10_nodes.t

@@ -85,7 +85,7 @@ sub test_node($vm_name,$node) {
 
     clean_remote_node($node);
 
-    { $node->vm };
+    eval{ $node->vm };
     is($@,'')   or return;
 
     ok($node->id) or return;

t/lib/Test/Ravada.pm

@@ -755,7 +755,8 @@ sub _activate_storage_pools($vm) {
     for my $sp (@sp) {
         next if $sp->is_active;
         diag("Activating sp ".$sp->get_name." on ".$vm->name);
-        $sp->create();
+        $sp->build() unless $sp->is_active;
+        $sp->create() unless $sp->is_active;
     }
 }
 sub _remove_old_disks_kvm {
@@ -1099,23 +1100,27 @@ sub _qemu_storage_pool {
     return $pool_name;
 }
 
-sub remove_qemu_pools {
+sub remove_qemu_pools($vm=undef) {
     return if !$VM_VALID{'KVM'} || $>;
-    my $vm;
-    eval { $vm = rvd_back->search_vm('KVM') };
-    if ($@ && $@ !~ /Missing qemu-img/) {
-        warn $@;
-    }
-    if  ( !$vm ) {
-        $VM_VALID{'KVM'} = 0;
-        return;
+    return if defined $vm && $vm->type eq 'Void';
+    if (!defined $vm) {
+        eval { $vm = rvd_back->search_vm('KVM') };
+        if ($@ && $@ !~ /Missing qemu-img/) {
+            warn $@;
+        }
+        if  ( !$vm ) {
+            $VM_VALID{'KVM'} = 0;
+            return;
+        }
     }
 
     my $base = base_pool_name();
     for my $pool  ( Ravada::VM::KVM::_list_storage_pools($vm->vm)) {
         my $name = $pool->get_name;
+        eval {$pool->build(Sys::Virt::StoragePool::BUILD_NEW); $pool->create() };
+        warn $@ if $@ && $@ !~ /already active/;
         next if $name !~ qr/^$base/;
-        diag("Removing ".$pool->get_name." storage_pool");
+        diag("Removing ".$vm->name." storage_pool ".$pool->get_name);
         _delete_qemu_pool($pool);
         for my $vol ( $pool->list_volumes ) {
             diag("Removing ".$pool->get_name." vol ".$vol->get_name);
@@ -1146,7 +1151,7 @@ sub _delete_qemu_pool($pool) {
     }
     if (-l $dir) {
         unlink $dir or die "$! $dir";
-    } else {
+    } elsif (-e $dir) {
         rmdir($dir) or die "$! $dir";
     }
 
@@ -1246,6 +1251,7 @@ sub clean_remote_node {
     wait_request(debug => 0);
     _remove_old_disks($node);
     flush_rules_node($node)  if !$node->is_local() && $node->is_active;
+    remove_qemu_pools($node);
 }
 
 sub _remove_old_disks {
@@ -2070,7 +2076,7 @@ sub shutdown_nodes {
     }
 }
 
-sub create_storage_pool($vm, $dir=undef) {
+sub create_storage_pool($vm, $dir=undef, $pool_name=new_pool_name()) {
     if (!ref($vm)) {
         $vm = rvd_back->search_vm($vm);
     }
@@ -2079,7 +2085,6 @@ sub create_storage_pool($vm, $dir=undef) {
 
     my $capacity = 1 * 1024 * 1024;
 
-    my $pool_name = new_pool_name();
     if (!$dir) {
         $dir = "/var/tmp/$pool_name";
     }
@@ -2106,9 +2111,11 @@ sub create_storage_pool($vm, $dir=undef) {
 </pool>"
 ;
     my $pool;
-    eval { $pool = $vm->vm->create_storage_pool($xml) };
+    eval { $pool = $vm->vm->define_storage_pool($xml) };
     ok(!$@,"Expecting \$@='', got '".($@ or '')."'") or return;
     ok($pool,"Expecting a pool , got ".($pool or ''));
+    $pool->build( Sys::Virt::StoragePool::BUILD_NEW );
+    $pool->create();
 
     return $pool_name;
 

t/nodes/40_storage.t

@@ -62,6 +62,29 @@ sub test_fail_different_storage_pools($node) {
     };
     like($@, qr'.');
 
+    $base->remove(user_admin);
+    $vm->clone_storage_pool('');
+}
+
+sub test_fail_storage_pools_different_path($vm,$node2) {
+
+    my $sp_name = create_storage_pool($vm);
+    my $dir2="/var/tmp/".new_pool_name();
+    my $sp2 = create_storage_pool($node2,$dir2, $sp_name);
+
+    my $sp_default = $vm->default_storage_pool_name();
+    $vm->default_storage_pool_name($sp_name);
+    my $base = create_domain($vm->type);
+    $base->remove_controller('disk',1);
+
+    eval {
+        $base->migrate($node2);
+    };
+    like(''.$@, qr/Error: Storage pool.*different/,"migrating to ".$node2->name) or BAIL_OUT();
+    diag($@);
+
+    $vm->default_storage_pool_name($sp_default);
+
     $base->remove(user_admin);
 }
 
@@ -123,6 +146,7 @@ for my $vm_name ( 'KVM' ) {
         my $node = remote_node($vm_name)  or next;
         clean_remote_node($node);
 
+        test_fail_storage_pools_different_path($vm, $node);
         test_fail_different_storage_pools($node);
 
         test_shared_conflict($vm, $node);