Change in vdsm[master]: vdsm: lastclient info in jsonrpc
by Nir Soffer
Nir Soffer has posted comments on this change.
Change subject: vdsm: lastclient info in jsonrpc
......................................................................
Patch Set 12:
(3 comments)
Nice! - added some suggestions for making it little more clear.
http://gerrit.ovirt.org/#/c/28817/12/lib/yajsonrpc/stompReactor.py
File lib/yajsonrpc/stompReactor.py:
Line 128: self._messageHandler = msgHandler
Line 129: self.check_read()
Line 130:
Line 131: def check_read(self):
Line 132: if isinstance(self.socket, SSLSocket) and self.socket.pending() > 0:
For another patch - do not check instance type. You can to this instead,
if hasattr(self.socket, 'pending') and self.socket.pending() > 0:
This allow you to replace the class with something else the provide the same interface.
Line 133: self._stompConn._dispatcher.handle_read()
Line 134:
Line 135: def send(self, message):
Line 136: self.log.debug("Sending response")
http://gerrit.ovirt.org/#/c/28817/12/tests/bridgeTests.py
File tests/bridgeTests.py:
Line 78: return '127.0.0.1'
Line 79:
Line 80: @property
Line 81: def server_address(self):
Line 82: return '127.0.0.1'
Do you really need this fake? can you use the real object instead?
Line 83:
Line 84:
Line 85: class BridgeTests(TestCaseBase):
Line 86:
http://gerrit.ovirt.org/#/c/28817/12/vdsm/rpc/Bridge.py
File vdsm/rpc/Bridge.py:
Line 59: def register(self, ctx):
Line 60: self._threadLocal.ctx = ctx
Line 61:
Line 62: def unregister(self):
Line 63: self._threadLocal.ctx = None
Not sure we need this - the caller can do instead:
bridge.register(None)
And it will be even more clear if we call it:
bridge.registerContext(ctx)
I like short method names but register is not an expected method of a bridge.
Line 64:
Line 65: def dispatch(self, name, argobj):
Line 66: methodName = name.replace('.', '_')
Line 67: result = None
--
To view, visit http://gerrit.ovirt.org/28817
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I402e15cb05f89a98dab14491d9da5985335e095e
Gerrit-PatchSet: 12
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
9 years, 11 months
Change in vdsm[master]: vdsm: Shuting down protocol detector fails
by Dan Kenigsberg
Dan Kenigsberg has posted comments on this change.
Change subject: vdsm: Shuting down protocol detector fails
......................................................................
Patch Set 1: Code-Review+1
--
To view, visit http://gerrit.ovirt.org/29556
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I82119a61835fe335f2aa5da29fb8d3f2b8ae33fc
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No
9 years, 11 months
Change in vdsm[master]: vdsm: Shuting down protocol detector fails
by Dan Kenigsberg
Dan Kenigsberg has posted comments on this change.
Change subject: vdsm: Shuting down protocol detector fails
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.ovirt.org/#/c/29556/1/vdsm/protocoldetector.py
File vdsm/protocoldetector.py:
Line 166: if e.errno == errno.EPIPE:
Line 167: pass
Line 168: if e.errno == errno.EINTR:
Line 169: self.wakeup()
Line 170: elif e.errno not in (errno.EAGAIN, errno.EWOULDBLOCK):
unrelated question: why do we swallow EAGAIN error? Why not try again?
Line 171: raise
Line 172:
Line 173: def _cleanup_wakeup_pipe(self):
Line 174: try:
--
To view, visit http://gerrit.ovirt.org/29556
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I82119a61835fe335f2aa5da29fb8d3f2b8ae33fc
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: Yes
9 years, 11 months
Change in vdsm[master]: vm: drop redundant hasattr()
by fromani@redhat.com
Francesco Romani has uploaded a new change for review.
Change subject: vm: drop redundant hasattr()
......................................................................
vm: drop redundant hasattr()
I don't see how it is possible for a Vm instance to *not* have the
updateGuestCpuRunning method, therefore I remove the hasattr() check
before to call it.
Change-Id: I23743b7c6f4a8b71a301d02670c348d53f23f124
Signed-off-by: Francesco Romani <fromani(a)redhat.com>
---
M vdsm/virt/vm.py
1 file changed, 2 insertions(+), 4 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/12/29112/1
diff --git a/vdsm/virt/vm.py b/vdsm/virt/vm.py
index 2b28473..d145122 100644
--- a/vdsm/virt/vm.py
+++ b/vdsm/virt/vm.py
@@ -2617,8 +2617,7 @@
self.log.error('cannot cont while %s', self.lastStatus)
return errCode['unexpected']
self._underlyingCont()
- if hasattr(self, 'updateGuestCpuRunning'):
- self.updateGuestCpuRunning()
+ self.updateGuestCpuRunning()
self._lastStatus = afterState
try:
with self._confLock:
@@ -2638,8 +2637,7 @@
with self._confLock:
self.conf['pauseCode'] = pauseCode
self._underlyingPause()
- if hasattr(self, 'updateGuestCpuRunning'):
- self.updateGuestCpuRunning()
+ self.updateGuestCpuRunning()
self._lastStatus = afterState
return {'status': doneCode, 'output': ['']}
finally:
--
To view, visit http://gerrit.ovirt.org/29112
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I23743b7c6f4a8b71a301d02670c348d53f23f124
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani(a)redhat.com>
9 years, 11 months
Change in vdsm[master]: spec: Remove non-existing requirement
by Nir Soffer
Nir Soffer has uploaded a new change for review.
Change subject: spec: Remove non-existing requirement
......................................................................
spec: Remove non-existing requirement
python-ioprocess package is not availabe yet on all supported platforms,
so we cannot require it and use it by default. This patch removes the
requirement and change the default to rfh.
People who want to test this package should install the package manually
and enable it in the vdsm config. Other people can continue to work
normally.
When the package is ready we can safely add the requirement and change
the default.
Change-Id: Ia1a1ac03145010fd0e3fa70201e62d883ce4dd4d
Signed-off-by: Nir Soffer <nsoffer(a)redhat.com>
---
M lib/vdsm/config.py.in
M vdsm.spec.in
2 files changed, 1 insertion(+), 4 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/04/29304/1
diff --git a/lib/vdsm/config.py.in b/lib/vdsm/config.py.in
index ab330be..3bbeea7 100644
--- a/lib/vdsm/config.py.in
+++ b/lib/vdsm/config.py.in
@@ -324,7 +324,7 @@
('use_volume_leases', 'false',
'Whether to use the volume leases or not.'),
- ('oop_impl', 'ioprocess',
+ ('oop_impl', 'rfh',
'Whether to use RFH or ioprocess implementation for oop.'
'Values can be either ioprocess or rfh.'),
]),
diff --git a/vdsm.spec.in b/vdsm.spec.in
index 008fd39..f750566 100644
--- a/vdsm.spec.in
+++ b/vdsm.spec.in
@@ -74,7 +74,6 @@
BuildRequires: python-inotify
BuildRequires: python-pthreading
BuildRequires: python-cpopen >= 1.3
-BuildRequires: python-ioprocess >= 0.5-1
BuildRequires: rpm-python
BuildRequires: libnl
BuildRequires: libselinux-python
@@ -118,7 +117,6 @@
Requires: python-inotify
Requires: python-argparse
Requires: python-cpopen >= 1.3
-Requires: python-ioprocess >= 0.5-1
Requires: python-ethtool >= 0.6-3
Requires: %{name}-python-zombiereaper = %{version}-%{release}
Requires: rpm-python
@@ -332,7 +330,6 @@
Requires: %{name}-python-zombiereaper = %{version}-%{release}
Requires: python-cpopen >= 1.2.3-5
Requires: m2crypto
-Requires: python-ioprocess >= 0.5-1
%description python
Shared libraries between the various VDSM packages.
--
To view, visit http://gerrit.ovirt.org/29304
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ia1a1ac03145010fd0e3fa70201e62d883ce4dd4d
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsoffer(a)redhat.com>
9 years, 11 months
Change in vdsm[master]: tests: make api check support falsey values
by fromani@redhat.com
Francesco Romani has uploaded a new change for review.
Change subject: tests: make api check support falsey values
......................................................................
tests: make api check support falsey values
the current vmApiTests assume truthy values in the replies.
The schema actually doesn't requires that, and falsey values
(e.g. empty lists) are actually valid.
This patch fixes the tests to support them.
Change-Id: I4f974b08589a077dedba3006b29f6bb29aee05ba
Signed-off-by: Francesco Romani <fromani(a)redhat.com>
---
M tests/vmApiTests.py
1 file changed, 2 insertions(+), 1 deletion(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/38/29438/1
diff --git a/tests/vmApiTests.py b/tests/vmApiTests.py
index 260c98a..bc68c6d 100644
--- a/tests/vmApiTests.py
+++ b/tests/vmApiTests.py
@@ -46,7 +46,8 @@
for apiItem, apiType in ref.items():
if apiItem[0] == '*':
# optional, may be absent and it is fine
- self.assertTrue(stats.get(apiItem[1:], True))
+ if apiItem[1:] in stats:
+ self.assertNotEqual(stats[apiItem[1:]], None)
else:
# mandatory
self.assertIn(apiItem, stats)
--
To view, visit http://gerrit.ovirt.org/29438
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I4f974b08589a077dedba3006b29f6bb29aee05ba
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani(a)redhat.com>
9 years, 11 months
Change in vdsm[ovirt-3.5]: ipwrapper: do not report devices that disappear
by asegurap@redhat.com
Hello Dan Kenigsberg,
I'd like you to do a code review. Please visit
http://gerrit.ovirt.org/29546
to review the following change.
Change subject: ipwrapper: do not report devices that disappear
......................................................................
ipwrapper: do not report devices that disappear
There was a race in getLinks that consisted on devices disappearing
between them being reported by netlink and information being
retrieved by the Link class (driver info).
This patch solves it by reporting just those links that suceed.
Change-Id: Ib41336f3ac2f2278db2706f7d4eb3a9b579f66d4
Bug-Url: https://bugzilla.redhat.com/1113948
Signed-off-by: Antoni S. Puimedon <asegurap(a)redhat.com>
Reviewed-on: http://gerrit.ovirt.org/29442
Reviewed-by: Dan Kenigsberg <danken(a)redhat.com>
---
M lib/vdsm/ipwrapper.py
1 file changed, 7 insertions(+), 1 deletion(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/46/29546/1
diff --git a/lib/vdsm/ipwrapper.py b/lib/vdsm/ipwrapper.py
index b1dd83d..cd4b7d4 100644
--- a/lib/vdsm/ipwrapper.py
+++ b/lib/vdsm/ipwrapper.py
@@ -286,7 +286,13 @@
def getLinks():
"""Returns a list of Link objects for each link in the system."""
- return [Link.fromDict(data) for data in netlink.iter_links()]
+ links = []
+ for data in netlink.iter_links():
+ try:
+ links.append(Link.fromDict(data))
+ except IOError: # If a link goes missing we just don't report it
+ continue
+ return links
def getLink(dev):
--
To view, visit http://gerrit.ovirt.org/29546
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib41336f3ac2f2278db2706f7d4eb3a9b579f66d4
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.5
Gerrit-Owner: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
9 years, 11 months