Yedidyah Bar David 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.
We want to use this for migration of a standalone engine machine to hosted
engine. So we definitely want to have something that will provision. You prefer
something like 'provision-engine-from-backup' and not just change
engine-backup? I do not mind also adding notes if you want.
....................................................
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}" ] && \
OK, deleted
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
I'll try to do better next time...
(now deleted per previous comment)
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' &&
I saw in many places that we prefer read (builtin) over cat (external). No?
/proc/pid/exe is linux-specific, and will merely return python (python2.7 in my
case). I can check cmdline, but it's not better than ps (ps simply prints
this). I do want to check the executable - it can die and another process can
get the same pid.
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 <[email protected]>
Gerrit-Reviewer: Alex Lourie <[email protected]>
Gerrit-Reviewer: Alon Bar-Lev <[email protected]>
Gerrit-Reviewer: Ofer Schreiber <[email protected]>
Gerrit-Reviewer: Sandro Bonazzola <[email protected]>
Gerrit-Reviewer: Yedidyah Bar David <[email protected]>
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
