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

Reply via email to