From 469f1e6798d3dea4c968ed5acb752fd85a62089a Mon Sep 17 00:00:00 2001 From: Francesc Guasch Date: Wed, 15 May 2024 12:51:33 +0200 Subject: [PATCH 1/7] wip: properly dettach host devices --- lib/Ravada.pm | 1 + lib/Ravada/Domain.pm | 48 +++- lib/Ravada/Domain/KVM.pm | 45 +++- lib/Ravada/Domain/Void.pm | 36 ++- lib/Ravada/HostDevice/Templates.pm | 20 ++ lib/Ravada/VM.pm | 4 +- lib/Ravada/VM/Void.pm | 3 +- t/device/40_mediated_device.t | 407 +++++++++++++++++++++++++++-- 8 files changed, 536 insertions(+), 28 deletions(-) diff --git a/lib/Ravada.pm b/lib/Ravada.pm index 54c6b8b57..e7d0ea53c 100644 --- a/lib/Ravada.pm +++ b/lib/Ravada.pm @@ -2271,6 +2271,7 @@ sub _sql_create_tables($self) { ,id_vm => 'integer NOT NULL references `vms`(`id`) ON DELETE CASCADE' ,id_domain => 'integer NOT NULL references `domains`(`id`) ON DELETE CASCADE' ,name => 'varchar(255)' + ,old => 'text' } ] ,[ diff --git a/lib/Ravada/Domain.pm b/lib/Ravada/Domain.pm index 670b6f57b..fb89cddd6 100644 --- a/lib/Ravada/Domain.pm +++ b/lib/Ravada/Domain.pm @@ -355,7 +355,7 @@ sub _around_start($orig, $self, @arg) { $arg{listen_ip} = $display_ip; } if ($enable_host_devices) { - $self->_add_host_devices(@arg); + $self->_attach_host_devices(@arg); } else { $self->_dettach_host_devices(); } @@ -7195,6 +7195,10 @@ sub list_host_devices_attached($self) { # adds host devices to domain instance # usually run right before startup sub _add_host_devices($self, @args) { + $self->_attach_host_devices(@args); +} + +sub _attach_host_devices($self, @args) { my @host_devices = $self->list_host_devices(); return if !@host_devices; return if $self->is_active(); @@ -7233,11 +7237,19 @@ sub _add_host_devices($self, @args) { $self->_lock_host_device($host_device, $device); + my %old; for my $entry( $host_device->render_template($device) ) { + my $old_entry; if ($entry->{type} eq 'node') { - $self->add_config_node($entry->{path}, $entry->{content}, $doc); + $old_entry = $self->add_config_node($entry->{path}, $entry->{content}, $doc); + die Dumper(["old entry should be ARRAY ref ", + ,$self->name + ,$old_entry + ])unless ref($old_entry) && ref($old_entry) eq 'ARRAY'; } elsif ($entry->{type} eq 'unique_node') { - $self->add_config_unique_node($entry->{path}, $entry->{content}, $doc); + $old_entry = $self->add_config_unique_node($entry->{path}, $entry->{content}, $doc); + $old_entry = undef if $old_entry && ref($old_entry) + && ref($old_entry) eq 'HASH' && !keys %$old_entry; } elsif($entry->{type} eq 'attribute') { $self->change_config_attribute($entry->{path}, $entry->{content}, $doc); } elsif($entry->{type} eq 'namespace') { @@ -7247,12 +7259,22 @@ sub _add_host_devices($self, @args) { ." template: ".$entry->{path} ." Unknown type ".($entry->{type} or ''); } + $old{$entry->{path}} = $old_entry if $old_entry; } + $self->_store_hd_old_config($device,\%old) if keys %old; } $self->reload_config($doc); } +sub _store_hd_old_config($self, $device, $old) { + my $sth = $self->_dbh->prepare("UPDATE host_devices_domain_locked " + ." SET old=? " + ." WHERE id_domain=? AND name=?" + ); + $sth->execute(encode_json($old), $self->id, $device); +} + sub _dettach_host_devices($self) { my @host_devices = $self->list_host_devices(); for my $host_device ( @host_devices ) { @@ -7267,10 +7289,30 @@ sub _dettach_host_device($self, $host_device, $doc=$self->get_config return if !defined $device or !length($device); + my $sth0 = $self->_dbh->prepare("SELECT * FROM host_devices_domain_locked " + ." WHERE id_domain=? AND name=? " + ); + $sth0->execute($self->id, $device); + my $row = $sth0->fetchrow_hashref(); + my $old = {}; + $old = decode_json($row->{old}) if $row->{old}; + for my $entry( $host_device->render_template($device) ) { + my $curr_old = $old->{$entry->{path}}; + if ($entry->{type} eq 'node') { + if ($curr_old) { + $self->set_config_node($entry->{path}, $curr_old, $doc); + next; + } $self->remove_config_node($entry->{path}, $entry->{content}, $doc); } elsif ($entry->{type} eq 'unique_node') { + if ($curr_old) { + $self->add_config_unique_node($entry->{path}, $curr_old + ,$doc); + next; + } + $self->remove_config_node($entry->{path}, $entry->{content}, $doc); } elsif($entry->{type} eq 'attribute') { $self->remove_config_attribute($entry->{path}, $entry->{content}, $doc); diff --git a/lib/Ravada/Domain/KVM.pm b/lib/Ravada/Domain/KVM.pm index 0a86f5fc3..224559c9c 100644 --- a/lib/Ravada/Domain/KVM.pm +++ b/lib/Ravada/Domain/KVM.pm @@ -3921,6 +3921,11 @@ sub remove_config_node($self, $path, $content, $doc) { sub _xml_equal_hostdev($doc1, $doc2) { return 1 if $doc1 eq $doc2; + + $doc1 =~ s/\n//g; + $doc2 =~ s/\n//g; + return 1 if $doc1 eq $doc2; + my $parser = XML::LibXML->new() or die $!; $doc1 =~ s{(parse_string($doc1); @@ -3966,12 +3971,17 @@ sub add_config_node($self, $path, $content, $doc) { die "Error: I found ".scalar(@parent)." nodes for $dir, expecting 1" unless scalar(@parent)==1; - my $element; + my @old ; + my @element; eval { - ($element) = $parent[0]->findnodes($entry); + (@element) = $parent[0]->findnodes($entry); }; die $@ if $@ && $@ !~ /Undefined namespace prefix/; - return if $element && $element->toString eq $content; + for my $element (@element) { + return if $element && $element->toString eq $content; + } + + @old = map { $_->toString } @element; if ($content =~ /appendWellBalancedChunk($content); } + return \@old; +} + +sub set_config_node($self, $path, $content, $doc) { + + my ($dir,$entry) = $path =~ m{(.*)/(.*)}; + confess "Error: missing entry in '$path'" if !$entry; + + my @parent = $doc->findnodes($dir); + if (scalar(@parent)==0) { + @parent = $self->_xml_create_path($doc, $dir); + } + + die "Error: I found ".scalar(@parent)." nodes for $dir, expecting 1" + unless scalar(@parent)==1; + + my $parent = $parent[0]; + + for my $child ($parent->findnodes($entry)) { + $parent->removeChild($child); + } + + for my $curr_entry (@$content) { + $parent->appendWellBalancedChunk($curr_entry); + } + } sub _fix_pci_slot($self, $content) { @@ -4046,7 +4082,9 @@ sub add_config_unique_node($self, $path, $content, $doc) { die $@ if $@ && $@ !~ /Undefined namespace prefix/; return if $element && $element->toString eq $content; + my $old; if ($element ) { + $old = $element->toString(); my $child = $parent[0]->removeChild($element); } if ($content =~ /appendWellBalancedChunk($content); } + return $old; } sub change_config_attribute($self, $path, $content, $doc) { diff --git a/lib/Ravada/Domain/Void.pm b/lib/Ravada/Domain/Void.pm index 2eae1a04f..9d7471058 100644 --- a/lib/Ravada/Domain/Void.pm +++ b/lib/Ravada/Domain/Void.pm @@ -13,6 +13,7 @@ use Hash::Util qw(lock_keys lock_hash unlock_hash); use IPC::Run3 qw(run3); use Mojo::JSON qw(decode_json); use Moose; +use Storable qw(dclone); use YAML qw(Load Dump LoadFile DumpFile); use Image::Magick; use MIME::Base64; @@ -1162,12 +1163,34 @@ sub add_config_node($self, $path, $content, $data) { $found = $found->{$item}; } + my $old; if (ref($found) eq 'ARRAY') { + $old = dclone($found); push @$found, ( $content_hash ); } else { my ($item) = keys %$content_hash; + $old = dclone($found->{$item}); $found->{$item} = $content_hash->{$item}; } + return $old; +} + +sub set_config_node($self, $path, $content, $data) { + my $found = $data; + my ($pre,$last) = $path =~ m{(.+)/(.*)}; + if ($pre) { + for my $item (split m{/}, $pre) { + next if !$item; + + confess "Error, no $item in ".Dumper($found) + if !exists $found->{$item}; + + $found = $found->{$item}; + } + } else { + ($last) = $path =~ m{.*/(.*)}; + } + $found->{$last} = $content; } sub remove_config_node($self, $path, $content, $data) { @@ -1211,8 +1234,8 @@ sub _equal_hash($a,$b) { } sub add_config_unique_node($self, $path, $content, $data) { - my $content_hash; - eval { $content_hash = Load($content) }; + my $content_hash = $content; + eval { $content_hash = Load($content) if !ref($content) }; confess $@."\n$content" if $@; $data->{hardware}->{host_devices} = [] @@ -1222,17 +1245,20 @@ sub add_config_unique_node($self, $path, $content, $data) { for my $item (split m{/}, $path ) { next if !$item; - confess "Error, no $item in ".Dumper($found) - if !exists $found->{$item}; - + if ( !exists $found->{$item} ) { + $found->{$item} ={}; + } $found = $found->{$item}; } + my $old; if (ref($found) eq 'ARRAY') { push @$found, ( $content_hash ); } else { my ($item) = keys %$content_hash; + $old = dclone($found); $found->{$item} = $content_hash->{$item}; } + return $old; } diff --git a/lib/Ravada/HostDevice/Templates.pm b/lib/Ravada/HostDevice/Templates.pm index 75dc5acb3..027e7c68a 100644 --- a/lib/Ravada/HostDevice/Templates.pm +++ b/lib/Ravada/HostDevice/Templates.pm @@ -177,6 +177,26 @@ our @TEMPLATES_VOID = ( } ] } + ,{ name => "GPU Mediated Device" + ,list_command => "lsusb " + ,list_filter => '.*' + ,template_args => encode_json( + { uuid => '^(.*?) '} + ) + + ,templates => [{path => "/hardware/host_devices" + ,type => 'node' + ,template => Dump( device => { + uuid => '<%= $uuid %>' + }) + } + ,{ path => "/features" + ,type => 'unique_node' + ,template => Dump( { hidden => 'on' } ) + } + + ] + } ); my %TEMPLATES = ( diff --git a/lib/Ravada/VM.pm b/lib/Ravada/VM.pm index 2f28630ae..2bba34ba5 100644 --- a/lib/Ravada/VM.pm +++ b/lib/Ravada/VM.pm @@ -483,6 +483,7 @@ sub _around_create_domain { confess "ERROR: Unknown args ".Dumper(\%args) if keys %args; $self->_check_duplicate_name($name); + my $create_volatile; if ($id_base) { my $vm_local = $self; $vm_local = $self->new( host => 'localhost') if !$vm_local->is_local; @@ -493,6 +494,7 @@ sub _around_create_domain { unless $owner->allowed_access($base->id); $volatile = $base->volatile_clones if (! defined($volatile)); + $create_volatile=$volatile if !$base->list_host_devices(); if ($add_to_pool) { confess "Error: you can't add to pool and also pick from pool" if $from_pool; $from_pool = 0; @@ -530,7 +532,7 @@ sub _around_create_domain { $args_create{listen_ip} = $self->listen_ip($remote_ip); } - my $domain = $self->$orig(%args_create, volatile => $volatile); + my $domain = $self->$orig(%args_create, volatile => $create_volatile); $self->_add_instance_db($domain->id); $domain->add_volume_swap( size => $swap ) if $swap; $domain->_data('is_compacted' => 1); diff --git a/lib/Ravada/VM/Void.pm b/lib/Ravada/VM/Void.pm index b68973f82..0e552e768 100644 --- a/lib/Ravada/VM/Void.pm +++ b/lib/Ravada/VM/Void.pm @@ -177,6 +177,7 @@ sub create_domain { $domain->change_hardware('network', $index ,{name => $network}) } } + $active = 0 if $domain_base->list_host_devices(); } elsif (!exists $args{config}) { $storage = $self->default_storage_pool_name() if !$storage; my ($vda_name) = "$args{name}-vda-".Ravada::Utils::random_name(4).".void"; @@ -194,7 +195,7 @@ sub create_domain { } $domain->set_memory($args{memory}) if $args{memory}; - if ( $volatile || $user->is_temporary ) { + if ( $active ) { $domain->_store( is_active => 1 ); } # $domain->start(); diff --git a/t/device/40_mediated_device.t b/t/device/40_mediated_device.t index ce3bda5cd..cfb0eb71c 100644 --- a/t/device/40_mediated_device.t +++ b/t/device/40_mediated_device.t @@ -4,6 +4,8 @@ use strict; use Carp qw(confess); use Data::Dumper; use Test::More; +use Mojo::JSON qw( encode_json decode_json ); +use YAML qw(Load Dump LoadFile DumpFile); use Ravada::HostDevice::Templates; @@ -14,6 +16,8 @@ no warnings "experimental::signatures"; use feature qw(signatures); my $BASE; +my $MOCK_MDEV; +my $N_TIMERS; #################################################################### @@ -37,18 +41,269 @@ sub _prepare_dir_mdev() { return $dir; } +sub _check_mdev($vm, $hd) { + + my $n_dev = $hd->list_available_devices(); + return _check_used_mdev($vm, $hd) if $n_dev; + + my $dir = _prepare_dir_mdev(); + $hd->_data('list_command' => "ls $dir"); + + $MOCK_MDEV=1 unless $vm->type eq 'Void'; + return $hd->list_available_devices;; +} + +sub _check_used_mdev($vm, $hd) { + return $hd->list_available_devices() if $vm->type eq 'Void'; + + my @active = $vm->vm->list_domains; + for my $dom (@active) { + my $doc = XML::LibXML->load_xml(string => $dom->get_xml_description); + + my $hd_path = "/domain/devices/hostdev/source/address"; + my ($hostdev) = $doc->findnodes($hd_path); + next if !$hostdev; + + my $uuid = $hostdev->getAttribute('uuid'); + if (!$uuid) { + warn "No uuid in ".$hostdev->toString; + next; + } + my $dom = $vm->import_domain($dom->get_name,user_admin); + $dom->add_host_device($hd->id); + my ($dev) = grep /^$uuid/, $hd->list_devices; + if (!$dev) { + warn "No $uuid found in mdevctl list"; + next; + } + $dom->_lock_host_device($hd, $dev); + } + return $hd->list_available_devices(); +} + sub test_mdev($vm) { my $templates = Ravada::HostDevice::Templates::list_templates($vm->id); my ($mdev) = grep { $_->{name} eq "GPU Mediated Device" } @$templates; ok($mdev,"Expecting PCI template in ".$vm->name) or return; + my $id = $vm->add_host_device(template => $mdev->{name}); + my $hd = Ravada::HostDevice->search_by_id($id); + + my $n_devices = _check_mdev($vm, $hd); + is( $hd->list_available_devices() , $n_devices); + + my $domain = $BASE->clone( + name =>new_domain_name + ,user => user_admin + ); + test_config_no_hd($domain); + $domain->add_host_device($id); + $domain->_attach_host_devices(); + is($hd->list_available_devices(), $n_devices-1); + test_config($domain); + + $domain->_dettach_host_devices(); + is($hd->list_available_devices(), $n_devices); + + test_config_no_hd($domain); + + return ($domain, $hd); +} + +sub _change_state_on($domain) { + if ($domain->type eq 'KVM') { + _change_kvm_state_on($domain); + } elsif ($domain->type eq 'Void') { + _change_void_state_on($domain); + } +} + +sub _change_void_state_on($domain) { + my $config = $domain->_load(); + my $features = ($config->{features} or {}); + $features->{hidden}='on'; + $domain->_store(features => $features); +} + +sub _change_kvm_state_on($domain) { + my $xml = $domain->xml_description(); + my $doc = XML::LibXML->load_xml(string => $xml); + + my $kvm_path = "/domain/features/kvm"; + my ($kvm) = $doc->findnodes($kvm_path); + if (!$kvm) { + my ($features) = $doc->findnodes("/domain/features"); + $kvm = $features->addNewChild(undef,'kvm'); + my $hidden = $kvm->addNewChild(undef, 'hidden'); + $hidden->setAttribute('state' => 'on'); + } + ok($kvm,"Expecting $kvm_path") or return; + my ($state) = $kvm->findnodes("hidden"); + is($state->getAttribute('state'),'on'); + + $domain->reload_config($doc); +} + +sub _change_timer($domain) { + if ($domain->type eq 'KVM') { + _change_timer_kvm($domain); + } elsif ($domain->type eq 'Void') { + _change_timer_void($domain); + } else { + die $domain->type; + } +} + +sub _base_timers_void($domain) { + my $config = $domain->_load(); + my $clock = ($config->{clock} or []); + my @timers = ( + { name => 'rtc', tickpolicy => 'catchup'} + ,{ name => 'pit', tickpolicy => 'delay'} + ,{ name => 'hpet', present => 'no'} + ); + for my $timer (@timers) { + push @$clock ,({timer => $timer }); + } + $domain->_store(clock => $clock); +} + +sub _change_timer_void($domain) { + _base_timers_void($domain); + my $config = $domain->_load(); + my $clock = $config->{clock}; + + $N_TIMERS = scalar(@$clock); + + $domain->_store(clock => $clock); +} + +sub _change_timer_kvm($domain) { + + my $doc = XML::LibXML->load_xml(string => $domain->xml_description()); + + my $timer; + for (my ($node) = $doc->findnodes("/domain/clock/timer") ) { + $timer = $node if $node->getAttribute('name' eq 'tsc'); + } + return if $timer; + my ($clock) = $doc->findnodes("/domain/clock"); + for my $timer ( $clock->findnodes("timer") ) { + $clock->removeChild($timer) if $timer->getAttribute('name') eq 'tsc'; + } + + my @timers = $doc->findnodes("/domain/clock/timer"); + $N_TIMERS = scalar(@timers); + + $domain->reload_config($doc); + +} + +sub test_timer($domain, $field, $value) { + my ($timers,$timer_tsc); + if ($domain->type eq 'KVM') { + ($timers, $timer_tsc)=_get_timer_kvm($domain); + } elsif ($domain->type eq 'Void') { + ($timers, $timer_tsc)= _get_timer_void($domain); + } else { + die $domain->type; + } + is(scalar(@$timers), $N_TIMERS+1); + is(scalar(@$timer_tsc), 1); + is($timer_tsc->[0]->{$field},$value) or confess Dumper([$domain->name,$timer_tsc]); + +} + +sub test_no_timer($domain) { + my ($timers,$list_timer_tsc); + my $expected_tsc = undef; + if ($domain->type eq 'KVM') { + ($timers, $list_timer_tsc)=_get_timer_kvm($domain); + } elsif ($domain->type eq 'Void') { + ($timers, $list_timer_tsc)= _get_timer_void($domain); + } else { + die $domain->type; + } + is(scalar(@$timers), $N_TIMERS) or confess; + is(scalar(@$list_timer_tsc),0); + my $timer_tsc = $list_timer_tsc->[0]; + is_deeply($timer_tsc, $expected_tsc) or confess Dumper($timer_tsc); + +} +sub _get_timer_kvm($domain) { + my $doc = XML::LibXML->load_xml(string => $domain->xml_description()); + my ($clock) = $doc->findnodes("/domain/clock"); + my @timers = $doc->findnodes("/domain/clock/timer"); + my @found_tsc; + for my $node (@timers) { + if ( $node->getAttribute('name') eq'tsc') { + my $found_tsc = {}; + for my $attrib ($node->attributes) { + $found_tsc->{$attrib->name}= $attrib->value; + } + push @found_tsc,($found_tsc); + } + } + return (\@timers,\@found_tsc); +} + +sub _get_timer_void($domain) { + my $config = $domain->_load(); + my $timers = $config->{clock}; + + my @found_tsc; + for my $timer (@$timers) { + die Dumper([$domain->name,$timer]) if ref($timer) ne 'HASH'; + die Dumper([$domain->name,$timer]) if !exists $timer->{timer}->{name}; + + if ( $timer->{timer}->{name} eq 'tsc') { + push @found_tsc , ($timer->{timer}); + } + } + return ($timers,\@found_tsc); +} + +sub _add_template_timer_void($hd) { + my $sth = connector->dbh->prepare( + "INSERT INTO host_device_templates " + ."( id_host_device, path, template,type )" + ."values( ?, '/clock', ? , 'node')" + ); + $sth->execute($hd->id,Dump({ timer => { name => 'tsc', present => 'yes' }})) +} + +sub _add_template_timer_kvm($hd) { + my $sth = connector->dbh->prepare( + "INSERT INTO host_device_templates " + ."( id_host_device, path, template,type )" + ."values( ?, '/domain/clock/timer', ? , 'node')" + ); + $sth->execute($hd->id,"" ) +} + +sub _add_template_timer($hd) { + my $vm = Ravada::VM->open($hd->id_vm); + if ($vm->type eq 'Void') { + _add_template_timer_void($hd); + } else { + _add_template_timer_kvm($hd); + } +} + +sub test_mdev_kvm_state($vm) { + + my $templates = Ravada::HostDevice::Templates::list_templates($vm->id); + my ($mdev) = grep { $_->{name} eq "GPU Mediated Device" } @$templates; + ok($mdev,"Expecting PCI template in ".$vm->name) or return; + my $dir = _prepare_dir_mdev(); my $id = $vm->add_host_device(template => $mdev->{name}); my $hd = Ravada::HostDevice->search_by_id($id); $hd->_data('list_command' => "ls $dir"); + _add_template_timer($hd); is( $hd->list_available_devices() , 2); @@ -56,16 +311,102 @@ sub test_mdev($vm) { name =>new_domain_name ,user => user_admin ); + _change_state_on($domain); + _change_timer($domain); + test_hidden($domain); + test_no_timer($domain); $domain->add_host_device($id); - $domain->_add_host_devices(); - is($hd->list_available_devices(), 1); + $domain->_attach_host_devices(); - test_xml($domain); + test_hidden($domain); + test_old_in_locked($domain); + test_timer($domain,'present' => 'yes'); - return ($domain, $hd); + $domain->_dettach_host_device($hd); + diag("Test hidden after remove"); + test_hidden($domain); + test_no_timer($domain); + + $domain->remove(user_admin); + $hd->remove(); + +} + +sub test_old_in_locked($domain) { + my $sth = connector->dbh->prepare("SELECT * FROM host_devices_domain_locked hdd " + ." WHERE id_domain=?" + ); + $sth->execute($domain->id); + my $row = $sth->fetchrow_hashref(); + like($row->{old},qr/\w/); +} + +sub test_hidden($domain) { + + if ($domain->type eq 'KVM') { + test_hidden_kvm($domain); + } else { + test_hidden_void($domain); + } +} + +sub test_hidden_void($domain) { + my $config = $domain->_load(); + is($config->{features}->{hidden},'on') or exit; } +sub test_hidden_kvm($domain) { + my $doc = XML::LibXML->load_xml(string => $domain->xml_description()); + my $state_path = "/domain/features/kvm/hidden"; + my ($hidden) = $doc->findnodes($state_path); + ok($hidden,"Missing $state_path ".$domain->name) or confess; +} + +sub test_config($domain) { + + if ($domain->type eq'KVM') { + test_xml($domain); + } elsif ($domain->type eq 'Void') { + test_yaml($domain); + } else { + die "unknown type ".$domain->type; + } +} + +sub test_config_no_hd($domain){ + if ($domain->type eq'KVM') { + test_xml_no_hd($domain); + } elsif ($domain->type eq 'Void') { + test_yaml_no_hd($domain); + } else { + die "unknown type ".$domain->type; + } + +} + +sub test_yaml($domain) { + my $config = $domain->_load(); + + my $hd= $config->{hardware}->{host_devices}; + ok($hd) or confess; + + my $features = $config->{features}; + ok($features); + +} + +sub test_yaml_no_hd($domain) { + my $config = $domain->_load(); + + my $hd= $config->{hardware}->{host_devices}; + ok(!$hd) or confess; + + my $features = $config->{features}; + ok(!$features || !keys(%$features)); +} + + sub test_xml($domain) { my $xml = $domain->xml_description(); @@ -87,6 +428,27 @@ sub test_xml($domain) { } +sub test_xml_no_hd($domain) { + + my $xml = $domain->xml_description(); + + my $doc = XML::LibXML->load_xml(string => $xml); + + my $hd_path = "/domain/devices/hostdev"; + my ($hostdev) = $doc->findnodes($hd_path); + ok(!$hostdev,"Expecting no $hd_path") or exit; + my $model = "/domain/devices/video/model"; + my ($video) = $doc->findnodes($model); + ok($video,"Expecting a $model ".$domain->name) or exit; + my $v_type = $video ->getAttribute('type'); + isnt($v_type,'none') or exit; + + my $kvm_path = "/domain/features/kvm/hidden"; + my ($kvm) = $doc->findnodes($kvm_path); + ok(!$kvm,"Expecting no $kvm_path in ".$domain->name) or confess; +} + + sub test_base($domain) { my @args = ( uid => user_admin->id ,id_domain => $domain->id); @@ -96,7 +458,7 @@ sub test_base($domain) { wait_request(); - test_xml($domain); + test_config_no_hd($domain); Ravada::Request->clone(@args, number => 2, remote_ip => '1.2.3.4'); wait_request(); @@ -104,8 +466,8 @@ sub test_base($domain) { for my $clone_data( $domain->clones ) { my $clone = Ravada::Domain->open($clone_data->{id}); - $clone->_add_host_devices(); - test_xml($clone); + $clone->_attach_host_devices(); + test_config($clone); $clone->remove(user_admin); } } @@ -116,27 +478,37 @@ sub test_volatile_clones($vm, $domain, $host_device) { $domain->shutdown_now(user_admin) if $domain->is_active; Ravada::Request->prepare_base(@args) if !$domain->is_base(); - wait_request(); - - is($host_device->list_available_devices(), 2) or exit; + my $n_devices = $host_device->list_available_devices(); + ok($n_devices) or exit; $domain->_data('volatile_clones' => 1); - my $n_clones = $domain->clones; + my @old_clones = $domain->clones; my $n=2; + my $max_n_device = $host_device->list_available_devices(); my $exp_avail = $host_device->list_available_devices()- $n; Ravada::Request->clone(@args, number => $n, remote_ip => '1.2.3.4'); wait_request(check_error => 0); - is(scalar($domain->clones), $n_clones+$n); + is(scalar($domain->clones), scalar(@old_clones)+$n); + + my $found_active=0; + for my $clone ($domain->clones) { + $found_active ++ if $clone->{status} =~ /(active|starting)/; + next if grep { $_->{name} eq $clone->{name} } @old_clones; + ok($clone->{status} =~ /(active|starting)/, $clone->{name}) or exit; + } + is($found_active, $n); my $n_device = $host_device->list_available_devices(); is($n_device,$exp_avail); for my $clone_data( $domain->clones ) { my $clone = Ravada::Domain->open($clone_data->{id}); - test_xml($clone); + is($clone->is_active,1) unless $MOCK_MDEV; + is($clone->is_volatile,1); + test_config($clone); $clone->shutdown_now(user_admin); $n_device = $host_device->list_available_devices(); @@ -147,16 +519,20 @@ sub test_volatile_clones($vm, $domain, $host_device) { my $clone_gone2 = $vm->search_domain($clone_data->{name}); ok(!$clone_gone2,"Expecting $clone_data->{name} removed on shutdown"); + + my $clone_gone3; + eval { $clone_gone3 = $vm->import_domain($clone_data->{name},user_admin,0) }; + ok(!$clone_gone3,"Expecting $clone_data->{name} removed on shutdown") or exit; } $domain->_data('volatile_clones' => 0); - is($host_device->list_available_devices(), 2) or exit; + is($host_device->list_available_devices(), $max_n_device) or exit; } #################################################################### clean(); -for my $vm_name ( 'KVM' ) { +for my $vm_name ('KVM', 'Void' ) { my $vm; eval { $vm = rvd_back->search_vm($vm_name) @@ -177,6 +553,7 @@ for my $vm_name ( 'KVM' ) { } else { $BASE = import_domain($vm); } + test_mdev_kvm_state($vm); my ($domain, $host_device) = test_mdev($vm); test_volatile_clones($vm, $domain, $host_device); test_base($domain); From 9d8c0ec7e5272e3cf211f29d25c44f31a2549c86 Mon Sep 17 00:00:00 2001 From: Francesc Guasch Date: Thu, 16 May 2024 10:36:17 +0200 Subject: [PATCH 2/7] wip: restore completely config from before hd --- lib/Ravada.pm | 4 +- lib/Ravada/Domain.pm | 65 ++++++++++++++------------------ lib/Ravada/Domain/KVM.pm | 17 ++++----- lib/Ravada/Domain/Void.pm | 18 ++++++++- t/device/00_host_device.t | 6 ++- t/device/40_mediated_device.t | 70 ++++++++++++++++++++++++++--------- 6 files changed, 110 insertions(+), 70 deletions(-) diff --git a/lib/Ravada.pm b/lib/Ravada.pm index e7d0ea53c..35074405f 100644 --- a/lib/Ravada.pm +++ b/lib/Ravada.pm @@ -2271,7 +2271,6 @@ sub _sql_create_tables($self) { ,id_vm => 'integer NOT NULL references `vms`(`id`) ON DELETE CASCADE' ,id_domain => 'integer NOT NULL references `domains`(`id`) ON DELETE CASCADE' ,name => 'varchar(255)' - ,old => 'text' } ] ,[ @@ -2953,6 +2952,7 @@ sub _upgrade_tables { $self->_upgrade_table('domains','auto_compact','int default NULL'); $self->_upgrade_table('domains','date_status_change' , 'datetime'); $self->_upgrade_table('domains','show_clones' , 'int not null default 1'); + $self->_upgrade_table('domains','config_no_hd' , 'text'); $self->_upgrade_table('domains_network','allowed','int not null default 1'); @@ -5648,7 +5648,7 @@ sub _cmd_refresh_machine($self, $request) { ,timeout => 60, retry => 10) if $is_active && $domain->ip && $domain->list_ports; - $domain->_unlock_host_devices() if !$is_active; + $domain->_dettach_host_devices() if !$is_active; } sub _cmd_refresh_machine_ports($self, $request) { diff --git a/lib/Ravada/Domain.pm b/lib/Ravada/Domain.pm index fb89cddd6..cc0c2cd06 100644 --- a/lib/Ravada/Domain.pm +++ b/lib/Ravada/Domain.pm @@ -913,7 +913,7 @@ sub _pre_prepare_base($self, $user, $request = undef ) { sleep 1; } } - $self->_unlock_host_devices() if !$self->is_active; + $self->_dettach_host_devices() if !$self->is_active; # $self->_post_remove_base(); if (!$self->is_local) { @@ -3126,7 +3126,7 @@ sub _post_shutdown { my $is_active = $self->is_active; if ( $self->is_known && !$self->is_volatile && !$is_active ) { - $self->_unlock_host_devices(); + $self->_dettach_host_devices(); if ($self->is_hibernated) { $self->_data(status => 'hibernated'); } else { @@ -7125,9 +7125,7 @@ sub add_host_device($self, $host_device) { sub remove_host_device($self, $host_device) { confess if !ref($host_device); - confess if $self->readonly; - $self->_dettach_host_device($host_device); my $id_hd = $host_device->id; @@ -7198,6 +7196,16 @@ sub _add_host_devices($self, @args) { $self->_attach_host_devices(@args); } +sub _backup_config_no_hd($self) { + $self->_data('config_no_hd' => $self->get_config_txt); +} + +sub _restore_config_no_hd($self) { + my $config_no_hd = $self->_data('config_no_hd'); + return if !$config_no_hd; + $self->reload_config($config_no_hd); +} + sub _attach_host_devices($self, @args) { my @host_devices = $self->list_host_devices(); return if !@host_devices; @@ -7212,6 +7220,7 @@ sub _attach_host_devices($self, @args) { $request = delete $args{request} if exists $args{request}; } + $self->_backup_config_no_hd(); my $doc = $self->get_config(); for my $host_device ( @host_devices ) { my $device_configured = $self->_device_already_configured($host_device); @@ -7237,19 +7246,11 @@ sub _attach_host_devices($self, @args) { $self->_lock_host_device($host_device, $device); - my %old; for my $entry( $host_device->render_template($device) ) { - my $old_entry; if ($entry->{type} eq 'node') { - $old_entry = $self->add_config_node($entry->{path}, $entry->{content}, $doc); - die Dumper(["old entry should be ARRAY ref ", - ,$self->name - ,$old_entry - ])unless ref($old_entry) && ref($old_entry) eq 'ARRAY'; + $self->add_config_node($entry->{path}, $entry->{content}, $doc); } elsif ($entry->{type} eq 'unique_node') { - $old_entry = $self->add_config_unique_node($entry->{path}, $entry->{content}, $doc); - $old_entry = undef if $old_entry && ref($old_entry) - && ref($old_entry) eq 'HASH' && !keys %$old_entry; + $self->add_config_unique_node($entry->{path}, $entry->{content}, $doc); } elsif($entry->{type} eq 'attribute') { $self->change_config_attribute($entry->{path}, $entry->{content}, $doc); } elsif($entry->{type} eq 'namespace') { @@ -7259,28 +7260,20 @@ sub _attach_host_devices($self, @args) { ." template: ".$entry->{path} ." Unknown type ".($entry->{type} or ''); } - $old{$entry->{path}} = $old_entry if $old_entry; } - $self->_store_hd_old_config($device,\%old) if keys %old; } $self->reload_config($doc); } -sub _store_hd_old_config($self, $device, $old) { - my $sth = $self->_dbh->prepare("UPDATE host_devices_domain_locked " - ." SET old=? " - ." WHERE id_domain=? AND name=?" - ); - $sth->execute(encode_json($old), $self->id, $device); -} - sub _dettach_host_devices($self) { my @host_devices = $self->list_host_devices(); for my $host_device ( @host_devices ) { $self->_dettach_host_device($host_device); } $self->remove_host_devices(); + + $self->_restore_config_no_hd(); } sub _dettach_host_device($self, $host_device, $doc=$self->get_config @@ -7301,19 +7294,12 @@ sub _dettach_host_device($self, $host_device, $doc=$self->get_config my $curr_old = $old->{$entry->{path}}; if ($entry->{type} eq 'node') { - if ($curr_old) { - $self->set_config_node($entry->{path}, $curr_old, $doc); - next; - } - $self->remove_config_node($entry->{path}, $entry->{content}, $doc); + $self->set_config_node($entry->{path}, $curr_old, $doc) + if $curr_old; } elsif ($entry->{type} eq 'unique_node') { - if ($curr_old) { - $self->add_config_unique_node($entry->{path}, $curr_old - ,$doc); - next; - } - - $self->remove_config_node($entry->{path}, $entry->{content}, $doc); + $self->add_config_unique_node($entry->{path}, $curr_old + ,$doc) + if $curr_old; } elsif($entry->{type} eq 'attribute') { $self->remove_config_attribute($entry->{path}, $entry->{content}, $doc); } elsif($entry->{type} eq 'namespace') { @@ -7356,7 +7342,10 @@ sub _lock_host_device($self, $host_device, $device=undef) { my $query = "INSERT INTO host_devices_domain_locked (id_domain,id_vm,name) VALUES(?,?,?)"; my $sth = $$CONNECTOR->dbh->prepare($query); - eval { $sth->execute($self->id,$self->_vm->id, $device) }; + my $id_vm = $self->_data('id_vm'); + $id_vm = $self->_vm->id if !$id_vm; + cluck if !$id_vm; + eval { $sth->execute($self->id,$id_vm, $device) }; if ($@) { warn $@; $self->_data(status => 'shutdown'); @@ -7392,7 +7381,7 @@ sub _check_host_device_already_used($self, $device) { ." WHERE id_vm=? AND name=?" ; my $sth = $$CONNECTOR->dbh->prepare($query); - $sth->execute($self->_vm->id, $device); + $sth->execute($self->_data('id_vm'), $device); my ($id_domain) = $sth->fetchrow; # warn "\n".($id_domain or '')." [".$self->id."] had locked $device\n"; diff --git a/lib/Ravada/Domain/KVM.pm b/lib/Ravada/Domain/KVM.pm index 224559c9c..a29a70abf 100644 --- a/lib/Ravada/Domain/KVM.pm +++ b/lib/Ravada/Domain/KVM.pm @@ -3651,6 +3651,9 @@ sub _validate_xml($self, $doc) { } sub reload_config($self, $doc) { + if (!ref($doc)) { + $doc = XML::LibXML->load_xml(string => $doc); + } $self->_validate_xml($doc) if $self->_vm->vm->get_major_version >= 4; my $new_domain; @@ -3693,6 +3696,10 @@ sub get_config($self) { return XML::LibXML->load_xml( string => $self->xml_description()); } +sub get_config_txt($self) { + return $self->xml_description(); +} + 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'; @@ -3852,6 +3859,7 @@ sub _add_xml_parse($parent, $content) { } sub remove_config_node($self, $path, $content, $doc) { + confess; my ($dir,$entry) = $path =~ m{(.*)/(.*)}; confess "Error: missing entry in '$path'" if !$entry; @@ -3971,7 +3979,6 @@ sub add_config_node($self, $path, $content, $doc) { die "Error: I found ".scalar(@parent)." nodes for $dir, expecting 1" unless scalar(@parent)==1; - my @old ; my @element; eval { (@element) = $parent[0]->findnodes($entry); @@ -3981,16 +3988,12 @@ sub add_config_node($self, $path, $content, $doc) { return if $element && $element->toString eq $content; } - @old = map { $_->toString } @element; - if ($content =~ /_fix_pci_slot(\$content); $parent[0]->appendWellBalancedChunk($content); } - - return \@old; } sub set_config_node($self, $path, $content, $doc) { @@ -4082,9 +4085,7 @@ sub add_config_unique_node($self, $path, $content, $doc) { die $@ if $@ && $@ !~ /Undefined namespace prefix/; return if $element && $element->toString eq $content; - my $old; if ($element ) { - $old = $element->toString(); my $child = $parent[0]->removeChild($element); } if ($content =~ /appendWellBalancedChunk($content); } - return $old; } sub change_config_attribute($self, $path, $content, $doc) { @@ -4135,7 +4135,6 @@ sub remove_host_devices($self) { my ($dev) = $doc->findnodes("/domain/devices"); for my $hostdev ( $dev->findnodes("hostdev") ) { $dev->removeChild($hostdev); - warn $hostdev->toString(); } $self->reload_config($doc); } diff --git a/lib/Ravada/Domain/Void.pm b/lib/Ravada/Domain/Void.pm index 9d7471058..6ad11390c 100644 --- a/lib/Ravada/Domain/Void.pm +++ b/lib/Ravada/Domain/Void.pm @@ -1242,12 +1242,14 @@ sub add_config_unique_node($self, $path, $content, $data) { if $path eq "/hardware/host_devices" && !exists $data->{hardware}->{host_devices}; my $found = $data; + my $parent; for my $item (split m{/}, $path ) { next if !$item; if ( !exists $found->{$item} ) { $found->{$item} ={}; } + $parent = $found; $found = $found->{$item}; } my $old; @@ -1255,8 +1257,13 @@ sub add_config_unique_node($self, $path, $content, $data) { push @$found, ( $content_hash ); } else { my ($item) = keys %$content_hash; - $old = dclone($found); - $found->{$item} = $content_hash->{$item}; + if ($item) { + $old = dclone($found); + $found->{$item} = $content_hash->{$item}; + } else { + my ($last) = $path =~ m{.*/(.*)}; + $parent->{$last} = $content_hash; + } } return $old; } @@ -1278,7 +1285,14 @@ sub get_config($self) { return $self->_load(); } +sub get_config_txt($self) { + return $self->_vm->read_file($self->_config_file); +} + sub reload_config($self, $data) { + if (!ref($data)) { + $data = Load($data); + } eval { DumpFile($self->_config_file(), $data) }; confess $@ if $@; } diff --git a/t/device/00_host_device.t b/t/device/00_host_device.t index 1e720c15e..69ad4da80 100644 --- a/t/device/00_host_device.t +++ b/t/device/00_host_device.t @@ -315,6 +315,7 @@ sub test_host_device_usb($vm) { _check_hostdev($clone, 1); shutdown_domain_internal($clone); + _check_hostdev($clone, 1) or exit; eval { $clone->start(user_admin) }; is(''.$@, '') or exit; _check_hostdev($clone, 1) or exit; @@ -422,7 +423,7 @@ sub test_host_device_usb_mock($vm, $n_hd=1) { push @clones,($clone); } $clones[0]->shutdown_now(user_admin); - _check_hostdev($clones[0], $n_hd); + _check_hostdev($clones[0], 0); my @devs_attached = $clones[0]->list_host_devices_attached(); is(scalar(@devs_attached), $n_hd); is($devs_attached[0]->{is_locked},0); @@ -553,8 +554,11 @@ sub test_host_device_gpu($vm) { test_hostdev_gpu($base); diag("Remove host device ".$list_hostdev[0]->name); + warn 1; $list_hostdev[0]->remove(); + warn 2; remove_domain($base); + warn 3; } sub test_xmlns($vm) { diff --git a/t/device/40_mediated_device.t b/t/device/40_mediated_device.t index cfb0eb71c..028637ba7 100644 --- a/t/device/40_mediated_device.t +++ b/t/device/40_mediated_device.t @@ -69,18 +69,49 @@ sub _check_used_mdev($vm, $hd) { warn "No uuid in ".$hostdev->toString; next; } - my $dom = $vm->import_domain($dom->get_name,user_admin); - $dom->add_host_device($hd->id); + my $dom_imported = rvd_front->search_domain($dom->get_name); + $dom_imported = $vm->import_domain($dom->get_name,user_admin) + unless $dom_imported; + + $dom_imported->add_host_device($hd->id); my ($dev) = grep /^$uuid/, $hd->list_devices; if (!$dev) { warn "No $uuid found in mdevctl list"; next; } - $dom->_lock_host_device($hd, $dev); + $dom_imported->_lock_host_device($hd, $dev); } return $hd->list_available_devices(); } +sub _req_start($domain) { + if ($MOCK_MDEV) { + $domain->_attach_host_devices(); + } else { + Ravada::Request->start_domain( + uid => user_admin->id + ,id_domain => $domain->id + ); + wait_request(); + } +} + +sub _req_shutdown($domain) { + # $domain->_dettach_host_devices(); + my $req = Ravada::Request->shutdown_domain( + uid => user_admin->id + ,id_domain => $domain->id + ); + wait_request(); + + for ( 1 .. 10 ) { + last if !$domain->is_active; + diag("Waiting for ".$domain." down"); + sleep 1; + } + +} + sub test_mdev($vm) { my $templates = Ravada::HostDevice::Templates::list_templates($vm->id); @@ -99,11 +130,13 @@ sub test_mdev($vm) { ); test_config_no_hd($domain); $domain->add_host_device($id); - $domain->_attach_host_devices(); + _req_start($domain); is($hd->list_available_devices(), $n_devices-1); test_config($domain); - $domain->_dettach_host_devices(); + diag("gonna shutdown ".$domain->name); + _req_shutdown($domain); + # $domain->_dettach_host_devices(); is($hd->list_available_devices(), $n_devices); test_config_no_hd($domain); @@ -225,7 +258,7 @@ sub test_no_timer($domain) { } else { die $domain->type; } - is(scalar(@$timers), $N_TIMERS) or confess; + is(scalar(@$timers), $N_TIMERS) or confess $domain->name; is(scalar(@$list_timer_tsc),0); my $timer_tsc = $list_timer_tsc->[0]; is_deeply($timer_tsc, $expected_tsc) or confess Dumper($timer_tsc); @@ -297,15 +330,14 @@ sub test_mdev_kvm_state($vm) { my ($mdev) = grep { $_->{name} eq "GPU Mediated Device" } @$templates; ok($mdev,"Expecting PCI template in ".$vm->name) or return; - my $dir = _prepare_dir_mdev(); - my $id = $vm->add_host_device(template => $mdev->{name}); my $hd = Ravada::HostDevice->search_by_id($id); - $hd->_data('list_command' => "ls $dir"); + my $n_devices = _check_mdev($vm, $hd); + _add_template_timer($hd); - is( $hd->list_available_devices() , 2); + is( $hd->list_available_devices() , $n_devices); my $domain = $BASE->clone( name =>new_domain_name @@ -317,14 +349,16 @@ sub test_mdev_kvm_state($vm) { test_no_timer($domain); $domain->add_host_device($id); - $domain->_attach_host_devices(); + _req_start($domain); test_hidden($domain); test_old_in_locked($domain); test_timer($domain,'present' => 'yes'); - $domain->_dettach_host_device($hd); - diag("Test hidden after remove"); + _req_shutdown($domain); + $domain->_dettach_host_devices(); + + diag("Test hidden after dettach"); test_hidden($domain); test_no_timer($domain); @@ -334,12 +368,12 @@ sub test_mdev_kvm_state($vm) { } sub test_old_in_locked($domain) { - my $sth = connector->dbh->prepare("SELECT * FROM host_devices_domain_locked hdd " - ." WHERE id_domain=?" + my $sth = connector->dbh->prepare("SELECT config_no_hd FROM domains d " + ." WHERE id=?" ); $sth->execute($domain->id); - my $row = $sth->fetchrow_hashref(); - like($row->{old},qr/\w/); + my ($old) = $sth->fetchrow(); + like($old,qr/\w/); } sub test_hidden($domain) { @@ -403,7 +437,7 @@ sub test_yaml_no_hd($domain) { ok(!$hd) or confess; my $features = $config->{features}; - ok(!$features || !keys(%$features)); + ok(!$features || !keys(%$features)) or confess(Dumper($features)); } From 7be7b5b92e61cf2eda0fd00cbe86ddd8bd7e8b69 Mon Sep 17 00:00:00 2001 From: Francesc Guasch Date: Thu, 16 May 2024 12:28:00 +0200 Subject: [PATCH 3/7] wip: dettach hds before start --- lib/Ravada/Domain.pm | 38 +++++++++++++++++------------------ lib/Ravada/Domain/KVM.pm | 1 - t/device/00_host_device.t | 3 --- t/device/10_templates.t | 25 ++++------------------- t/device/40_mediated_device.t | 1 - 5 files changed, 22 insertions(+), 46 deletions(-) diff --git a/lib/Ravada/Domain.pm b/lib/Ravada/Domain.pm index cc0c2cd06..b9f8599b1 100644 --- a/lib/Ravada/Domain.pm +++ b/lib/Ravada/Domain.pm @@ -300,6 +300,7 @@ sub _vm_disconnect { sub _around_start($orig, $self, @arg) { $self->_post_hibernate() if $self->is_hibernated && !$self->_data('post_hibernated'); + $self->_dettach_host_devices() if !$self->is_active; $self->_start_preconditions(@arg); @@ -7223,26 +7224,18 @@ sub _attach_host_devices($self, @args) { $self->_backup_config_no_hd(); my $doc = $self->get_config(); for my $host_device ( @host_devices ) { + next if !$host_device->enabled(); my $device_configured = $self->_device_already_configured($host_device); + my $device; if ( $device_configured ) { if ( $host_device->enabled() && $host_device->is_device($device_configured) && $self->_lock_host_device($host_device) ) { - next; + $device = $device_configured; } else { $self->_dettach_host_device($host_device, $doc, $device_configured); } } - next if !$host_device->enabled(); - - my ($device) = $host_device->list_available_devices(); - if ( !$device ) { - $device = _refresh_domains_with_locked_devices($host_device); - if (!$device) { - $self->_data(status => 'down'); - $self->_unlock_host_devices(); - die "Error: No available devices in ".$host_device->name."\n"; - } - } + $device = $self->_search_free_device($host_device) if !$device; $self->_lock_host_device($host_device, $device); @@ -7266,13 +7259,24 @@ sub _attach_host_devices($self, @args) { } +sub _search_free_device($self, $host_device) { + my ($device) = $host_device->list_available_devices(); + if ( !$device ) { + $device = _refresh_domains_with_locked_devices($host_device); + if (!$device) { + $self->_data(status => 'down'); + $self->_unlock_host_devices(); + die "Error: No available devices in ".$host_device->name."\n"; + } + } + return $device; +} + sub _dettach_host_devices($self) { my @host_devices = $self->list_host_devices(); for my $host_device ( @host_devices ) { $self->_dettach_host_device($host_device); } - $self->remove_host_devices(); - $self->_restore_config_no_hd(); } @@ -7313,12 +7317,6 @@ sub _dettach_host_device($self, $host_device, $doc=$self->get_config $self->reload_config($doc); $self->_unlock_host_device($device); - my $sth = $$CONNECTOR->dbh->prepare( - "UPDATE host_devices_domain SET name=NULL " - ." WHERE id_domain=? AND id_host_device=?" - ); - $sth->execute($self->id, $host_device->id); - } # marks a host device as being used by a domain diff --git a/lib/Ravada/Domain/KVM.pm b/lib/Ravada/Domain/KVM.pm index a29a70abf..37f9f9066 100644 --- a/lib/Ravada/Domain/KVM.pm +++ b/lib/Ravada/Domain/KVM.pm @@ -3859,7 +3859,6 @@ sub _add_xml_parse($parent, $content) { } sub remove_config_node($self, $path, $content, $doc) { - confess; my ($dir,$entry) = $path =~ m{(.*)/(.*)}; confess "Error: missing entry in '$path'" if !$entry; diff --git a/t/device/00_host_device.t b/t/device/00_host_device.t index 69ad4da80..c58051b8f 100644 --- a/t/device/00_host_device.t +++ b/t/device/00_host_device.t @@ -554,11 +554,8 @@ sub test_host_device_gpu($vm) { test_hostdev_gpu($base); diag("Remove host device ".$list_hostdev[0]->name); - warn 1; $list_hostdev[0]->remove(); - warn 2; remove_domain($base); - warn 3; } sub test_xmlns($vm) { diff --git a/t/device/10_templates.t b/t/device/10_templates.t index f3fd80b60..c613648e4 100644 --- a/t/device/10_templates.t +++ b/t/device/10_templates.t @@ -60,7 +60,7 @@ sub test_hostdev_not_in_domain_config($domain) { sub test_hostdev_in_domain_void($domain) { my $config = $domain->_load(); - ok(exists $config->{hardware}->{host_devices}) or return; + ok(exists $config->{hardware}->{host_devices}) or confess $domain->name; ok(scalar(@{ $config->{hardware}->{host_devices}})) or die "Expecting host_devices" ." in ".Dumper($config->{hardware}); } @@ -211,7 +211,7 @@ sub test_grab_free_device($base) { ok($down_dev->{name}); ($up_dev) = $up->list_host_devices_attached(); is($up_dev->{is_locked},0); - test_hostdev_in_domain_config($up, $expect_feat_kvm); + test_hostdev_not_in_domain_config($up); test_hostdev_in_domain_config($down, $expect_feat_kvm); eval { $up->start(user_admin) }; @@ -238,7 +238,7 @@ sub test_grab_free_device($base) { my ($third_dev_down) = $third->list_host_devices_attached(); is($third_dev_down->{is_locked},0) or die Dumper($third_dev_down); test_hostdev_in_domain_config($up, $expect_feat_kvm); - test_hostdev_in_domain_config($third, $expect_feat_kvm); + test_hostdev_not_in_domain_config($third); } @@ -356,8 +356,6 @@ sub test_templates_start_nohd($vm) { is($req->error, '') or exit; test_hostdev_not_in_domain_config($domain); $domain->shutdown_now(user_admin); - my $device_configured = $domain->_device_already_configured($hd); - is($device_configured, undef); $req = Ravada::Request->start_domain( uid => user_admin->id ,id_domain => $domain->id @@ -535,13 +533,6 @@ sub _mangle_dom_hd($domain) { sub _mangle_dom_hd_void($domain) { my $config = $domain->_load(); - die "Error: ho host devices in ".$domain->name - if !exists $config->{hardware}->{host_devices} - || !exists $config->{hardware}->{host_devices}->[0] - || !defined $config->{hardware}->{host_devices}->[0] - ; - my ($hd) = $config->{hardware}->{host_devices}->[0]; - my $vendor_id = $hd->{vendor_id}; my $sth = connector->dbh->prepare("SELECT id,name FROM host_devices_domain " ." WHERE id_domain=?"); $sth->execute($domain->id); @@ -549,9 +540,6 @@ sub _mangle_dom_hd_void($domain) { my ($id_hd, $device_name) = $sth->fetchrow; $device_name =~ s/(.* ID ).*?(:.*)/${1}$new_id$2/; - $config->{hardware}->{host_devices}->[0]->{vendor_id} = $new_id; - $domain->_store(hardware => $config->{hardware}); - $sth = connector->dbh->prepare("UPDATE host_devices_domain set name=?" ." WHERE id=?"); $sth->execute($device_name, $id_hd); @@ -566,20 +554,15 @@ sub _mangle_dom_hd_kvm($domain) { my ($current) = $line =~ /Device (\d+)/; $device = $current if $current>$device; } - my $xml = XML::LibXML->load_xml(string => $domain->domain->get_xml_description); - - my ($address) = $xml->findnodes("/domain/devices/hostdev/source/address"); - my ($old_device) = $address->getAttribute('device'); my $sth = connector->dbh->prepare("SELECT id,name FROM host_devices_domain " ." WHERE id_domain=?"); $sth->execute($domain->id); my ($id_hd, $device_name) = $sth->fetchrow; - $address->setAttribute(device => ++$device); + $device++; $device_name =~ s/(.*Device )\d+(.*)/$1$device$2/; $sth = connector->dbh->prepare("UPDATE host_devices_domain set name=?" ." WHERE id=?"); $sth->execute($device_name, $id_hd); - $domain->reload_config($xml); } sub _create_host_devices($vm, $n) { diff --git a/t/device/40_mediated_device.t b/t/device/40_mediated_device.t index 028637ba7..45e97b382 100644 --- a/t/device/40_mediated_device.t +++ b/t/device/40_mediated_device.t @@ -134,7 +134,6 @@ sub test_mdev($vm) { is($hd->list_available_devices(), $n_devices-1); test_config($domain); - diag("gonna shutdown ".$domain->name); _req_shutdown($domain); # $domain->_dettach_host_devices(); is($hd->list_available_devices(), $n_devices); From 8b56bfba0f4a179a821db06bfef3b12dc89797e9 Mon Sep 17 00:00:00 2001 From: Francesc Guasch Date: Thu, 16 May 2024 14:48:28 +0200 Subject: [PATCH 4/7] wip: remove hd before backup --- lib/Ravada/Domain.pm | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/Ravada/Domain.pm b/lib/Ravada/Domain.pm index b9f8599b1..385211e85 100644 --- a/lib/Ravada/Domain.pm +++ b/lib/Ravada/Domain.pm @@ -7198,6 +7198,7 @@ sub _add_host_devices($self, @args) { } sub _backup_config_no_hd($self) { + $self->remove_host_devices(); $self->_data('config_no_hd' => $self->get_config_txt); } From d9c5511e60cccf02e1035933f6cc3bab756a7899 Mon Sep 17 00:00:00 2001 From: Francesc Guasch Date: Thu, 16 May 2024 15:34:27 +0200 Subject: [PATCH 5/7] wip: dettach hds and show disabled at run request --- lib/Ravada/Domain.pm | 4 ++-- templates/main/run_request.html.ep | 6 +++++- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/lib/Ravada/Domain.pm b/lib/Ravada/Domain.pm index 385211e85..12a87d768 100644 --- a/lib/Ravada/Domain.pm +++ b/lib/Ravada/Domain.pm @@ -7184,8 +7184,8 @@ sub list_host_devices_attached($self) { $sth_locked->execute($self->id, $row->{name}); my ($is_locked) = $sth_locked->fetchrow(); $row->{is_locked} = 1 if $is_locked; + push @found,($row); } - push @found,($row); } return @found; @@ -7198,7 +7198,7 @@ sub _add_host_devices($self, @args) { } sub _backup_config_no_hd($self) { - $self->remove_host_devices(); + $self->_dettach_host_devices(); $self->_data('config_no_hd' => $self->get_config_txt); } diff --git a/templates/main/run_request.html.ep b/templates/main/run_request.html.ep index 3da1bd68f..3ff5f921f 100644 --- a/templates/main/run_request.html.ep +++ b/templates/main/run_request.html.ep @@ -48,7 +48,12 @@
  • Host Devices
    • + + <%=l 'disabled' %> + + {{device.name}} +
    • @@ -106,7 +111,6 @@ -
    {{request.error}}
    From 9c9a1dc036ee01f2476466a03f7c25c4172de8d4 Mon Sep 17 00:00:00 2001 From: Francesc Guasch Date: Thu, 16 May 2024 15:41:40 +0200 Subject: [PATCH 6/7] wip: first time will not show attached unless available --- t/device/00_host_device.t | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/device/00_host_device.t b/t/device/00_host_device.t index c58051b8f..b384be9ba 100644 --- a/t/device/00_host_device.t +++ b/t/device/00_host_device.t @@ -418,8 +418,8 @@ sub test_host_device_usb_mock($vm, $n_hd=1) { like( ''.$@,qr(Did not find USB device)) if $vm->type eq 'KVM'; is( ''.$@, '' ) if $vm->type eq 'Void'; _check_hostdev($clone, $n_hd); + is(scalar($clone->list_host_devices_attached()), $n_hd, $clone->name); } - is(scalar($clone->list_host_devices_attached()), $n_hd, $clone->name); push @clones,($clone); } $clones[0]->shutdown_now(user_admin); From aee06f352dc1066ab0195710846bbb19c44b3903 Mon Sep 17 00:00:00 2001 From: Francesc Guasch Date: Thu, 16 May 2024 16:01:11 +0200 Subject: [PATCH 7/7] remove the old way just in case --- lib/Ravada/Domain.pm | 16 ++-------------- lib/Ravada/Domain/KVM.pm | 3 +++ 2 files changed, 5 insertions(+), 14 deletions(-) diff --git a/lib/Ravada/Domain.pm b/lib/Ravada/Domain.pm index 12a87d768..57266fb8b 100644 --- a/lib/Ravada/Domain.pm +++ b/lib/Ravada/Domain.pm @@ -7287,24 +7287,12 @@ sub _dettach_host_device($self, $host_device, $doc=$self->get_config return if !defined $device or !length($device); - my $sth0 = $self->_dbh->prepare("SELECT * FROM host_devices_domain_locked " - ." WHERE id_domain=? AND name=? " - ); - $sth0->execute($self->id, $device); - my $row = $sth0->fetchrow_hashref(); - my $old = {}; - $old = decode_json($row->{old}) if $row->{old}; - for my $entry( $host_device->render_template($device) ) { - my $curr_old = $old->{$entry->{path}}; if ($entry->{type} eq 'node') { - $self->set_config_node($entry->{path}, $curr_old, $doc) - if $curr_old; + $self->remove_config_node($entry->{path}, $entry->{content}, $doc); } elsif ($entry->{type} eq 'unique_node') { - $self->add_config_unique_node($entry->{path}, $curr_old - ,$doc) - if $curr_old; + $self->remove_config_node($entry->{path}, $entry->{content}, $doc); } elsif($entry->{type} eq 'attribute') { $self->remove_config_attribute($entry->{path}, $entry->{content}, $doc); } elsif($entry->{type} eq 'namespace') { diff --git a/lib/Ravada/Domain/KVM.pm b/lib/Ravada/Domain/KVM.pm index 37f9f9066..06fab8788 100644 --- a/lib/Ravada/Domain/KVM.pm +++ b/lib/Ravada/Domain/KVM.pm @@ -3912,6 +3912,8 @@ sub remove_config_node($self, $path, $content, $doc) { if ( _xml_equal_hostdev($content, $element_s) ) { $parent->removeChild($element); } else { +=pod + my @lines_c = split /\n/,$content; my @lines_e = split /\n/,$element_s; warn $element->getName." ".(scalar(@lines_c)." ".scalar(@lines_e)); @@ -3921,6 +3923,7 @@ sub remove_config_node($self, $path, $content, $doc) { } warn $content; die $self->name if $element->getName eq 'hostdev'; +=cut } } }