From 9ae8d74ee9fc2c4b902b0d7ac37a7a0d97da6128 Mon Sep 17 00:00:00 2001
From: Francesc Guasch <frankie@etsetb.upc.edu>
Date: Tue, 11 Mar 2025 11:52:34 +0100
Subject: [PATCH 1/6] test: check spice removed and added
issue #2157
---
t/kvm/s10_spice.t | 68 +++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 66 insertions(+), 2 deletions(-)
diff --git a/t/kvm/s10_spice.t b/t/kvm/s10_spice.t
index f0cfdfeb2..74897f4ea 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,68 @@ 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 $req = Ravada::Request->remove_hardware(
+ uid => user_admin->id
+ ,id_domain => $domain->id
+ ,name => 'display'
+ ,index => 0
+ );
+ wait_request();
+
+ is($req->error,'');
+
+ my $doc =XML::LibXML->load_xml(string => $domain->domain->get_xml_description());
+ my ($spice) = $doc->findnodes("/domain/devices/graphics");
+ ok(!$spice);
+
+ my $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[\@type='spice']");
+ is(scalar(@audio),1);
+
+ my @channel = $doc->findnodes("/domain/devices/channel[\@type='spicevmc']");
+ is(scalar(@channel),1);
}
+sub test_remove_spice($domain) {
+ $domain->shutdown_now(user_admin) if $domain->is_active;
+
+ 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 +139,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();
From add8d9a0b4309755c96fe38d60c9c26502eeacf4 Mon Sep 17 00:00:00 2001
From: Francesc Guasch <frankie@etsetb.upc.edu>
Date: Tue, 11 Mar 2025 16:12:47 +0100
Subject: [PATCH 2/6] wip: remove and add spice and related hardware
issue #2157
---
lib/Ravada/Domain/KVM.pm | 57 +++++++++++++++++++++++++++++++++++++++-
public/js/ravada.js | 4 +++
t/kvm/s10_spice.t | 47 +++++++++++++++++++++++++++++----
3 files changed, 102 insertions(+), 6 deletions(-)
diff --git a/lib/Ravada/Domain/KVM.pm b/lib/Ravada/Domain/KVM.pm
index 96fd9045e..15ecde2b8 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,11 @@ 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);
+ }
+
$self->_vm->connect if !$self->_vm->vm;
$self->reload_config($doc);
@@ -2706,6 +2713,54 @@ sub _remove_device($self, $index, $device, $attribute_name0=undef, $attribute_va
." not removed, only ".($ind)." found in ".$self->name."\n";
}
+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 74897f4ea..64a7a9fc5 100644
--- a/t/kvm/s10_spice.t
+++ b/t/kvm/s10_spice.t
@@ -54,21 +54,48 @@ sub test_spice {
}
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($info->{hardware}->{display});
+
+ ok(defined $index_tls) or die "No spice-tls found in hardwre"
+ .Dumper($info->{hardware}->{display});
+
my $req = Ravada::Request->remove_hardware(
uid => user_admin->id
,id_domain => $domain->id
,name => 'display'
- ,index => 0
+ ,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);
- my $info = $domain->info(user_admin);
+ $info = $domain->info(user_admin);
my ($hw_spice) = grep { $_->{driver} =~ /spice/ } @{$info->{hardware}->{display}};
ok(!$hw_spice);
}
@@ -89,15 +116,25 @@ sub _add_display($domain,$driver='spice') {
my @redir = $doc->findnodes("/domain/devices/redirdev[\@type=\'spicevmc\']");
ok(scalar(@redir)>2);
- my @audio = $doc->findnodes("/domain/devices/audio[\@type='spice']");
- is(scalar(@audio),1);
+ 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) {
- $domain->shutdown_now(user_admin) if $domain->is_active;
+ 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");
From 46bd3f03f9bb2b08e02e72c99e055369c175fc72 Mon Sep 17 00:00:00 2001
From: Francesc Guasch <frankie@telecos.upc.edu>
Date: Wed, 12 Mar 2025 12:50:05 +0100
Subject: [PATCH 3/6] wip: show only display types found
---
t/kvm/s10_spice.t | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/t/kvm/s10_spice.t b/t/kvm/s10_spice.t
index 64a7a9fc5..98875ecee 100644
--- a/t/kvm/s10_spice.t
+++ b/t/kvm/s10_spice.t
@@ -71,10 +71,11 @@ sub _remove_display($domain) {
}
ok(defined $index) or die "No spice found in hardwre"
- .Dumper($info->{hardware}->{display});
+ .Dumper([ map { $_->{driver} } @{$info->{hardware}->{display}}] );
+
ok(defined $index_tls) or die "No spice-tls found in hardwre"
- .Dumper($info->{hardware}->{display});
+ .Dumper([ map { $_->{driver} } @{$info->{hardware}->{display}}] );
my $req = Ravada::Request->remove_hardware(
uid => user_admin->id
From 32e9b36b88843d7153e300d9184e4071a11c255d Mon Sep 17 00:00:00 2001
From: Francesc Guasch <frankie@telecos.upc.edu>
Date: Thu, 13 Mar 2025 08:52:13 +0100
Subject: [PATCH 4/6] wip: deny change hw to active domains with host devs
issue #2157
---
lib/Ravada/Domain.pm | 7 +++++++
1 file changed, 7 insertions(+)
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');
From 3d46e158f60eb408f6698304f5a90d34a76d361c Mon Sep 17 00:00:00 2001
From: Francesc Guasch <frankie@etsetb.upc.edu>
Date: Tue, 18 Mar 2025 11:38:20 +0100
Subject: [PATCH 5/6] wip: set minimal none video in KVM
issue #2157
---
lib/Ravada/Domain/KVM.pm | 13 +++++++++++++
t/nodes/60_hardware.t | 9 +++++++--
2 files changed, 20 insertions(+), 2 deletions(-)
diff --git a/lib/Ravada/Domain/KVM.pm b/lib/Ravada/Domain/KVM.pm
index 15ecde2b8..508275327 100644
--- a/lib/Ravada/Domain/KVM.pm
+++ b/lib/Ravada/Domain/KVM.pm
@@ -2696,6 +2696,8 @@ sub _remove_device($self, $index, $device, $attribute_name0=undef, $attribute_va
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;
@@ -2713,6 +2715,17 @@ 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);
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);
From b8e452663b3e6875d67e980f3078d30daf666a18 Mon Sep 17 00:00:00 2001
From: Francesc Guasch <frankie@etsetb.upc.edu>
Date: Tue, 18 Mar 2025 12:48:35 +0100
Subject: [PATCH 6/6] wip: force refresh machine to fix conflict
---
t/vm/20_base.t | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
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);