7:23 a.m.
From Yaniv Bronhaim <ybronhei(a)redhat.com>:
Yaniv Bronhaim has uploaded a new change for review.
Change subject: check-merged: split functions by logic and few nits
......................................................................
check-merged: split functions by logic and few nits
adding local, var renames
Change-Id: I74f9326289b972abaef4507ab4d2582a8d641778
Signed-off-by: Yaniv Bronhaim <ybronhei(a)redhat.com>
---
M automation/check-merged.sh
1 file changed, 27 insertions(+), 18 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/25/70825/1
diff --git a/automation/check-merged.sh b/automation/check-merged.sh
index ef81436..e6933b9 100755
--- a/automation/check-merged.sh
+++ b/automation/check-merged.sh
@@ -6,17 +6,25 @@
# direct backend, but without KVM(much slower).
! [[ -c "/dev/kvm" ]] && mknod /dev/kvm c 10 232
+# Fix when running in an el* chroot in fc2* host
+[[ -e /usr/bin/qemu-kvm ]] \
+|| ln -s /usr/libexec/qemu-kvm /usr/bin/qemu-kvm
+# ENV vars
DISTRO='el7'
VM_NAME="vdsm_functional_tests_host-${DISTRO}"
AUTOMATION="$PWD"/automation
PREFIX="$AUTOMATION"/vdsm_functional
EXPORTS="$PWD"/exported-artifacts
-function prepare {
+function setup_env {
+ # TODO: ask lago to create this folder
+ mkdir "$EXPORTS"/lago-logs
+
# Creates RPMS
"$AUTOMATION"/build-artifacts.sh
+ # TODO: jenkins mock env should take care of that
if [[ -d "$PREFIX" ]]; then
pushd "$PREFIX"
echo 'cleaning old lago env'
@@ -25,15 +33,14 @@
rm -rf "$PREFIX"
fi
- # Fix when running in an el* chroot in fc2* host
- [[ -e /usr/bin/qemu-kvm ]] \
- || ln -s /usr/libexec/qemu-kvm /usr/bin/qemu-kvm
lago init \
"$PREFIX" \
"$AUTOMATION"/lago-env.yml
cd "$PREFIX"
+
+ # TODO: remove --reposync-yum-config /dev/null if not needed
lago ovirt reposetup \
--reposync-yum-config /dev/null \
--custom-source "dir:$EXPORTS"
@@ -84,9 +91,6 @@
}
function run {
- mkdir "$EXPORTS"/lago-logs
- failed=0
-
lago start "$VM_NAME"
prepare_and_copy_yum_conf
@@ -95,28 +99,33 @@
# otherwise
lago ovirt deploy
+ # TODO: ask lago to have cooler way to keep the localrepo up
lago ovirt serve &
- PID=$!
+ local httpd_pid=$!
- fake_ksm_in_vm
+ fake_ksm_in_vm && run_all_tests
+ local failed=$?
+ kill $httpd_pid
+
+ return $failed
+}
+
+function run_all_tests {
run_infra_tests | tee "$EXPORTS/functional_tests_stdout.$DISTRO.log"
failed="${PIPESTATUS[0]}"
run_network_tests | tee -a "$EXPORTS/functional_tests_stdout.$DISTRO.log"
res="${PIPESTATUS[0]}"
[ "$res" -ne 0 ] && failed="$res"
+}
- kill $PID
-
- lago copy-from-vm \
- "$VM_NAME" \
- "/tmp/nosetests-${DISTRO}.xml" \
- "$EXPORTS/nosetests-${DISTRO}.xml" || :
+function collect_logs {
+ lago copy-from-vm "$VM_NAME" \
+ "/tmp/nosetests-${DISTRO}.xml" \
+ "$EXPORTS/nosetests-${DISTRO}.xml" || :
lago collect --output "$EXPORTS"/lago-logs
-
cp "$PREFIX"/current/logs/*.log "$EXPORTS"/lago-logs
- return $failed
}
function cleanup {
@@ -124,5 +133,5 @@
lago cleanup
}
-prepare && run && cleanup
+setup_env && run && collect_logs && cleanup
exit $?
--
To view, visit https://gerrit.ovirt.org/70825
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I74f9326289b972abaef4507ab4d2582a8d641778
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybronhei(a)redhat.com>
7:29 a.m.
From Yaniv Bronhaim <ybronhei(a)redhat.com>:
Yaniv Bronhaim has posted comments on this change.
Change subject: check-merged: split functions by logic and few nits
......................................................................
Patch Set 1:
http://jenkins.ovirt.org/job/vdsm_master_check-merged-el7-x86_64/1091/
--
To view, visit https://gerrit.ovirt.org/70825
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I74f9326289b972abaef4507ab4d2582a8d641778
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Irit Goihman <igoihman(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
Gerrit-HasComments: No
7:46 a.m.
From Yaniv Bronhaim <ybronhei(a)redhat.com>:
Yaniv Bronhaim has posted comments on this change.
Change subject: check-merged: split functions by logic and few nits
......................................................................
Patch Set 1:
something went around with first run, trying again -
http://jenkins.ovirt.org/job/vdsm_master_check-merged-el7-x86_64/1093/
--
To view, visit https://gerrit.ovirt.org/70825
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I74f9326289b972abaef4507ab4d2582a8d641778
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Irit Goihman <igoihman(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
Gerrit-HasComments: No
8:07 a.m.
From Yaniv Bronhaim <ybronhei(a)redhat.com>:
Yaniv Bronhaim has posted comments on this change.
Change subject: check-merged: split functions by logic and few nits
......................................................................
Patch Set 1: Verified+1
works great
--
To view, visit https://gerrit.ovirt.org/70825
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I74f9326289b972abaef4507ab4d2582a8d641778
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Irit Goihman <igoihman(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
Gerrit-HasComments: No
8:23 a.m.
From Yaniv Bronhaim <ybronhei(a)redhat.com>:
Yaniv Bronhaim has posted comments on this change.
Change subject: check-merged: split functions by logic and few nits
......................................................................
Patch Set 2:
http://jenkins.ovirt.org/job/vdsm_master_check-merged-el7-x86_64/1096/
--
To view, visit https://gerrit.ovirt.org/70825
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I74f9326289b972abaef4507ab4d2582a8d641778
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Irit Goihman <igoihman(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
Gerrit-HasComments: No
9:25 a.m.
From Yaniv Bronhaim <ybronhei(a)redhat.com>:
Yaniv Bronhaim has posted comments on this change.
Change subject: check-merged: split functions by logic and few nits
......................................................................
Patch Set 3:
http://jenkins.ovirt.org/job/vdsm_master_check-merged-el7-x86_64/1097/
--
To view, visit https://gerrit.ovirt.org/70825
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I74f9326289b972abaef4507ab4d2582a8d641778
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Irit Goihman <igoihman(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
Gerrit-HasComments: No
5:56 a.m.
From Yaniv Bronhaim <ybronhei(a)redhat.com>:
Yaniv Bronhaim has posted comments on this change.
Change subject: check-merged: split functions by logic and few nits
......................................................................
Patch Set 3: Code-Review-1
Thanks for the review Nadav! will fix.
--
To view, visit https://gerrit.ovirt.org/70825
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I74f9326289b972abaef4507ab4d2582a8d641778
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Irit Goihman <igoihman(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nadav Goldin <ngoldin(a)redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
Gerrit-HasComments: No
9:35 a.m.
From Yaniv Bronhaim <ybronhei(a)redhat.com>:
Yaniv Bronhaim has posted comments on this change.
Change subject: check-merged: split functions by logic and few nits
......................................................................
Patch Set 4:
(1 comment)
http://jenkins.ovirt.org/job/vdsm_master_check-merged-el7-x86_64/1112/
https://gerrit.ovirt.org/#/c/70825/3/automation/check-merged.sh
File automation/check-merged.sh:
Line 116:
Line 117: run_network_tests | tee -a
"$EXPORTS/functional_tests_stdout.$DISTRO.log"
Line 118: res="${PIPESTATUS[0]}"
Line 119: [ "$res" -ne 0 ] && failed="$res"
Line 120: }
Lago supports configuring directories to collect logs from in the
init file
this will be done separately to this patch
Line 121:
Line 122: function collect_logs {
Line 123: lago copy-from-vm "$VM_NAME" \
Line 124: "/tmp/nosetests-${DISTRO}.xml" \
--
To view, visit https://gerrit.ovirt.org/70825
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I74f9326289b972abaef4507ab4d2582a8d641778
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Irit Goihman <igoihman(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nadav Goldin <ngoldin(a)redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
Gerrit-HasComments: Yes
6:23 a.m.
From Yaniv Bronhaim <ybronhei(a)redhat.com>:
Yaniv Bronhaim has posted comments on this change.
Change subject: check-merged: split functions by logic and few nits
......................................................................
Patch Set 6:
http://jenkins.ovirt.org/job/vdsm_master_check-merged-el7-x86_64/1118/
--
To view, visit https://gerrit.ovirt.org/70825
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I74f9326289b972abaef4507ab4d2582a8d641778
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Irit Goihman <igoihman(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Leon Goldberg <lgoldber(a)redhat.com>
Gerrit-Reviewer: Nadav Goldin <ngoldin(a)redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
Gerrit-HasComments: No
7:56 a.m.
From Yaniv Bronhaim <ybronhei(a)redhat.com>:
Yaniv Bronhaim has posted comments on this change.
Change subject: check-merged: split functions by logic and few nits
......................................................................
Patch Set 7:
http://jenkins.ovirt.org/job/vdsm_master_check-merged-el7-x86_64/1119/
--
To view, visit https://gerrit.ovirt.org/70825
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I74f9326289b972abaef4507ab4d2582a8d641778
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Irit Goihman <igoihman(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Leon Goldberg <lgoldber(a)redhat.com>
Gerrit-Reviewer: Nadav Goldin <ngoldin(a)redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
Gerrit-HasComments: No
6:25 a.m.
From Dan Kenigsberg <danken(a)redhat.com>:
Dan Kenigsberg has posted comments on this change.
Change subject: check-merged: split functions by logic and few nits
......................................................................
Patch Set 9: Code-Review-1
(3 comments)
https://gerrit.ovirt.org/#/c/70825/9/automation/check-merged.sh
File automation/check-merged.sh:
PS9, Line 9: Fix when running in an el* chroot in fc2* host
I don't understand this comment.
is this a fix? or should we fix this?
Line 125: lago stop "$VM_NAME"
Line 126: lago cleanup
Line 127: }
Line 128:
Line 129: function quit {
it feels like what you need is
trap collect_and_clean EXIT
Line 130: collect_logs
Line 131: cleanup
Line 132: exit $0
Line 133: }
PS9, Line 136: if [ $? -ne 0 ]; then
: echo 'check-merged.sh: ERROR during setup_env'
: quit 1
: fi
does this work? the script is running with -e, so the script would explode on the very
first non-zero return value, and would never get to here.
--
To view, visit https://gerrit.ovirt.org/70825
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I74f9326289b972abaef4507ab4d2582a8d641778
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Irit Goihman <igoihman(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Leon Goldberg <lgoldber(a)redhat.com>
Gerrit-Reviewer: Nadav Goldin <ngoldin(a)redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
Gerrit-HasComments: Yes
9:15 a.m.
From Yaniv Bronhaim <ybronhei(a)redhat.com>:
Yaniv Bronhaim has posted comments on this change.
Change subject: check-merged: split functions by logic and few nits
......................................................................
Patch Set 9:
(1 comment)
https://gerrit.ovirt.org/#/c/70825/9/automation/check-merged.sh
File automation/check-merged.sh:
PS9, Line 9: Fix when running in an el* chroot in fc2* host
I don't understand this comment.
:) good question. the next
2 lines are the fix which is required when the script is running over fedora>20 hosts
with el* chroot
--
To view, visit https://gerrit.ovirt.org/70825
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I74f9326289b972abaef4507ab4d2582a8d641778
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Irit Goihman <igoihman(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Leon Goldberg <lgoldber(a)redhat.com>
Gerrit-Reviewer: Nadav Goldin <ngoldin(a)redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
Gerrit-HasComments: Yes
10:10 a.m.
From Yaniv Bronhaim <ybronhei(a)redhat.com>:
Yaniv Bronhaim has posted comments on this change.
Change subject: check-merged: split functions by logic and few nits
......................................................................
Patch Set 9:
(1 comment)
https://gerrit.ovirt.org/#/c/70825/9/automation/check-merged.sh
File automation/check-merged.sh:
PS9, Line 136: if [ $? -ne 0 ]; then
: echo 'check-merged.sh: ERROR during setup_env'
: quit 1
: fi
does this work? the script is running with -e, so the script would
explode
so it supports also without -e run .. why is it bad?
--
To view, visit https://gerrit.ovirt.org/70825
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I74f9326289b972abaef4507ab4d2582a8d641778
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Irit Goihman <igoihman(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Leon Goldberg <lgoldber(a)redhat.com>
Gerrit-Reviewer: Nadav Goldin <ngoldin(a)redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
Gerrit-HasComments: Yes
5:55 a.m.
From Dan Kenigsberg <danken(a)redhat.com>:
Dan Kenigsberg has posted comments on this change.
Change subject: check-merged: split functions by logic and few nits
......................................................................
Patch Set 11: Code-Review-1
(5 comments)
https://gerrit.ovirt.org/#/c/70825/11/automation/check-merged.sh
File automation/check-merged.sh:
Line 5: # ensure /dev/kvm exists, otherwise it will still use
Line 6: # direct backend, but without KVM(much slower).
Line 7: ! [[ -c "/dev/kvm" ]] && mknod /dev/kvm c 10 232
Line 8:
Line 9: # Fix when running in an el* chroot in fc2* host
comment still not readable
Line 10: [[ -e /usr/bin/qemu-kvm ]] \
Line 11: || ln -s /usr/libexec/qemu-kvm /usr/bin/qemu-kvm
Line 12:
Line 13: # ENV vars
PS11, Line 96: ret
other functions call it res
PS11, Line 110: failed
why not stay with "res"?
PS11, Line 113: res
always declare as local
PS11, Line 138: if [ $? -ne 0 ]; then
: echo 'check-merged.sh: ERROR during setup_env'
: exit 1
: fi
again, with -e, this could would never be reached.
--
To view, visit https://gerrit.ovirt.org/70825
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I74f9326289b972abaef4507ab4d2582a8d641778
Gerrit-PatchSet: 11
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Irit Goihman <igoihman(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Leon Goldberg <lgoldber(a)redhat.com>
Gerrit-Reviewer: Nadav Goldin <ngoldin(a)redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
Gerrit-HasComments: Yes
8:04 a.m.
From Yaniv Bronhaim <ybronhei(a)redhat.com>:
Yaniv Bronhaim has posted comments on this change.
Change subject: check-merged: split functions by logic and few nits
......................................................................
Patch Set 12: Verified+1
please review - the patch is ready
http://jenkins.ovirt.org/job/vdsm_master_check-merged-el7-x86_64/1373/con...
--
To view, visit https://gerrit.ovirt.org/70825
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I74f9326289b972abaef4507ab4d2582a8d641778
Gerrit-PatchSet: 12
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Irit Goihman <igoihman(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Leon Goldberg <lgoldber(a)redhat.com>
Gerrit-Reviewer: Nadav Goldin <ngoldin(a)redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
Gerrit-HasComments: No
9:51 a.m.
From Dan Kenigsberg <danken(a)redhat.com>:
Dan Kenigsberg has posted comments on this change.
Change subject: check-merged: split functions by logic and few nits
......................................................................
Patch Set 12: Code-Review-1
please see comments to ps11. they apply to ps12 too.
--
To view, visit https://gerrit.ovirt.org/70825
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I74f9326289b972abaef4507ab4d2582a8d641778
Gerrit-PatchSet: 12
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Irit Goihman <igoihman(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Leon Goldberg <lgoldber(a)redhat.com>
Gerrit-Reviewer: Nadav Goldin <ngoldin(a)redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
Gerrit-HasComments: No
2:09 a.m.
From Yaniv Bronhaim <ybronhei(a)redhat.com>:
Yaniv Bronhaim has posted comments on this change.
Change subject: check-merged: split functions by logic and few nits
......................................................................
Patch Set 11:
(2 comments)
https://gerrit.ovirt.org/#/c/70825/11/automation/check-merged.sh
File automation/check-merged.sh:
Line 5: # ensure /dev/kvm exists, otherwise it will still use
Line 6: # direct backend, but without KVM(much slower).
Line 7: ! [[ -c "/dev/kvm" ]] && mknod /dev/kvm c 10 232
Line 8:
Line 9: # Fix when running in an el* chroot in fc2* host
comment still not readable
its not related to the patch
though...
Line 10: [[ -e /usr/bin/qemu-kvm ]] \
Line 11: || ln -s /usr/libexec/qemu-kvm /usr/bin/qemu-kvm
Line 12:
Line 13: # ENV vars
PS11, Line 138: if [ $? -ne 0 ]; then
: echo 'check-merged.sh: ERROR during setup_env'
: exit 1
: fi
again, with -e, this could would never be reached.
I removed
the -e
--
To view, visit https://gerrit.ovirt.org/70825
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I74f9326289b972abaef4507ab4d2582a8d641778
Gerrit-PatchSet: 11
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Irit Goihman <igoihman(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Leon Goldberg <lgoldber(a)redhat.com>
Gerrit-Reviewer: Nadav Goldin <ngoldin(a)redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
Gerrit-HasComments: Yes
4:06 a.m.
From Yaniv Bronhaim <ybronhei(a)redhat.com>:
Yaniv Bronhaim has posted comments on this change.
Change subject: check-merged: split functions by logic and few nits
......................................................................
Patch Set 13:
http://jenkins.ovirt.org/job/vdsm_master_check-merged-el7-x86_64/1378/
--
To view, visit https://gerrit.ovirt.org/70825
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I74f9326289b972abaef4507ab4d2582a8d641778
Gerrit-PatchSet: 13
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Irit Goihman <igoihman(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Leon Goldberg <lgoldber(a)redhat.com>
Gerrit-Reviewer: Nadav Goldin <ngoldin(a)redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
Gerrit-HasComments: No
5:07 a.m.
From Yaniv Bronhaim <ybronhei(a)redhat.com>:
Yaniv Bronhaim has posted comments on this change.
Change subject: check-merged: split functions by logic and few nits
......................................................................
Patch Set 14: Verified+1
--
To view, visit https://gerrit.ovirt.org/70825
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I74f9326289b972abaef4507ab4d2582a8d641778
Gerrit-PatchSet: 14
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Irit Goihman <igoihman(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Leon Goldberg <lgoldber(a)redhat.com>
Gerrit-Reviewer: Nadav Goldin <ngoldin(a)redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
Gerrit-HasComments: No
11:38 a.m.
From Dan Kenigsberg <danken(a)redhat.com>:
Dan Kenigsberg has posted comments on this change.
Change subject: check-merged: split functions by logic and few nits
......................................................................
Patch Set 14: Code-Review-1
(1 comment)
https://gerrit.ovirt.org/#/c/70825/14/automation/check-merged.sh
File automation/check-merged.sh:
Line 1: #!/bin/bash -x
why? running a bash script with no -e is a bad practice.
it means that we would never notice a failure to run a command - processing would just go
on.
Line 2:
Line 3: export LIBGUESTFS_BACKEND=direct
Line 4:
Line 5: # ensure /dev/kvm exists, otherwise it will still use
--
To view, visit https://gerrit.ovirt.org/70825
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I74f9326289b972abaef4507ab4d2582a8d641778
Gerrit-PatchSet: 14
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Irit Goihman <igoihman(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Leon Goldberg <lgoldber(a)redhat.com>
Gerrit-Reviewer: Nadav Goldin <ngoldin(a)redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
Gerrit-HasComments: Yes
5:14 a.m.
From Yaniv Bronhaim <ybronhei(a)redhat.com>:
Yaniv Bronhaim has posted comments on this change.
Change subject: check-merged: split functions by logic and few nits
......................................................................
Patch Set 14:
(1 comment)
https://gerrit.ovirt.org/#/c/70825/14/automation/check-merged.sh
File automation/check-merged.sh:
Line 1: #!/bin/bash -x
why? running a bash script with no -e is a bad practice.
I
know, that's how I prefer to run this script, otherwise it exits and its hard to
understand why so for me its easier to flow my echos
Line 2:
Line 3: export LIBGUESTFS_BACKEND=direct
Line 4:
Line 5: # ensure /dev/kvm exists, otherwise it will still use
--
To view, visit https://gerrit.ovirt.org/70825
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I74f9326289b972abaef4507ab4d2582a8d641778
Gerrit-PatchSet: 14
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Irit Goihman <igoihman(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Leon Goldberg <lgoldber(a)redhat.com>
Gerrit-Reviewer: Nadav Goldin <ngoldin(a)redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
Gerrit-HasComments: Yes
5:58 a.m.
From Dan Kenigsberg <danken(a)redhat.com>:
Dan Kenigsberg has posted comments on this change.
Change subject: check-merged: split functions by logic and few nits
......................................................................
Patch Set 14:
(1 comment)
https://gerrit.ovirt.org/#/c/70825/14/automation/check-merged.sh
File automation/check-merged.sh:
Line 1: #!/bin/bash -x
I know, that's how I prefer to run this script, otherwise it
exits and its
Running bash without -e is just like writing Python with
try:
statement
except:
pass
around each statement. It's a great way to quickly generate code with undetected
bugs.
non-trivial code must not use it.
Line 2:
Line 3: export LIBGUESTFS_BACKEND=direct
Line 4:
Line 5: # ensure /dev/kvm exists, otherwise it will still use
--
To view, visit https://gerrit.ovirt.org/70825
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I74f9326289b972abaef4507ab4d2582a8d641778
Gerrit-PatchSet: 14
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Irit Goihman <igoihman(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Leon Goldberg <lgoldber(a)redhat.com>
Gerrit-Reviewer: Nadav Goldin <ngoldin(a)redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
Gerrit-HasComments: Yes
6:09 a.m.
From Yaniv Bronhaim <ybronhei(a)redhat.com>:
Yaniv Bronhaim has posted comments on this change.
Change subject: check-merged: split functions by logic and few nits
......................................................................
Patch Set 15:
http://jenkins.ovirt.org/job/vdsm_master_check-merged-el7-x86_64/1394/
--
To view, visit https://gerrit.ovirt.org/70825
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I74f9326289b972abaef4507ab4d2582a8d641778
Gerrit-PatchSet: 15
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Irit Goihman <igoihman(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Leon Goldberg <lgoldber(a)redhat.com>
Gerrit-Reviewer: Nadav Goldin <ngoldin(a)redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
Gerrit-HasComments: No
6:55 a.m.
From Yaniv Bronhaim <ybronhei(a)redhat.com>:
Yaniv Bronhaim has posted comments on this change.
Change subject: check-merged: split functions by logic and few nits
......................................................................
Patch Set 15: Code-Review+1
--
To view, visit https://gerrit.ovirt.org/70825
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I74f9326289b972abaef4507ab4d2582a8d641778
Gerrit-PatchSet: 15
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Irit Goihman <igoihman(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Leon Goldberg <lgoldber(a)redhat.com>
Gerrit-Reviewer: Nadav Goldin <ngoldin(a)redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
Gerrit-HasComments: No
9:53 a.m.
From Yaniv Bronhaim <ybronhei(a)redhat.com>:
Yaniv Bronhaim has posted comments on this change.
Change subject: check-merged: split functions by logic and few nits
......................................................................
Patch Set 15: Verified+1
please review again.. I think its ready
--
To view, visit https://gerrit.ovirt.org/70825
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I74f9326289b972abaef4507ab4d2582a8d641778
Gerrit-PatchSet: 15
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Irit Goihman <igoihman(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Leon Goldberg <lgoldber(a)redhat.com>
Gerrit-Reviewer: Nadav Goldin <ngoldin(a)redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
Gerrit-HasComments: No
5:19 a.m.
From Dan Kenigsberg <danken(a)redhat.com>:
Dan Kenigsberg has posted comments on this change.
Change subject: check-merged: split functions by logic and few nits
......................................................................
Patch Set 15: Code-Review+2
--
To view, visit https://gerrit.ovirt.org/70825
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I74f9326289b972abaef4507ab4d2582a8d641778
Gerrit-PatchSet: 15
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Irit Goihman <igoihman(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Leon Goldberg <lgoldber(a)redhat.com>
Gerrit-Reviewer: Nadav Goldin <ngoldin(a)redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
Gerrit-HasComments: No
5:19 a.m.
From Dan Kenigsberg <danken(a)redhat.com>:
Dan Kenigsberg has submitted this change and it was merged.
Change subject: check-merged: split functions by logic and few nits
......................................................................
check-merged: split functions by logic and few nits
Change-Id: I74f9326289b972abaef4507ab4d2582a8d641778
Signed-off-by: Yaniv Bronhaim <ybronhei(a)redhat.com>
---
M automation/check-merged.sh
1 file changed, 48 insertions(+), 35 deletions(-)
Approvals:
Piotr Kliczewski: Looks good to me, but someone else must approve
Yaniv Bronhaim: Verified; Looks good to me, but someone else must approve
Jenkins CI: Passed CI tests
Dan Kenigsberg: Looks good to me, approved
Leon Goldberg: Looks good to me, but someone else must approve
--
To view, visit https://gerrit.ovirt.org/70825
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I74f9326289b972abaef4507ab4d2582a8d641778
Gerrit-PatchSet: 16
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Irit Goihman <igoihman(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Leon Goldberg <lgoldber(a)redhat.com>
Gerrit-Reviewer: Nadav Goldin <ngoldin(a)redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
2584
days inactive
2654
days old
vdsm-patches@lists.fedorahosted.org
26 comments
1 participants
participants (1)
-
Code Review