vdsm-patches December 2015

vdsm-patches@lists.fedorahosted.org

91 discussions

Change in vdsm[master]: storage: Do not count garbage volumes as children
by alitke＠redhat.com 13 Aug '16

13 Aug '16

Adam Litke has uploaded a new change for review. Change subject: storage: Do not count garbage volumes as children ...................................................................... storage: Do not count garbage volumes as children Change-Id: Ice3c70249cbc3577b239bd11d224955f2e29b211 Signed-off-by: Adam Litke <alitke(a)redhat.com> --- M vdsm/storage/blockVolume.py 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/73/44573/1 diff --git a/vdsm/storage/blockVolume.py b/vdsm/storage/blockVolume.py index 51f60ea..9baa3d2 100644 --- a/vdsm/storage/blockVolume.py +++ b/vdsm/storage/blockVolume.py @@ -184,7 +184,7 @@ lvs = lvm.lvsByTag(self.sdUUID, "%s%s" % (TAG_PREFIX_PARENT, self.volUUID)) - return tuple(lv.name for lv in lvs if TAG_VOL_UNINIT not in lv.tags) + return tuple(lv.name for lv in lvs if TAG_VOL_GARBAGE not in lv.tags) def getImage(self): """ -- To view, visit https://gerrit.ovirt.org/44573 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ice3c70249cbc3577b239bd11d224955f2e29b211 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke <alitke(a)redhat.com>

3 3

Change in vdsm[master]: storage: export volume lease state in getVolumeInfo
by alitke＠redhat.com 12 Aug '16

12 Aug '16

Adam Litke has uploaded a new change for review. Change subject: storage: export volume lease state in getVolumeInfo ...................................................................... storage: export volume lease state in getVolumeInfo In order to support an entity-based polling methodology (ie. in the new SDM verbs) we must know whether a volume is currently locked by a long running operation. Extend the getVolumeInfo API to report whether the lease is free or held. Change-Id: I55f062a4be15593fdc98518fd0a113976cbe0ae7 Signed-off-by: Adam Litke <alitke(a)redhat.com> --- M vdsm/rpc/vdsmapi-schema.json M vdsm/storage/volume.py 2 files changed, 31 insertions(+), 1 deletion(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/23/38623/1 diff --git a/vdsm/rpc/vdsmapi-schema.json b/vdsm/rpc/vdsmapi-schema.json index 988ce64..cba1008 100644 --- a/vdsm/rpc/vdsmapi-schema.json +++ b/vdsm/rpc/vdsmapi-schema.json @@ -7897,6 +7897,19 @@ {'enum': 'VolumeStatus', 'data': ['OK', 'INVALID', 'ILLEGAL']} ## +# @VolumeLeaseState: +# +# An enumeration of Volume lease states. +# +# @FREE: The lease is free +# +# @HELD: The lease is held +# +# Since: 4.10.0 +## +{'enum': 'VolumeLeaseState', 'data': ['FREE', 'HELD']} + +## # @VolumeInfo: # # Information about a Volume. @@ -7937,6 +7950,8 @@ # # @children: A list of decendent Volumes that depend on this Volume # +# @leaseState: #optional The state of the volume lease +# # Since: 4.10.0 ## {'type': 'VolumeInfo', @@ -7946,7 +7961,8 @@ 'description': 'str', 'pool': 'UUID', 'domain': 'UUID', 'image': 'UUID', 'ctime': 'int', '*mtime': 'uint', 'legality': 'VolumeLegality', 'apparentsize': 'uint', - 'truesize': 'uint', 'status': 'VolumeStatus', 'children': ['UUID']}} + 'truesize': 'uint', 'status': 'VolumeStatus', 'children': ['UUID'], + '*leaseState': 'VolumeLeaseState'}} ## # @Volume.getInfo: diff --git a/vdsm/storage/volume.py b/vdsm/storage/volume.py index 980b6e3..2e16efc 100644 --- a/vdsm/storage/volume.py +++ b/vdsm/storage/volume.py @@ -33,6 +33,7 @@ import fileUtils import task from threadLocal import vars +from clusterlock import InquireNotSupportedError import resourceFactories import resourceManager as rm rmanager = rm.ResourceManager.getInstance() @@ -85,6 +86,10 @@ ILLEGAL_VOL = "ILLEGAL" LEGAL_VOL = "LEGAL" FAKE_VOL = "FAKE" + +# Volume lease states +LEASE_FREE = "FREE" +LEASE_HELD = "HELD" log = logging.getLogger('Storage.Volume') @@ -839,6 +844,11 @@ cls.createMetadata(metaId, meta) return meta + def getLeaseState(self): + ver, owners = sdCache.produce(self.sdUUID).inquireVolumeLease( + self.imgUUID, self.volUUID) + return LEASE_HELD if owners else LEASE_FREE + def getInfo(self): """ Get volume info @@ -857,6 +867,10 @@ info['apparentsize'] = str(vsize) info['truesize'] = str(avsize) info['status'] = "OK" + try: + info['leaseState'] = self.getLeaseState() + except InquireNotSupportedError: + pass except se.StorageException as e: self.log.debug("exception: %s:%s" % (str(e.message), str(e.value))) info['apparentsize'] = "0" -- To view, visit https://gerrit.ovirt.org/38623 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I55f062a4be15593fdc98518fd0a113976cbe0ae7 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke <alitke(a)redhat.com>

