Dan Kenigsberg has submitted this change and it was merged.
Change subject: vm: Improve error handling when Vm._dom is None
......................................................................
vm: Improve error handling when Vm._dom is None
Vm._dom is initialized to None on when creating a vm, and set to None if
the underlying libvirt domain has died. Since Vm._dom is modified by
multiple threads, it is impossible to check for None before using it.
Even if it was possible, we don't want to litter the code with None
checks everywhere.
Some code was using this pattern:
try:
self._dom.doSomething()
except AttributeError:
# Oh, it was None
This code is not communicating well our intent. Worse, it hides
AttributeError in doSomthing()!
Most code never check self._dom before using it. In the rare cases it is
None, we fail with this Traceback:
Thread-460::ERROR::2015-07-04
19:53:21,977::__init__::520::jsonrpc.JsonRpcServer::(_serveRequest)
Internal server error
Traceback (most recent call last):
File "/usr/lib/python2.7/site-packages/yajsonrpc/__init__.py", line 515,
in _serveRequest
res = method(**params)
File "/usr/share/vdsm/rpc/Bridge.py", line 277, in _dynamicMethod
result = fn(*methodArgs)
File "/usr/share/vdsm/API.py", line 727, in freeze
return v.freeze()
File "/usr/share/vdsm/virt/vm.py", line 2882, in freeze
frozen = self._dom.fsFreeze()
AttributeError: 'NoneType' object has no attribute 'fsFreeze'
This traceback is a poor way to say "the vm is not running".
This patch introduces the DisconnectedVirDomain class. This object will
raise NotConnectedError for any attribute access. Vm._dom is initialzied
to DisconnectedVirDomain(vmid) on startup and after underlying libvirt
domain has died.
Code trying to talk with a dead vm will fail now with:
NotConnectedError: Vm '681f6b09-a9c3-4422-a7e2-2f607368718b' is not
running yet or was shut down.
Code handling disconnected state is using now:
try:
self._dom.doSomething()
except NotConnctedError:
...
This communicates our intent, and does not hide any error from the
underlying code.
Code checking for None is using now:
if self._dom.connected:
...
Change-Id: I5349ec51c7accf3b417b3bc9489c7eed5bfd8733
Signed-off-by: Nir Soffer <nsof...@redhat.com>
Reviewed-on: https://gerrit.ovirt.org/43225
Continuous-Integration: Jenkins CI
Reviewed-by: Vinzenz Feenstra <vfeen...@redhat.com>
Reviewed-by: Francesco Romani <from...@redhat.com>
---
M tests/vmTests.py
M tests/vmfakelib.py
M vdsm/virt/sampling.py
M vdsm/virt/vm.py
4 files changed, 47 insertions(+), 20 deletions(-)
Approvals:
Nir Soffer: Verified
Jenkins CI: Passed CI tests
Vinzenz Feenstra: Looks good to me, but someone else must approve
Francesco Romani: Looks good to me, approved
--
To view, visit https://gerrit.ovirt.org/43225
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I5349ec51c7accf3b417b3bc9489c7eed5bfd8733
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsof...@redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com>
Gerrit-Reviewer: Daniel Erez <de...@redhat.com>
Gerrit-Reviewer: Francesco Romani <from...@redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik <mpoled...@redhat.com>
Gerrit-Reviewer: Nir Soffer <nsof...@redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeen...@redhat.com>
Gerrit-Reviewer: automat...@ovirt.org
_______________________________________________
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
