Alon Bar-Lev has posted comments on this change.
Change subject: packaging: Add backup and restore utility
......................................................................
Patch Set 9: (15 inline comments)
Hi,
The major usability:
1. restore to more recent z-stream should be supported.
2. we should add digest verification to backup, so we won't try to restore
corrupted package.
3. back up while engine is up? is it supported?
4. nfs and such? what is the policy?
5. backup folders before we overwrite.
4. instruction for user how to create the database?
....................................................
File packaging/bin/engine-backup.sh
Line 39: engine-backup: backup and restore ovirt-engine
Line 40: USAGE:
Line 41: engine-backup [--mode=<backup|restore>] [--scope=<all|dbonly>]
[--file=/path/to/bacup/or/restore/FILE]
Line 42: Where:
Line 43: --mode= the required mode (backup or restore)
no tabs within usage please.
--mode=STRING ...
look at cp --help, ls --help, tar --help
Line 44: --scope= the elements to backup
Line 45: --file= the backup FILE to restore
Line 46:
Line 47: __EOF__
Line 86: done
Line 87: }
Line 88:
Line 89: verifyArgs() {
Line 90: # Verify backup OR restore specified
comment is required?
Line 91: [ -n "${MODE}" ] || die "--mode=<backup|restore> is missing"
Line 92: [ -n "${FILE}" ] || die "--file is missing"
Line 93:
Line 94: # TODO: Verify scope
Line 88:
Line 89: verifyArgs() {
Line 90: # Verify backup OR restore specified
Line 91: [ -n "${MODE}" ] || die "--mode=<backup|restore> is missing"
Line 92: [ -n "${FILE}" ] || die "--file is missing"
add comment above?
# Verify file was provided
:)
Line 93:
Line 94: # TODO: Verify scope
Line 95: }
Line 96:
Line 90: # Verify backup OR restore specified
Line 91: [ -n "${MODE}" ] || die "--mode=<backup|restore> is missing"
Line 92: [ -n "${FILE}" ] || die "--file is missing"
Line 93:
Line 94: # TODO: Verify scope
comment required?
Line 95: }
Line 96:
Line 97: dobackup() {
Line 98: # Create temporart folder
Line 98: # Create temporart folder
Line 99: local tardir="${TEMP_FOLDER}/tar"
Line 100: mkdir "${tardir}" || die "Cannot create '${tardir}'"
Line 101: mkdir "${tardir}/files" || die "Cannot create '${tardir}/files'"
Line 102: mkdir "${tardir}/db" || die "Cannot create '${tardir}/db'"
the /db and /files should be constants as well? well, if you put the
DB_BACKUP_FILE_NAME as constant...
Line 103:
Line 104: backupFiles "${BACKUP_FOLDERS}" "${tardir}/files"
Line 105: backupDB "${tardir}/db/${DB_BACKUP_FILE_NAME}"
Line 106: cp -a "${VERSION_FILE}" "${tardir}"
Line 102: mkdir "${tardir}/db" || die "Cannot create '${tardir}/db'"
Line 103:
Line 104: backupFiles "${BACKUP_FOLDERS}" "${tardir}/files"
Line 105: backupDB "${tardir}/db/${DB_BACKUP_FILE_NAME}"
Line 106: cp -a "${VERSION_FILE}" "${tardir}"
I suggest we add:
PACKAGE_VERSION="@PACKAGE_VERSION@"
to prolog.sh, so you can use it directly, I want to delete this version file.
Line 107: tar -C "${tardir}" -cjf "${FILE}" . || die "Cannot create
'${FILE}'"
Line 108: }
Line 109:
Line 110: backupFiles() {
Line 111: local folders="$1"
Line 112: local target="$2"
Line 113: echo "${folders}" | while read folder; do
Line 114: local dirname="$(dirname ${folder})"
Line 115: mkdir -p "${tardir}/files/${dirname}"
|| die?
Line 116: cp -a "${folder}" "${target}/${dirname}"
Line 117: done
Line 118: }
Line 119:
Line 112: local target="$2"
Line 113: echo "${folders}" | while read folder; do
Line 114: local dirname="$(dirname ${folder})"
Line 115: mkdir -p "${tardir}/files/${dirname}"
Line 116: cp -a "${folder}" "${target}/${dirname}"
|| die?
Line 117: done
Line 118: }
Line 119:
Line 120: backupDB() {
Line 113: echo "${folders}" | while read folder; do
Line 114: local dirname="$(dirname ${folder})"
Line 115: mkdir -p "${tardir}/files/${dirname}"
Line 116: cp -a "${folder}" "${target}/${dirname}"
Line 117: done
|| die
Line 118: }
Line 119:
Line 120: backupDB() {
Line 121: local file="$1"
Line 142:
Line 143: verifyVersion() {
Line 144: local installed_version="$(cat ${VERSION_FILE})"
Line 145: local backup_version="$(cat ${TEMP_FOLDER}/version)"
Line 146: [ "${installed_version}" == "${backup_version}" ] \
shouldn't we support z-stream restore? and if we do, we need to run database
upgrade after restore
Line 147: || die "Backup version '${backup_version}' doesn't
match installed version"
Line 148: }
Line 149:
Line 150: restoreDB() {
Line 148: }
Line 149:
Line 150: restoreDB() {
Line 151: # TODO: Find a way to supress psql output
Line 152: # TODO: Verify and drop exiting db before restore?
I think database must be empty before restore.
I am not sure you can assume same password... well, user should know to set it
up with right name and password...
Line 153: local backupfile="$1"
Line 154: PGPASSFILE="${MYPGPASS}" psql \
Line 155: -w \
Line 156: -q \
Line 156: -q \
Line 157: -U "${ENGINE_DB_USER}" \
Line 158: -h "${ENGINE_DB_HOST}" \
Line 159: -p "${ENGINE_DB_PORT}" \
Line 160: -f "${backupfile}" \
please move as last argument
Line 161: -d "${ENGINE_DB_DATABASE}" \
Line 162: || die "Database restore failed"
Line 163: }
Line 164:
Line 163: }
Line 164:
Line 165: restoreFiles() {
Line 166: local source="$1"
Line 167: ls "${source}" | while read folder; do
why ls? why not go over the list we know?
Line 168: cp -a "${source}/${folder}" "/"
Line 169: done
Line 170:
Line 171: }
Line 164:
Line 165: restoreFiles() {
Line 166: local source="$1"
Line 167: ls "${source}" | while read folder; do
Line 168: cp -a "${source}/${folder}" "/"
|| die?
please try to avoid as much as possible messing up with /, this has the
potential of damaging the system.
copy only directories we backed.
Line 169: done
Line 170:
Line 171: }
Line 172:
Line 165: restoreFiles() {
Line 166: local source="$1"
Line 167: ls "${source}" | while read folder; do
Line 168: cp -a "${source}/${folder}" "/"
Line 169: done
|| die
Line 170:
Line 171: }
Line 172:
Line 173: generatePgPass() {
--
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: 9
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Ofer Schreiber <[email protected]>
Gerrit-Reviewer: Alon Bar-Lev <[email protected]>
Gerrit-Reviewer: Eli Mesika <[email protected]>
Gerrit-Reviewer: Kiril Nesenko <[email protected]>
Gerrit-Reviewer: Moran Goldboim <[email protected]>
Gerrit-Reviewer: Ofer Schreiber <[email protected]>
Gerrit-Reviewer: Ohad Basan <[email protected]>
Gerrit-Reviewer: Sandro Bonazzola <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
