Dan Kenigsberg has posted comments on this change. Change subject: storage: introducing vdsm-dump-chains script ......................................................................
Patch Set 7: (6 comments) https://gerrit.ovirt.org/#/c/38281/7//COMMIT_MSG Commit Message: Line 8: Line 9: This script queries VDSM about the existing structure of image Line 10: volumes and prints them in an ordered fashion with optional Line 11: additional info per volume Line 12: > Showing here example output can be nice Please also mark which part of dumpStorageTable.py this new script obsoletes. dumpStorageTable is used by support engineers, and the should become acquainted with the new tools. Line 13: Change-Id: I428c443bb7d6b2a504a6f77efcd4838f7ae6c404 https://gerrit.ovirt.org/#/c/38281/7/client/Makefile.am File client/Makefile.am: Line 22: Line 23: dist_vdsm_PYTHON = \ Line 24: vdsClient.py \ Line 25: vdsClientGluster.py \ Line 26: vdsm_dump_chains.py while at it, please add the NULL trick here. Line 27: Line 28: nodist_bin_SCRIPTS = \ Line 29: vdsClient \ Line 30: vdsm-dump-chains Line 26: vdsm_dump_chains.py Line 27: Line 28: nodist_bin_SCRIPTS = \ Line 29: vdsClient \ Line 30: vdsm-dump-chains and here Line 31: Line 32: dist_man1_MANS = \ Line 33: vdsClient.1 Line 34: https://gerrit.ovirt.org/#/c/38281/7/client/vdsm-dump-chains.in File client/vdsm-dump-chains.in: Line 3: CLIENT="@VDSMDIR@/vdsm_dump_chains.py" Line 4: [ -e "$CLIENT" ] || CLIENT="$CLIENT"c Line 5: Line 6: [ -n "$PYTHONPATH" ] && PYTHON_FORMATTED_PATH=":$PYTHONPATH" Line 7: PYTHONPATH="@VDSMDIR@$PYTHON_FORMATTED_PATH" python "$CLIENT" "$@" > I don't know why we need to start additional shell process for running /usr or, we can simply place the new python module here. https://gerrit.ovirt.org/#/c/38281/7/debian/vdsm-dump-chains.install File debian/vdsm-dump-chains.install: Line 1: ./usr/bin/vdsm-dump-chains Line 2: ./usr/share/vdsm/vdsm_dump_chains.py > We should move it to vdsm/storage/dump_chains.py I'm not sure about that - vdsm.storage is a server-side package; the new client-side module does not really belong there. However, I accept whatever location Nir prefers. https://gerrit.ovirt.org/#/c/38281/7/vdsm.spec.in File vdsm.spec.in: Line 1579: %{_bindir}/vdsClient Line 1580: %{_bindir}/vdsm-dump-chains Line 1581: %{_datadir}/%{vdsm_name}/vdsClient.py* Line 1582: %{_datadir}/%{vdsm_name}/vdsClientGluster.py* Line 1583: %{_datadir}/%{vdsm_name}/vdsm_dump_chains.py* is this file really needed? we can just drop the Python script in _bindir. Line 1584: %{_sysconfdir}/bash_completion.d/vdsClient Line 1585: %{_mandir}/man1/vdsClient.1* Line 1586: Line 1587: %files xmlrpc -- 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: 7 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
