Francesco Romani has uploaded a new change for review.

Change subject: vm: graphics: fix console open after restore
......................................................................

vm: graphics: fix console open after restore

When restoring a VM from hibernation (aka migration from file),
if we don't set a SPICE password, libvirt automatically sets
the 'disable-ticketing' QEMU option, which will prevent
any future vm.setTicket (thus any graphical connection) to work.

This behaviour is easily reproduced by:
a. booting a VM with SPICE graphic device
b. do NOT connect to it using SPICE
c. hibernate the VM
d. restore the VM
e. inspect the QEMU command line for the disable-ticketing parameter

Reproduced on RHEL/CentOS 7.1, most likely being there since long time.

The automatic restore using virDomainRestore() does not set the passwd.
This behaviour may be surprising, but makes sense.

To fix this, we
1. always use virDomainRestoreFlags() in the restore path
2. amend the VM Domain XML, injecting the passwd the same way we do for
  VM startup.

X-Backport-To: 3.6
Change-Id: Ic9f11e2d87cc715c77d305c005c1cd7f502d506d
Bug-Url: https://bugzilla.redhat.com/1234197
Signed-off-by: Francesco Romani <[email protected]>
---
M tests/devices/data/Makefile.am
A tests/devices/data/vm_restore_spice_after.xml
A tests/devices/data/vm_restore_spice_before.xml
M tests/vmTests.py
M vdsm/virt/vm.py
M vdsm/virt/vmdevices/graphics.py
6 files changed, 354 insertions(+), 7 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/42/44842/1

diff --git a/tests/devices/data/Makefile.am b/tests/devices/data/Makefile.am
index 179a5de..6a5f543 100644
--- a/tests/devices/data/Makefile.am
+++ b/tests/devices/data/Makefile.am
@@ -37,4 +37,6 @@
        usb_1_1.xml \
        usb_1_1_4.xml \
        usb_usb1.xml \
+       vm_restore_spice_after.xml \
+       vm_restore_spice_before.xml \
        $(NULL)
diff --git a/tests/devices/data/vm_restore_spice_after.xml 
b/tests/devices/data/vm_restore_spice_after.xml
new file mode 100644
index 0000000..bc08203
--- /dev/null
+++ b/tests/devices/data/vm_restore_spice_after.xml
@@ -0,0 +1,145 @@
+<domain type='kvm' id='6'>
+  <name>ftest1</name>
+  <uuid>42e30017-b29c-4646-aa75-e42c4d0fe659</uuid>
+  <memory unit='KiB'>4194304</memory>
+  <currentMemory unit='KiB'>4194304</currentMemory>
+  <vcpu placement='static' current='2'>16</vcpu>
+  <cputune>
+    <shares>1020</shares>
+  </cputune>
+  <numatune>
+    <memory mode='interleave' nodeset='0'/>
+  </numatune>
+  <resource>
+    <partition>/machine</partition>
+  </resource>
+    <sysinfo type='smbios'>
+      <system>
+        <entry name='manufacturer'>Red Hat</entry>
+        <entry name='product'>RHEV Hypervisor</entry>
+        <entry name='version'>7.2-1.el7</entry>
+        <entry name='serial'>4C4C4544-0059-4410-8043-B6C04F4E5A31</entry>
+        <entry name='uuid'>42e30017-b29c-4646-aa75-e42c4d0fe659</entry>
+      </system>
+    </sysinfo>
+  <os>
+    <type arch='x86_64' machine='pc-i440fx-rhel7.1.0'>hvm</type>
+    <smbios mode='sysinfo'/>
+  </os>
+  <features>
+    <acpi/>
+  </features>
+  <cpu mode='custom' match='exact'>
+    <model fallback='allow'>SandyBridge</model>
+    <topology sockets='16' cores='1' threads='1'/>
+    <numa>
+      <cell id='0' cpus='0,1,2,3,4,5,6,7' memory='4194304'/>
+    </numa>
+  </cpu>
+  <clock offset='variable' adjustment='0' basis='utc'>
+    <timer name='rtc' tickpolicy='catchup'/>
+    <timer name='pit' tickpolicy='delay'/>
+    <timer name='hpet' present='no'/>
+  </clock>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>destroy</on_crash>
+  <devices>
+    <emulator>/usr/libexec/qemu-kvm</emulator>
+    <disk type='file' device='cdrom'>
+      <driver name='qemu' type='raw'/>
+      <source 
file='/rhev/data-center/mnt/192.168.1.20:_srv_virtstore_nfs_generic_iso/b0390d9e-2f1d-43c0-a56c-a20af032e934/images/11111111-1111-1111-1111-111111111111/Fedora-Live-Workstation-x86_64-21-5.iso'
 startupPolicy='optional'>
+        <seclabel model='selinux' labelskip='yes'/>
+      </source>
+      <backingStore/>
+      <target dev='hdc' bus='ide'/>
+      <readonly/>
+      <serial></serial>
+      <boot order='1'/>
+      <alias name='ide0-1-0'/>
+      <address type='drive' controller='0' bus='1' target='0' unit='0'/>
+    </disk>
+    <disk type='file' device='disk' snapshot='no'>
+      <driver name='qemu' type='raw' cache='none' error_policy='stop' 
io='threads'/>
+      <source 
file='/rhev/data-center/00000001-0001-0001-0001-000000000178/f856ad64-22a3-4460-bdd7-5846573b3c01/images/c088d8c8-2599-46c9-aeb8-2e33671a4237/b267133d-fe7b-4d7d-b9ea-b0fed7db4490'>
+        <seclabel model='selinux' labelskip='yes'/>
+      </source>
+      <backingStore/>
+      <target dev='vda' bus='virtio'/>
+      <serial>c088d8c8-2599-46c9-aeb8-2e33671a4237</serial>
+      <boot order='2'/>
+      <alias name='virtio-disk0'/>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x05' 
function='0x0'/>
+    </disk>
+    <controller type='usb' index='0'>
+      <alias name='usb0'/>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' 
function='0x2'/>
+    </controller>
+    <controller type='pci' index='0' model='pci-root'>
+      <alias name='pci.0'/>
+    </controller>
+    <controller type='ide' index='0'>
+      <alias name='ide0'/>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' 
function='0x1'/>
+    </controller>
+    <controller type='virtio-serial' index='0'>
+      <alias name='virtio-serial0'/>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x04' 
function='0x0'/>
+    </controller>
+    <interface type='bridge'>
+      <mac address='00:1a:4a:16:01:51'/>
+      <source bridge='ovirtmgmt'/>
+      <bandwidth>
+      </bandwidth>
+      <target dev='vnet0'/>
+      <model type='virtio'/>
+      <filterref filter='vdsm-no-mac-spoofing'/>
+      <link state='up'/>
+      <boot order='3'/>
+      <alias name='net0'/>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x03' 
function='0x0'/>
+    </interface>
+    <console type='unix'>
+      <source mode='bind' 
path='/var/run/ovirt-vmconsole-console/42e30017-b29c-4646-aa75-e42c4d0fe659.sock'/>
+      <target type='virtio' port='0'/>
+      <alias name='console0'/>
+    </console>
+    <channel type='unix'>
+      <source mode='bind' 
path='/var/lib/libvirt/qemu/channels/42e30017-b29c-4646-aa75-e42c4d0fe659.com.redhat.rhevm.vdsm'/>
+      <target type='virtio' name='com.redhat.rhevm.vdsm' state='disconnected'/>
+      <alias name='channel0'/>
+      <address type='virtio-serial' controller='0' bus='0' port='1'/>
+    </channel>
+    <channel type='unix'>
+      <source mode='bind' 
path='/var/lib/libvirt/qemu/channels/42e30017-b29c-4646-aa75-e42c4d0fe659.org.qemu.guest_agent.0'/>
+      <target type='virtio' name='org.qemu.guest_agent.0' state='connected'/>
+      <alias name='channel1'/>
+      <address type='virtio-serial' controller='0' bus='0' port='2'/>
+    </channel>
+    <channel type='spicevmc'>
+      <target type='virtio' name='com.redhat.spice.0' state='connected'/>
+      <alias name='channel2'/>
+      <address type='virtio-serial' controller='0' bus='0' port='3'/>
+    </channel>
+    <input type='mouse' bus='ps2'/>
+    <input type='keyboard' bus='ps2'/>
+    <graphics type='spice' port='5900' tlsPort='5901' autoport='yes' 
listen='0' passwd='*****' passwdValidTo='1970-01-01T00:00:01'>
+      <listen type='address' address='0'/>
+    </graphics>
+    <video>
+      <model type='qxl' ram='65536' vram='32768' vgamem='16384' heads='1'/>
+      <alias name='video0'/>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x02' 
function='0x0'/>
+    </video>
+    <memballoon model='none'>
+      <alias name='balloon0'/>
+    </memballoon>
+  </devices>
+  <seclabel type='dynamic' model='selinux' relabel='yes'>
+    <label>system_u:system_r:svirt_t:s0:c215,c257</label>
+    <imagelabel>system_u:object_r:svirt_image_t:s0:c215,c257</imagelabel>
+  </seclabel>
+</domain>
+
+
+
diff --git a/tests/devices/data/vm_restore_spice_before.xml 
b/tests/devices/data/vm_restore_spice_before.xml
new file mode 100644
index 0000000..0b8ccec
--- /dev/null
+++ b/tests/devices/data/vm_restore_spice_before.xml
@@ -0,0 +1,145 @@
+<domain type='kvm' id='6'>
+  <name>ftest1</name>
+  <uuid>42e30017-b29c-4646-aa75-e42c4d0fe659</uuid>
+  <memory unit='KiB'>4194304</memory>
+  <currentMemory unit='KiB'>4194304</currentMemory>
+  <vcpu placement='static' current='2'>16</vcpu>
+  <cputune>
+    <shares>1020</shares>
+  </cputune>
+  <numatune>
+    <memory mode='interleave' nodeset='0'/>
+  </numatune>
+  <resource>
+    <partition>/machine</partition>
+  </resource>
+    <sysinfo type='smbios'>
+      <system>
+        <entry name='manufacturer'>Red Hat</entry>
+        <entry name='product'>RHEV Hypervisor</entry>
+        <entry name='version'>7.2-1.el7</entry>
+        <entry name='serial'>4C4C4544-0059-4410-8043-B6C04F4E5A31</entry>
+        <entry name='uuid'>42e30017-b29c-4646-aa75-e42c4d0fe659</entry>
+      </system>
+    </sysinfo>
+  <os>
+    <type arch='x86_64' machine='pc-i440fx-rhel7.1.0'>hvm</type>
+    <smbios mode='sysinfo'/>
+  </os>
+  <features>
+    <acpi/>
+  </features>
+  <cpu mode='custom' match='exact'>
+    <model fallback='allow'>SandyBridge</model>
+    <topology sockets='16' cores='1' threads='1'/>
+    <numa>
+      <cell id='0' cpus='0,1,2,3,4,5,6,7' memory='4194304'/>
+    </numa>
+  </cpu>
+  <clock offset='variable' adjustment='0' basis='utc'>
+    <timer name='rtc' tickpolicy='catchup'/>
+    <timer name='pit' tickpolicy='delay'/>
+    <timer name='hpet' present='no'/>
+  </clock>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>destroy</on_crash>
+  <devices>
+    <emulator>/usr/libexec/qemu-kvm</emulator>
+    <disk type='file' device='cdrom'>
+      <driver name='qemu' type='raw'/>
+      <source 
file='/rhev/data-center/mnt/192.168.1.20:_srv_virtstore_nfs_generic_iso/b0390d9e-2f1d-43c0-a56c-a20af032e934/images/11111111-1111-1111-1111-111111111111/Fedora-Live-Workstation-x86_64-21-5.iso'
 startupPolicy='optional'>
