diff --git a/lib/Ravada/Domain.pm b/lib/Ravada/Domain.pm index 114c67c87..4fe278fe7 100644 --- a/lib/Ravada/Domain.pm +++ b/lib/Ravada/Domain.pm @@ -6445,6 +6445,10 @@ sub _add_hardware_disk($orig, $self, $index, $data) { sub _around_add_hardware($orig, $self, $hardware, $index, $data=undef) { confess "Error: minimal add hardware index>=0 , got '$index'" if defined $index && $index <0; + + die "Error: Virtual Machines with host devices can not be modified while running" + if $self->is_active && $self->list_host_devices_locked; + my $data_orig = undef; $data_orig = dclone($data ) if ref($data); @@ -6478,6 +6482,9 @@ sub _delete_db_display_by_driver($self, $driver) { sub _around_remove_hardware($orig, $self, $hardware, $index=undef, $options=undef) { confess "Error: supply either index or options when removing hardware " if !defined $index && !defined $options; + die "Error: Virtual Machines with host devices can not be modified while running" + if $self->is_active && $self->list_host_devices_locked; + my $id_filesystem; if ( $hardware eq 'filesystem') { my @fs = $self->get_controller('filesystem'); diff --git a/lib/Ravada/Domain/KVM.pm b/lib/Ravada/Domain/KVM.pm index 96fd9045e..508275327 100644 --- a/lib/Ravada/Domain/KVM.pm +++ b/lib/Ravada/Domain/KVM.pm @@ -2537,8 +2537,8 @@ sub _set_controller_display_spice($self, $number, $data) { $graphic = $g0; } } + my ($devices) = $doc->findnodes("/domain/devices"); if (!$graphic) { - my ($devices) = $doc->findnodes("/domain/devices"); $graphic = $devices->addNewChild(undef,'graphics'); } @@ -2569,6 +2569,8 @@ sub _set_controller_display_spice($self, $number, $data) { my $item = $graphic->addNewChild(undef, $name); $item->setAttribute($attrib => $value); } + _add_spice_related($devices); + $self->reload_config($doc); } @@ -2691,6 +2693,13 @@ sub _remove_device($self, $index, $device, $attribute_name0=undef, $attribute_va $file = $source->getAttribute('file') if $source; $devices->removeChild($controller); + + if ($device eq 'graphics' && $controller->getAttribute('type') eq 'spice') { + $self->_remove_spice_related($devices); + } elsif ($device eq 'video') { + $self->_set_minimal_video($devices); + } + $self->_vm->connect if !$self->_vm->vm; $self->reload_config($doc); @@ -2706,6 +2715,65 @@ sub _remove_device($self, $index, $device, $attribute_name0=undef, $attribute_va ." not removed, only ".($ind)." found in ".$self->name."\n"; } +sub _set_minimal_video($self, $devices) { + my $string = 'video'; + my @current = $devices->findnodes($string); + if (!@current) { + my $video = $devices->addNewChild(undef,'video'); + my $model = $video->addNewChild(undef,'model'); + $model->setAttribute('type' => 'none'); + } + +} + +sub _remove_spice_related($self, $devices) { + for my $redir ($devices->findnodes("redirdev[\@type=\'spicevmc\']")) { + $devices->removeChild($redir); + } + my ($audio) = $devices->findnodes("audio[\@type='spice']"); + $devices->removeChild($audio) if $audio; + + my ($channel) = $devices->findnodes("channel[\@type='spicevmc']"); + $devices->removeChild($channel) if $channel; +} + +sub _add_spice_related($devices) { + + if (!$devices->findnodes("redirdev[\@type=\'spicevmc\']")) { + for ( 1 .. 3 ) { + my $redir = $devices->addNewChild(undef,'redirdev'); + $redir->setAttribute('type' => 'spicevmc'); + $redir->setAttribute('bus' => 'usb'); + } + } + my @audio = $devices->findnodes("audio"); + my ($none) = grep { $_->getAttribute('type') eq 'none' } @audio; + + if (!@audio || $none) { + $devices->removeChild($none) if $none; + + my $audio= $devices->addNewChild(undef,'audio'); + $audio->setAttribute(type =>'spice'); + $audio->setAttribute(id => 1); + } + + if (!$devices->findnodes("channel[\@type=\'spicevmc\']")) { + my $channel = $devices->addNewChild(undef,'channel'); + $channel->setAttribute('type' => 'spicevmc'); + + my $target = $channel->addNewChild(undef,'target'); + $target->setAttribute('type' => 'virtio'); + $target->setAttribute('name' => 'com.redhat.spice.0'); + + my $address = $channel->addNewChild(undef,'address'); + $address->setAttribute('type' => 'virtio-serial'); + $address->setAttribute('controller' => 0); + $address->setAttribute('bus' => 0); + $address->setAttribute('port' => 1); + + } +} + sub _remove_controller_display($self, $index, $attribute_name=undef, $attribute_value=undef) { $self->_remove_device($index,'graphics', $attribute_name,$attribute_value ); } diff --git a/public/js/ravada.js b/public/js/ravada.js index 348c57a9e..5eaf97d47 100644 --- a/public/js/ravada.js +++ b/public/js/ravada.js @@ -940,6 +940,10 @@ } } + if(typeof($scope.showmachine.hardware[hardware][index]["_index"]) + !== 'undefined') { + index=$scope.showmachine.hardware[hardware][index]._index; + } $scope.request('remove_hardware',{ 'id_domain': $scope.showmachine.id ,'name': hardware diff --git a/t/kvm/s10_spice.t b/t/kvm/s10_spice.t index f0cfdfeb2..98875ecee 100644 --- a/t/kvm/s10_spice.t +++ b/t/kvm/s10_spice.t @@ -6,6 +6,9 @@ use Data::Dumper; use IPC::Run3; use Test::More; +use feature qw(signatures); +no warnings "experimental::signatures"; + use lib 't/lib'; use Test::Ravada; @@ -15,7 +18,7 @@ init(); my @VMS = vm_names(); my $USER = create_user("foo","bar", 1); -my $TLS; +my $TLS=0; ####################################################### @@ -47,8 +50,106 @@ sub test_spice { my ($port_f) = $display_file =~ m{port=(.*)}mx; is($ip_d, $ip_f); is($port_d, $port_f); + return $domain; } +sub _remove_display($domain) { + + my $info = $domain->info(user_admin); + my $index; + my $index_tls; + my $n = 0; + for my $item ( @{$info->{hardware}->{display}} ) { + if ( $item->{driver} eq 'spice' ) { + $index=$item->{_index}; + } + if ( $item->{driver} eq 'spice-tls' ) { + $index_tls=$n; + } + last if $index && $index_tls; + $n++; + } + + ok(defined $index) or die "No spice found in hardwre" + .Dumper([ map { $_->{driver} } @{$info->{hardware}->{display}}] ); + + + ok(defined $index_tls) or die "No spice-tls found in hardwre" + .Dumper([ map { $_->{driver} } @{$info->{hardware}->{display}}] ); + + my $req = Ravada::Request->remove_hardware( + uid => user_admin->id + ,id_domain => $domain->id + ,name => 'display' + ,index => $index + ); + wait_request(); + + is($req->error,''); + Ravada::Request->force_shutdown( + uid => user_admin->id + ,id_domain => $domain->id + ); + wait_request(); + + my $doc =XML::LibXML->load_xml(string => $domain->domain->get_xml_description()); + my ($spice) = $doc->findnodes("/domain/devices/graphics"); + ok(!$spice); + + $info = $domain->info(user_admin); + my ($hw_spice) = grep { $_->{driver} =~ /spice/ } @{$info->{hardware}->{display}}; + ok(!$hw_spice); +} + +sub _add_display($domain,$driver='spice') { + my $req = Ravada::Request->add_hardware( + uid => user_admin->id + ,id_domain => $domain->id + ,name => 'display' + ,data => { driver => $driver} + ); + wait_request(); + + my $doc =XML::LibXML->load_xml(string => $domain->domain->get_xml_description()); + my ($spice) = $doc->findnodes("/domain/devices/graphics[\@type='$driver']"); + ok($spice); + + my @redir = $doc->findnodes("/domain/devices/redirdev[\@type=\'spicevmc\']"); + ok(scalar(@redir)>2); + + my @audio = $doc->findnodes("/domain/devices/audio"); + ok(scalar(@audio)); + + my ($none) = grep { $_->getAttribute('type') eq 'none' } @audio; + ok(!$none,"Expecting no 'none' audio"); + + my @audio_spice = $doc->findnodes("/domain/devices/audio[\@type='spice']"); + is(scalar(@audio_spice),1); + + my @channel = $doc->findnodes("/domain/devices/channel[\@type='spicevmc']"); + is(scalar(@channel),1); +} + +sub test_remove_spice($domain) { + Ravada::Request->start_domain( + uid => user_admin->id + ,id_domain => $domain->id + ); + wait_request(); + + my $doc =XML::LibXML->load_xml(string => $domain->domain->get_xml_description()); + my ($spice) = $doc->findnodes("/domain/devices/graphics"); + die "Error: no spice found in ".$domain->name if !$spice; + + _remove_display($domain); + + _add_display($domain, 'spice'); + + $domain->start(user_admin); + + my $info = $domain->info(user_admin); + like($info->{hardware}->{display}->[0]->{driver},qr'spice'); +} ####################################################### @@ -76,7 +177,8 @@ SKIP: { $TLS = 1 if check_libvirt_tls() && $vm_name eq 'KVM'; - test_spice($vm_name); + my $domain = test_spice($vm_name); + test_remove_spice($domain); } end(); diff --git a/t/nodes/60_hardware.t b/t/nodes/60_hardware.t index e2ed8b9de..f5cf246ad 100644 --- a/t/nodes/60_hardware.t +++ b/t/nodes/60_hardware.t @@ -225,7 +225,7 @@ sub test_change_hardware($vm, @nodes) { } my @hardware = grep (!/^disk$/, sort keys %{$info->{hardware}}); push @hardware,("disk"); - for my $hardware ( @hardware) { + for my $hardware (reverse @hardware) { next if $hardware =~ /cpu|features|memory/; my $tls = 0; $tls = grep {$_->{driver} =~ /-tls/} @{$info->{hardware}->{$hardware}} @@ -260,9 +260,14 @@ sub test_change_hardware($vm, @nodes) { my $info2 = $clone2->info(user_admin); my $devices2 = $info2->{hardware}->{$hardware}; - is( scalar(@$devices2),$n_expected + if ($hardware eq 'video' && $vm->type eq 'KVM') { + is( scalar(@$devices2),1); + is($devices2->[0]->{type},'none'); + } else { + is( scalar(@$devices2),$n_expected , $clone2->name.": Expecting 1 $hardware device less in instance in node ".$node->name) or die Dumper($devices2); + } } my $clone_fresh = Ravada::Domain->open($clone->id); diff --git a/t/vm/20_base.t b/t/vm/20_base.t index 6653d502e..24ed1bac2 100644 --- a/t/vm/20_base.t +++ b/t/vm/20_base.t @@ -523,7 +523,7 @@ sub test_display_info($vm) { my $exp_driver = 'spice'; $exp_driver .= "-tls" if $TLS; is($display_h->[0]->{driver}, $exp_driver) if $domain->type eq 'KVM'; - like($display_h->[0]->{password},qr{..+}, $domain_f->name) or exit if $domain->type eq 'KVM'; + like($display_h->[0]->{password},qr{..+}, $domain_f->name) or confess Dumper($display_h->[0]) if $domain->type eq 'KVM'; is($display_h->[0]->{id_exposed_port},undef); # spice doesn't need exposed port is($display_h->[1+$TLS]->{driver}, 'rdp'); @@ -1481,7 +1481,7 @@ sub test_display_drivers($vm, $remove) { Ravada::Request->start_domain(uid => user_admin->id ,id_domain => $domain->id ); - for ( 1 .. 10 ) { + for my $n ( 1 .. 20 ) { wait_request(debug => 0); last if !$req->error || $req->error !~ /Retry/i; sleep 1; @@ -1608,10 +1608,11 @@ sub test_display_conflict($vm) { for ( 1 .. 10 ) { $port3 = $domain->exposed_port(22); last if $port3->{public_port} && $port3->{public_port} != $display_builtin->{port}; - Ravada::Request->refresh_machine(uid => user_admin->id ,id_domain => $domain->id); + Ravada::Request->refresh_machine(uid => user_admin->id ,id_domain => $domain->id + , _force => 1); wait_request(debug => 0); } - isnt($port3->{public_port},$display_builtin->{port}) or die; + isnt($port3->{public_port},$display_builtin->{port}) or die $domain->id." ".$domain->name; $domain->remove(user_admin);