fix: adding usb spice requires graphics spice too

[frankiejol]

When SPICE display has been removed it is not possible to add USB spicevmc devices. Add the display when required.

[Copilot]

Pull request overview

This PR addresses a KVM/SPICE configuration edge case in Ravada: when a domain’s SPICE display is removed, adding a USB spicevmc redirection device fails because the domain XML no longer contains a SPICE <graphics> device.

Changes:

• Update KVM domain XML mutation logic to (re)add a SPICE <graphics> device when adding usb/spicevmc redirection.
• Add a regression test that removes the display and then attempts to add USB spicevmc.
• Adjust hardware clone test normalization to ignore filesystem.id_domain during deep comparisons.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
lib/Ravada/Domain/KVM.pm	Adds _add_spice_display and refactors SPICE graphics defaults to support USB spicevmc even when SPICE display was removed.
t/kvm/s10_spice.t	Adds a regression test for “remove display then add usb spicevmc”.
t/request/30_hardware_clones.t	Normalizes filesystem hardware clone comparisons by deleting id_domain.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

_add_spice_display creates a <graphics type="spice"> node but doesn't set the listen attribute / <listen> child or autoport. Other code paths (eg display_info -> _display_info_spice) read listen from the XML to populate the display IP, so this new graphics node can end up with an undefined IP and incomplete SPICE config. Consider passing $self (or at least the listen IP) into _add_spice_display and initializing the same core attributes/children that _set_controller_display_spice sets (listen, <listen type="address" ...>, and autoport/port defaults).

Suggested change

	# Ensure core SPICE graphics attributes are set so other code paths
	# (e.g. display_info/_display_info_spice) can reliably read them.
	my $listen_address = '0.0.0.0';
	# Set default listen and autoport if not already present
	$graphic->setAttribute(listen => $listen_address)
	if !$graphic->hasAttribute('listen');
	$graphic->setAttribute(autoport => 'yes')
	if !$graphic->hasAttribute('autoport');
	# Ensure there is a <listen type="address" ...> child
	my ($listen_node) = $graphic->findnodes('./listen[@type="address"]');
	if (!$listen_node) {
	$listen_node = $graphic->addNewChild(undef, 'listen');
	$listen_node->setAttribute(type => 'address');
	}
	$listen_node->setAttribute(address => $listen_address)
	if !$listen_node->hasAttribute('address');

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

This strict string comparison might be too fragile. The newly created SPICE graphics element may have different attributes or child element ordering compared to the original, even if functionally equivalent. Consider comparing specific attributes instead of the entire XML string, or use a more flexible XML comparison method.

Suggested change

	is($spice_graphics->toString, $spice_graphics0->toString);
	is($spice_graphics->getAttribute('type'), 'spice', "graphics type is 'spice'");
	my $autoport = $spice_graphics->getAttribute('autoport');
	ok(defined($autoport) && $autoport eq 'yes', "SPICE graphics autoport is 'yes'");
	is($spice_graphics->getAttribute('type'),
	$spice_graphics0->getAttribute('type'),
	"graphics type matches original SPICE graphics");
	my $autoport = $spice_graphics->getAttribute('autoport');
	my $autoport_orig = $spice_graphics0->getAttribute('autoport');
	is($autoport, $autoport_orig, "SPICE graphics autoport matches original");