Alon Bar-Lev has posted comments on this change. Change subject: packaging: Allow restore to different db location ......................................................................
Patch Set 3: (9 comments) .................................................... File packaging/bin/engine-backup.sh Line 151 Line 152 Line 153 Line 154 Line 155 folders are now files, no? Line 29: /etc/httpd/conf.d/z-ovirt-engine-proxy.conf Line 30: /etc/yum/pluginconf.d/versionlock.list Line 31: /etc/firewalld/services/ovirt-https.xml Line 32: /etc/firewalld/services/ovirt-http.xml Line 33: /etc/firewalld/services/ovirt-postgres.xml" please separate to different patch. Line 34: MYPGPASS="" Line 35: TEMP_FOLDER="" Line 36: FILE="" Line 37: DB_BACKUP_FILE_NAME="engine_backup.db" Line 111: DB_PASSFILE="${v}" Line 112: if ! [ -r "${DB_PASSFILE}" ]; then Line 113: die "Can not read password file ${DB_PASSFILE}" Line 114: fi Line 115: read DB_PASSWORD < "${DB_PASSFILE}" please support also plain password from cmdline Line 116: ;; Line 117: --db-name=*) Line 118: DB_NAME="${v}" Line 119: ;; Line 119: ;; Line 120: --db-secured=*) Line 121: DB_SECURED="${v}" Line 122: case "${DB_SECURED}" in Line 123: True|False) ;; this can be boolean parameter --db-secured sets True while default is False Line 124: *) die "invalid 'sec' value '${DB_SECURED}' for --db-secured" Line 125: esac Line 126: ;; Line 127: --db-sec-validation=*) Line 123: True|False) ;; Line 124: *) die "invalid 'sec' value '${DB_SECURED}' for --db-secured" Line 125: esac Line 126: ;; Line 127: --db-sec-validation=*) same... boolean Line 128: DB_SEC_VALIDATION="${v}" Line 129: case "${DB_SEC_VALIDATION}" in Line 130: True|False) ;; Line 131: *) die "invalid 'valid' value '${DB_SEC_VALIDATION}' for --db-sec-validation" Line 250: verifyConnection Line 251: log "Restoring database backup at ${TEMP_FOLDER}/db/${DB_BACKUP_FILE_NAME}" Line 252: restoreDB "${TEMP_FOLDER}/db/${DB_BACKUP_FILE_NAME}" Line 253: output "Starting services:" Line 254: service ovirt-engine start I am not sure we need to start automatically, maybe there are manual task to do before. Line 255: service httpd restart Line 256: output "Note: you might need to manually fix firewalld conf and autostart of ovirt-engine service" Line 257: } Line 258: Line 296: if [ -e "${backup}" ]; then Line 297: cp -a "${backup}" "${dirname}" || logdie "Cannot copy '${backup}' to '${dirname}'" Line 298: selinuxenabled && restorecon -R "${folder}" Line 299: else Line 300: echo "Skipping not-existent ${folder}" usually, the short side/non operation block of the condition - better to place it first. Line 301: fi Line 302: done || logdie "Cannot read ${folders}" Line 303: changeDBConf Line 304: } Line 314: DB_USER Line 315: DB_PASSWORD Line 316: DB_NAME Line 317: DB_SECURED Line 318: DB_SEC_VALIDATION" please indent this, to something like: local vars=" var1 var2 " this will be clear that vars are part of block and will enable adding more at future in clean diff Line 319: Line 320: echo "${vars}" | while read var; do Line 321: eval "value=\${$var}" Line 322: if [ -n "${value}" ]; then Line 316: DB_NAME Line 317: DB_SECURED Line 318: DB_SEC_VALIDATION" Line 319: Line 320: echo "${vars}" | while read var; do I am unsure why not just create our own file from scratch if we were provided external credentials. Line 321: eval "value=\${$var}" Line 322: if [ -n "${value}" ]; then Line 323: if [ "${enabled}" = "0" ]; then Line 324: log "Backing up ${conf} to ${backup}" -- To view, visit http://gerrit.ovirt.org/20258 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie2c84111808c638a1c589cf3c14c271184a0a7c2 Gerrit-PatchSet: 3 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Yedidyah Bar David <d...@redhat.com> Gerrit-Reviewer: Alex Lourie <alou...@redhat.com> Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com> Gerrit-Reviewer: Ofer Schreiber <oschr...@redhat.com> Gerrit-Reviewer: Sandro Bonazzola <sbona...@redhat.com> Gerrit-Reviewer: Yedidyah Bar David <d...@redhat.com> Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches