Change in vdsm[master]: vmdevices: remove duplicate slot in audio device
by Martin Polednik
Martin Polednik has uploaded a new change for review.
Change subject: vmdevices: remove duplicate slot in audio device
......................................................................
vmdevices: remove duplicate slot in audio device
Alias slot is created by Base device, no need to re-create it.
Change-Id: I40e78734eb12798d801f66a9e85ac53034568902
Signed-off-by: Martin Polednik <mpolednik(a)redhat.com>
---
M vdsm/virt/vmdevices/core.py
1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/70/38370/1
diff --git a/vdsm/virt/vmdevices/core.py b/vdsm/virt/vmdevices/core.py
index 9a8b4c7..1eb50d0 100644
--- a/vdsm/virt/vmdevices/core.py
+++ b/vdsm/virt/vmdevices/core.py
@@ -123,7 +123,7 @@
class Sound(Base):
- __slots__ = ('address', 'alias')
+ __slots__ = ('address')
def getXML(self):
"""
--
To view, visit https://gerrit.ovirt.org/38370
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I40e78734eb12798d801f66a9e85ac53034568902
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik <mpolednik(a)redhat.com>
9 years, 2 months
Change in vdsm[master]: storage: introducing vdsm-dump-chains script
by ibarkan@redhat.com
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
this is impossible. just add a comment about it.
Line 26:
Line 27: _USAGE = "usage: vdsm-dump-chains [options] <sd_UUID>"
Line 28:
Line 29:
Line 23: import errno
Line 24: from vdsm import vdscli
Line 25: BLANK_UUID = '00000000-0000-0000-0000-000000000000'
Line 26:
Line 27: _USAGE = "usage: vdsm-dump-chains [options] <sd_UUID>"
vdsm-dump-images ?
Line 28:
Line 29:
Line 30: class ServerError(Exception):
Line 31: pass
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 nice error. different types can inherit from this one.
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
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"
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"
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"
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):
> vdscli implementation implies an order of an API call per volume. It might
server should be first argument
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?
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?
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 'double parent' situation. this can later be converted to a dict easily. (comment about this)
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 should be in _get_volume_chain)
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
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 can be initially set as BLANK_UUID and the append call can move to be the last statement in the loop.
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
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)
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?)
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
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:
- only a base volume
- an empty volume chain
- no base
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
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 attribute in this case
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
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 <ibarkan(a)redhat.com>
Gerrit-Reviewer: Allon Mureinik <amureini(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Darshan N <dnarayan(a)redhat.com>
Gerrit-Reviewer: Ido Barkan <ibarkan(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: Yaniv Dary <ydary(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
9 years, 2 months
Change in vdsm[master]: storage: introducing vdsm-dump-chains script
by dnarayan@redhat.com
Darshan N has posted comments on this change.
Change subject: storage: introducing vdsm-dump-chains script
......................................................................
Patch Set 5:
(1 comment)
https://gerrit.ovirt.org/#/c/38281/5/client/vdsm_dump_chains.py
File client/vdsm_dump_chains.py:
Line 152: )
> yea but shouldn't we support rhel6+Python2.6?
you are right. sorry for the confusion, I thought argparse was added in 2.6
--
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: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ido Barkan <ibarkan(a)redhat.com>
Gerrit-Reviewer: Allon Mureinik <amureini(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Darshan N <dnarayan(a)redhat.com>
Gerrit-Reviewer: Ido Barkan <ibarkan(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: Yaniv Dary <ydary(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
9 years, 2 months
Change in vdsm[master]: storage: introducing vdsm-dump-chains script
by ibarkan@redhat.com
Ido Barkan has posted comments on this change.
Change subject: storage: introducing vdsm-dump-chains script
......................................................................
Patch Set 5:
(2 comments)
https://gerrit.ovirt.org/#/c/38281/5/tests/vdsmDumpChainsTests.py
File tests/vdsmDumpChainsTests.py:
Line 22:
Line 23: from vdsm_dump_chains import _volume_dict_to_volume_chains
Line 24:
Line 25:
Line 26: class vdsmDumpChainsTest(TestCaseBase):
> also test the base where there is no parent (parent is BLANK_UUID)
Done
Line 27: def test_single_chain(self):
Line 28: d = {'a': 'b', 'b': None}
Line 29: self.assertEqual(_volume_dict_to_volume_chains(d), [['a', 'b', None]])
Line 30:
Line 28: d = {'a': 'b', 'b': None}
Line 29: self.assertEqual(_volume_dict_to_volume_chains(d), [['a', 'b', None]])
Line 30:
Line 31: def test_two_disconnected_chains(self):
Line 32: d = {'a': 'b', 'c': 'd', 'b': None, 'd': None}
> there is no such case (to chains)
Done
Line 33: self.assertEqual(
Line 34: _volume_dict_to_volume_chains(d).sort(),
Line 35: [['a', 'b', None], ['c', 'd', None]].sort())
Line 36:
--
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: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ido Barkan <ibarkan(a)redhat.com>
Gerrit-Reviewer: Allon Mureinik <amureini(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Darshan N <dnarayan(a)redhat.com>
Gerrit-Reviewer: Ido Barkan <ibarkan(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: Yaniv Dary <ydary(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
9 years, 2 months
Change in vdsm[master]: storage: introducing vdsm-dump-chains script
by ibarkan@redhat.com
Ido Barkan has posted comments on this change.
Change subject: storage: introducing vdsm-dump-chains script
......................................................................
Patch Set 6:
(3 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 current packaging of this script is not decided yet but in anyway this should be imported as a constant from a storage package. if it is already possible to import it now, please show me how.
Line 26:
Line 27: _USAGE = "usage: vdsm-dump-chains [options] <sd_UUID>"
Line 28:
Line 29:
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")
Line 69: parser.add_option('-H', '--host', default=vdscli._ADDRESS)
Line 70: parser.add_option('-p', '--port', default=vdscli._PORT)
_ADDRESS and _PORT should be made public in a different patch
Line 71: options, args = parser.parse_args()
Line 72: if len(args) != 1:
Line 73: parser.error(_USAGE)
Line 74: return args, options
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):
vdscli implementation implies an order of an API call per volume. It might be worhtwhile to implement a getVolumesList option that retrieves volume info for each volume it returns.
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}
--
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 <ibarkan(a)redhat.com>
Gerrit-Reviewer: Allon Mureinik <amureini(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Darshan N <dnarayan(a)redhat.com>
Gerrit-Reviewer: Ido Barkan <ibarkan(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: Yaniv Dary <ydary(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
9 years, 2 months
Change in vdsm[master]: storage: introducing vdsm-dump-chains script
by ibarkan@redhat.com
Ido Barkan has posted comments on this change.
Change subject: storage: introducing vdsm-dump-chains script
......................................................................
Patch Set 5:
(23 comments)
https://gerrit.ovirt.org/#/c/38281/5/client/vdsm_dump_chains.py
File client/vdsm_dump_chains.py:
Line 28: class ServerError(Exception):
Line 29: pass
Line 30:
Line 31:
Line 32: def _volume_dict_to_volume_chains(dct):
> assuming we only have a single volume without a parent (base) we can simpli
Done
Line 33:
Line 34: def _tails_indices(chains_):
Line 35: return dict(
Line 36: (chain[-1], chain_index)
Line 38:
> How about having a single function and passing index as an argument ?
I completely rewrote _volume_dict_to_volume_chains
Line 82: return res
Line 83:
Line 84:
Line 85: def _get_volumes_chains(sd_uuid, server):
Line 86: pools = _call_server(server.getConnectedStoragePoolsList)
> factor out, and add a comment about the single pool
Done
Line 87: sp_uuid, = pools['poollist']
Line 88: images = _call_server(server.getImagesList, sd_uuid)
Line 89:
Line 90: dct_image_volume_chains = {} # {image_uuid -> vol_chains}
Line 84:
Line 85: def _get_volumes_chains(sd_uuid, server):
Line 86: pools = _call_server(server.getConnectedStoragePoolsList)
Line 87: sp_uuid, = pools['poollist']
Line 88: images = _call_server(server.getImagesList, sd_uuid)
> maybe create a small wrapper for each api call to clean up this code
Done
Line 89:
Line 90: dct_image_volume_chains = {} # {image_uuid -> vol_chains}
Line 91: dct_volumes_info = {} # {vol_uuid-> vol_info}
Line 92: for img_uuid in images['imageslist']:
Line 88: images = _call_server(server.getImagesList, sd_uuid)
Line 89:
Line 90: dct_image_volume_chains = {} # {image_uuid -> vol_chains}
Line 91: dct_volumes_info = {} # {vol_uuid-> vol_info}
Line 92: for img_uuid in images['imageslist']:
> factor put this get
Done
Line 93: volumes = _call_server(
Line 94: server.getVolumesList, sd_uuid, sp_uuid, img_uuid)
Line 95:
Line 96: dct_volumes = {} # {parent_vol_uuid-> child_vol_uuid}
Line 92: for img_uuid in images['imageslist']:
Line 93: volumes = _call_server(
Line 94: server.getVolumesList, sd_uuid, sp_uuid, img_uuid)
Line 95:
Line 96: dct_volumes = {} # {parent_vol_uuid-> child_vol_uuid}
> volumes_children
Done
Line 97:
Line 98: for vol_uuid in volumes['uuidlist']:
Line 99: vol_info_answer = _call_server(server.getVolumeInfo, sd_uuid,
Line 100: sp_uuid, img_uuid, vol_uuid)
Line 97:
Line 98: for vol_uuid in volumes['uuidlist']:
Line 99: vol_info_answer = _call_server(server.getVolumeInfo, sd_uuid,
Line 100: sp_uuid, img_uuid, vol_uuid)
Line 101: if 'info' not in vol_info_answer:
> can this really happen?
no
Line 102: print 'ERROR getting info for volume %s:%s' % (
Line 103: vol_uuid, vol_info_answer)
Line 104: continue
Line 105: vol_info = vol_info_answer['info']
Line 98: for vol_uuid in volumes['uuidlist']:
Line 99: vol_info_answer = _call_server(server.getVolumeInfo, sd_uuid,
Line 100: sp_uuid, img_uuid, vol_uuid)
Line 101: if 'info' not in vol_info_answer:
Line 102: print 'ERROR getting info for volume %s:%s' % (
> write to stderr
Done
Line 103: vol_uuid, vol_info_answer)
Line 104: continue
Line 105: vol_info = vol_info_answer['info']
Line 106: dct_volumes_info[vol_uuid] = vol_info
Line 105: vol_info = vol_info_answer['info']
Line 106: dct_volumes_info[vol_uuid] = vol_info
Line 107:
Line 108: parent_uuid = vol_info['parent']
Line 109: if vdsClient.BLANK_UUID == parent_uuid:
> define here or try to import from vdsm
Done
Line 110: dct_volumes[None] = vol_uuid
Line 111: else:
Line 112: dct_volumes[parent_uuid] = vol_uuid
Line 113:
Line 106: dct_volumes_info[vol_uuid] = vol_info
Line 107:
Line 108: parent_uuid = vol_info['parent']
Line 109: if vdsClient.BLANK_UUID == parent_uuid:
Line 110: dct_volumes[None] = vol_uuid
> use BLANK_UUID?
Done
Line 111: else:
Line 112: dct_volumes[parent_uuid] = vol_uuid
Line 113:
Line 114: dct_image_volume_chains[img_uuid] = _volume_dict_to_volume_chains(
Line 139: :
> if vol: must be sufficient
not really needed anymore, since I changed the implementation of _volume_dict_to_volume_chains so None cannot be included in a volume chain
Line 145: relevant_info if verbose else ''
Line 146: )
Line 147:
Line 148:
Line 149: if __name__ == '__main__':
> just call main() here and implement it at the top of the module.
Done
Line 150:
Line 151: usage = "usage: vdsm-dump-chains [options] <ip|0> <storage_domain_UUID>"
Line 152: parser = optparse.OptionParser(usage)
Line 153: parser.add_option("-v", "--verbose", action="store_true", dest="verbose",
Line 148:
Line 149: if __name__ == '__main__':
Line 150:
Line 151: usage = "usage: vdsm-dump-chains [options] <ip|0> <storage_domain_UUID>"
Line 152: parser = optparse.OptionParser(usage)
> localhost + ssl should be the default. add options to override it.
Done
Line 153: parser.add_option("-v", "--verbose", action="store_true", dest="verbose",
Line 154: help="show additional volume info")
Line 155: parser.add_option("-s", "--SSL", action="store_true", dest="use_ssl",
Line 156: help="use SSL")
Line 152: )
> its better to use argparse instead of optparse as this module is deprecated
yea but shouldn't we support rhel6+Python2.6?
Line 154: help="show additional volume info")
Line 155: parser.add_option("-s", "--SSL", action="store_true", dest="use_ssl",
Line 156: help="use SSL")
Line 157:
Line 158: (options, args) = parser.parse_args()
> remove (,)
Done
Line 159: if len(args) != 2:
Line 160: parser.error('incorrect number of arguments')
Line 161:
Line 162: ip, sd_uuid = args
Line 156: help="use SSL")
Line 157:
Line 158: (options, args) = parser.parse_args()
Line 159: if len(args) != 2:
Line 160: parser.error('incorrect number of arguments')
> show a more verbose error message (what should have been entered)
Done
Line 161:
Line 162: ip, sd_uuid = args
Line 163:
Line 164: host_port = vdscli.cannonizeHostPort(ip)
Line 160: parser.error('incorrect number of arguments')
Line 161:
Line 162: ip, sd_uuid = args
Line 163:
Line 164: host_port = vdscli.cannonizeHostPort(ip)
> additional parameter for port (and use it directly without requiring cannon
Done
Line 165: service = vdsClient.service()
Line 166: service.useSSL = bool(options.use_ssl)
Line 167:
Line 168: try:
Line 161:
Line 162: ip, sd_uuid = args
Line 163:
Line 164: host_port = vdscli.cannonizeHostPort(ip)
Line 165: service = vdsClient.service()
> use vdsm/lib/vdsm/vdscli.py directly instead of vdsClient
Done
Line 166: service.useSSL = bool(options.use_ssl)
Line 167:
Line 168: try:
Line 169: service.do_connect(host_port)
Line 167:
Line 168: try:
Line 169: service.do_connect(host_port)
Line 170: except socket.error as e:
Line 171: if e[0] == 111:
> use errno
Done
Line 172: print "Connection to %s refused" % host_port
Line 173: else:
Line 174: traceback.print_exc()
Line 175: sys.exit(-1)
Line 168: try:
Line 169: service.do_connect(host_port)
Line 170: except socket.error as e:
Line 171: if e[0] == 111:
Line 172: print "Connection to %s refused" % host_port
> write to stderr (maybe use a proper func for this) and exit
Done
Line 173: else:
Line 174: traceback.print_exc()
Line 175: sys.exit(-1)
Line 176:
Line 170: except socket.error as e:
Line 171: if e[0] == 111:
Line 172: print "Connection to %s refused" % host_port
Line 173: else:
Line 174: traceback.print_exc()
> just raise
Done
Line 175: sys.exit(-1)
Line 176:
Line 177: volume_chains, dct_volumes_info = _get_volumes_chains(sd_uuid, service.s)
Line 171: if e[0] == 111:
Line 172: print "Connection to %s refused" % host_port
Line 173: else:
Line 174: traceback.print_exc()
Line 175: sys.exit(-1)
> 1
Done
Line 176:
Line 177: volume_chains, dct_volumes_info = _get_volumes_chains(sd_uuid, service.s)
Line 173: else:
Line 174: traceback.print_exc()
Line 175: sys.exit(-1)
Line 176:
Line 177: volume_chains, dct_volumes_info = _get_volumes_chains(sd_uuid, service.s)
> there could be one, and only one chain per image- need to reflect it in dat
Done
--
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: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ido Barkan <ibarkan(a)redhat.com>
Gerrit-Reviewer: Allon Mureinik <amureini(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Darshan N <dnarayan(a)redhat.com>
Gerrit-Reviewer: Ido Barkan <ibarkan(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: Yaniv Dary <ydary(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
9 years, 2 months
Change in vdsm[master]: storage: introducing vdsm-dump-chains script
by automation@ovirt.org
automation(a)ovirt.org has posted comments on this change.
Change subject: storage: introducing vdsm-dump-chains script
......................................................................
Patch Set 6:
* Update tracker::IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
--
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 <ibarkan(a)redhat.com>
Gerrit-Reviewer: Allon Mureinik <amureini(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Darshan N <dnarayan(a)redhat.com>
Gerrit-Reviewer: Ido Barkan <ibarkan(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: Yaniv Dary <ydary(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
9 years, 2 months
Change in vdsm[master]: storage: introducing vdsm-dump-chains script
by Nir Soffer
Nir Soffer has posted comments on this change.
Change subject: storage: introducing vdsm-dump-chains script
......................................................................
Patch Set 5:
(1 comment)
https://gerrit.ovirt.org/#/c/38281/5/client/vdsm_dump_chains.py
File client/vdsm_dump_chains.py:
Line 71: chains.append([child, parent])
Line 72: return chains
Line 73:
Line 74:
Line 75: def _call_server(method, *args):
_check_rpc()?
Later we need to move this into vdscli.py, and provide a way to use it for all calls transparently.
Line 76: res = method(*args)
Line 77: if res['status']['code']:
Line 78: raise ServerError(
Line 79: '%s%s failed. code:%s msg:%s' % (
--
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: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ido Barkan <ibarkan(a)redhat.com>
Gerrit-Reviewer: Allon Mureinik <amureini(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Darshan N <dnarayan(a)redhat.com>
Gerrit-Reviewer: Ido Barkan <ibarkan(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: Yaniv Dary <ydary(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
9 years, 2 months
Change in vdsm[master]: storage: introducing vdsm-dump-chains script
by dnarayan@redhat.com
Darshan N has posted comments on this change.
Change subject: storage: introducing vdsm-dump-chains script
......................................................................
Patch Set 5:
(3 comments)
https://gerrit.ovirt.org/#/c/38281/5/client/vdsm_dump_chains.py
File client/vdsm_dump_chains.py:
Line 38:
How about having a single function and passing index as an argument ?
Line 139: :
if vol: must be sufficient
Line 152: )
its better to use argparse instead of optparse as this module is deprecated since 2.7
--
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: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ido Barkan <ibarkan(a)redhat.com>
Gerrit-Reviewer: Allon Mureinik <amureini(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Darshan N <dnarayan(a)redhat.com>
Gerrit-Reviewer: Ido Barkan <ibarkan(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: Yaniv Dary <ydary(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
9 years, 2 months