+        <seclabel model='selinux' labelskip='yes'/>
+      </source>
+      <backingStore/>
+      <target dev='hdc' bus='ide'/>
+      <readonly/>
+      <serial></serial>
+      <boot order='1'/>
+      <alias name='ide0-1-0'/>
+      <address type='drive' controller='0' bus='1' target='0' unit='0'/>
+    </disk>
+    <disk type='file' device='disk' snapshot='no'>
+      <driver name='qemu' type='raw' cache='none' error_policy='stop' 
io='threads'/>
+      <source 
file='/rhev/data-center/00000001-0001-0001-0001-000000000178/f856ad64-22a3-4460-bdd7-5846573b3c01/images/c088d8c8-2599-46c9-aeb8-2e33671a4237/b267133d-fe7b-4d7d-b9ea-b0fed7db4490'>
+        <seclabel model='selinux' labelskip='yes'/>
+      </source>
+      <backingStore/>
+      <target dev='vda' bus='virtio'/>
+      <serial>c088d8c8-2599-46c9-aeb8-2e33671a4237</serial>
+      <boot order='2'/>
+      <alias name='virtio-disk0'/>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x05' 
function='0x0'/>
+    </disk>
+    <controller type='usb' index='0'>
+      <alias name='usb0'/>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' 
function='0x2'/>
+    </controller>
+    <controller type='pci' index='0' model='pci-root'>
+      <alias name='pci.0'/>
+    </controller>
+    <controller type='ide' index='0'>
+      <alias name='ide0'/>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' 
function='0x1'/>
+    </controller>
+    <controller type='virtio-serial' index='0'>
+      <alias name='virtio-serial0'/>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x04' 
function='0x0'/>
+    </controller>
+    <interface type='bridge'>
+      <mac address='00:1a:4a:16:01:51'/>
+      <source bridge='ovirtmgmt'/>
+      <bandwidth>
+      </bandwidth>
+      <target dev='vnet0'/>
+      <model type='virtio'/>
+      <filterref filter='vdsm-no-mac-spoofing'/>
+      <link state='up'/>
+      <boot order='3'/>
+      <alias name='net0'/>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x03' 
function='0x0'/>
+    </interface>
+    <console type='unix'>
+      <source mode='bind' 
path='/var/run/ovirt-vmconsole-console/42e30017-b29c-4646-aa75-e42c4d0fe659.sock'/>
+      <target type='virtio' port='0'/>
+      <alias name='console0'/>
+    </console>
+    <channel type='unix'>
+      <source mode='bind' 
path='/var/lib/libvirt/qemu/channels/42e30017-b29c-4646-aa75-e42c4d0fe659.com.redhat.rhevm.vdsm'/>
+      <target type='virtio' name='com.redhat.rhevm.vdsm' state='disconnected'/>
+      <alias name='channel0'/>
+      <address type='virtio-serial' controller='0' bus='0' port='1'/>
+    </channel>
+    <channel type='unix'>
+      <source mode='bind' 
path='/var/lib/libvirt/qemu/channels/42e30017-b29c-4646-aa75-e42c4d0fe659.org.qemu.guest_agent.0'/>
+      <target type='virtio' name='org.qemu.guest_agent.0' state='connected'/>
+      <alias name='channel1'/>
+      <address type='virtio-serial' controller='0' bus='0' port='2'/>
+    </channel>
+    <channel type='spicevmc'>
+      <target type='virtio' name='com.redhat.spice.0' state='connected'/>
+      <alias name='channel2'/>
+      <address type='virtio-serial' controller='0' bus='0' port='3'/>
+    </channel>
+    <input type='mouse' bus='ps2'/>
+    <input type='keyboard' bus='ps2'/>
+    <graphics type='spice' port='5900' tlsPort='5901' autoport='yes' 
listen='0' passwdValidTo='1970-01-01T00:00:01'>
+      <listen type='address' address='0'/>
+    </graphics>
+    <video>
+      <model type='qxl' ram='65536' vram='32768' vgamem='16384' heads='1'/>
+      <alias name='video0'/>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x02' 
function='0x0'/>
+    </video>
+    <memballoon model='none'>
+      <alias name='balloon0'/>
+    </memballoon>
+  </devices>
+  <seclabel type='dynamic' model='selinux' relabel='yes'>
+    <label>system_u:system_r:svirt_t:s0:c215,c257</label>
+    <imagelabel>system_u:object_r:svirt_image_t:s0:c215,c257</imagelabel>
+  </seclabel>
+</domain>
+
+
+
diff --git a/tests/vmTests.py b/tests/vmTests.py
index 7dd7ac9..fdcf220 100644
--- a/tests/vmTests.py
+++ b/tests/vmTests.py
@@ -21,6 +21,7 @@
 
 from itertools import product
 import logging
