Alon Bar-Lev has posted comments on this change. Change subject: packaging: Allow restore to different db location ......................................................................
Patch Set 4: (7 comments) .................................................... File packaging/bin/engine-backup.sh Line 86 Line 87 Line 88 Line 89 Line 90 we need an option to allow to use different database configuration. something --use-alternate-database-configuration to make the option bellow effective or not. Line 120: ;; Line 121: --db-name=*) Line 122: ENGINE_DB_NAME="${v}" Line 123: ;; Line 124: --db-secured=*) Again, I disagree... --db-secured is a boolean parameter, and it it gets a value, it should not depend on the actual value we going to put in file. Same for db-sec-valid... Line 125: ENGINE_DB_SECURED="${v}" Line 126: case "${ENGINE_DB_SECURED}" in Line 127: True|False) ;; Line 128: *) die "invalid 'sec' value '${ENGINE_DB_SECURED}' for --db-secured" Line 303: cp -a "${backup}" "${dirname}" || logdie "Cannot copy '${backup}' to '${dirname}'" Line 304: selinuxenabled && restorecon -R "${path}" Line 305: fi Line 306: done || logdie "Cannot read ${paths}" Line 307: changeDBConf only if we enabled alternate database configuration. Line 308: } Line 309: Line 310: changeDBConf() { Line 311: local conf="/etc/ovirt-engine/engine.conf.d/10-setup-database.conf" Line 307: changeDBConf Line 308: } Line 309: Line 310: changeDBConf() { Line 311: local conf="/etc/ovirt-engine/engine.conf.d/10-setup-database.conf" please include engine-prolog.sh and use variables from there. Line 312: [ -f "${conf}" ] || return Line 313: Line 314: local enabled=0 Line 315: local backup="${conf}.$(date +"%Y%m%d%H%M%S")" Line 308: } Line 309: Line 310: changeDBConf() { Line 311: local conf="/etc/ovirt-engine/engine.conf.d/10-setup-database.conf" Line 312: [ -f "${conf}" ] || return this is an error Line 313: Line 314: local enabled=0 Line 315: local backup="${conf}.$(date +"%Y%m%d%H%M%S")" Line 316: Line 313: Line 314: local enabled=0 Line 315: local backup="${conf}.$(date +"%Y%m%d%H%M%S")" Line 316: Line 317: while read var; do I think we should create the file, and keep the script sync with what we need. Nothing should be left from original configuration if we use different database profile. Just like a fresh setup. Line 318: eval "value=\${$var}" Line 319: if [ -n "${value}" ]; then Line 320: if [ "${enabled}" = "0" ]; then Line 321: log "Backing up ${conf} to ${backup}" Line 327: sed -i -e "s/^${var}=.*/${var}=\"${value}\"/" "${conf}" || \ Line 328: die "Failed to edit ${conf}" Line 329: fi Line 330: done << __END_OF_VARS__ Line 331: ENGINE_DB_HOST I would not have indented these... but not that important. Line 332: ENGINE_DB_PORT Line 333: ENGINE_DB_USER Line 334: ENGINE_DB_PASSWORD Line 335: ENGINE_DB_NAME -- 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: 4 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