Alon Bar-Lev has posted comments on this change.
Change subject: Extend vdsm-tool: moving configure libvirt to external shell
script
......................................................................
Patch Set 19: (3 inline comments)
....................................................
File lib/vdsm/tool/libvirt_configure.sh.in
Line 45: echo 'FAILED: Could not read SSL configuration' 1>&2
Line 46: return 3
Line 47: fi
Line 48:
Line 49: if [ "$ssl" = "true" ]; then
all variables should go into "${xxx}" notation.
Line 50: echo "SUCCESS: ssl configured to true. No conflicts"
Line 51: return 0
Line 52: fi
Line 53:
Line 56: local spice_tls="$(get_libvirt_conf_item "${qconf}" spice_tls)"
Line 57:
Line 58: if [ "$listen_tcp" = "1" -a \
Line 59: "$auth_tcp" = '"none"' -a \
Line 60: "$spice_tls" = "0" ]; then
decide tabs or spaces, but to the entire file. I love tabs... but it is a
python project...
Line 61: echo "SUCCESS: No conflicts between configuration files"
Line 62: return 0
Line 63: else
Line 64: echo "FAILED: conflicting vdsm and libvirt-qemu tls
configuration."
Line 283: # Conf files as variables to allow pass them as parameter
Line 284: start_configure "$2" "$3" "$4" "$5" "$6"
Line 285: else
Line 286: start_configure "$LCONF" "$QCONF" "$LDCONF" "$QLCONF" "$2"
Line 287: fi
I think you should switch to long arguments and not positional.
[--force] [--lconf=xxx] [--qconf=yyy] ...
See example[1] of simple argument parsing.
now that you pass transparent arguments no change in python is required.
[1]
http://gerrit.ovirt.org/gitweb?p=ovirt-engine.git;a=blob;f=packaging/bin/pki-create-ca.sh;hb=HEAD#l102
Line 288: RETVAL=$?
Line 289: ;;
Line 290: test_conflict_configurations)
Line 291: echo "Checking conflicts ..."
--
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: 19
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