fix: properly check free storage size (#1980)

fix: properly check free storage size
UPC · Sep 20, 2023 · 5fe0f05 · 5fe0f05
1 parent 6db872a
commit 5fe0f05
Show file tree

Hide file tree

Showing 4 changed files with 73 additions and 21 deletions.
diff --git a/lib/Ravada/Domain.pm b/lib/Ravada/Domain.pm
@@ -704,7 +704,9 @@ sub _around_add_volume {
         ($name) = $file =~ m{.*/(.*)} if !$name && $file;
         $name = $self->name if !$name;
 
-        $name .= "-".$args{target}."-".Ravada::Utils::random_name(4);
+        $name .= "-".$args{target}."-".Ravada::Utils::random_name(4)
+        if $name !~ /\.iso$/;
+
         $args{name} = $name;
     }
 
@@ -717,10 +719,12 @@ sub _around_add_volume {
     $args{allocation} = Ravada::Utils::size_to_number($args{allocation})
         if exists $args{allocation} && defined $args{allocation};
 
-    my $free = $self->_vm->free_disk();
+    my $storage = $args{storage};
+
+    my $free = $self->_vm->free_disk($storage);
     my $free_out = int($free / 1024 / 1024 / 1024 ) * 1024 *1024 *1024;
 
-    confess "Error creating volume, out of space $size . Disk free: "
+    die "Error creating volume, out of space $size . Disk free: "
             .Ravada::Utils::number_to_size($free_out)
             ."\n"
         if exists $args{size} && $args{size} && $args{size} >= $free;
@@ -1945,9 +1949,6 @@ sub display($self, $user) {
 
     my ($display_info) = grep { $_->{driver} !~ /-tls$/ } @display_info;
 
-    confess "Error: I can't find builtin display info for ".$self->name." ".ref($self)."\n".Dumper($display_info)
-    if !exists $display_info->{port};
-
     return '' if !$display_info->{driver} || !$display_info->{ip}
     || !$display_info->{port};
 

diff --git a/t/lib/Test/Ravada.pm b/t/lib/Test/Ravada.pm
@@ -86,6 +86,8 @@ create_domain
     remove_old_users
     remove_old_users_ldap
 
+    remove_qemu_pools
+
     mangle_volume
     test_volume_contents
     test_volume_format
@@ -273,6 +275,7 @@ sub create_domain_v2(%args) {
     my $user = (delete $args{user} or $USER_ADMIN);
     my $iso_name = delete $args{iso_name};
     my $id_iso = delete $args{id_iso};
+    my $disk = delete $args{disk};
     croak "Error: supply either iso_name or id_iso ".Dumper(\%args)
     if $iso_name && $id_iso;
 
@@ -298,13 +301,12 @@ sub create_domain_v2(%args) {
     if keys %args;
 
     my $iso;
-    my $disk;
     if ($vm->type eq 'KVM' && (!$iso_name || $iso_name !~ /Alpine/i)) {
         $iso = $vm->_search_iso($id_iso);
-        $disk = ($iso->{min_disk_size} or 2 );
+        $disk = ($iso->{min_disk_size} or 2 ) if !$disk;
         diag("Creating [$id_iso] $iso->{name}");
     } else {
-        $disk = 2;
+        $disk = 2 if !$disk;
     }
 
     if ($vm->type eq 'KVM' && !$options ) {
@@ -1510,6 +1512,7 @@ sub remove_old_storage_pools_void() {
 }
 
 sub remove_old_storage_pools_kvm() {
+    remove_qemu_pools();
 }
 
 sub remove_old_storage_pools() {

diff --git a/t/vm/20_base.t b/t/vm/20_base.t
@@ -300,17 +300,15 @@ sub test_displays_added_on_refresh($domain, $n_expected, $delete=1) {
 
     _wait_ip($domain);
 
+    my $domain_f0 = Ravada::Front::Domain->open($domain->id);
+    my $display0_raw = $domain_f0->info(user_admin)->{hardware}->{display};
+
+    my @display0 = grep { $_->{is_secondary} == 0 } @$display0_raw;
+    my $n_expected2 = scalar(@display0);
+
     if ($delete) {
         my $sth = connector->dbh->prepare("DELETE FROM domain_displays WHERE id_domain=?");
         $sth->execute($domain->id);
-    }else {
-        my $sth = connector->dbh->prepare("SELECT count(*) FROM domain_displays WHERE id_domain=?");
-        $sth->execute($domain->id);
-        my ($n_exp2) = $sth->fetchrow;
-        if ($n_exp2 && $n_expected != $n_exp2) {
-            cluck "n_expected was $n_expected, but it should be $n_exp2";
-            $n_expected = $n_exp2;
-        }
     }
     my $req;
     for ( 1 .. 3 ) {
@@ -333,7 +331,7 @@ sub test_displays_added_on_refresh($domain, $n_expected, $delete=1) {
     my $count;
     for ( 1 .. 10 ) {
         my $sth_count = connector->dbh->prepare(
-            "SELECT count(*) FROM domain_displays WHERE id_domain=?");
+            "SELECT count(*) FROM domain_displays WHERE id_domain=? AND is_secondary=0");
         $sth_count->execute($domain->id);
         ($count) = $sth_count->fetchrow;
         last if $count;
@@ -344,11 +342,13 @@ sub test_displays_added_on_refresh($domain, $n_expected, $delete=1) {
         );
         wait_request();
     }
-    ok($count>=$n_expected,"Got $count, expecting >$n_expected displays on table domain_displays for ".$domain->name) or confess;
+    ok($count>=$n_expected || $count >=$n_expected2,"Got $count, expecting >$n_expected or $n_expected2 displays on table domain_displays for ".$domain->name) or confess;
 
     my $domain_f = Ravada::Front::Domain->open($domain->id);
-    my $display = $domain_f->info(user_admin)->{hardware}->{display};
-    is(scalar(@$display), $n_expected,"Expecting $n_expected displays on info->{hardware}->{display} in ".$domain->name) or confess Dumper($display);
+    my $display_raw = $domain_f->info(user_admin)->{hardware}->{display};
+    my @display = grep { $_->{is_secondary} == 0 } @$display_raw;
+    ok(scalar(@display) == $n_expected || scalar(@display) == $n_expected2
+        ,"Expecting $n_expected or $n_expected2 displays on info->{hardware}->{display} in ".$domain->name) or confess Dumper(@display);
 
 }
 

diff --git a/t/vm/s30_storage.t b/t/vm/s30_storage.t
@@ -85,6 +85,48 @@ sub _clean_local {
     my $file = "$dir/check_storage";# or die "$!";
     unlink $file or die "$! $file" if -e $file;
 }
+
+sub test_storage_full($vm) {
+
+    my $dir = "/run";
+    $dir.="/user/".$< if $<;
+    my $storage_name = new_domain_name();
+
+    $dir.= "/".$storage_name;
+
+    mkdir $dir or die "$! $dir" if ! -e $dir;
+
+    if (! grep { $_ eq $storage_name} $vm->list_storage_pools) {
+        $vm->create_storage_pool($storage_name,$dir);
+    }
+
+    my($out,$err) = $vm->run_command("df");
+
+    my ($available) = $out =~ m{(\d+)\s+\d+\% /run}ms;
+
+    $available = $available*10;
+
+    $vm->default_storage_pool_name($storage_name);
+
+    my $name = new_domain_name;
+    my $req = Ravada::Request->create_domain(
+        vm => $vm->type
+        ,id_owner => user_admin->id
+        ,name => $name
+        ,disk => $available
+        ,storage => 'default'
+        ,id_iso => search_id_iso('Alpine%64')
+    );
+    wait_request(debug => 0);
+
+    my $domain = $vm->search_domain($name);
+    for my $vol ($domain->list_volumes) {
+        next if $vol =~ /iso$/;
+        unlike($vol,qr{^/run});
+    }
+
+}
+
 ###########################################################
 
 _clean_local();
@@ -101,6 +143,12 @@ for my $vm_name (vm_names() ) {
         }
 
         skip($msg,10)   if !$vm;
+
+        if ($vm->type eq 'KVM') {
+            remove_qemu_pools($vm);
+        }
+        test_storage_full($vm);
+
         test_storage_pools($vm);
         test_storage_pools_fail($vm);
     }