+import os.path
 import re
 import threading
 import time
@@ -950,6 +951,29 @@
                 testvm._guestSocketFile,
                 testvm._makeChannelPath(vm._VMCHANNEL_DEVICE_NAME))
 
+    def test_spice_restore_set_passwd(self):
+        # stolen from VDSM logs
+        conf = {
+            'tlsPort': u'5901',
+            u'specParams': {
+                u'fileTransferEnable': u'true',
+                u'copyPasteEnable': u'true',
+                'displayIp': '0'
+            },
+            'deviceType': u'graphics',
+            u'deviceId': u'dbb9db4e-1a30-45b7-b76d-608afc797be9',
+            u'device': u'spice',
+            u'type': u'graphics',
+            'port': u'5900'
+        }
+
+        with fake.VM(devices=[conf], create_device_objects=True)as testvm:
+            out_dom_xml = testvm._correctSpiceConfiguration(
+                _load_xml('vm_restore_spice_before.xml'))
+
+        self.assertXMLEqual(out_dom_xml,
+                            _load_xml('vm_restore_spice_after.xml'))
+
 
 @expandPermutations
 class TestVmOperations(TestCaseBase):
@@ -1639,3 +1663,11 @@
     err = libvirt.libvirtError(defmsg=message)
     err.err = [code]
     raise err
