Alon Bar-Lev has posted comments on this change.
Change subject: packaging: Fix problems in engine-backup.sh
......................................................................
Patch Set 6:
(3 comments)
I do not think restore should provision database. Although I understand that
this may be comfortable.
Restore assume database already configured with user and password, and this
derives also configure for network access. It can be either previous
installation user and password or entirely new one for restore into different
setup.
We can put some notes regarding this in the --help output, similar to what we
have in README.developer.
....................................................
File packaging/bin/engine-backup.sh
Line 183: || logdie "Database backup failed"
Line 184: }
Line 185:
Line 186: dorestore() {
Line 187: if [ -n "${ENGINE_UP_MARK}" ] && \
as far as I know it will always contain something.
Line 188: [ -r "${ENGINE_UP_MARK}" ]; then
Line 189: local engine_pid
Line 190: read engine_pid < "${ENGINE_UP_MARK}"
Line 191: ps "${engine_pid}" | grep -q 'ovirt-engine.py' &&
Line 184: }
Line 185:
Line 186: dorestore() {
Line 187: if [ -n "${ENGINE_UP_MARK}" ] && \
Line 188: [ -r "${ENGINE_UP_MARK}" ]; then
if you indent several statement, please make it clear where block begins...
if [ ] && \
[ ]
then
xxx
fi
or similar...
Line 189: local engine_pid
Line 190: read engine_pid < "${ENGINE_UP_MARK}"
Line 191: ps "${engine_pid}" | grep -q 'ovirt-engine.py' &&
Line 192: logdie "Engine service is active - can not
restore backup"
Line 187: if [ -n "${ENGINE_UP_MARK}" ] && \
Line 188: [ -r "${ENGINE_UP_MARK}" ]; then
Line 189: local engine_pid
Line 190: read engine_pid < "${ENGINE_UP_MARK}"
Line 191: ps "${engine_pid}" | grep -q 'ovirt-engine.py' &&
can be:
ps $(cat "${ENGINE_UP_MARK}")
but anyway... should be:
kill -0 "$(cat "${ENGINE_UP_MARK}")"
but if really like to check executable... better?
readlink /proc/pid/exe
Line 192: logdie "Engine service is active - can not
restore backup"
Line 193: fi
Line 194: output "Restoring..."
Line 195: log "Opening tarball ${FILE} to ${TEMP_FOLDER}"
--
To view, visit http://gerrit.ovirt.org/20264
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I464e795627af23712696212e4589e4b8480bb8a0
Gerrit-PatchSet: 6
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
