Yedidyah Bar David has posted comments on this change.

Change subject: packaging: setup: Log setup/cleanup/upgrade actions
......................................................................


Patch Set 7: (6 inline comments)

....................................................
File packaging/bin/ovirt-engine-log-setup-event.sh.in
Line 52:        esac
Line 53: done
Line 54: 
Line 55: get_distro_name() {
Line 56:        python -c 'import platform; print 
platform.linux_distribution(full_distribution_name=0)[0]'
Added ()

I'd rather use something like 'lsb_release -i' but it's not always installed by 
default and I wouldn't want to depend on it.

Moran wants to have the option to add more distros in the future, and we do 
rely on python anyway.
Line 57: }
Line 58: 
Line 59: get_distro_packages() {
Line 60:        distro=$(get_distro_name)


Line 56:        python -c 'import platform; print 
platform.linux_distribution(full_distribution_name=0)[0]'
Line 57: }
Line 58: 
Line 59: get_distro_packages() {
Line 60:        distro=$(get_distro_name)
Done
Line 61: 
Line 62:        case "${distro}" in
Line 63:                redhat|centos|fedora|SuSE)
Line 64:                        rpm -qa --last


Line 66:                debian|ubuntu)
Line 67:                        dpkg -l | awk '/^ii/ {print $2, $3}'
Line 68:                        ;;
Line 69:                *)
Line 70:                        echo "$0: distribution ${distro} not supported"
No reason to die. I do log other useful information.
Line 71:                        ;;
Line 72:        esac
Line 73: }
Line 74: 


Line 72:        esac
Line 73: }
Line 74: 
Line 75: prefix_cmd(){
Line 76:        prefix="$1"
Done
Line 77:        shift
Line 78:        "$@" 2>&1 | sed "s/^/${prefix}: /"
Line 79: }
Line 80: 


Line 75: prefix_cmd(){
Line 76:        prefix="$1"
Line 77:        shift
Line 78:        "$@" 2>&1 | sed "s/^/${prefix}: /"
Line 79: }
But they each have a different prefix.

Why more efficient?

The only advantage I can think of is not relying on sed, which I do not 
consider important - it's in the minimal installation of every common 
distribution.
Line 80: 
Line 81: (
Line 82:        echo SETUP-EVENT-BEGIN
Line 83:        prefix_cmd DATE date +'%Y-%m-%d %H:%M:%S'


Line 88:        prefix_cmd VIRSH_VER virsh --version
Line 89:        prefix_cmd UNAME_A uname -a
Line 90:        prefix_cmd PACKAGE get_distro_packages | sort
Line 91:        echo SETUP-EVENT-END
Line 92: ) >> @ENGINE_VAR@/setup-history.txt
see above.

I decided to add more commands. This can potentially help catch 
issues/conflicts between package management and what's actually installed.


-- 
To view, visit http://gerrit.ovirt.org/15863
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iae455ff0ef6475f00243906738474134f6f4e6e3
Gerrit-PatchSet: 7
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: Kiril Nesenko <knese...@redhat.com>
Gerrit-Reviewer: Moran Goldboim <mgold...@redhat.com>
Gerrit-Reviewer: Sandro Bonazzola <sbona...@redhat.com>
Gerrit-Reviewer: Yedidyah Bar David <d...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to