6 40

Change in vdsm[master]: misc: remove cp parameter
by ykaplan＠redhat.com 06 Aug '16

06 Aug '16

Yeela Kaplan has uploaded a new change for review. Change subject: misc: remove cp parameter ...................................................................... misc: remove cp parameter All usages of cp parameter have been extinguished. Therefore, no longer needed. Change-Id: I0906bfd7dfa128c323aa399810bbd75883618434 Signed-off-by: Yeela Kaplan <ykaplan(a)redhat.com> --- M vdsm/storage/misc.py 1 file changed, 5 insertions(+), 13 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/13/45613/1 diff --git a/vdsm/storage/misc.py b/vdsm/storage/misc.py index b14fd26..b0146c8 100644 --- a/vdsm/storage/misc.py +++ b/vdsm/storage/misc.py @@ -449,7 +449,7 @@ return n -def rotateFiles(directory, prefixName, gen, cp=False, persist=False): +def rotateFiles(directory, prefixName, gen, persist=False): log.debug("dir: %s, prefixName: %s, versions: %s" % (directory, prefixName, gen)) gen = int(gen) @@ -477,24 +477,16 @@ for key in keys: oldName = os.path.join(directory, fd[key]['old']) newName = os.path.join(directory, fd[key]['new']) - if utils.isOvirtNode() and persist and not cp: + if utils.isOvirtNode() and persist: try: utils.unpersist(oldName) utils.unpersist(newName) except: pass - try: - if cp: - execCmd([constants.EXT_CP, oldName, newName], sudo=True) - if (utils.isOvirtNode() and - persist and not os.path.exists(newName)): - utils.persist(newName) - else: - os.rename(oldName, newName) - except: - pass - if utils.isOvirtNode() and persist and not cp: + os.rename(oldName, newName) + + if utils.isOvirtNode() and persist: try: utils.persist(newName) except: -- To view, visit https://gerrit.ovirt.org/45613 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I0906bfd7dfa128c323aa399810bbd75883618434 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yeela Kaplan <ykaplan(a)redhat.com>

5 17

Change in vdsm[master]: freeze: Freeze guest even when creating memory snapshot
by Nir Soffer 30 Jul '16

30 Jul '16

