Change in vdsm[master]: storage: introducing vdsm-dump-chains script (part of vdsm-t...

nsoffer at redhat.com nsoffer at redhat.com
Sun Mar 29 12:36:22 UTC 2015


Nir Soffer has posted comments on this change.

Change subject: storage: introducing vdsm-dump-chains script (part of vdsm-tool)
......................................................................


Patch Set 9:

(4 comments)

https://gerrit.ovirt.org/#/c/38281/9/lib/vdsm/tool/vdsm_dump_chains.py
File lib/vdsm/tool/vdsm_dump_chains.py:

> please rename. it doesn't really matter but more appropriate. and why do we
This is leftover from the version were this was a standalone tool
Line 1: # Copyright 2015 Red Hat, Inc.
Line 2: #
Line 3: # This program is free software; you can redistribute it and/or modify
Line 4: # it under the terms of the GNU General Public License as published by


Line 88:      e.g: (BLANK_UUID<-a), (a<-b), (c<-d)"""
Line 89: 
Line 90: 
Line 91: @expose(_NAME)
Line 92: def main(*args):
Do we get args from vdsm-tool in this argument? If not, remove args.
Line 93:     """
Line 94:     dump-volume-chains
Line 95:     Query VDSM about the existing structure of image volumes and prints
Line 96:     them in an ordered fashion with optional additional info per volume.


Line 93:     """
Line 94:     dump-volume-chains
Line 95:     Query VDSM about the existing structure of image volumes and prints
Line 96:     them in an ordered fashion with optional additional info per volume.
Line 97:     """
> you can validate if vdsmd is up before trying to connect
Better try to connect and fail, less racy (assuming that this fails fast).
Line 98:     try:
Line 99:         options, args = _parse_args()
Line 100:         sd_uuid, = args
Line 101:         server = _connect_to_server(options)


Line 95:     Query VDSM about the existing structure of image volumes and prints
Line 96:     them in an ordered fashion with optional additional info per volume.
Line 97:     """
Line 98:     try:
Line 99:         options, args = _parse_args()
If we get args using *args, you should use them to parse the arguments, instead of sys.argv.
Line 100:         sd_uuid, = args
Line 101:         server = _connect_to_server(options)
Line 102:         image_chains, volumes_info = _get_volumes_chains(server, sd_uuid)
Line 103: 


-- 
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: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ido Barkan <ibarkan at redhat.com>
Gerrit-Reviewer: Adam Litke <alitke at redhat.com>
Gerrit-Reviewer: Allon Mureinik <amureini at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Darshan N <dnarayan at redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Ido Barkan <ibarkan at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Vladik Romanovsky <vladik.romanovsky at gmail.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: Yaniv Dary <ydary at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list