Change in vdsm[ovirt-3.5]: vm: more generic recovery check

fromani at redhat.com fromani at redhat.com
Tue Aug 12 14:42:41 UTC 2014


Hello Dan Kenigsberg,

I'd like you to do a code review.  Please visit

    http://gerrit.ovirt.org/31411

to review the following change.

Change subject: vm: more generic recovery check
......................................................................

vm: more generic recovery check

On recovery, VDSM checks each VM listed by libvirt
to see if it should take care of it.
The check is done by using some system properties
added in the smbios domain element, which is lacking
outside x86_64.

As result, recovery finds no VMs on PPC64.

This patch makes VDSM use the vm guest agent channels
to do the recovery check, since they are avaialble
on all plataforms.

Change-Id: Ic0cc57129e9c8e6545f9a947329adf1f9e82648f
Bug-Url: https://bugzilla.redhat.com/1126887
Signed-off-by: Francesco Romani <fromani at redhat.com>
Reviewed-on: http://gerrit.ovirt.org/31241
Reviewed-by: Dan Kenigsberg <danken at redhat.com>
---
M tests/vmTests.py
M tests/vmTestsData.py
M vdsm/clientIF.py
M vdsm/virt/vm.py
4 files changed, 166 insertions(+), 56 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/11/31411/1

diff --git a/tests/vmTests.py b/tests/vmTests.py
index 7ee2eb8..c1284ea 100644
--- a/tests/vmTests.py
+++ b/tests/vmTests.py
@@ -40,6 +40,7 @@
 from monkeypatch import MonkeyPatch, MonkeyPatchScope
 from vmTestsData import CONF_TO_DOMXML_X86_64
 from vmTestsData import CONF_TO_DOMXML_PPC64
+from vmTestsData import CONF_TO_DOMXML_NO_VDSM
 
 
 class ConnectionMock:
@@ -53,10 +54,14 @@
         return []
 
 
-class FakeDomain:
-    def __init__(self, xml=''):
+class FakeDomain(object):
+    def __init__(self, xml='',  vmId=''):
         self._xml = xml
         self.devXml = ''
+        self._vmId = vmId
+
+    def UUIDString(self):
+        return self._vmId
 
     def info(self):
         raise libvirt.libvirtError(defmsg='')
@@ -884,13 +889,15 @@
 
 
 @contextmanager
-def FakeVM(params=None, devices=None, runCpu=False):
+def FakeVM(params=None, devices=None, runCpu=False,
+           arch=caps.Architecture.X86_64):
     with namedTemporaryDir() as tmpDir:
         with MonkeyPatchScope([(constants, 'P_VDSM_RUN', tmpDir + '/'),
                                (libvirtconnection, 'get', ConnectionMock)]):
             vmParams = {'vmId': 'TESTING'}
             vmParams.update({} if params is None else params)
             fake = vm.Vm(None, vmParams)
+            fake.arch = arch
             fake.guestAgent = FakeGuestAgent()
             fake.conf['devices'] = [] if devices is None else devices
             fake._guestCpuRunning = runCpu
@@ -1239,3 +1246,40 @@
                    {'type': 'graphics', 'device': devType}]
         with FakeVM(self.conf, devices) as fake:
             self.assertRaises(ValueError, fake.buildConfDevices)
