Hello Dan Kenigsberg,
I'd like you to do a code review. Please visit
http://gerrit.ovirt.org/31421
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 <[email protected]>
Reviewed-on: http://gerrit.ovirt.org/31241
Reviewed-by: Dan Kenigsberg <[email protected]>
---
M tests/vmTests.py
M tests/vmTestsData.py
M vdsm/clientIF.py
M vdsm/vm.py
4 files changed, 171 insertions(+), 55 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/21/31421/1
diff --git a/tests/vmTests.py b/tests/vmTests.py
index bd5a904..fc56f1a 100644
--- a/tests/vmTests.py
+++ b/tests/vmTests.py
@@ -29,12 +29,14 @@
from vdsm import constants
from testrunner import VdsmTestCase as TestCaseBase
from testrunner import namedTemporaryDir
+from testrunner import permutations, expandPermutations
import caps
from vdsm import utils
from vdsm import libvirtconnection
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:
@@ -42,7 +44,15 @@
pass
-class FakeDomain:
+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='')
@@ -682,7 +692,8 @@
@contextmanager
-def FakeVM(params=None, devices=None):
+def FakeVM(params=None, devices=None,
+ arch=caps.Architecture.X86_64):
with namedTemporaryDir() as tmpDir:
with MonkeyPatchScope([(constants, 'P_VDSM_RUN', tmpDir + '/'),
(libvirtconnection, 'get',
@@ -690,6 +701,7 @@
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
yield fake
@@ -721,3 +733,40 @@
mock_stats_thread._getBalloonStats(res)
self.assertIn('balloonInfo', res)
self.assertIn('balloon_cur', res['balloonInfo'])
+
+
+@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 b75c9dc..d95fe58 100644
--- a/tests/vmTestsData.py
+++ b/tests/vmTestsData.py
@@ -144,3 +144,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 987b7f3..cc19197 100644
--- a/vdsm/clientIF.py
+++ b/vdsm/clientIF.py
@@ -22,7 +22,6 @@
import time
import threading
import pickle
-from xml.dom import minidom
import uuid
import alignmentScan
@@ -37,7 +36,7 @@
from vdsm import utils
import caps
from vmChannels import Listener
-from vm import Vm
+from vm import Vm, getVDSMDomains
import blkid
import supervdsm
import sampling
@@ -393,9 +392,8 @@
caps.CpuTopology().cores())
vm.MigrationSourceThread.setMaxOutgoingMigrations(mog)
- vdsmVms = self._getVDSMVms()
- #Recover
- for v in vdsmVms:
+ # Recover
+ for v in getVDSMDomains():
vmId = v.UUIDString()
if not self._recoverVm(vmId):
#RH qemu proc without recovery
@@ -436,54 +434,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:
- vms.append(vm)
- return [vm for vm in vms if self.isVDSMVm(vm)]
def _recoverVm(self, vmid):
try:
diff --git a/vdsm/vm.py b/vdsm/vm.py
index 3b947c1..16037fa 100644
--- a/vdsm/vm.py
+++ b/vdsm/vm.py
@@ -92,6 +92,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'],
@@ -2987,6 +3024,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/31421
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.4
Gerrit-Owner: Francesco Romani <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
