Ido Barkan has posted comments on this change. Change subject: storage: introducing vdsm-dump-chains script ......................................................................
Patch Set 6: (22 comments) https://gerrit.ovirt.org/#/c/38281/6/client/vdsm_dump_chains.py File client/vdsm_dump_chains.py: Line 21: import sys Line 22: import optparse Line 23: import errno Line 24: from vdsm import vdscli Line 25: BLANK_UUID = '00000000-0000-0000-0000-000000000000' > I am not sure how to properly import this into the current directory. The c Done Line 26: Line 27: _USAGE = "usage: vdsm-dump-chains [options] <sd_UUID>" Line 28: Line 29: Line 21: import sys Line 22: import optparse Line 23: import errno Line 24: from vdsm import vdscli Line 25: BLANK_UUID = '00000000-0000-0000-0000-000000000000' > this is impossible. just add a comment about it. Done Line 26: Line 27: _USAGE = "usage: vdsm-dump-chains [options] <sd_UUID>" Line 28: Line 29: Line 30: class ServerError(Exception): Line 31: pass Line 32: Line 33: Line 34: class ChainError(Exception): > can put here the dictionary of vol_uuid and provide an __str__ to print a n Done Line 35: pass Line 36: Line 37: Line 38: def main(): Line 46: Line 47: def _connect_to_server(options): Line 48: host_port = "%s:%s" % (options.host, options.port) Line 49: try: Line 50: server = vdscli.connect(host_port, options.use_ssl) > just return here Done Line 51: except socket.error as e: Line 52: if e[0] == errno.ECONNREFUSED: Line 53: _write_error_and_exit("Connection to %s refused" % (host_port,)) Line 54: raise Line 63: def _parse_args(): Line 64: parser = optparse.OptionParser(_USAGE) Line 65: parser.add_option('-v', '--verbose', action='store_true', dest='verbose', Line 66: default=False, help="show additional volume info") Line 67: parser.add_option('-u', '--NOSSL', action='store_false', dest='use_ssl', > "--unsecured" Done Line 68: default=True, help="use SSL") Line 69: parser.add_option('-H', '--host', default=vdscli._ADDRESS) Line 70: parser.add_option('-p', '--port', default=vdscli._PORT) Line 71: options, args = parser.parse_args() Line 64: parser = optparse.OptionParser(_USAGE) Line 65: parser.add_option('-v', '--verbose', action='store_true', dest='verbose', Line 66: default=False, help="show additional volume info") Line 67: parser.add_option('-u', '--NOSSL', action='store_false', dest='use_ssl', Line 68: default=True, help="use SSL") > "use unsecured connection" Done Line 69: parser.add_option('-H', '--host', default=vdscli._ADDRESS) Line 70: parser.add_option('-p', '--port', default=vdscli._PORT) Line 71: options, args = parser.parse_args() Line 72: if len(args) != 1: Line 69: parser.add_option('-H', '--host', default=vdscli._ADDRESS) Line 70: parser.add_option('-p', '--port', default=vdscli._PORT) Line 71: options, args = parser.parse_args() Line 72: if len(args) != 1: Line 73: parser.error(_USAGE) > the parameter might be redundant. print "only sd_uuid is required" Done Line 74: return args, options Line 75: Line 76: Line 77: def _get_vol_info(server, vol_uuid, img_uuid, sd_uuid, sp_uuid): Line 80: vol_info = vol_info_answer['info'] Line 81: return vol_info Line 82: Line 83: Line 84: def _get_volumes_chains(sd_uuid, server): > server should be first argument Done Line 85: sp_uuid = _get_sp_uuid(server) Line 86: images = _get_all_images(server, sd_uuid) Line 87: Line 88: volume_chains = {} # {image_uuid -> vol_chain} Line 82: Line 83: Line 84: def _get_volumes_chains(sd_uuid, server): Line 85: sp_uuid = _get_sp_uuid(server) Line 86: images = _get_all_images(server, sd_uuid) > images_uuids? Done Line 87: Line 88: volume_chains = {} # {image_uuid -> vol_chain} Line 89: volumes_info = {} # {vol_uuid-> vol_info} Line 90: Line 84: def _get_volumes_chains(sd_uuid, server): Line 85: sp_uuid = _get_sp_uuid(server) Line 86: images = _get_all_images(server, sd_uuid) Line 87: Line 88: volume_chains = {} # {image_uuid -> vol_chain} > images_chains? Done Line 89: volumes_info = {} # {vol_uuid-> vol_info} Line 90: Line 91: for img_uuid in images: Line 92: volumes = _get_volumes_for_image(server, img_uuid, sd_uuid, sp_uuid) Line 90: Line 91: for img_uuid in images: Line 92: volumes = _get_volumes_for_image(server, img_uuid, sd_uuid, sp_uuid) Line 93: Line 94: volumes_children = {} # {parent_vol_uuid-> child_vol_uuid} > having a list of tuples preserves all the information and overcomes a 'doub Done Line 95: Line 96: for vol_uuid in volumes: Line 97: vol_info = _get_vol_info(server, vol_uuid, img_uuid, sd_uuid, Line 98: sp_uuid) Line 98: sp_uuid) Line 99: volumes_info[vol_uuid] = vol_info Line 100: Line 101: parent_uuid = vol_info['parent'] Line 102: volumes_children[parent_uuid] = vol_uuid > check if we override an existing paren and raise ChainError (this test shou Done Line 103: Line 104: try: Line 105: volume_chains[img_uuid] = _get_volume_chain(volumes_children) Line 106: except ChainError as e: Line 119: return res['uuidlist'] Line 120: Line 121: Line 122: def _get_volume_chain(volumes_children): Line 123: parent_vol = volumes_children.get(BLANK_UUID) > child_vol Done Line 124: if not parent_vol: Line 125: raise ChainError('base volume was not found.') Line 126: Line 127: chain = [] # ordered vol_UUIDs Line 132: raise ChainError('there is a loop in the volume chain.') Line 133: elif child_vol is None: Line 134: break # end of chain Line 135: else: Line 136: parent_vol = child_vol > renaming of parent to child will make the last else go away. also, parent c Done Line 137: Line 138: if len(chain) < len(volumes_children): Line 139: raise ChainError('there are orphan volumes in chain. ' Line 140: 'connected volumes: %s' % (chain,)) Line 145: def _call_server(method, *args): Line 146: res = method(*args) Line 147: if res['status']['code']: Line 148: raise ServerError( Line 149: '%s%s failed. code:%s msg:%s' % ( > this code can be implemented in a dedicated object Done Line 150: method, args, Line 151: res['status']['code'], res['status']['message'])) Line 152: return res Line 153: Line 171: def _print_volume_chains(volume_chains, dct_volumes_info, verbose=False): Line 172: for img_uuid, vol_chain in volume_chains.iteritems(): Line 173: print Line 174: print 'image: %s' % (img_uuid,) Line 175: > add a header: images volume chains (base volume first) Done Line 176: if not vol_chain: Line 177: print '\t- no volumes' Line 178: continue Line 179: for index, vol in enumerate(vol_chain): Line 178: continue Line 179: for index, vol in enumerate(vol_chain): Line 180: vol_info = _get_relevant_volume_info(dct_volumes_info, vol) Line 181: print '%s\t%s\t%s' % ( Line 182: 'base->' if index == 0 else '', > drop this (and add another \t?) Done Line 183: vol, Line 184: vol_info if verbose else '' Line 185: ) Line 186: Line 180: vol_info = _get_relevant_volume_info(dct_volumes_info, vol) Line 181: print '%s\t%s\t%s' % ( Line 182: 'base->' if index == 0 else '', Line 183: vol, Line 184: vol_info if verbose else '' > format this as a separate line and not as a raw python dict Done Line 185: ) Line 186: Line 187: Line 188: if __name__ == '__main__': https://gerrit.ovirt.org/#/c/38281/6/tests/vdsmDumpChainsTests.py File tests/vdsmDumpChainsTests.py: Line 20: from testlib import VdsmTestCase as TestCaseBase Line 21: from vdsm_dump_chains import _get_volume_chain, ChainError, BLANK_UUID Line 22: Line 23: Line 24: class VdsmDumpChainsTest(TestCaseBase): > test for: Done Line 25: def test_single_chain(self): Line 26: d = {BLANK_UUID: 'a', 'a': 'b'} Line 27: self.assertEqual(_get_volume_chain(d), ['a', 'b']) Line 28: Line 21: from vdsm_dump_chains import _get_volume_chain, ChainError, BLANK_UUID Line 22: Line 23: Line 24: class VdsmDumpChainsTest(TestCaseBase): Line 25: def test_single_chain(self): > remove this- there is another similar Done Line 26: d = {BLANK_UUID: 'a', 'a': 'b'} Line 27: self.assertEqual(_get_volume_chain(d), ['a', 'b']) Line 28: Line 29: def test_disconnected_chains(self): Line 30: d = {BLANK_UUID: 'a', 'a': 'b', 'c': 'd'} Line 31: with self.assertRaises(ChainError) as cm: Line 32: _get_volume_chain(d) Line 33: self.assertEqual( Line 34: str(cm.exception), "there are orphan volumes in chain. " > test an exception type instead of message. also, possibly have an exception Done Line 35: "connected volumes: ['a', 'b']") Line 36: Line 37: def test_simple_chain(self): Line 38: d = {BLANK_UUID: 'a', 'a': 'b', 'b': 'c'} Line 33: self.assertEqual( Line 34: str(cm.exception), "there are orphan volumes in chain. " Line 35: "connected volumes: ['a', 'b']") Line 36: Line 37: def test_simple_chain(self): > only leave one no-error scenario Done Line 38: d = {BLANK_UUID: 'a', 'a': 'b', 'b': 'c'} Line 39: self.assertEqual(_get_volume_chain(d), ['a', 'b', 'c']) Line 40: Line 41: def test_longer_chain(self): -- To view, visit https://gerrit.ovirt.org/38281 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I428c443bb7d6b2a504a6f77efcd4838f7ae6c404 Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ido Barkan <[email protected]> Gerrit-Reviewer: Allon Mureinik <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Darshan N <[email protected]> Gerrit-Reviewer: Ido Barkan <[email protected]> Gerrit-Reviewer: Nir Soffer <[email protected]> Gerrit-Reviewer: Yaniv Bronhaim <[email protected]> Gerrit-Reviewer: Yaniv Dary <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