+
+
+ at expandPermutations
+class TestVmFunctions(TestCaseBase):
+
+    _CONFS = {
+        caps.Architecture.X86_64: CONF_TO_DOMXML_X86_64,
+        caps.Architecture.PPC64: CONF_TO_DOMXML_PPC64,
+        'novdsm': CONF_TO_DOMXML_NO_VDSM}
+
+    def _buildAllDomains(self, arch):
+        for conf, _ in self._CONFS[arch]:
+            with FakeVM(conf, arch=arch) as v:
+                domXml = v._buildCmdLine()
+                yield FakeDomain(domXml, vmId=v.id), domXml
+
+    def _getAllDomains(self, arch):
+        for conf, rawXml in self._CONFS[arch]:
+            domXml = rawXml % conf
+            yield FakeDomain(domXml, vmId=conf['vmId']), domXml
+
+    def _getAllDomainIds(self, arch):
+        return [conf['vmId'] for conf, _ in self._CONFS[arch]]
+
+    @permutations([[caps.Architecture.X86_64], [caps.Architecture.PPC64]])
+    def testGetVDSMDomains(self, arch):
+        with MonkeyPatchScope([(vm, '_listDomains',
+                                lambda: self._buildAllDomains(arch))]):
+            self.assertEqual([v.UUIDString() for v in vm.getVDSMDomains()],
+                             self._getAllDomainIds(arch))
+
+    # VDSM (of course) builds correct config, so we need static examples
+    # of incorrect/not-compliant data
+    def testSkipNotVDSMDomains(self):
+        with MonkeyPatchScope([(vm, '_listDomains',
+                                lambda: self._getAllDomains('novdsm'))]):
+            self.assertFalse(vm.getVDSMDomains())
diff --git a/tests/vmTestsData.py b/tests/vmTestsData.py
index 3cee160..69d2c6b 100644
--- a/tests/vmTestsData.py
+++ b/tests/vmTestsData.py
@@ -143,3 +143,79 @@
                 </qemu:commandline>
             </domain>
 """, )]
+
+
+# valid libvirt domain XML, but not generated by VDSM
+CONF_TO_DOMXML_NO_VDSM = [({
+    'vmId': ''},
+
+    """<domain type="kvm">
+        <name>SuperTiny_C0</name>
+        <uuid>%(vmId)s</uuid>
+        <memory>65536</memory>
+        <currentMemory>65536</currentMemory>
+        <vcpu current="1">160</vcpu>
+        <memtune>
+            <min_guarantee>16384</min_guarantee>
+        </memtune>
+        <devices>
+            <channel type="unix">
+                <target name="org.qemu.guest_agent.0" type="virtio"/>
+                <source mode="bind"
+        path="/var/lib/libvirt/qemu/channels/%(vmId)s.org.qemu.guest_agent.0"/>
+            </channel>
+            <input bus="ps2" type="mouse"/>
+            <memballoon model="none"/>
+            <controller index="0" model="virtio-scsi" type="scsi">
+                <address bus="0x00" domain="0x0000" function="0x0" slot="0x03"
+                    type="pci"/>
+            </controller>
+            <video>
+                <address bus="0x00" domain="0x0000" function="0x0" slot="0x02"
+                    type="pci"/>
+                <model heads="1" type="qxl" vram="32768"/>
+            </video>
+            <graphics autoport="yes" keymap="en-us" passwd="*****"
+        passwdValidTo="1970-01-01T00:00:01"
+        port="-1" tlsPort="-1" type="spice">
+                <listen network="vdsm-ovirtmgmt" type="network"/>
+            </graphics>
+            <disk device="cdrom" snapshot="no" type="file">
+                <address bus="1" controller="0" target="0"
+        type="drive" unit="0"/>
+                <source file="" startupPolicy="optional"/>
+                <target bus="ide" dev="hdc"/>
+                <readonly/>
+                <serial/>
+                <boot order="1"/>
+            </disk>
+            <channel type="spicevmc">
+                <target name="com.redhat.spice.0" type="virtio"/>
+            </channel>
+        </devices>
+        <os>
+            <type arch="x86_64" machine="rhel6.5.0">hvm</type>
+            <smbios mode="sysinfo"/>
+        </os>
+        <sysinfo type="smbios">
+            <system>
+                <entry name="manufacturer">oVirt</entry>
+                <entry name="product">oVirt Node</entry>
+                <entry name="version">6Server-6.5.0.1.el6</entry>
+                <entry name="uuid">%(vmId)s</entry>
+            </system>
+        </sysinfo>
+        <clock adjustment="0" offset="variable">
+            <timer name="rtc" tickpolicy="catchup"/>
+            <timer name="pit" tickpolicy="delay"/>
+            <timer name="hpet" present="no"/>
+        </clock>
+        <features>
+            <acpi/>
+        </features>
+        <cpu match="exact">
+            <model>SandyBridge</model>
+            <topology cores="1" sockets="160" threads="1"/>
+        </cpu>
+    </domain>
+""", )]
diff --git a/vdsm/clientIF.py b/vdsm/clientIF.py
index fc3e573..b6cd405 100644
--- a/vdsm/clientIF.py
+++ b/vdsm/clientIF.py
@@ -22,7 +22,6 @@
 import os.path
 import time
 import threading
-from xml.dom import minidom
 import uuid
 
 import alignmentScan
@@ -45,7 +44,7 @@
 from virt import sampling
 from virt import vm
 from virt import vmstatus
-from virt.vm import Vm
+from virt.vm import Vm, getVDSMDomains
 from virt.vmchannels import Listener
 try:
     import gluster.api as gapi
@@ -420,9 +419,8 @@
                       caps.CpuTopology().cores())
             migration.SourceThread.setMaxOutgoingMigrations(mog)
 
-            vdsmVms = self._getVDSMVms()
             # Recover
-            for v in vdsmVms:
+            for v in getVDSMDomains():
                 vmId = v.UUIDString()
                 if not self._recoverVm(vmId):
                     # RH qemu proc without recovery
@@ -476,55 +474,6 @@
         except:
             self.log.error("Vm's recovery failed", exc_info=True)
             raise
-
-    def isVDSMVm(self, vm):
-        """
-        Return True if vm seems as if it was created by vdsm.
-        """
-        try:
-            vmdom = minidom.parseString(vm.XMLDesc(0))
-            sysinfo = vmdom.getElementsByTagName("sysinfo")[0]
-        except libvirt.libvirtError as e:
-            if e.get_error_code() == libvirt.VIR_ERR_NO_DOMAIN:
-                self.log.error("domId: %s is dead", vm.UUIDString())
-            else:
-                raise
-        except IndexError:
-            pass  # no sysinfo in xml
-        else:
-            systype = sysinfo.getAttribute("type")
-            if systype == "smbios":
-                entries = sysinfo.getElementsByTagName("entry")
-                for entry in entries:
-                    if entry.getAttribute("name") == "product":
-                        prod = entry.firstChild.data
-                        if prod in (caps.OSName.RHEL, caps.OSName.OVIRT,
-                                    caps.OSName.RHEVH, caps.OSName.FEDORA,
-                                    caps.OSName.DEBIAN):
-                            return True
-        return False
-
-    def _getVDSMVms(self):
-        """
-        Return a list of vdsm created VM's.
-        """
-        libvirtCon = libvirtconnection.get()
-        domIds = libvirtCon.listDomainsID()
-        vms = []
-        for domId in domIds:
-            try:
-                vm = libvirtCon.lookupByID(domId)
-            except libvirt.libvirtError as e:
-                if e.get_error_code() == libvirt.VIR_ERR_NO_DOMAIN:
-                    self.log.error("domId: %s is dead", domId, exc_info=True)
-                else:
-                    self.log.error("Can't look for domId: %s, code: %s",
-                                   domId, e.get_error_code(), exc_info=True)
-                    raise
-            else:
-                if self.isVDSMVm(vm):
-                    vms.append(vm)
-        return vms
 
     def _getVDSMVmsFromRecovery(self):
         vms = []
diff --git a/vdsm/virt/vm.py b/vdsm/virt/vm.py
index b33ed0c..4309c8e 100644
--- a/vdsm/virt/vm.py
+++ b/vdsm/virt/vm.py
@@ -111,6 +111,43 @@
     return all(k in drive for k in required)
 
 
+def _has_channel(domXML, name):
+    domObj = _domParseStr(domXML)
+    devices = domObj.getElementsByTagName('devices')
+
+    if len(devices) == 1:
+        for chan in devices[0].getElementsByTagName('channel'):
+            targets = chan.getElementsByTagName('target')
+            if len(targets) == 1:
+                if targets[0].getAttribute('name') == name:
+                    return True
+
+    return False
+
+
+def _listDomains():
+    libvirtCon = libvirtconnection.get()
+    for domId in libvirtCon.listDomainsID():
+        try:
+            vm = libvirtCon.lookupByID(domId)
+            xmlDom = vm.XMLDesc(0)
+        except libvirt.libvirtError as e:
+            if e.get_error_code() == libvirt.VIR_ERR_NO_DOMAIN:
+                logging.exception("domId: %s is dead", domId)
+            else:
+                raise
+        else:
+            yield vm, xmlDom
+
+
+def getVDSMDomains():
+    """
+    Return a list of Domains created by VDSM.
+    """
+    return [vm for vm, xmlDom in _listDomains()
+            if _has_channel(xmlDom, _VMCHANNEL_DEVICE_NAME)]
+
+
 def _filterSnappableDiskDevices(diskDeviceXmlElements):
         return filter(lambda(x): not(x.getAttribute('device')) or
                       x.getAttribute('device') in ['disk', 'lun'],
@@ -3050,6 +3087,10 @@
         if utils.tobool(self.conf.get('vmchannel', 'true')):
             domxml._appendAgentDevice(self._guestSocketFile.decode('utf-8'),
                                       _VMCHANNEL_DEVICE_NAME)
+        else:
+            self.log.warning('detected disabled vmchannel.'
+                             ' This should be done ONLY for debug purposes'
+                             ' and never in production environments')
         if utils.tobool(self.conf.get('qgaEnable', 'true')):
             domxml._appendAgentDevice(
                 self._qemuguestSocketFile.decode('utf-8'),


-- 
To view, visit http://gerrit.ovirt.org/31411
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ic0cc57129e9c8e6545f9a947329adf1f9e82648f
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.5
Gerrit-Owner: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>


More information about the vdsm-patches mailing list