Zhou Zheng Sheng has posted comments on this change. Change subject: Extend vdsm-tool: moving configure libvirt to external shell script ......................................................................
Patch Set 11: (3 inline comments) .................................................... File lib/vdsm/tool/libvirt_configure.py Line 77: 3. libvirtd Line 78: 4. qemu-sanlock.conf Line 79: For using default files, don't pass parameters. Line 80: """ Line 81: raise RuntimeError(usageMsg) I think it's ok to write the above message to stderr. Maybe the name of the variable should be different for error message. we can use "errorMsg" as the variable name. Line 82: Line 83: sys.stdout.write(usageMsg) Line 84: Line 85: .................................................... File lib/vdsm/tool/Makefile.am Line 30: validate_ovirt_certs.py \ Line 31: $(NULL) Line 32: Line 33: nodist_vdsmtool_SCRIPTS = \ Line 34: libvirt_configure.sh \ Looks like a tab is needed. Line 35: $(NULL) Line 36: Line 37: dist_vdsmtool_DATA = \ Line 38: __init__.py \ Line 45: vdsm-id.py \ Line 46: $(NULL) Line 47: Line 48: CLEANFILES = \ Line 49: config.log \ For consistency, maybe tab is better than space. Line 50: $(nodist_vdsmtool_DATA) \ Line 51: $(nodist_vdsmtool_SCRIPTS) \ -- To view, visit http://gerrit.ovirt.org/15216 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id58b129afbf141a47a85b421961bf5b1776b41e4 Gerrit-PatchSet: 11 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim <[email protected]> Gerrit-Reviewer: Alon Bar-Lev <[email protected]> Gerrit-Reviewer: Bala.FA <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Giuseppe Vallarelli <[email protected]> Gerrit-Reviewer: Yaniv Bronhaim <[email protected]> Gerrit-Reviewer: Zhou Zheng Sheng <[email protected]> Gerrit-Reviewer: oVirt Jenkins CI Server _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
