Commit 13a83387 authored by Francesc Guasch's avatar Francesc Guasch Committed by Francesc Guasch
Browse files

Fix shared storage change hardware (#1521)

fix(backend): redefine instances on change hardware

* test(nodes): remove base shared storage
* test(nodes): shared storage
* refactor(backend): deal with gone volumes

issue #1340
parent 16bee86e
......@@ -370,6 +370,9 @@ sub _around_start($orig, $self, @arg) {
$error = $@;
last if !$error;
warn "WARNING: $error ".$self->_vm->name." ".$self->_vm->enabled if $error;
next if $error && ref($error) && $error->code == 1;# pool has asynchronous jobs running.
if ($error && $self->id_base && !$self->is_local && $self->_vm->enabled) {
$self->_request_set_base();
next;
......@@ -1250,8 +1253,6 @@ sub _insert_display( $self, $display ) {
$display->{n_order} = $self->_max_n_order_display()+1
if !exists $display->{n_order};
confess if $display->{driver} eq 'rdp' && exists $display->{builtin} && !$display->{is_builtin};
$display->{is_builtin} = $self->_is_display_builtin($display->{driver})
if !defined $display->{is_builtin};
......@@ -2079,8 +2080,14 @@ sub _after_remove_domain {
}
sub _remove_all_volumes($self) {
my $vm_local = $self->_vm;
$vm_local = $self->_vm->new( host => 'localhost' ) if !$self->is_local;
for my $vol (@{$self->{_volumes}}) {
next if $vol =~ /iso$/;
if (!$self->is_local) {
my ($dir) = $vol =~ m{(.*)/};
next if $vm_local->shared_storage($self->_vm, $dir);
}
$self->remove_volume($vol);
}
}
......@@ -2121,7 +2128,23 @@ sub _remove_domain_data_db($id, $type=undef) {
_remove_domain_custom_db($id, $type);
my $sth = $$CONNECTOR->dbh->prepare("DELETE FROM domains WHERE id=?");
$sth->execute($id);
}
sub _redefine_instances($self) {
my $domain_name = $self->name or confess "Unknown my self name $self ".Dumper($self->{_data});
my @instances = $self->list_instances();
for my $instance ( @instances ) {
next if $instance->{id_vm} == $self->_vm->id;
my $vm;
eval { $vm = Ravada::VM->open($instance->{id_vm}) };
die $@ if $@ && $@ !~ /I can't find VM/i;
next if !$vm || !$vm->is_active;
my $domain;
$@ = '';
eval { $domain = $vm->search_domain($domain_name) } if $vm;
warn $@ if $@;
$domain->copy_config($self);
}
}
sub _remove_domain_custom_db($id, $type=undef) {
......@@ -2422,22 +2445,23 @@ sub _do_remove_base($self, $user) {
my $vm_local = $self->_vm->new( host => 'localhost' );
for my $vol ($self->list_volumes_info) {
next if !$vol->file || $vol->file =~ /\.iso$/;
my ($dir) = $vol->file =~ m{(.*)/};
next if !$self->is_local && $self->_vm->shared_storage($vm_local, $dir);
my $backing_file = $vol->backing_file;
next if !$backing_file;
# confess "Error: no backing file for ".$vol->file if !$backing_file;
if (!$self->is_local) {
my ($dir) = $backing_file =~ m{(.*/)};
if ( $self->_vm->shared_storage($vm_local, $dir) ) {
next;
}
next if $self->_vm->shared_storage($vm_local, $dir);
$self->_vm->remove_file($vol->file);
$self->_vm->remove_file($backing_file);
$self->_vm->refresh_storage_pools();
return ;
next;
}
$vol->block_commit();
unlink $vol->file or die "$! ".$vol->file;
my @stat = stat($backing_file);
my @stat = stat($backing_file) or confess "Error: missing $backing_file";
move($backing_file, $vol->file) or die "$! $backing_file -> ".$vol->file;
my $mask = oct(7777);
my $mode = $stat[2] & $mask;
......@@ -2449,8 +2473,11 @@ sub _do_remove_base($self, $user) {
for my $file ($self->list_files_base) {
next if $file =~ /\.iso$/i;
next if ! -e $file;
unlink $file or die "$! unlinking $file";
next if ! $self->_vm->file_exists($file);
my ($dir) = $file =~ m{(.*/)};
next if !$self->_vm->is_local && $self->_vm->shared_storage($vm_local, $dir);
$self->_vm->remove_file($file);
}
$self->storage_refresh() if $self->storage();
......@@ -5080,27 +5107,30 @@ sub needs_restart($self, $value=undef) {
return $self->_data('needs_restart',$value);
}
sub _pre_change_hardware($self, @) {
sub _pre_remove_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($self, $hardware, $index, $data=undef) {
if ($hardware eq 'disk' && ( defined $index || $data ) && $self->is_known() ) {
my $sth = $$CONNECTOR->dbh->prepare("DELETE FROM volumes WHERE id_domain=?");
$sth->execute($self->id);
my @volumes = $self->list_volumes_info();
$self->list_volumes_info();
}
$self->info(Ravada::Utils::user_daemon) if $self->is_known();
$self->needs_restart(1) if $self->is_known && $self->_data('status') eq 'active';
}
sub _around_change_hardware($orig, $self, @args) {
my ($hardware, $index, $data) = @args;
sub _around_change_hardware($orig, $self, $hardware, $index=undef, $data=undef) {
my $real_id_vm;
if ($hardware eq 'disk' && !$self->_vm->is_local) {
$real_id_vm = $self->_vm->id;
my $vm_local = $self->_vm->new( host => 'localhost' );
$self->_set_vm($vm_local, 1);
}
if ($hardware eq 'display') {
......@@ -5113,9 +5143,19 @@ sub _around_change_hardware($orig, $self, @args) {
}
$self->_store_display($data, $current_data);
}
unless ( $hardware eq 'display' && !$self->_is_display_builtin($index, $data)) {
$orig->($self, $hardware, $index,$data);
$self->_redefine_instances() if $self->is_known();
}
_change_instances_hardware($orig, $self,@args);
$self->_post_change_hardware(@args);
if ( $real_id_vm ) {
my $id_vm = $real_id_vm;
my $vm = Ravada::VM->open($id_vm);
$self->_set_vm($vm, 1);
}
$self->_post_change_hardware($hardware, $index, $data);
}
sub _get_display_port($self, $display) {
......@@ -5134,59 +5174,77 @@ sub _get_display_port($self, $display) {
$display->{driver} = $selected->{value};
}
sub _around_add_hardware($orig, $self, $hardware, $index, $data=undef) {
confess "Error: minimal add hardware index>=0 , got '$index'" if defined $index && $index <0;
sub _add_hardware_display($orig, $self, $index, $data) {
confess "Error: missing driver ".Dumper($data) if !exists $data->{driver};
if ($hardware eq 'display' ) {
confess "Error: missing driver ".Dumper($data) if !exists $data->{driver};
die "Error: display ".$data->{driver}." duplicated.\n"
if $self->_get_display($data->{driver});
die "Error: display ".$data->{driver}." duplicated.\n"
if $self->_get_display($data->{driver});
my $is_builtin = $self->_is_display_builtin($data->{driver});
$self->_get_display_port($data)
if exists $data->{driver}
&& !$self->_is_display_builtin( $data->{driver})
&& (!exists $data->{port} || !defined $data->{port});
$self->_get_display_port($data)
if exists $data->{driver}
&& !$is_builtin
&& (!exists $data->{port} || !defined $data->{port});
if ( !$self->_is_display_builtin($data->{driver}) && exists $data->{port}
if ( !$is_builtin && exists $data->{port}
&& defined $data->{port} && $data->{port} ne 'auto') {
die "Error: display $data->{driver} can not be used because port $data->{port} "
." is already exported. Remove it from hardware / ports\n"
if $self->exposed_port($data->{port});
die "Error: display $data->{driver} can not be used because port $data->{port} "
." is already exported. Remove it from hardware / ports\n"
if $self->exposed_port($data->{port});
my $public_port = $self->expose( port => $data->{port}
, name => $data->{driver}
, restricted => 1
);
my $port = $self->exposed_port($data->{port});
$data->{port} = $public_port;
$data->{id_domain_port} = $port->{id};
}
$self->_store_display($data);
my $public_port = $self->expose( port => $data->{port}
, name => $data->{driver}
, restricted => 1
);
my $port = $self->exposed_port($data->{port});
$data->{port} = $public_port;
$data->{id_domain_port} = $port->{id};
}
_change_instances_hardware($orig, $self, $hardware, $index, $data);
$self->_post_change_hardware( $hardware, $index, $data);
$self->_store_display($data);
$orig->($self, 'display', $index, $data) if $is_builtin;
}
sub _change_instances_hardware($orig, $self, @args) {
my ($hardware, $index, $data) = @args;
sub _add_hardware_disk($orig, $self, $index, $data) {
my $real_id_vm;
if (!$self->_vm->is_local) {
$real_id_vm = $self->_vm->id;
my $vm_local = $self->_vm->new( host => 'localhost' );
$self->_set_vm($vm_local, 1);
}
return if $hardware eq 'display' && !$self->_is_display_builtin($index, $data);
$orig->($self, 'disk', $index, $data);
my $vm_orig = $self->_vm;
$self->$orig(@args);
my %changed = ( $self->_vm->id => 1 );
for my $instance ( $self->list_instances ) {
next if $changed{$instance->{id_vm}}++;
if (( defined $index || $data ) && $self->is_known() ) {
my $sth = $$CONNECTOR->dbh->prepare("DELETE FROM volumes WHERE id_domain=?");
$sth->execute($self->id);
my @volumes = $self->list_volumes_info();
}
if ($self->_vm->id != $instance->{id_vm}) {
my $vm = Ravada::VM->open($instance->{id_vm});
$self->_set_vm($vm, 1);
}
if ( $real_id_vm ) {
my $id_vm = $real_id_vm;
my $vm = Ravada::VM->open($id_vm);
$self->_set_vm($vm, 1);
}
}
$self->$orig(@args);
sub _around_add_hardware($orig, $self, $hardware, $index, $data=undef) {
confess "Error: minimal add hardware index>=0 , got '$index'" if defined $index && $index <0;
if ($hardware eq 'display' ) {
_add_hardware_display($orig, $self, $index, $data);
} elsif ($hardware eq 'disk') {
_add_hardware_disk($orig, $self, $index, $data);
} else {
$orig->($self, $hardware, $index, $data);
}
if ( $self->is_known() && !$self->is_base ) {
$self->_redefine_instances();
}
$self->_set_vm($vm_orig, 1) if $vm_orig->id != $self->_vm->id;
$self->needs_restart(1) if $self->is_known && $self->_data('status') eq 'active';
$self->_post_change_hardware( $hardware, $index, $data);
}
sub _delete_db_display($self, $index) {
......@@ -5209,15 +5267,22 @@ sub _delete_db_display($self, $index) {
}
sub _around_remove_hardware($orig, $self, $hardware, $index) {
my $display;
if ( $hardware eq 'display' ) {
my $display = $self->_get_display_by_index($index);
$display = $self->_get_display_by_index($index);
if ( !$display->{is_builtin} ) {
my $port = $self->exposed_port($display->{driver});
$self->remove_expose($port->{internal_port}) if $port;
}
$self->_delete_db_display($index);
}
_change_instances_hardware($orig, $self, $hardware, $index);
$orig->($self, $hardware, $index)
unless $hardware eq 'display' && !$display->{is_builtin};
if ( $self->is_known() && !$self->is_base ) {
$self->_redefine_instances();
}
$self->_post_change_hardware( $hardware, $index);
}
......
......@@ -2589,6 +2589,16 @@ sub reload_config($self, $doc) {
$self->info(Ravada::Utils::user_daemon);
}
sub copy_config($self, $domain) {
my $doc = XML::LibXML->load_xml(string => $domain->xml_description(Sys::Virt::Domain::XML_INACTIVE));
my ($uuid) = $doc->findnodes("/domain/uuid/text()");
confess "I cant'find /domain/uuid in ".$self->name if !$uuid;
$uuid->setData($self->domain->get_uuid_string);
my $new_domain = $self->_vm->vm->define_domain($doc->toString);
$self->domain($new_domain);
}
sub _change_xml_address($self, $doc, $address, $bus) {
my $type_def = $address->getAttribute('type');
return $self->_change_xml_address_ide($doc, $address, 1, 1) if $bus eq 'ide';
......
......@@ -455,14 +455,31 @@ Returns true if the file exists in this virtual manager storage
sub file_exists($self, $file) {
for my $pool ($self->vm->list_all_storage_pools ) {
$pool->refresh();
for my $vol ( $pool->list_all_volumes ) {
return 1 if $vol->get_path eq $file;
$self->_wait_storage( sub { $pool->refresh() } );
my @volumes = $self->_wait_storage( sub { $pool->list_all_volumes });
for my $vol ( @volumes ) {
my $found;
eval { $found = 1 if $vol->get_path eq $file };
# volume was removed in the nick of time
die $@ if $@ && ( !ref($@) || $@->code != 50);
return 1 if $found;
}
}
return 0;
}
sub _wait_storage($self, $sub) {
my @ret;
for ( 1 .. 10 ) {
eval { @ret=$sub->() };
last if !$@;
die $@ if !ref($@) || $@->code != 1;
warn "Warning: $@ [retrying $_]";
sleep 1;
};
return @ret;
}
=head2 dir_img
Returns the directory where disk images are stored in this Virtual Manager
......
......@@ -109,8 +109,9 @@ sub test_prepare_base {
eval {
$domain->remove_base( user_admin);
$domain->prepare_base( user_admin );
$@ = '';
};
is($@,'');
is(''.$@,'');
ok($domain->is_base);
my $name_clone = new_domain_name();
......
......@@ -136,7 +136,7 @@ sub test_remove_domain {
sub test_base {
my $domain = shift;
eval { $domain->prepare_base( user_admin ) };
is($@,'',"Prepare base") or exit;
is(''.$@,'',"Prepare base") or exit;
my @files_base = $domain->list_files_base();
is(scalar @files_base, 2);
......
......@@ -596,6 +596,7 @@ sub test_bases_node {
is($domain->base_in_vm($node->id), undef);
$domain->migrate($node);
is($domain->_vm->id, $node->id) or exit;
is($domain->base_in_vm($node->id), 1);
for my $file ( $domain->list_files_base ) {
......@@ -606,6 +607,9 @@ sub test_bases_node {
$domain->set_base_vm(vm => $node, value => 0, user => user_admin);
is($domain->base_in_vm($node->id), 0);
for my $file ( $domain->list_files_base ) {
ok($vm->file_exists($file)) or die "File $file doesn't exist in ".$vm->name;
}
$domain->set_base_vm(vm => $vm, value => 0, user => user_admin);
is($domain->is_base(),0);
......
......@@ -53,6 +53,7 @@ create_domain
hibernate_domain_internal
remote_node
remote_node_2
remote_node_shared
add_ubuntu_minimal_iso
create_ldap_user
connector
......@@ -793,7 +794,13 @@ sub _remove_old_disks_kvm {
next if $volume->get_name !~ /^${name}_\d+.*\.(img|raw|ro\.qcow2|qcow2|void|backup)$/;
eval { $volume->delete() };
warn $@ if $@;
if ($@) {
if ($@->code == 38 ) {
$vm->remove_file($volume->get_path);
} else {
warn "Error $@ removing ".$volume->get_name." in ".$vm->name if $@;
}
}
}
}
eval {
......@@ -1801,6 +1808,15 @@ sub remote_node_2($vm_name) {
return @nodes;
}
sub remote_node_shared($vm_name) {
my $remote_config = {
'name' => 'ztest-shared'
,'host' => '192.168.122.153'
,public_ip => '192.168.130.153'
};
return _do_remote_node($vm_name, $remote_config);
}
sub _do_remote_node($vm_name, $remote_config) {
my $vm = rvd_back->search_vm($vm_name);
......
......@@ -812,8 +812,7 @@ sub test_remove_base($vm, $node, $volatile) {
$base->remove_base_vm(node => $node, user => user_admin);
for my $file ( @volumes , @volumes0 ) {
my ($out, $err) = $node->run_command("ls $file");
ok(!$out, "Expecting no file '$file' in ".$node->name) or exit;
ok(!$node->file_exists($file));
ok(-e $file, "Expecting file '$file' in local") or exit;
}
isnt($base->_data('id_vm'), $node->id);
......
......@@ -161,6 +161,7 @@ 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) {
for ( 1 .. 10 ) {
......@@ -184,11 +185,10 @@ sub test_change_hardware($vm, @nodes) {
$devices{$hardware} = scalar(@{$info->{hardware}->{$hardware}});
}
for my $hardware ( sort keys %{$info->{hardware}} ) {
#TODO disk volumes in Void
next if $vm->type eq 'Void' && $hardware =~ /disk|volume/;
#next if $vm->type eq 'Void' && $hardware =~ /disk|volume/;
# diag("Testing remove $hardware");
diag("Testing remove $hardware");
my $current_vm = $clone->_vm;
$clone->remove_controller($hardware,0);
......@@ -199,8 +199,14 @@ sub test_change_hardware($vm, @nodes) {
$n_expected = 0 if $n_expected<0;
my $count_instances = $domain->list_instances();
is($count_instances,1+scalar(@nodes),"Expecting other instances not removed when hardware $hardware removed");
for my $node ($vm, @nodes) {
my $clone2 = $node->search_domain($clone->name);
ok($clone2,"Expecting clone ".$clone->name." in remote node ".$node->name
." when removing $hardware") or next;
my $info2 = $clone2->info(user_admin);
my $devices2 = $info2->{hardware}->{$hardware};
is( scalar(@$devices2),$n_expected
......@@ -208,15 +214,12 @@ sub test_change_hardware($vm, @nodes) {
or exit;
}
is($clone->_vm->id, $current_vm->id) or exit;
my $clone3 = Ravada::Domain->open($clone->id);
my $info3 = $clone3->info(user_admin);
$devices{$hardware}--;
for my $item (keys %devices) {
is(scalar(@{$info3->{hardware}->{$item}}), $devices{$item},$item)
or exit;
my $clone_fresh = Ravada::Domain->open($clone->id);
is($clone_fresh->_vm->is_local, 1) or exit;
for (@volumes) {
ok(-e $_,$_) or exit;
}
}
$clone->remove(user_admin);
$domain->remove(user_admin);
......
......@@ -12,20 +12,20 @@ use Test::Ravada;
no warnings "experimental::signatures";
use feature qw(signatures);
my $SHARED_SP = "pool_tst";
my $SHARED_SP = "pool_shared";
my $DIR_SHARED = "/home2/pool_shared";
init();
my $BASE_NAME = "zz-test-base-alpine";
my $BASE;
#################################################################################
sub test_shared($vm, $node) {
$vm->default_storage_pool_name($SHARED_SP);
my $domain = create_domain($vm);
my $storage_path = $vm->_storage_path($SHARED_SP);
is($vm->shared_storage($node, $storage_path),1,"Expecting $SHARED_SP shared") or exit;
for my $vol ($domain->list_disks) {
like($vol,qr(^$storage_path), $vol);
}
......@@ -43,6 +43,17 @@ sub test_shared($vm, $node) {
is($domain->base_in_vm($vm->id),1);
my @files_base = $domain->list_files_base();
for my $vol (@files_base) {
my $ok;
for ( 1 .. 5 ) {
$ok = -e $vol;
last if $ok;
sleep 1;
}
ok($ok,"Volume $vol should exist") or exit;
ok($node->file_exists($vol), "Volume $vol should exist in ".$node->name);
}
$req = Ravada::Request->set_base_vm(
uid => user_admin->id
......@@ -65,12 +76,173 @@ sub test_shared($vm, $node) {
last if $ok;
sleep 1;
}
ok($ok,"Volume $vol should exists");
ok($ok,"Volume $vol should exist") or exit;
ok($node->file_exists($vol), "Volume $vol should exist in ".$node->name);
}
$domain->remove(user_admin);
}
sub test_is_shared($vm, $node) {
is($vm->shared_storage($node,$DIR_SHARED),1) or exit;
is($node->shared_storage($vm,$DIR_SHARED),1) or exit;
my $sth = connector->dbh->prepare("SELECT * FROM storage_nodes "
." WHERE dir=?"
);
my $dir_shared = $DIR_SHARED;
$dir_shared.="/" unless $dir_shared =~ m{/$};
$sth->execute($dir_shared);
my $row = $sth->fetchrow_hashref;
is($row->{is_shared},1) or die Dumper($row);
}
sub _add_disk($domain) {
my $req = Ravada::Request->add_hardware(
uid => user_admin->id
,id_domain => $domain->id
,name=> 'disk'
,data => { size => 512 * 1024 }
);
wait_request(debug => 0);
is($req->error,'');
}
sub _change_ram($domain) {
my $mem = $domain->info(user_admin)->{memory};
my $new_mem = int($mem * 0.8 ) - 1;
my $req = Ravada::Request->change_hardware(
uid => user_admin->id
,id_domain => $domain->id
,hardware => 'memory'
,data => {memory => $new_mem }
);
wait_request(debug => 0);
is($req->error,'');