Hello Dan Kenigsberg,
I'd like you to do a code review. Please visit
https://gerrit.ovirt.org/45378
to review the following change.
Change subject: virt: introduce virdomain module
......................................................................
virt: introduce virdomain module
Add virdomain module to hold the
libvirt Domain wrapper that virt code needs,
to avoid ugly circular dependencies between
periodic.py, sampling.py and vm.py, and last
but not least, to shed some load from vm.py
This patch does the code move, but it is peppered
with trivial rename and trivial fix (inherit from object).
Bug-Url: https://bugzilla.redhat.com/1154389
Backport-To: 3.6
Change-Id: I527ca3aa5733a16b2c81c9fa433125bc189c64ca
Signed-off-by: Francesco Romani <[email protected]>
Reviewed-on: https://gerrit.ovirt.org/44812
Continuous-Integration: Jenkins CI
Reviewed-by: Dan Kenigsberg <[email protected]>
---
M debian/vdsm.install
M tests/vmTests.py
M vdsm.spec.in
M vdsm/virt/Makefile.am
M vdsm/virt/sampling.py
A vdsm/virt/virdomain.py
M vdsm/virt/vm.py
7 files changed, 98 insertions(+), 72 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/78/45378/1
diff --git a/debian/vdsm.install b/debian/vdsm.install
index a26c790..17ec403 100644
--- a/debian/vdsm.install
+++ b/debian/vdsm.install
@@ -148,6 +148,7 @@
./usr/share/vdsm/virt/periodic.py
./usr/share/vdsm/virt/sampling.py
./usr/share/vdsm/virt/secret.py
+./usr/share/vdsm/virt/virdomain.py
./usr/share/vdsm/virt/vm.py
./usr/share/vdsm/virt/vmchannels.py
./usr/share/vdsm/virt/vmexitreason.py
diff --git a/tests/vmTests.py b/tests/vmTests.py
index 7dd7ac9..a6b631c 100644
--- a/tests/vmTests.py
+++ b/tests/vmTests.py
@@ -34,6 +34,7 @@
from virt import vmexitreason
from virt.vmdevices import hwclass
from virt.vmtune import io_tune_merge, io_tune_dom_to_values, io_tune_to_dom
+from virt import virdomain
from virt import vmxml
from virt import vmstats
from virt import vmstatus
@@ -1189,7 +1190,7 @@
</devices>''' % device
def _fail(*args):
- raise vm.TimeoutError(defmsg=message)
+ raise virdomain.TimeoutError(defmsg=message)
domain = fake.Domain(domXml)
domain.updateDeviceFlags = _fail
diff --git a/vdsm.spec.in b/vdsm.spec.in
index d17338d..2e037a0 100644
--- a/vdsm.spec.in
+++ b/vdsm.spec.in
@@ -810,6 +810,7 @@
%{_datadir}/%{vdsm_name}/virt/periodic.py*
%{_datadir}/%{vdsm_name}/virt/sampling.py*
%{_datadir}/%{vdsm_name}/virt/secret.py*
+%{_datadir}/%{vdsm_name}/virt/virdomain.py*
%{_datadir}/%{vdsm_name}/virt/vmchannels.py*
%{_datadir}/%{vdsm_name}/virt/vmstats.py*
%{_datadir}/%{vdsm_name}/virt/vmstatus.py*
diff --git a/vdsm/virt/Makefile.am b/vdsm/virt/Makefile.am
index 745e7dc..5a0d062 100644
--- a/vdsm/virt/Makefile.am
+++ b/vdsm/virt/Makefile.am
@@ -31,6 +31,7 @@
periodic.py \
sampling.py \
secret.py \
+ virdomain.py \
vm.py \
vmchannels.py \
vmexitreason.py \
diff --git a/vdsm/virt/sampling.py b/vdsm/virt/sampling.py
index 90eeec0..389d27e 100644
--- a/vdsm/virt/sampling.py
+++ b/vdsm/virt/sampling.py
@@ -37,6 +37,7 @@
from vdsm.config import config
import v2v
+from . import virdomain
from .utils import ExpiringCache
import caps
@@ -583,7 +584,6 @@
self._stopEvent.set()
def run(self):
- import vm
try:
# wait a bit before starting to sample
time.sleep(self._sampleInterval)
@@ -598,7 +598,7 @@
diff = sample.connlog_diff(second_last)
if diff:
self._CONNLOG.debug('%s', diff)
- except vm.TimeoutError:
+ except virdomain.TimeoutError:
self._log.exception("Timeout while sampling stats")
self._stopEvent.wait(self._sampleInterval)
except:
diff --git a/vdsm/virt/virdomain.py b/vdsm/virt/virdomain.py
new file mode 100644
index 0000000..f8b3398
--- /dev/null
+++ b/vdsm/virt/virdomain.py
@@ -0,0 +1,78 @@
+#
+# Copyright 2008-2015 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+#
+# Refer to the README and COPYING files for full details of the license
+#
+
+import libvirt
+
+
+class NotConnectedError(Exception):
+ """
+ Raised when trying to talk with a vm that was not started yet or was shut
+ down.
+ """
+
+
+class TimeoutError(libvirt.libvirtError):
+ pass
+
+
+class Disconnected(object):
+
+ def __init__(self, vmid):
+ self.vmid = vmid
+
+ @property
+ def connected(self):
+ return False
+
+ def __getattr__(self, name):
+ raise NotConnectedError("VM %r was not started yet or was shut down"
+ % self.vmid)
+
+
+class Notifying(object):
+ # virDomain wrapper that notifies vm when a method raises an exception with
+ # get_error_code() = VIR_ERR_OPERATION_TIMEOUT
+
+ def __init__(self, dom, tocb):
+ self._dom = dom
+ self._cb = tocb
+
+ @property
+ def connected(self):
+ return True
+
+ def __getattr__(self, name):
+ attr = getattr(self._dom, name)
+ if not callable(attr):
+ return attr
+
+ def f(*args, **kwargs):
+ try:
+ ret = attr(*args, **kwargs)
+ self._cb(False)
+ return ret
+ except libvirt.libvirtError as e:
+ if e.get_error_code() == libvirt.VIR_ERR_OPERATION_TIMEOUT:
+ self._cb(True)
+ toe = TimeoutError(e.get_error_message())
+ toe.err = e.err
+ raise toe
+ raise
+ return f
diff --git a/vdsm/virt/vm.py b/vdsm/virt/vm.py
index 5f4fcf0..a354312 100644
--- a/vdsm/virt/vm.py
+++ b/vdsm/virt/vm.py
@@ -62,6 +62,7 @@
from . import guestagent
from . import migration
from . import sampling
+from . import virdomain
from . import vmdevices
from . import vmexitreason
from . import vmstats
@@ -138,13 +139,6 @@
pass
-class NotConnectedError(Exception):
- """
- Raised when trying to talk with a vm that was not started yet or was shut
- down.
- """
-
-
VALID_STATES = (vmstatus.DOWN, vmstatus.MIGRATION_DESTINATION,
vmstatus.MIGRATION_SOURCE, vmstatus.PAUSED,
vmstatus.POWERING_DOWN, vmstatus.REBOOT_IN_PROGRESS,
@@ -189,56 +183,6 @@
VolumeSize = namedtuple("VolumeSize",
["apparentsize", "truesize"])
-
-
-class TimeoutError(libvirt.libvirtError):
- pass
-
-
-class DisconnectedVirDomain(object):
-
- def __init__(self, vmid):
- self.vmid = vmid
-
- @property
- def connected(self):
- return False
-
- def __getattr__(self, name):
- raise NotConnectedError("VM %r was not started yet or was shut down"
- % self.vmid)
-
-
-class NotifyingVirDomain:
- # virDomain wrapper that notifies vm when a method raises an exception with
- # get_error_code() = VIR_ERR_OPERATION_TIMEOUT
-
- def __init__(self, dom, tocb):
- self._dom = dom
- self._cb = tocb
-
- @property
- def connected(self):
- return True
-
- def __getattr__(self, name):
- attr = getattr(self._dom, name)
- if not callable(attr):
- return attr
-
- def f(*args, **kwargs):
- try:
- ret = attr(*args, **kwargs)
- self._cb(False)
- return ret
- except libvirt.libvirtError as e:
- if e.get_error_code() == libvirt.VIR_ERR_OPERATION_TIMEOUT:
- self._cb(True)
- toe = TimeoutError(e.get_error_message())
- toe.err = e.err
- raise toe
- raise
- return f
class MigrationError(Exception):
@@ -302,7 +246,7 @@
:param recover: Signal if the Vm is recovering;
:type recover: bool
"""
- self._dom = DisconnectedVirDomain(params["vmId"])
+ self._dom = virdomain.Disconnected(params["vmId"])
self.recovering = recover
self.conf = {'pid': '0', '_blockJobs': {}, 'clientIp': ''}
self.conf.update(params)
@@ -830,7 +774,7 @@
def _onQemuDeath(self):
self.log.info('underlying process disconnected')
- self._dom = DisconnectedVirDomain(self.id)
+ self._dom = virdomain.Disconnected(self.id)
# Try release VM resources first, if failed stuck in 'Powering Down'
# state
result = self.releaseVm()
@@ -1590,7 +1534,7 @@
if e.get_error_code() == libvirt.VIR_ERR_OPERATION_INVALID:
return response.error('migCancelErr')
raise
- except NotConnectedError:
+ except virdomain.NotConnectedError:
return response.error('migCancelErr')
finally:
self._guestCpuLock.release()
@@ -1706,7 +1650,7 @@
def _isDomainRunning(self):
try:
status = self._dom.info()
- except NotConnectedError:
+ except virdomain.NotConnectedError:
# Known reasons for this:
# * on migration destination, and migration not yet completed.
# * self._dom may be disconnected asynchronously (_onQemuDeath).
@@ -1881,7 +1825,7 @@
self.log.info(domxml)
if self.recovering:
- self._dom = NotifyingVirDomain(
+ self._dom = virdomain.Notifying(
self._connection.lookupByUUIDString(self.id),
self._timeoutExperienced)
elif 'migrationDest' in self.conf:
@@ -1903,7 +1847,7 @@
finally:
self.cif.teardownVolumePath(self.conf['restoreState'])
- self._dom = NotifyingVirDomain(
+ self._dom = virdomain.Notifying(
self._connection.lookupByUUIDString(self.id),
self._timeoutExperienced)
else:
@@ -1912,7 +1856,7 @@
flags |= libvirt.VIR_DOMAIN_START_PAUSED
self.conf['pauseCode'] = 'NOERR'
del self.conf['launchPaused']
- self._dom = NotifyingVirDomain(
+ self._dom = virdomain.Notifying(
self._connection.createXML(domxml, flags),
self._timeoutExperienced)
hooks.after_vm_start(self._dom.XMLDesc(0), self.conf)
@@ -2709,7 +2653,7 @@
"""
try:
state, details, stateTime = self._dom.controlInfo()
- except NotConnectedError:
+ except virdomain.NotConnectedError:
# this method may be called asynchronously by periodic
# operations. Thus, we must use a try/except block
# to avoid racy checks.
@@ -2778,7 +2722,7 @@
try:
# Would fail if migration isn't successful,
# or restart vdsm if connection to libvirt was lost
- self._dom = NotifyingVirDomain(
+ self._dom = virdomain.Notifying(
self._connection.lookupByUUIDString(self.id),
self._timeoutExperienced)
@@ -3575,7 +3519,7 @@
disconnectAction = params.get('disconnectAction',
ConsoleDisconnectAction.LOCK_SCREEN)
self._consoleDisconnectAction = disconnectAction
- except TimeoutError as tmo:
+ except virdomain.TimeoutError as tmo:
res = response.error('ticketErr', unicode(tmo))
else:
hooks.after_vm_set_ticket(self._domain.xml, self.conf, params)
@@ -4464,7 +4408,7 @@
# _completeIncomingMigration didn't update it yet.
try:
domxml = self._dom.XMLDesc(0)
- except NotConnectedError:
+ except virdomain.NotConnectedError:
pass
else:
hooks.after_vm_pause(domxml, self.conf)
@@ -4480,7 +4424,7 @@
# callback however we do not use it.
try:
domxml = self._dom.XMLDesc(0)
- except NotConnectedError:
+ except virdomain.NotConnectedError:
pass
else:
hooks.after_vm_cont(domxml, self.conf)
--
To view, visit https://gerrit.ovirt.org/45378
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I527ca3aa5733a16b2c81c9fa433125bc189c64ca
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.6
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