+
+
+def _load_xml(name):
+    test_path = os.path.realpath(__file__)
+    data_path = os.path.join(os.path.split(test_path)[0], 'devices', 'data')
+
+    with open(os.path.join(data_path, name), 'r') as f:
+        return f.read()
diff --git a/vdsm/virt/vm.py b/vdsm/virt/vm.py
index 8fb2289..a4b6e88 100644
--- a/vdsm/virt/vm.py
+++ b/vdsm/virt/vm.py
@@ -1891,15 +1891,13 @@
             srcDomXML = self.conf.pop('_srcDomXML')
             if fromSnapshot:
                 srcDomXML = self._correctDiskVolumes(srcDomXML)
+            srcDomXML = self._correctSpiceConfiguration(srcDomXML)
             hooks.before_vm_dehibernate(srcDomXML, self.conf,
                                         {'FROM_SNAPSHOT': str(fromSnapshot)})
 
             fname = self.cif.prepareVolumePath(self.conf['restoreState'])
             try:
-                if fromSnapshot:
-                    self._connection.restoreFlags(fname, srcDomXML, 0)
-                else:
-                    self._connection.restore(fname)
+                self._connection.restoreFlags(fname, srcDomXML, 0)
             finally:
                 self.cif.teardownVolumePath(self.conf['restoreState'])
 
@@ -1957,6 +1955,15 @@
 
         return parsedSrcDomXML.toxml()
 
+    def _correctSpiceConfiguration(self, srcDomXML):
+        """
+        TODO: documentation
+        """
+        outDomXML = srcDomXML
+        for dev in self._devices[hwclass.GRAPHICS]:
+            outDomXML = dev.updateXML(outDomXML)
+        return outDomXML
+
     def _changeDisk(self, diskDeviceXmlElement):
         diskType = diskDeviceXmlElement.getAttribute('type')
 
diff --git a/vdsm/virt/vmdevices/graphics.py b/vdsm/virt/vmdevices/graphics.py
index 03d9a20..c7f5c61 100644
--- a/vdsm/virt/vmdevices/graphics.py
+++ b/vdsm/virt/vmdevices/graphics.py
@@ -19,6 +19,7 @@
 #
 
 import logging
+import xml.etree.ElementTree as etree
 
 import libvirt
 
@@ -102,9 +103,7 @@
         if self.device == 'spice':
             graphicsAttrs['tlsPort'] = self.tlsPort
 
-        if not utils.tobool(self.specParams.get('disableTicketing', False)):
-            graphicsAttrs['passwd'] = '*****'
-            graphicsAttrs['passwdValidTo'] = '1970-01-01T00:00:01'
+        self._setPasswd(graphicsAttrs)
 
         if 'keyMap' in self.specParams:
             graphicsAttrs['keymap'] = self.specParams['keyMap']
@@ -134,6 +133,23 @@
 
         return graphics
 
+    def _setPasswd(self, attrs):
+        if not utils.tobool(self.specParams.get('disableTicketing', False)):
+            attrs['passwd'] = '*****'
+            attrs['passwdValidTo'] = '1970-01-01T00:00:01'
+
+    def updateXML(self, domXML):
+        if self.device == 'spice':
+            domObj = etree.fromstring(domXML)
+            devices = domObj.findall('devices')
+
+            for graphdev in devices[0].findall('graphics'):
+                if graphdev.attrib['type'] == 'spice':
+                    self._setPasswd(graphdev.attrib)
+
+            return etree.tostring(domObj)
+        return domXML
+
 
 def isSupportedDisplayType(vmParams):
     display = vmParams.get('display')


-- 
To view, visit https://gerrit.ovirt.org/44842
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ic9f11e2d87cc715c77d305c005c1cd7f502d506d
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <[email protected]>
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to