Nir Soffer has uploaded a new change for review. Change subject: freeze: Freeze guest even when creating memory snapshot ...................................................................... freeze: Freeze guest even when creating memory snapshot We used to skip freezing the guest if creating memory snapshot. This was probably done because qemu is pausing the vm for creating memory snapshot. However, this is not consistent with snapshots of external disks such as network disks, where we always freeze the vm before taking the snapshot. Also, it is probably safer to freeze even when creating memory snapshot, giving applications on the guest chance to pause in consistent state. This patch removes the check for memory snapshot, and freeze the guest unless it is already frozen. Change-Id: I8aa7ac0dea8690ca33df8067f84734d788da8bf8 Signed-off-by: Nir Soffer <nsoffer(a)redhat.com> --- M vdsm/virt/vm.py 1 file changed, 3 insertions(+), 6 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/99/43299/1 diff --git a/vdsm/virt/vm.py b/vdsm/virt/vm.py index 332281b..ecbbe1d 100644 --- a/vdsm/virt/vm.py +++ b/vdsm/virt/vm.py @@ -3093,9 +3093,6 @@ else: snapFlags |= libvirt.VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY - # When creating memory snapshot libvirt will pause the vm - should_freeze = not (memoryParams or frozen) - snapxml = snap.toprettyxml() # TODO: this is debug information. For 3.6.x we still need to # see the XML even with 'info' as default level. @@ -3109,7 +3106,7 @@ self.stopDisksStatsCollection() try: - if should_freeze: + if not frozen: freezed = self.freeze() try: self._dom.snapshotCreateXML(snapxml, snapFlags) @@ -3120,7 +3117,7 @@ # Must always thaw, even if freeze failed; in case the guest # did freeze the filesystems, but failed to reply in time. # Libvirt is using same logic (see src/qemu/qemu_driver.c). - if should_freeze: + if not frozen: self.thaw() # We are padding the memory volume with block size of zeroes @@ -3148,7 +3145,7 @@ # Returning quiesce to notify the manager whether the guest agent # froze and flushed the filesystems or not. return {'status': doneCode, - 'quiesce': should_freeze and freezed["status"]["code"] == 0} + 'quiesce': not frozen and freezed["status"]["code"] == 0} def diskReplicateStart(self, srcDisk, dstDisk): try: -- To view, visit https://gerrit.ovirt.org/43299 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I8aa7ac0dea8690ca33df8067f84734d788da8bf8 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer <nsoffer(a)redhat.com>

4 14

Change in vdsm[master]: log: Use INFO log level as default
by Nir Soffer 27 Jul '16

27 Jul '16

Nir Soffer has uploaded a new change for review. Change subject: log: Use INFO log level as default ...................................................................... log: Use INFO log level as default The current logs are much too verbose which cause trouble for users, and make us look unprofessional. Mature project should not use debug log by default. To debug issues that are not clear enough using INFO logs, the relevant logger level can be modified on a user machine as needed. Change-Id: I767dcd9bad7b9fbeebb438e9ef13cb0ec3f042ee Signed-off-by: Nir Soffer <nsoffer(a)redhat.com> --- M vdsm/logger.conf.in 1 file changed, 4 insertions(+), 4 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/04/32504/1 diff --git a/vdsm/logger.conf.in b/vdsm/logger.conf.in index 64b154f..8e963dd 100644 --- a/vdsm/logger.conf.in +++ b/vdsm/logger.conf.in @@ -8,18 +8,18 @@ keys=long,simple,none,sysform [logger_root] -level=DEBUG +level=INFO handlers=syslog,logfile propagate=0 [logger_vds] -level=DEBUG +level=INFO handlers=syslog,logfile qualname=vds propagate=0 [logger_Storage] -level=DEBUG +level=INFO handlers=logfile qualname=Storage propagate=0 @@ -31,7 +31,7 @@ propagate=1 [logger_connectivity] -level=DEBUG +level=INFO handlers=connlogfile qualname=connectivity propagate=0 -- To view, visit http://gerrit.ovirt.org/32504 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I767dcd9bad7b9fbeebb438e9ef13cb0ec3f042ee Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer <nsoffer(a)redhat.com>

9 23

Change in vdsm[master]: tests: JsonRpcClient tests suite
by Piotr Kliczewski 25 Jul '16

25 Jul '16

