Francesco Romani 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:
...
Bug-Url: https://bugzilla.redhat.com/1154389
Change-Id: I5349ec51c7accf3b417b3bc9489c7eed5bfd8733
Signed-off-by: Nir Soffer <[email protected]>
Reviewed-on: https://gerrit.ovirt.org/43225
Continuous-Integration: Jenkins CI
Reviewed-by: Vinzenz Feenstra <[email protected]>
Reviewed-by: Francesco Romani <[email protected]>
Reviewed-on: https://gerrit.ovirt.org/45377
Tested-by: Francesco Romani <[email protected]>
Reviewed-by: Martin Polednik <[email protected]>
---
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: Looks good to me, but someone else must approve
Jenkins CI: Passed CI tests
Francesco Romani: Verified; Looks good to me, approved
Martin Polednik: Looks good to me, but someone else must approve
--
To view, visit https://gerrit.ovirt.org/45377
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I5349ec51c7accf3b417b3bc9489c7eed5bfd8733
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.6
Gerrit-Owner: Francesco Romani <[email protected]>
Gerrit-Reviewer: Francesco Romani <[email protected]>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik <[email protected]>
Gerrit-Reviewer: Nir Soffer <[email protected]>
Gerrit-Reviewer: [email protected]
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
