[virt-manager PATCH] Fix checking for graphics spice for Xen

[Cole Robinson]

On 2/9/22 4:53 PM, Charles Arnold wrote:
> On 2/9/22 11:32 AM, Charles Arnold wrote:
>> On 2/9/22 10:32 AM, Cole Robinson wrote:
>>> On 2/9/22 9:50 AM, Charles Arnold wrote:
>>>> Advertising graphics support doesn't necessarily mean spice support.
>>>> self.devices.graphics.supported seems to not be spice specific
>>>> but rather more generic in indicating whether graphics are supported.
>>>> For Xen, spice is not supported so fallback to the old logic.
>>>>
>>>> Signed-off-by: Charles Arnold <carnold at suse.com>
>>>>
>>>> diff --git a/virtinst/domcapabilities.py b/virtinst/domcapabilities.py
>>>> index 67bceaa3..ad6e3363 100644
>>>> --- a/virtinst/domcapabilities.py
>>>> +++ b/virtinst/domcapabilities.py
>>>> @@ -382,7 +382,7 @@ class DomainCapabilities(XMLBuilder):
>>>>           return len(models) > 0 and bool("emulator" in backends)
>>>>
>>>>       def supports_graphics_spice(self):
>>>> -        if not self.devices.graphics.supported:
>>>> +        if not self.devices.graphics.supported or self.conn.is_xen():
>>>>               # domcaps is too old, or the driver doesn't advertise
>>>> graphics
>>>>               # support. Use our pre-existing logic
>>>>               if not self.conn.is_qemu() and not self.conn.is_test():
>>>>
>>>>
>>> Hmm but does that mean domcapabilities for xen is reporting that spice
>>> is available? If that's the case, seems like a domcapabilities bug in
>>> libvirt xen driver. Or am I missing something?
>>>
>>> Thanks,
>>> Cole
>>>
>>
>> Booted into Xen, virsh domcapabilities reports,
>>
>> <domainCapabilities>
>>   <path>/usr/bin/qemu-system-x86_64</path>
>>   <domain>xen</domain>
>>   <machine>xenpv</machine>
>>   <arch>x86_64</arch>
>>   <vcpu max='512'/>
>>   <iothreads supported='no'/>
>>   <os supported='yes'>
>>     <loader supported='no'/>
>>   </os>
>>   <cpu>
>>     <mode name='host-passthrough' supported='no'/>
>>     <mode name='maximum' supported='no'/>
>>     <mode name='host-model' supported='no'/>
>>     <mode name='custom' supported='no'/>
>>   </cpu>
>>   <devices>
>>     <disk supported='yes'>
>>       <enum name='diskDevice'>
>>         <value>disk</value>
>>         <value>cdrom</value>
>>       </enum>
>>       <enum name='bus'>
>>         <value>ide</value>
>>         <value>scsi</value>
>>         <value>xen</value>
>>       </enum>
>>       <enum name='model'/>
>>     </disk>
>>     <graphics supported='yes'>
>>       <enum name='type'>
>>         <value>sdl</value>
>>         <value>vnc</value>
>>         <value>spice</value>
>>       </enum>
>>     </graphics>
>>     <video supported='yes'>
>>       <enum name='modelType'>
>>         <value>vga</value>
>>         <value>cirrus</value>
>>         <value>xen</value>
>>       </enum>
>>     </video>
>>     <hostdev supported='yes'>
>>       <enum name='mode'>
>>         <value>subsystem</value>
>>       </enum>
>>       <enum name='startupPolicy'>
>>         <value>default</value>
>>         <value>mandatory</value>
>>         <value>requisite</value>
>>         <value>optional</value>
>>       </enum>
>>       <enum name='subsysType'>
>>         <value>usb</value>
>>         <value>pci</value>
>>       </enum>
>>       <enum name='capsType'/>
>>       <enum name='pciBackend'>
>>         <value>xen</value>
>>       </enum>
>>     </hostdev>
>>   </devices>
>>   <features>
>>     <gic supported='no'/>
>>     <vmcoreinfo supported='no'/>
>>     <genid supported='no'/>
>>     <sev supported='no'/>
>>   </features>
>> </domainCapabilities>
>>

Thanks, this could be useful to add to the test suite. Is output the
mostly the same for `virsh domcapabilities --machine xenfv` too?

> 
> The more precise error I'm trying to fix is while attempting to install
> a Xen HVM guest it reports,
> 
> details=Traceback (most recent call last):
>   File "./virtManager/asyncjob.py", line 72, in cb_wrapper
>     callback(asyncjob, *args, **kwargs)
>   File "./virtManager/createvm.py", line 2008, in _do_async_install
>     installer.start_install(guest, meter=meter)
>   File "./virtinst/install/installer.py", line 704, in start_install
>     doboot, transient)
>   File "./virtinst/install/installer.py", line 649, in _create_guest
>     domain = self.conn.createXML(install_xml or final_xml, 0)
>   File "/usr/lib64/python3.6/site-packages/libvirt.py", line 4393, in
> createXML
>     raise libvirtError('virDomainCreateXML() failed')
> libvirt.libvirtError: unsupported configuration: cannot add redirected
> USB device: USB is disabled for this domain
> 
> So in the latest code the VM is marked as supporting spice and later
> wants to add a DeviceRedirdev. The 3.2.0 code always just failed for
> spice if 'is_qemu()' was false.

Okay, interesting. Can you provide the full --debug output? Probably our
code that adds usb2/usb3 support by default is not triggering for xen.
There might be something we need to tweak here. This could be a bug on
its own since it probably means explicit `--graphics spice` on the cli
doesn't work when it should.

Can you also try adding `--redirdev none` on the cli and see if that
fixes it, or hits another error?


But my change wasn't intended to change the default for xen from vnc to
spice. I didn't think xen supported spice so I didn't expect it to be
advertised in domcaps. Rather than hack the domcaps check, I think we
should make this explicit at the source: devices/graphics.py
_default_type function. I've pushed this bit which I think will fix your
case:

diff --git a/virtinst/devices/graphics.py b/virtinst/devices/graphics.py
index a91db289..0c3499ef 100644
--- a/virtinst/devices/graphics.py
+++ b/virtinst/devices/graphics.py
@@ -155,10 +155,16 @@ class DeviceGraphics(Device):
     def _default_type(self, guest):
         gtype = guest.default_graphics_type
         log.debug("Using default_graphics=%s", gtype)
+
+        if self.conn.is_xen():
+            # Xen domcaps can advertise spice support, but we have
historically
+            # not defaulted to it for xen, so force vnc.
+            log.debug("Not defaulting to spice for xen driver. Using vnc.")
+            gtype = "vnc"
+


Thanks,
Cole