Eli Mesika has posted comments on this change. Change subject: core: Remove Windows ProductKey* from db ......................................................................
Patch Set 3: (2 comments) .................................................... File packaging/dbscripts/upgrade/03_03_0950_extract_product_keys.sh.in Line 42: execute_command "${CMD}" "${DATABASE}" "${SERVERNAME}" "${PORT}" | \ Line 43: tail -n+2 | \ Line 44: sed -e 's/^ *//' -e 's/ *$//' -e 's/ *| */|/g' | \ Line 45: grep -v '|$' | \ Line 46: sed -e $SED_RULES > $TMP_FILE I think that this is not correct,the script will return 0 which means success , so how do you know that file creation succeeded or not You don't give indication to the user what occurred , I don't think this is a friendly behavior Line 47: Line 48: if [ -s $TMP_FILE ]; then Line 49: cp $TMP_FILE $OSINFO_FILE Line 50: fi Line 47: Line 48: if [ -s $TMP_FILE ]; then Line 49: cp $TMP_FILE $OSINFO_FILE Line 50: fi Line 51: rm $TMP_FILE More than that, if you fail to create the file above , attempts to remove i here will anyway exit with error code 1 Line 52: Line 53: # cleanup Line 54: for O in $OPTIONS_NAMES; do Line 55: CMD="select fn_db_delete_config_value('$O', 'general');" -- To view, visit http://gerrit.ovirt.org/19743 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I876894e7ba5fcd28ee0d435b4a2561f662140174 Gerrit-PatchSet: 3 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Frank Kobzik <fkob...@redhat.com> Gerrit-Reviewer: Eli Mesika <emes...@redhat.com> Gerrit-Reviewer: Frank Kobzik <fkob...@redhat.com> Gerrit-Reviewer: Itamar Heim <ih...@redhat.com> Gerrit-Reviewer: Omer Frenkel <ofren...@redhat.com> Gerrit-Reviewer: Roy Golan <rgo...@redhat.com> Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches