Change in vdsm[master]: json-rpc: Protocol detection
by smizrahi@redhat.com
Saggi Mizrahi has posted comments on this change.
Change subject: json-rpc: Protocol detection
......................................................................
Patch Set 33: Code-Review+2
--
To view, visit http://gerrit.ovirt.org/26300
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Id739a40e2b37dcc175137ec91cd5ec166ad24a75
Gerrit-PatchSet: 33
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: Barak Azulay <bazulay(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Liron Ar <laravot(a)redhat.com>
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(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: Vinzenz Feenstra <vfeenstr(a)redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: Yeela Kaplan <ykaplan(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
9 years, 11 months
Change in vdsm[master]: json-rpc: Protocol detection
by smizrahi@redhat.com
Saggi Mizrahi has posted comments on this change.
Change subject: json-rpc: Protocol detection
......................................................................
Patch Set 32: Code-Review-1
(4 comments)
http://gerrit.ovirt.org/#/c/26300/32/lib/vdsm/utils.py
File lib/vdsm/utils.py:
Line 229: self.end_headers()
Line 230: self.wfile.write(response)
Line 231:
Line 232:
Line 233: class ConnectedTCPServer(SocketServer.TCPServer, object):
Why are you also inheriting from object?
Line 234:
Line 235: def __init__(self, RequestHandlerClass):
Line 236: super(ConnectedTCPServer, self).__init__(None, RequestHandlerClass,
Line 237: False)
http://gerrit.ovirt.org/#/c/26300/32/lib/yajsonrpc/__init__.py
File lib/yajsonrpc/__init__.py:
Line 488: res = True if res is None else res
Line 489:
Line 490: ctx.requestDone(JsonRpcResponse(res, None, req.id))
Line 491:
Line 492: @traceback(on=log.name)
I don't know who added that traceback function to the code but I don't like it. Please just use plain old try\except.
This kind of code makes things needlessly complex and it makes the log errors confusingly similar. Whenever you log an error you need to give context in addition to the traceback.
Line 493: def serve_requests(self):
Line 494: while True:
Line 495: self.log.debug("Waiting for request")
Line 496: obj = self._workQueue.get()
http://gerrit.ovirt.org/#/c/26300/32/vdsm/protocolDetector.py
File vdsm/protocolDetector.py:
Line 136:
Line 137: def wakeup(self):
Line 138: try:
Line 139: os.write(self._write_fd, '1')
Line 140: except OSError as e:
You would never get to the second condition since if e.errno == errno.EINTR it is necessarily not in (errno.EAGAIN, errno.EWOULDBLOCK). Switch the order of conditions.
Line 141: if e.errno not in (errno.EAGAIN, errno.EWOULDBLOCK):
Line 142: raise
Line 143: elif e.errno == errno.EINTR:
Line 144: self.wakeup()
Line 174: try:
Line 175: data = client_socket.recv(self._required_size, socket.MSG_PEEK)
Line 176: except socket.error as e:
Line 177: if e.errno not in (errno.EAGAIN, errno.EWOULDBLOCK):
Line 178: self.log.warning("Not able to read data")
s/Not Able/Unable/
Line 179: self._remove_connection(client_socket)
Line 180: client_socket.close()
Line 181: return
Line 182:
--
To view, visit http://gerrit.ovirt.org/26300
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Id739a40e2b37dcc175137ec91cd5ec166ad24a75
Gerrit-PatchSet: 32
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: Barak Azulay <bazulay(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Liron Ar <laravot(a)redhat.com>
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(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: Vinzenz Feenstra <vfeenstr(a)redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: Yeela Kaplan <ykaplan(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
9 years, 11 months
Re: [oVirt Jenkins] vdsm_master_unit_tests - Build # 3043 - Failure!
by Dan Kenigsberg
Francesco, Yaniv,
Have you found the time to dig into the coredump
of these Segmentation faults?
(It is unrelated to Liron's new patches)
On Fri, May 30, 2014 at 01:14:03PM +0100, Jenkins ci oVirt Server wrote:
> Project: http://jenkins.ovirt.org/job/vdsm_master_unit_tests/
> Build: http://jenkins.ovirt.org/job/vdsm_master_unit_tests/3043/
> Build Number: 3043
> Build Status: Failure
> Triggered By: Started by an SCM change
>
> -------------------------------------
> Changes Since Last Success:
> -------------------------------------
> Changes for Build #3043
> [Federico Simoncelli] hsm: unify vm ovf management lock
>
> [Liron Aravot] core: imageSharing - export logic to functions
>
> [Liron Aravot] core: removal of unneeded callback passing
>
> [Liron Aravot] core: generify "streamDownloadImage" related methods
>
> [Liron Aravot] core: BindingXMLRPC - exporting logic out from do_PUT.
>
> [Liron Aravot] core: introducing uploadImageToStream
>
>
>
>
> -----------------
> Failed Tests:
> -----------------
> 1 tests failed.
> FAILED: nosetests.xml.<init>
>
> Error Message:
>
>
> Stack Trace:
> Test report file /home/jenkins/workspace/vdsm_master_unit_tests/tests/nosetests.xml was length 0
>
>
>
9 years, 11 months
Change in vdsm[master]: drop ominous log for libvirt errors
by Dan Kenigsberg
Dan Kenigsberg has uploaded a new change for review.
Change subject: drop ominous log for libvirt errors
......................................................................
drop ominous log for libvirt errors
Patch http://gerrit.ovirt.org/13990 has introduced a log line whenever a
libvirt exception flows through our libvirtconnection.
E.g. Unknown libvirterror: ecode: 9 edom: 19 level: 2 message: operation
failed: network 'vdsm-ovirtmgmt' already exists...
The log line appears out of context and is repeated once its traceback
is properly caught.
Change-Id: Icd2a53ffee7fb78cb1c8d171093e93e233ed5ad4
Signed-off-by: Dan Kenigsberg <danken(a)redhat.com>
---
M lib/vdsm/libvirtconnection.py
1 file changed, 0 insertions(+), 4 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/24/17324/1
diff --git a/lib/vdsm/libvirtconnection.py b/lib/vdsm/libvirtconnection.py
index cdba57c..9891691 100644
--- a/lib/vdsm/libvirtconnection.py
+++ b/lib/vdsm/libvirtconnection.py
@@ -95,10 +95,6 @@
if killOnFailure:
log.error('taking calling process down.')
os.kill(os.getpid(), signal.SIGTERM)
- else:
- log.debug('Unknown libvirterror: ecode: %d edom: %d '
- 'level: %d message: %s', ecode, edom,
- e.get_error_level(), e.get_error_message())
raise
wrapper.__name__ = f.__name__
wrapper.__doc__ = f.__doc__
--
To view, visit http://gerrit.ovirt.org/17324
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Icd2a53ffee7fb78cb1c8d171093e93e233ed5ad4
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg <danken(a)redhat.com>
9 years, 11 months
Change in vdsm[master]: IGNORE: testing gerrit triggered jobs
by dcaroest@redhat.com
David Caro has uploaded a new change for review.
Change subject: IGNORE: testing gerrit triggered jobs
......................................................................
IGNORE: testing gerrit triggered jobs
Change-Id: Ic8946fd09d50e68f5111860a90572c12d14c7326
Signed-off-by: David Caro <dcaroest(a)redhat.com>
---
M vdsm/sos/vdsm.py.in
1 file changed, 5 insertions(+), 3 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/70/26570/1
diff --git a/vdsm/sos/vdsm.py.in b/vdsm/sos/vdsm.py.in
index d90940f..b94bd6a 100644
--- a/vdsm/sos/vdsm.py.in
+++ b/vdsm/sos/vdsm.py.in
@@ -119,9 +119,11 @@
self.collectExtOutput(vdsclient + "getDeviceList")
self.collectExtOutput(vdsclient + "getAllTasksInfo")
self.collectExtOutput(vdsclient + "getAllTasksStatuses")
- p = subprocess.Popen(vdsclient + "getConnectedStoragePoolsList",
- shell=True, stdout=subprocess.PIPE,
- stderr=subprocess.PIPE)
+ p = subprocess.Popen(
+ vdsclient + "getConnectedStoragePoolsList",
+ shell=True,
+ stdout=subprocess.PIPE,
+ stderr=subprocess.PIPE)
out, err = p.communicate()
for line in out.splitlines()[1:-1]:
pool = line.strip()
--
To view, visit http://gerrit.ovirt.org/26570
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ic8946fd09d50e68f5111860a90572c12d14c7326
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: David Caro <dcaroest(a)redhat.com>
9 years, 11 months
Change in vdsm[master]: mutipath: Remove unneeded and dangerous -r parameter
by Nir Soffer
Nir Soffer has uploaded a new change for review.
Change subject: mutipath: Remove unneeded and dangerous -r parameter
......................................................................
mutipath: Remove unneeded and dangerous -r parameter
Since commit dbf2089488 (Jul 9 2013) multipath call was change to use
the -r flag, forcing a reload of the device map. This was tested to fix
a case where new lun is created on the storage server, while a host was
connected, and the new device is not available when issuing the
getDeviceList command. According to a comment on gerrit, the change was
tested for ISCSI and FC storage types, but there is no documentation of
the testing procedure. The related bug was verified, but has no
information about how it was verified.
We have two related bugs:
- Bug 1078879 tell us that invoking multipath with the -r flag sometimes
triggers a segfault in the multipathd daemon. In the bug, multipath
developer suggests that as long as multipathd daemon is running,
there is no need to invoke multipath to detect new devices, and
"multipath -r really isn't useful for much of anything".
- Bug 1071654 tell us that devices rescanning is broken on FC storage
domains (although the -r flag is used). I reproduced this bug using
storage QE FC server.
This patch removes the -r flag. To be on the safe side, I left the
multipath call as it was since the first multipath commit in 2009. We
will work with kernel and multipath developers further on removing this
call if it is indeed unneeded.
Bug-Url: https://bugzilla.redhat.com/1078879
Relates-to: https://bugzilla.redhat.com/1071654
Relates-to: http://gerrit.ovirt.org/17263
Change-Id: I880ab5343df3e0030638901e188320b20570747d
Signed-off-by: Nir Soffer <nsoffer(a)redhat.com>
---
M vdsm/storage/multipath.py
1 file changed, 1 insertion(+), 2 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/42/27242/1
diff --git a/vdsm/storage/multipath.py b/vdsm/storage/multipath.py
index 69a758e..ba98866 100644
--- a/vdsm/storage/multipath.py
+++ b/vdsm/storage/multipath.py
@@ -107,8 +107,7 @@
supervdsm.getProxy().hbaRescan()
# Now let multipath daemon pick up new devices
- cmd = [constants.EXT_MULTIPATH, "-r"]
- misc.execCmd(cmd, sudo=True)
+ misc.execCmd([constants.EXT_MULTIPATH], sudo=True)
def isEnabled():
--
To view, visit http://gerrit.ovirt.org/27242
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I880ab5343df3e0030638901e188320b20570747d
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]: sp: ensure that master domain is active
by Federico Simoncelli
Federico Simoncelli has uploaded a new change for review.
Change subject: sp: ensure that master domain is active
......................................................................
sp: ensure that master domain is active
Change-Id: I4e8fe07ebf8e59e9b028678fecd0a24578348b6c
Signed-off-by: Federico Simoncelli <fsimonce(a)redhat.com>
---
M vdsm/storage/sp.py
1 file changed, 5 insertions(+), 2 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/31/28331/1
diff --git a/vdsm/storage/sp.py b/vdsm/storage/sp.py
index 482bdca..3f983b6 100644
--- a/vdsm/storage/sp.py
+++ b/vdsm/storage/sp.py
@@ -1172,9 +1172,12 @@
domUUIDs = self.getDomains(activeOnly=True).keys()
# msdUUID should be present and active in getDomains result.
- # TODO: Consider remove if clause.
- if msdUUID in domUUIDs:
+ try:
domUUIDs.remove(msdUUID)
+ except ValueError:
+ self.log.error('master storage domain %s not found in the pool '
+ 'domains or not active', msdUUID)
+ raise se.StoragePoolWrongMaster(self.spUUID, msdUUID)
# TODO: Consider to remove this whole block. UGLY!
# We want to avoid looking up (vgs) of unknown block domains.
--
To view, visit http://gerrit.ovirt.org/28331
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I4e8fe07ebf8e59e9b028678fecd0a24578348b6c
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli <fsimonce(a)redhat.com>
9 years, 11 months
Change in vdsm[ovirt-3.4]: vm: fix odd migration progress reporting
by fromani@redhat.com
Francesco Romani has uploaded a new change for review.
Change subject: vm: fix odd migration progress reporting
......................................................................
vm: fix odd migration progress reporting
migration progress should not start from 100,
otherwise the user will see smething like:
progress: 100 -> 0% ... X% ... 100%
A real-word scenario that shows the need for this patch is
the following:
If during the the initialization of migration we experience
some delay, for example DNS resolve issues, libvirt returns 0
for all the progress values, and that incorrectly triggered
the old shortcut, leading to an incorrect initial progress
value of 100.
Change-Id: Ib2bfefb3ffc7d21f6a554f00eac6934fa89ab669
Bug-Url: https://bugzilla.redhat.com/1104670
Signed-off-by: Francesco Romani <fromani(a)redhat.com>
---
M vdsm/vm.py
1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/62/28362/1
diff --git a/vdsm/vm.py b/vdsm/vm.py
index 69f7cb4..a21d08b 100644
--- a/vdsm/vm.py
+++ b/vdsm/vm.py
@@ -765,7 +765,7 @@
def run(self):
def calculateProgress(remaining, total):
- if remaining == 0:
+ if remaining == 0 and total:
return 100
progress = 100 - 100 * remaining / total if total else 0
return progress if (progress < 100) else 99
--
To view, visit http://gerrit.ovirt.org/28362
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib2bfefb3ffc7d21f6a554f00eac6934fa89ab669
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.4
Gerrit-Owner: Francesco Romani <fromani(a)redhat.com>
9 years, 11 months