Piotr Kliczewski has uploaded a new change for review. Change subject: tests: JsonRpcClient tests suite ...................................................................... tests: JsonRpcClient tests suite Signed-off-by: pkliczewski <piotr.kliczewski(a)gmail.com> Change-Id: I48c209915f4047958fd9b7e5dd6a202ca14a50cb --- M tests/Makefile.am A tests/jsonrpcClientTests.py M tests/stompAdapterTests.py 3 files changed, 159 insertions(+), 0 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/82/43582/1 diff --git a/tests/Makefile.am b/tests/Makefile.am index 9a7f81f..a010bb3 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -57,6 +57,7 @@ iproute2Tests.py \ ipwrapperTests.py \ iscsiTests.py \ + jsonrpcClientTests.py \ jsonrpcServerTests.py \ libvirtconnectionTests.py \ lvmTests.py \ diff --git a/tests/jsonrpcClientTests.py b/tests/jsonrpcClientTests.py new file mode 100644 index 0000000..5d4304f --- /dev/null +++ b/tests/jsonrpcClientTests.py @@ -0,0 +1,148 @@ +# +# Copyright 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 +# +from Queue import Queue +from threading import Event + +from testlib import VdsmTestCase as TestCaseBase +from yajsonrpc.stompreactor import ClientRpcTransportAdapter +from yajsonrpc import JsonRpcClient, JsonRpcRequest, JsonRpcResponse +from yajsonrpc.stomp import Frame, Headers +from vdsm.compat import json + +from stompAdapterTests import TestSubscription + + +ID = 'e8a936a6-d886-4cfa-97b9-2d54209053ff' + + +class TestClient(object): + + def __init__(self): + self._queue = Queue() + self.closed = False + + def send(self, data, destination, headers): + self._queue.put_nowait((data, destination, headers)) + + def pop_message(self): + return self._queue.get(True, 3) + + @property + def has_outgoing_messages(self): + return (self._queue.qsize() > 0) + + def close(self): + self.closed = True + + +class JsonrpcClientTest(TestCaseBase): + + def test_call(self): + subscription = TestSubscription('jms.topic.vdsm_responses', + 'ad052acb-a934-4e10-8ec3-00c7417ef8d1') + client = TestClient() + subscription.set_client(client) + + json_client = JsonRpcClient( + ClientRpcTransportAdapter(subscription, + 'jms.topic.vdsm_requests', + client) + ) + + request = JsonRpcRequest('my_method', (), ID) + json_client.call(request, timeout=0) + + self.assertTrue(client.has_outgoing_messages) + data, destination, headers = client.pop_message() + + req = JsonRpcRequest.fromRawObject(json.loads(data)[0]) + self.assertEquals(req.method, request.method) + self.assertEquals(req.id, request.id) + + self.assertEquals('jms.topic.vdsm_requests', destination) + + self.assertEquals(headers[Headers.CONTENT_TYPE], 'application/json') + self.assertEquals(headers[Headers.REPLY_TO], + 'jms.topic.vdsm_responses') + + def test_async_call_with_response(self): + subscription = TestSubscription('jms.topic.vdsm_responses', + 'ad052acb-a934-4e10-8ec3-00c7417ef8d1') + client = TestClient() + subscription.set_client(client) + + json_client = JsonRpcClient( + ClientRpcTransportAdapter(subscription, + 'jms.topic.vdsm_requests', + client) + ) + + request = JsonRpcRequest('my_method', (), ID) + call = json_client.call_async(request) + + self.assertTrue(client.has_outgoing_messages) + + response = JsonRpcResponse(result='my_response', reqId=ID) + subscription.process(Frame(body=response.encode())) + + self.assertTrue(call.isSet()) + self.assertEquals('my_response', call.responses[0].result) + + def test_receive_event(self): + subscription = TestSubscription('jms.topic.vdsm_responses', + 'ad052acb-a934-4e10-8ec3-00c7417ef8d1') + client = TestClient() + subscription.set_client(client) + ev = Event() + + json_client = JsonRpcClient( + ClientRpcTransportAdapter(subscription, + 'jms.topic.vdsm_requests', + client) + ) + + def callback(client, event, params): + self.assertEquals(event, 'localhost|component|operation|id') + self.assertEquals(params['my_property'], 'my_value') + ev.set() + + json_client.registerEventCallback(callback) + + event = JsonRpcRequest('localhost|component|operation|id', + {'my_property': 'my_value'}) + + subscription.process(Frame(body=event.encode())) + ev.wait(timeout=3) + self.assertTrue(ev.is_set()) + + def test_close(self): + subscription = TestSubscription('jms.topic.vdsm_responses', + 'ad052acb-a934-4e10-8ec3-00c7417ef8d1') + client = TestClient() + + json_client = JsonRpcClient( + ClientRpcTransportAdapter(subscription, + 'jms.topic.vdsm_requests', + client) + ) + + json_client.close() + self.assertTrue(client.closed) + self.assertTrue(subscription.unsubscribed) diff --git a/tests/stompAdapterTests.py b/tests/stompAdapterTests.py index a0be47f..68fae77 100644 --- a/tests/stompAdapterTests.py +++ b/tests/stompAdapterTests.py @@ -60,6 +60,7 @@ def __init__(self, destination, id): self._destination = destination self._id = id + self.unsubscribed = False def set_client(self, client): self._client = TestConnection(client) @@ -76,6 +77,15 @@ def client(self): return self._client + def set_message_handler(self, message_handler): + self.handler = message_handler + + def process(self, data): + self.handler(None, data) + + def unsubscribe(self): + self.unsubscribed = True + class ConnectFrameTest(TestCaseBase): -- To view, visit https://gerrit.ovirt.org/43582 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I48c209915f4047958fd9b7e5dd6a202ca14a50cb Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>

