Alon Bar-Lev has posted comments on this change.
Change subject: packaging: Add backup and restore utility
......................................................................
Patch Set 22:
(5 comments)
Are you sure everything is working? have you checked if psql is down, database
is dirty, permission of files restored etc?
....................................................
File packaging/bin/engine-backup.sh
Line 39: usage() {
Line 40: cat << __EOF__
Line 41: engine-backup: backup and restore ovirt-engine environment
Line 42: USAGE:
Line 43: engine-backup [--mode=MODE] [--scope=SCOPE] [--file=FILE]
[--backup=FILE] [--log=FILE]
please add any parameter to usage.
see pki scripts.
Line 44: MODE is one of the following:
Line 45: backup backup system into FILE
Line 46: restore restore system from FILE
Line 47: SCOPE is one of the following:
Line 76: ;;
Line 77: --file=*)
Line 78: FILE="${v}"
Line 79: ;;
Line 80: --backup=*)
I still think this is misleading, and better to rename/copy directories with
files suffix before, or use clear statement and parameter naming.
Line 81: BACKUP_FILE="${v}"
Line 82: ;;
Line 83: --log=*)
Line 84: LOG="${v}"
Line 133:
Line 134: createtar() {
Line 135: local dir="$1"
Line 136: local file="$2"
Line 137: tar -C "${dir}" -cjf "${file}" . || logdie "Cannot create
'${file}'"
I think you should add -pSs for backups
Line 138: }
Line 139:
Line 140: createmd5() {
Line 141: local tardir="$1"
Line 179:
Line 180: dorestore() {
Line 181: output "Restoring..."
Line 182: log "Opening tarball ${FILE} to ${TEMP_FOLDER}"
Line 183: tar -C "${TEMP_FOLDER}" -xf "${FILE}" || logdie "cannot open
${TEMP_FOLDER}"
I think you should add -pSs for backups
Line 184: log "Verifying md5"
Line 185: verifymd5 "${TEMP_FOLDER}" "md5sum"
Line 186: log "Verifying version"
Line 187: verifyVersion
....................................................
File packaging/bin/engine-prolog.sh.in
Line 2: export ENGINE_DEFAULTS="${ENGINE_DEFAULTS:-@ENGINE_DEFAULTS@}"
Line 3: export ENGINE_VARS="${ENGINE_VARS:-@ENGINE_VARS@}"
Line 4: export PACKAGE_NAME="@PACKAGE_NAME@"
Line 5: export PACKAGE_VERSION="@PACKAGE_VERSION@"
Line 6: export DISPLAY_VERSION="@DISPLAY_VERSION@"
you do not need export for these... as these should not be populated to java
etc...
Line 7:
Line 8: die() {
Line 9: local m="$1"
Line 10: echo "FATAL: ${m}" >&2
--
To view, visit http://gerrit.ovirt.org/15276
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I7f6c386a0f48ccd520978193639120999e00cf2a
Gerrit-PatchSet: 22
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Ofer Schreiber <oschr...@redhat.com>
Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com>
Gerrit-Reviewer: Eli Mesika <emes...@redhat.com>
Gerrit-Reviewer: Kiril Nesenko <knese...@redhat.com>
Gerrit-Reviewer: Moran Goldboim <mgold...@redhat.com>
Gerrit-Reviewer: Ofer Schreiber <oschr...@redhat.com>
Gerrit-Reviewer: Ohad Basan <oba...@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