2 7

Change in vdsm[master]: tests: JsonRpcServer tests suite
by Piotr Kliczewski 25 Jul '16

25 Jul '16

Piotr Kliczewski has uploaded a new change for review. Change subject: tests: JsonRpcServer tests suite ...................................................................... tests: JsonRpcServer tests suite Change-Id: I1090764c7289544abe331a13ec765ceed2a53afe Signed-off-by: pkliczewski <piotr.kliczewski(a)gmail.com> --- M lib/yajsonrpc/__init__.py M tests/Makefile.am A tests/jsonrpcServerTests.py 3 files changed, 133 insertions(+), 1 deletion(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/81/43581/1 diff --git a/lib/yajsonrpc/__init__.py b/lib/yajsonrpc/__init__.py index 68af242..a307c7d 100644 --- a/lib/yajsonrpc/__init__.py +++ b/lib/yajsonrpc/__init__.py @@ -293,7 +293,12 @@ self._responses.append(response) def requestDone(self, response): - del self._requests[response.id] + try: + del self._requests[response.id] + except KeyError: + # ignore when request had no id + pass + self.addResponse(response) self.sendReply() diff --git a/tests/Makefile.am b/tests/Makefile.am index 15dbd7b..9a7f81f 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -57,6 +57,7 @@ iproute2Tests.py \ ipwrapperTests.py \ iscsiTests.py \ + jsonrpcServerTests.py \ libvirtconnectionTests.py \ lvmTests.py \ main.py \ diff --git a/tests/jsonrpcServerTests.py b/tests/jsonrpcServerTests.py new file mode 100644 index 0000000..6c508c2 --- /dev/null +++ b/tests/jsonrpcServerTests.py @@ -0,0 +1,126 @@ +# +# Copyright 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 threading +from contextlib import contextmanager +from Queue import Queue + +from testlib import VdsmTestCase as TestCaseBase +from yajsonrpc import JsonRpcServer, JsonRpcResponse, JsonRpcRequest + + +ID = 'e8a936a6-d886-4cfa-97b9-2d54209053ff' + + +class FakeBridge(object): + + def get_text(self): + return 'my text' + + def register_server_address(self, server_address): + self.server_address = server_address + + def unregister_server_address(self): + self.server_address = None + + +class TestClient(object): + + def __init__(self): + self._queue = Queue() + + def send(self, data): + self._queue.put_nowait(data) + + def pop_message(self): + return self._queue.get(True, 3) + + @property + def has_outgoing_messages(self): + return (self._queue.qsize() > 0) + + +class TestConnection(object): + + def get_local_address(self): + return 'localhost' + + +@contextmanager +def create_server(bridge): + server = JsonRpcServer(bridge, 3600) + t = threading.Thread(target=server.serve_requests, + name='JsonRpcServer') + t.setDaemon(True) + t.start() + + try: + yield server + finally: + server.stop() + + +class JsonrpcServerTest(TestCaseBase): + + def test_method_call(self): + request = JsonRpcRequest(method='get_text', params=(), reqId=ID) + client = TestClient() + + with create_server(FakeBridge()) as server: + server.queueRequest((client, TestConnection(), request.encode())) + + response = client.pop_message() + self.assertIsNot(response, None) + res = JsonRpcResponse.decode(response) + self.assertEquals(res.result, 'my text') + + def test_not_existing_method(self): + request = JsonRpcRequest(method='abcd', params=(), reqId=ID) + client = TestClient() + + with create_server(FakeBridge()) as server: + server.queueRequest((client, TestConnection(), request.encode())) + + response = client.pop_message() + self.assertIsNot(response, None) + res = JsonRpcResponse.decode(response) + self.assertEquals(res.error['code'], -32601) + + def test_wrong_param(self): + request = JsonRpcRequest(method='get_text', params=('param',), + reqId=ID) + client = TestClient() + + with create_server(FakeBridge()) as server: + server.queueRequest((client, TestConnection(), request.encode())) + + response = client.pop_message() + self.assertIsNot(response, None) + res = JsonRpcResponse.decode(response) + self.assertEquals(res.error['code'], -32603) + + def test_no_request_id(self): + request = JsonRpcRequest(method='get_text', params=()) + client = TestClient() + + with create_server(FakeBridge()) as server: + server.queueRequest((client, TestConnection(), request.encode())) + + response = client.pop_message() + self.assertIsNot(response, None) -- To view, visit https://gerrit.ovirt.org/43581 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I1090764c7289544abe331a13ec765ceed2a53afe Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>

2 8

Change in vdsm[master]: vm: Cleanup waiting for xml update
by Nir Soffer 24 Jul '16

24 Jul '16

Nir Soffer has uploaded a new change for review. Change subject: vm: Cleanup waiting for xml update ...................................................................... vm: Cleanup waiting for xml update This patch cleans up a bit the code for waiting until libvirt xml is updated after pivot was completed. - Clarify confusing log message claiming that pivot failed after it completed successfully - Cleanup creation of volumes lists using generator expression - More clear logic for checking current volumes list - Replace detailed log message and unhelpful exception with detailed exception - Move comment out of the loop to make the loop more clear - Remove unneeded keys() calls when looking up alias in chains This code was added as temporary solution until libvirt is fixed, but I think we would like keep a simplified version of it even after libvirt is fixed, verifying that the operation was successful. Change-Id: I9fec5416a62736bad461ddd0b54093d23960b7a6 Signed-off-by: Nir Soffer <nsoffer(a)redhat.com> --- M vdsm/virt/vm.py 1 file changed, 27 insertions(+), 24 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/38/39938/1 diff --git a/vdsm/virt/vm.py b/vdsm/virt/vm.py index efadbdb..8ece47b 100644 --- a/vdsm/virt/vm.py +++ b/vdsm/virt/vm.py @@ -5100,40 +5100,43 @@ # synchronized and we may start the vm with a stale volume in the # future. See https://bugzilla.redhat.com/show_bug.cgi?id=1202719 for # more details. - # TODO: Remove once we depend on a libvirt with this bug fixed. # We expect libvirt to show that the original leaf has been removed # from the active volume chain. origVols = sorted([x['volumeID'] for x in self.drive.volumeChain]) - expectedVols = origVols[:] - expectedVols.remove(self.drive.volumeID) + expectedVols = [v for v in origVols if v != self.driveVolumeID] alias = self.drive['alias'] self.vm.log.info("Waiting for libvirt to update the XML after pivot " "of drive %s completed", alias) - while True: - # This operation should complete in either one or two iterations of - # this loop. Until libvirt updates the XML there is nothing to do - # but wait. While we wait we continue to tell engine that the job - # is ongoing. If we are still in this loop when the VM is powered - # off, the merge will be resolved manually by engine using the - # reconcileVolumeChain verb. - chains = self.vm._driveGetActualVolumeChain([self.drive]) - if alias not in chains.keys(): - raise RuntimeError("Failed to retrieve volume chain for " - "drive %s. Pivot failed.", alias) - curVols = sorted([entry.uuid for entry in chains[alias]]) - if curVols == origVols: - time.sleep(1) - elif curVols == expectedVols: + # This operation should complete in either one or two iterations of + # this loop. Until libvirt updates the XML there is nothing to do + # but wait. While we wait we continue to tell engine that the job + # is ongoing. If we are still in this loop when the VM is powered + # off, the merge will be resolved manually by engine using the + # reconcileVolumeChain verb. + # TODO: Check once when we depend on a libvirt with this bug fixed. + + while True: + chains = self.vm._driveGetActualVolumeChain([self.drive]) + if alias not in chains: + raise RuntimeError("Failed to retrieve volume chain for " + "drive %s after pivot completed", alias) + + curVols = sorted(entry.uuid for entry in chains[alias]) + + if curVols == expectedVols: self.vm.log.info("The XML update has been completed") - break - else: - self.log.error("Bad volume chain found for drive %s. Previous " - "chain: %s, Expected chain: %s, Actual chain: " - "%s", alias, origVols, expectedVols, curVols) - raise RuntimeError("Bad volume chain found") + return + + if curVols != origVols: + raise RuntimeError( + "Bad volume chain after pivot for drive %s. Previous " + "chain: %s, Expected chain: %s, Actual chain: %s" % + (alias, origVols, expectedVols, curVols)) + + time.sleep(1) def _devicesWithAlias(domXML): -- To view, visit https://gerrit.ovirt.org/39938 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I9fec5416a62736bad461ddd0b54093d23960b7a6 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer <nsoffer(a)redhat.com>

3 7

Change in vdsm[master]: fc-scan: Use utilities from vdsm library.
by Nir Soffer 24 Jul '16

24 Jul '16

Nir Soffer has uploaded a new change for review. Change subject: fc-scan: Use utilities from vdsm library. ...................................................................... fc-scan: Use utilities from vdsm library. Replace low level threading code with simpler concurrent.tmap() call and duplicate monotonic_time() with utils.monotonic_time(). Change-Id: Ic48748d6a43d41e034e16cb4f636ebe627881590 Signed-off-by: Nir Soffer <nsoffer(a)redhat.com> --- M vdsm/storage/fc-scan 1 file changed, 24 insertions(+), 46 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/66/38466/1 diff --git a/vdsm/storage/fc-scan b/vdsm/storage/fc-scan index 344345d..c746ea4 100755 --- a/vdsm/storage/fc-scan +++ b/vdsm/storage/fc-scan @@ -38,43 +38,11 @@ import logging import os import sys -import threading + +from vdsm import concurrent +from vdsm import utils log = logging.getLogger("fc-scan") - - -class Scan(object): - - def __init__(self, host): - self.host = host - self.succeeded = False - self.thread = None - - def start(self): - self.thread = threading.Thread(target=self.run) - self.thread.daemon = True - self.thread.start() - - def wait(self): - self.thread.join() - - def run(self): - try: - path = "/sys/class/scsi_host/%s/scan" % self.host - log.debug("Scanning %s", path) - start = monotonic_time() - fd = os.open(path, os.O_WRONLY) - try: - os.write(fd, "- - -") - finally: - os.close(fd) - self.succeeded = True - elapsed = monotonic_time() - start - log.debug("Scanned %s in %.2f seconds", path, elapsed) - except OSError as e: - log.error("Scanning %s failed: %s", path, e) - except Exception: - log.exception("Scanning %s failed", path) def main(args): @@ -93,22 +61,32 @@ log.debug("No fc_host found") return 0 - scans = [] - - for host in hosts: - s = Scan(host) - s.start() - scans.append(s) - - for s in scans: - s.wait() + scans = concurrent.tmap(scan_host, hosts) if not all(s.succeeded for s in scans): return 1 + return 0 -def monotonic_time(): - return os.times()[4] + +def scan_host(name): + try: + path = "/sys/class/scsi_host/%s/scan" % name + log.debug("Scanning %s", path) + start = utils.monotonic_time() + fd = os.open(path, os.O_WRONLY) + try: + os.write(fd, "- - -") + finally: + os.close(fd) + elapsed = utils.monotonic_time() - start + log.debug("Scanned %s in %.2f seconds", path, elapsed) + except OSError as e: + log.error("Scanning %s failed: %s", path, e) + raise + except Exception: + log.exception("Scanning %s failed", path) + raise if __name__ == '__main__': -- To view, visit https://gerrit.ovirt.org/38466 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ic48748d6a43d41e034e16cb4f636ebe627881590 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer <nsoffer(a)redhat.com>

8 24

Change in vdsm[master]: lib: Revert and refine error handling in tmap()
by Nir Soffer 24 Jul '16

24 Jul '16

Nir Soffer has uploaded a new change for review. Change subject: lib: Revert and refine error handling in tmap() ...................................................................... lib: Revert and refine error handling in tmap() In commit 2b7155b696 (lib: Simplify and generalize concurrent.tmap()), we simplified error handling by returning a named tuple with function results. This turned out less useful then the original error handling. This patch returns the previous error handling: - Functions passed to tmap() should not raise - if they raise, this is considered a bug in the function. - The last error is raised by tmap() instead of returning the result. This make it easier to fail loudly for unexpected errors. - The original exception is re-raised now with the original traceback. - Error handling is documented properly now Previously you had to make sure function raises to signal failures: def func(): try: code that should not fail... code that may fail... code that should not fail... except ExpectedError: log.error(...) raise except Exception: log.exception(...) raise results = concurrent.tmap(func, values) if not all(r.succeeded for r in results): ... Returning the result as is lets us have nicer code: def func(): code that should not fail... try: code that may fail... except ExpectedError: log.error(...) return False code that should not fail... return True succeeded = concurrent.tmap(func, values) if not all(succeeded): ... We can ignore unexpected errors, since tmap() will log them and fail loudly. We can also minimize try except block for expected errors. Change-Id: I0154b28ff7822c63e77181bbbf444c712bd0c31e Signed-off-by: Nir Soffer <nsoffer(a)redhat.com> --- M lib/vdsm/concurrent.py M tests/concurrentTests.py 2 files changed, 45 insertions(+), 19 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/11/39211/1 diff --git a/lib/vdsm/concurrent.py b/lib/vdsm/concurrent.py index 64e072d..5498052 100644 --- a/lib/vdsm/concurrent.py +++ b/lib/vdsm/concurrent.py @@ -18,22 +18,42 @@ # Refer to the README and COPYING files for full details of the license # +import logging import threading -from collections import namedtuple - - -Result = namedtuple("Result", ["succeeded", "value"]) +import sys def tmap(func, iterable): + """ + Run func with arguments from iterable in multiple threads, a returning the + output in order of arguments. + + func should not raise exceptions - we consider this a bug in func, and will + fail the call and re-raise the exception in the caller thread. + + Expected exceptions should be handled in func. If the caller likes to + handle the error later, func should return it: + + def func(value): + try: + return something(value) + except ExpectedError as e: + return e + + Unexpected exceptions should not be handled, as they are logged in the + worker threads and re-raised in the caller thread. If multiple excpetions + raised, only the last one will be re-raised in the caller thread. + """ args = list(iterable) results = [None] * len(args) + error = [None] def worker(i, f, arg): try: - results[i] = Result(True, f(arg)) - except Exception as e: - results[i] = Result(False, e) + results[i] = f(arg) + except Exception: + error[0] = sys.exc_info() + logging.exception("Unhandled exception in tmap worker thread") threads = [] for i, arg in enumerate(args): @@ -45,4 +65,8 @@ for t in threads: t.join() + if error[0] is not None: + t, v, tb = error[0] + raise t, v, tb + return results diff --git a/tests/concurrentTests.py b/tests/concurrentTests.py index 307e397..5c0646b 100644 --- a/tests/concurrentTests.py +++ b/tests/concurrentTests.py @@ -26,13 +26,16 @@ from vdsm import concurrent +class Error(Exception): + pass + + class TMapTests(VdsmTestCase): def test_results(self): values = tuple(range(10)) results = concurrent.tmap(lambda x: x, values) - expected = [concurrent.Result(True, x) for x in values] - self.assertEqual(results, expected) + self.assertEqual(results, list(values)) def test_results_order(self): def func(x): @@ -40,8 +43,7 @@ return x values = tuple(random.random() * 0.1 for x in range(10)) results = concurrent.tmap(func, values) - expected = [concurrent.Result(True, x) for x in values] - self.assertEqual(results, expected) + self.assertEqual(results, list(values)) def test_concurrency(self): start = time.time() @@ -49,12 +51,12 @@ elapsed = time.time() - start self.assertTrue(0.1 < elapsed < 0.2) - def test_error(self): - error = RuntimeError("No result for you!") - + def test_raise_last_error(self): def func(x): - raise error - - results = concurrent.tmap(func, range(10)) - expected = [concurrent.Result(False, error)] * 10 - self.assertEqual(results, expected) + raise Error(x) + try: + concurrent.tmap(func, (1, 2, 3)) + except Error as e: + self.assertEqual(e.args, (3,)) + else: + self.fail("Exception was not raised") -- To view, visit https://gerrit.ovirt.org/39211 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I0154b28ff7822c63e77181bbbf444c712bd0c31e Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer <nsoffer(a)redhat.com>

4 10

← Newer
1
2
3
4
5
6
7
8
9
10
Older →

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

vdsm-patches December 2015