Change in vdsm[master]: Restart needed services after reconfigure libvirt

Yaniv Bronhaim has uploaded a new change for review.

Change subject: Restart needed services after reconfigure libvirt
......................................................................

Restart needed services after reconfigure libvirt

Without starting libvirtd service after reconfigure, nwfilter fails
to run.

Change-Id: Icf70a749454ea341d5b52220e16e9567b90431a0
Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1013371
Signed-off-by: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
---
M init/vdsmd_init_common.sh.in
1 file changed, 2 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/37/19737/1

diff --git a/init/vdsmd_init_common.sh.in b/init/vdsmd_init_common.sh.in
index 38da542..5c8895c 100644
--- a/init/vdsmd_init_common.sh.in
+++ b/init/vdsmd_init_common.sh.in
@@ -55,7 +55,8 @@
 
 
 task_reconfigure_libvirt(){
-    "$VDSM_TOOL" libvirt-configure
+    "$VDSM_TOOL" libvirt-configure &&
+        "$VDSM_TOOL" libvirt-configure-services-restart
 }
 
 


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Icf70a749454ea341d5b52220e16e9567b90431a0
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;

Change in vdsm[master]: Removing libvirt_configure from pre-start and ask for manual...

[Dan Kenigsberg]

Dan Kenigsberg has submitted this change and it was merged.

Change subject: Removing libvirt_configure from pre-start and ask for manual run
......................................................................


Removing libvirt_configure from pre-start and ask for manual run

Before starting vdsmd we need to add vdsm configuration to
libvirtd.conf. This part is performed by vdsm-tool libvirt_configure
verb. Currently we call it as part of init script pre-start function,
but doing that must follow restart of depended services as libvirtd. We
don't want to restart services as part of vdsmd init script, so this
patch detach the auto reconfigure and report to user to perform it
manually if attempt to start vdsmd without having libvirtd.conf
configured.
Performing start\stop actions on external services during service start
is invalid and can lead to bugs in different service managers.

Change-Id: Icf70a749454ea341d5b52220e16e9567b90431a0
Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1013371
Signed-off-by: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Reviewed-on: http://gerrit.ovirt.org/19737
Reviewed-by: Dan Kenigsberg <danken(a)redhat.com&gt;
---
M init/vdsmd_init_common.sh.in
M lib/vdsm/tool/libvirt_configure.py
M lib/vdsm/tool/libvirt_configure.sh.in
3 files changed, 70 insertions(+), 31 deletions(-)

Approvals:
  Yaniv Bronhaim: Verified
  Dan Kenigsberg: Looks good to me, approved



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

Gerrit-MessageType: merged
Gerrit-Change-Id: Icf70a749454ea341d5b52220e16e9567b90431a0
Gerrit-PatchSet: 12
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: Alon Bar-Lev <alonbl(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server

Change in vdsm[master]: Removing libvirt_configure from pre-start and ask for manual...

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: Removing libvirt_configure from pre-start and ask for manual run
......................................................................


Patch Set 11: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Icf70a749454ea341d5b52220e16e9567b90431a0
Gerrit-PatchSet: 11
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: Alon Bar-Lev <alonbl(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

Change in vdsm[master]: Removing libvirt_configure from pre-start and ask for manual...

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: Removing libvirt_configure from pre-start and ask for manual run
......................................................................


Patch Set 9:

(1 comment)

....................................................
File lib/vdsm/tool/libvirt_configure.sh.in
Line 151: 
Line 152: is_already_configured() {
Line 153:     local rc=0
Line 154:     while [ -n "$1" ]; do
Line 155:         if not grep -q "${BY_VDSM_VERS}" "$1"; then
that was a close one!
Line 156:             rc=1
Line 157:             break
Line 158:         fi
Line 159:         shift


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Icf70a749454ea341d5b52220e16e9567b90431a0
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: Alon Bar-Lev <alonbl(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes

Change in vdsm[master]: Removing libvirt_configure from pre-start and ask for manual...

Yaniv Bronhaim has posted comments on this change.

Change subject: Removing libvirt_configure from pre-start and ask for manual run
......................................................................


Patch Set 11: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Icf70a749454ea341d5b52220e16e9567b90431a0
Gerrit-PatchSet: 11
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: Alon Bar-Lev <alonbl(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

Change in vdsm[master]: Removing libvirt_configure from pre-start and ask for manual...

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Removing libvirt_configure from pre-start and ask for manual run
......................................................................


Patch Set 11:

Build Successful 

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/4892/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/4007/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/4817/ : SUCCESS

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Icf70a749454ea341d5b52220e16e9567b90431a0
Gerrit-PatchSet: 11
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: Alon Bar-Lev <alonbl(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

Change in vdsm[master]: Removing libvirt_configure from pre-start and ask for manual...

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Removing libvirt_configure from pre-start and ask for manual run
......................................................................


Patch Set 10:

Build Successful 

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/4801/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/4877/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/3992/ : SUCCESS

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Icf70a749454ea341d5b52220e16e9567b90431a0
Gerrit-PatchSet: 10
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: Alon Bar-Lev <alonbl(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

Change in vdsm[master]: Removing libvirt_configure from pre-start and ask for manual...

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Removing libvirt_configure from pre-start and ask for manual run
......................................................................


Patch Set 9:

Build Successful 

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/4795/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/4871/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/3986/ : SUCCESS

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Icf70a749454ea341d5b52220e16e9567b90431a0
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: Alon Bar-Lev <alonbl(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

Change in vdsm[master]: Removing libvirt_configure from pre-start and ask for manual...

[Alon Bar-Lev]

Alon Bar-Lev has posted comments on this change.

Change subject: Removing libvirt_configure from pre-start and ask for manual run
......................................................................


Patch Set 8: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Icf70a749454ea341d5b52220e16e9567b90431a0
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: Alon Bar-Lev <alonbl(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

Change in vdsm[master]: Removing libvirt_configure from pre-start and ask for manual...

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Removing libvirt_configure from pre-start and ask for manual run
......................................................................


Patch Set 8:

Build Successful 

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/4787/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/4863/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/3978/ : SUCCESS

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Icf70a749454ea341d5b52220e16e9567b90431a0
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: Alon Bar-Lev <alonbl(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

Change in vdsm[master]: Removing libvirt_configure from pre-start and ask for manual...

[Alon Bar-Lev]

Alon Bar-Lev has posted comments on this change.

Change subject: Removing libvirt_configure from pre-start and ask for manual run
......................................................................


Patch Set 7: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Icf70a749454ea341d5b52220e16e9567b90431a0
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: Alon Bar-Lev <alonbl(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

Change in vdsm[master]: Removing libvirt_configure from pre-start and ask for manual...

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Removing libvirt_configure from pre-start and ask for manual run
......................................................................


Patch Set 7:

Build Successful 

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/4769/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/4845/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/3960/ : SUCCESS

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Icf70a749454ea341d5b52220e16e9567b90431a0
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: Alon Bar-Lev <alonbl(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

Change in vdsm[master]: Removing libvirt_configure from pre-start and ask for manual...

[Alon Bar-Lev]

Alon Bar-Lev has posted comments on this change.

Change subject: Removing libvirt_configure from pre-start and ask for manual run
......................................................................


Patch Set 6:

(2 comments)

....................................................
File init/vdsmd_init_common.sh.in
Line 219:             "
Line 220:         ;;
Line 221:     --post-stop)
Line 222:             run_tasks " \
Line 223:             run_final_hooks \
indent missing
Line 224:             "
Line 225:         ;;
Line 226:     --help|*)
Line 227:         usage


....................................................
File lib/vdsm/tool/libvirt_configure.sh.in
Line 199:     #
Line 200:     if [ "${force_reconfigure}" != "--force" ] && \
Line 201:       ! [ -f "${FORCE_RECONFIGURE}" ] && \
Line 202:       is_already_configured "${lconf}" "${qconf}"
"${ldconf}" "${qlconf}"; then
Line 203:         echo "Not forcing reconfigure"
wrong indent?
Line 204:         return 0
Line 205:     fi
Line 206: 
Line 207:     # Remove a previous configuration (if present)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Icf70a749454ea341d5b52220e16e9567b90431a0
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: Alon Bar-Lev <alonbl(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes

Yaniv Bronhaim has posted comments on this change.

Change subject: Restart needed services after reconfigure libvirt
......................................................................


Patch Set 1:

Well, following the attached bug says we decide to remove the reconfigure_libvirt part
from --pre-start. Any comments about that?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Icf70a749454ea341d5b52220e16e9567b90431a0
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: Alon Bar-Lev <alonbl(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

Yaniv Bronhaim has posted comments on this change.

Change subject: Restart needed services after reconfigure libvirt
......................................................................


Patch Set 1:

(1 comment)

....................................................
File init/vdsmd_init_common.sh.in
Line 55
Line 56
Line 57
Line 58
Line 59
what do you mean? its part of vdsmd_init_common.sh --pre-start and it is used also over
systemd and upstart. It must be called from here


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Icf70a749454ea341d5b52220e16e9567b90431a0
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: Alon Bar-Lev <alonbl(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes

[Alon Bar-Lev]

Alon Bar-Lev has posted comments on this change.

Change subject: Restart needed services after reconfigure libvirt
......................................................................


Patch Set 1:

(1 comment)

....................................................
File init/vdsmd_init_common.sh.in
Line 55
Line 56
Line 57
Line 58
Line 59
I thought we removed this from the common, it should not be used anywhere, and should be
removed.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Icf70a749454ea341d5b52220e16e9567b90431a0
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: Alon Bar-Lev <alonbl(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes

Yaniv Bronhaim has posted comments on this change.

Change subject: Restart needed services after reconfigure libvirt
......................................................................


Patch Set 1: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Icf70a749454ea341d5b52220e16e9567b90431a0
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: Alon Bar-Lev <alonbl(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

Change in vdsm[master]: Removing libvirt_configure from pre-start and ask for manual...

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Removing libvirt_configure from pre-start and ask for manual run
......................................................................


Patch Set 6:

Build Successful 

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/4730/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/4806/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/3921/ : SUCCESS

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Icf70a749454ea341d5b52220e16e9567b90431a0
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: Alon Bar-Lev <alonbl(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

Change in vdsm[master]: Removing libvirt_configure from pre-start and ask for manual...

[Alon Bar-Lev]

Alon Bar-Lev has posted comments on this change.

Change subject: Removing libvirt_configure from pre-start and ask for manual run
......................................................................


Patch Set 4:

(4 comments)

....................................................
File init/sysvinit/vdsmd.init.in
Line 126
Line 127
Line 128
Line 129
Line 130
I think the above should be at the pre-start... not sure why was it left here.

export DAEMON_COREFILE_LIMIT=unlimited can be called always be added to bellow daemon
command, and dropped at the daemonAdapter as the vdsm_nice. At systemd there is LimitCORE
that should be added to unit.


Line 128
Line 129
Line 130
Line 131
Line 132
I would have added || return 1 here as well to be consistent making
shutdown_conflicting_srv return fail if something really goes wrong.


Line 130
Line 131
Line 132
Line 133
Line 134
I would have added || return 1 here as well to be consistent... making sure that the
start_needed_srv will return fail if something really goes wrong.


....................................................
File init/vdsmd_init_common.sh.in
Line 220:         ;;
Line 221:     --post-stop)
Line 222:         run_tasks " \
Line 223: 	    run_final_hooks \
Line 224: 	    "
tabs, can we just convert the file to use tabs?
Line 225:         ;;
Line 226:     --help|*)
Line 227:         usage
Line 228:         exit 1


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Icf70a749454ea341d5b52220e16e9567b90431a0
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: Alon Bar-Lev <alonbl(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes

Change in vdsm[master]: Removing libvirt_configure from pre-start and ask for manual...

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Removing libvirt_configure from pre-start and ask for manual run
......................................................................


Patch Set 4:

Build Successful 

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/4710/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/4786/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/3901/ : SUCCESS

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Icf70a749454ea341d5b52220e16e9567b90431a0
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: Alon Bar-Lev <alonbl(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

Change in vdsm[master]: Removing libvirt_configure from pre-start and ask for manual...

[Alon Bar-Lev]

Alon Bar-Lev has posted comments on this change.

Change subject: Removing libvirt_configure from pre-start and ask for manual run
......................................................................


Patch Set 2:

(2 comments)

....................................................
File init/sysvinit/vdsmd.init.in
Line 140:         -0 /dev/null -1 /dev/null -2 /dev/null --syslog
"@VDSMDIR@/respawn" --minlifetime 10 \
Line 141:         --daemon --masterpid "${RESPAWNPIDFILE}"
"${VDSM_BIN}" --pidfile "${PIDFILE}"
Line 142:     RETVAL=$?
Line 143:     [ "$RETVAL" = 0 ] && touch "${LOCK_FILE}"
Line 144:     return "$RETVAL"
using this retval is the ugliest approach :))))

 local RETVAL=1
 if "${VDSMD_INIT_COMMON}" --pre-start; then
     echo $"Starting up vdsm daemon: "
     if NICELEVEL="${NICE_LOWEST}" daemon --user=vdsm
"@VDSMDIR@/daemonAdapter" \
   ...; then
         touch "${LOCK_FILE}"
         RETVAL=0
 fi
 return "${RETVAL}"

or... use the "|| return 1" structure...
Line 145: }
Line 146: 
Line 147: stop() {
Line 148:     echo $"Shutting down vdsm daemon: "


....................................................
File init/vdsmd_init_common.sh.in
Line 207:         run_tasks " \
Line 208:             run_init_hooks gencerts check_libvirt_configure \
Line 209:             reconfigure_sanlock syslog_available nwfilter dummybr \
Line 210:             load_needed_modules tune_system mkdirs test_space test_lo \
Line 211:             test_conflicting_conf"
please move closing quotes to new line as well to allow adding last.
Line 212:         ;;
Line 213:     --post-stop)
Line 214:         run_tasks "run_final_hooks"
Line 215:         ;;


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Icf70a749454ea341d5b52220e16e9567b90431a0
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: Alon Bar-Lev <alonbl(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes

Change in vdsm[master]: Removing libvirt_configure from pre-start and ask for manual...

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Removing libvirt_configure from pre-start and ask for manual run
......................................................................


Patch Set 3:

Build Successful 

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/4684/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/4760/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/3875/ : SUCCESS

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Icf70a749454ea341d5b52220e16e9567b90431a0
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: Alon Bar-Lev <alonbl(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

Change in vdsm[master]: Removing libvirt_configure from pre-start and ask for manual...

Yaniv Bronhaim has posted comments on this change.

Change subject: Removing libvirt_configure from pre-start and ask for manual run
......................................................................


Patch Set 2:

(2 comments)

Thanks!

....................................................
File init/vdsmd_init_common.sh.in
Line 205: case "$1" in
Line 206:     --pre-start)
Line 207:         run_tasks " \
Line 208:             run_init_hooks gencerts check_libvirt_configure \
Line 209:             reconfigure_sanlock syslog_available nwfilter dummybr \
host-deploy uses vdsm-tool to reconfigure that does only the libvirt part. I intend to do
it separately to this patch
Line 210:             load_needed_modules tune_system mkdirs test_space test_lo \
Line 211:             test_conflicting_conf"
Line 212:         ;;
Line 213:     --post-stop)


....................................................
File lib/vdsm/tool/libvirt_configure.sh.in
Line 196:     # - not configured
Line 197:     #
Line 198:     if [ "${force_reconfigure}" != "--force" ] && \
Line 199:       ! [ -f "${FORCE_RECONFIGURE}" ] && \
Line 200:       is_already_configured "${lconf}" "${qconf}"
"${ldconf}" "${glconf}"; then
could be a mess.. thanks for catching
Line 201:         echo "Not forcing reconfigure"
Line 202:         return 0
Line 203:     fi
Line 204: 


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Icf70a749454ea341d5b52220e16e9567b90431a0
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: Alon Bar-Lev <alonbl(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes

Change in vdsm[master]: Removing libvirt_configure from pre-start and ask for manual...

[Alon Bar-Lev]

Alon Bar-Lev has posted comments on this change.

Change subject: Removing libvirt_configure from pre-start and ask for manual run
......................................................................


Patch Set 2:

(3 comments)

....................................................
File init/sysvinit/vdsmd.init.in
Line 137
Line 138
Line 139
Line 140
Line 141
please modify:

 xxxxx || return 1
 touch "${LOCK_FILE}"
 return 0

at least be consistent... :)


Line 132:     shutdown_conflicting_srv "${CONFLICTING_SERVICES}"
Line 133: 
Line 134:     start_needed_srv "${NEEDED_SERVICES}"
Line 135: 
Line 136:     "${VDSMD_INIT_COMMON}" --pre-start || return 1
your code...

if you want to cleanup on function return, or add proper logging this structure is
unsuitable.

but again, your code.
Line 137: 
Line 138:     echo $"Starting up vdsm daemon: "
Line 139:     NICELEVEL="${NICE_LOWEST}" daemon --user=vdsm
"@VDSMDIR@/daemonAdapter" \
Line 140:         -0 /dev/null -1 /dev/null -2 /dev/null --syslog
"@VDSMDIR@/respawn" --minlifetime 10 \


....................................................
File init/vdsmd_init_common.sh.in
Line 205: case "$1" in
Line 206:     --pre-start)
Line 207:         run_tasks " \
Line 208:             run_init_hooks gencerts check_libvirt_configure \
Line 209:             reconfigure_sanlock syslog_available nwfilter dummybr \
not exactly, as nothing actually configure it... I mean, host deploy does not.
Line 210:             load_needed_modules tune_system mkdirs test_space test_lo \
Line 211:             test_conflicting_conf"
Line 212:         ;;
Line 213:     --post-stop)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Icf70a749454ea341d5b52220e16e9567b90431a0
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: Alon Bar-Lev <alonbl(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes

Change in vdsm[master]: Removing libvirt_configure from pre-start and ask for manual...

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: Removing libvirt_configure from pre-start and ask for manual run
......................................................................


Patch Set 2:

(3 comments)

....................................................
File init/sysvinit/vdsmd.init.in
Line 132:     shutdown_conflicting_srv "${CONFLICTING_SERVICES}"
Line 133: 
Line 134:     start_needed_srv "${NEEDED_SERVICES}"
Line 135: 
Line 136:     "${VDSMD_INIT_COMMON}" --pre-start || return 1
The alternative, of putting the following lines in an "else" block, is not
better imo.
Line 137: 
Line 138:     echo $"Starting up vdsm daemon: "
Line 139:     NICELEVEL="${NICE_LOWEST}" daemon --user=vdsm
"@VDSMDIR@/daemonAdapter" \
Line 140:         -0 /dev/null -1 /dev/null -2 /dev/null --syslog
"@VDSMDIR@/respawn" --minlifetime 10 \


....................................................
File init/vdsmd_init_common.sh.in
Line 205: case "$1" in
Line 206:     --pre-start)
Line 207:         run_tasks " \
Line 208:             run_init_hooks gencerts check_libvirt_configure \
Line 209:             reconfigure_sanlock syslog_available nwfilter dummybr \
Usually I love multiple patches, but the problem with reconfigure_sanlock is the same,
right? Let's fix it in this patch.
Line 210:             load_needed_modules tune_system mkdirs test_space test_lo \
Line 211:             test_conflicting_conf"
Line 212:         ;;
Line 213:     --post-stop)


....................................................
File lib/vdsm/tool/libvirt_configure.sh.in
Line 196:     # - not configured
Line 197:     #
Line 198:     if [ "${force_reconfigure}" != "--force" ] && \
Line 199:       ! [ -f "${FORCE_RECONFIGURE}" ] && \
Line 200:       is_already_configured "${lconf}" "${qconf}"
"${ldconf}" "${glconf}"; then
q became g?
Line 201:         echo "Not forcing reconfigure"
Line 202:         return 0
Line 203:     fi
Line 204: 


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Icf70a749454ea341d5b52220e16e9567b90431a0
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: Alon Bar-Lev <alonbl(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes

Change in vdsm[master]: Removing libvirt_configure from pre-start and ask for manual...

Yaniv Bronhaim has posted comments on this change.

Change subject: Removing libvirt_configure from pre-start and ask for manual run
......................................................................


Patch Set 2:

(1 comment)

....................................................
File lib/vdsm/tool/libvirt_configure.sh.in
Line 329:         test_conflicting_conf "$LCONF" "$QCONF"
Line 330:         RETVAL=$?
Line 331:         ;;
Line 332:      check_if_configured)
Line 333: 	is_already_configured "$LCONF" "$QCONF" "$LDCONF"
"$QLCONF"
errr.. I'll omit the tabs next ps
Line 334:         RETVAL=$?
Line 335: 	;;
Line 336:      *)
Line 337:         echo "Usage: $0 {reconfigure *conf-files [--force] |
test_conflict_configurations *conf-files | check_if_configured}"


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Icf70a749454ea341d5b52220e16e9567b90431a0
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: Alon Bar-Lev <alonbl(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Restart needed services after reconfigure libvirt
......................................................................


Patch Set 1:

Build Successful 

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/3838/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/4723/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/4647/ : SUCCESS

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Icf70a749454ea341d5b52220e16e9567b90431a0
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: Alon Bar-Lev <alonbl(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

Change in vdsm[master]: Removing libvirt_configure from pre-start and ask for manual...

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Removing libvirt_configure from pre-start and ask for manual run
......................................................................


Patch Set 2:

Build Successful 

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/4673/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/4749/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/3864/ : SUCCESS

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Icf70a749454ea341d5b52220e16e9567b90431a0
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: Alon Bar-Lev <alonbl(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

Change in vdsm[master]: Removing libvirt_configure from pre-start and ask for manual...

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: Removing libvirt_configure from pre-start and ask for manual run
......................................................................


Patch Set 5: Code-Review-1

(1 comment)

other than the tabs, I'm happy with this patch.

....................................................
File init/vdsmd_init_common.sh.in
Line 219: 	    "
Line 220:         ;;
Line 221:     --post-stop)
Line 222:         run_tasks " \
Line 223: 	    run_final_hooks \
please replace the tabs.
Line 224: 	    "
Line 225:         ;;
Line 226:     --help|*)
Line 227:         usage


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Icf70a749454ea341d5b52220e16e9567b90431a0
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: Alon Bar-Lev <alonbl(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes

Change in vdsm[master]: Removing libvirt_configure from pre-start and ask for manual...

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: Removing libvirt_configure from pre-start and ask for manual run
......................................................................


Patch Set 7: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Icf70a749454ea341d5b52220e16e9567b90431a0
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: Alon Bar-Lev <alonbl(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

Change in vdsm[master]: Removing libvirt_configure from pre-start and ask for manual...

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Removing libvirt_configure from pre-start and ask for manual run
......................................................................


Patch Set 5:

Build Successful 

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/4713/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/4789/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/3904/ : SUCCESS

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Icf70a749454ea341d5b52220e16e9567b90431a0
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: Alon Bar-Lev <alonbl(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

Change in vdsm[master]: Removing libvirt_configure from pre-start and ask for manual...

Yaniv Bronhaim has posted comments on this change.

Change subject: Removing libvirt_configure from pre-start and ask for manual run
......................................................................


Patch Set 2:

(1 comment)

....................................................
File init/sysvinit/vdsmd.init.in
Line 137
Line 138
Line 139
Line 140
Line 141
for now I'll keep the RETVAL usage and modify both locations again in separate patch


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Icf70a749454ea341d5b52220e16e9567b90431a0
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: Alon Bar-Lev <alonbl(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes

Change in vdsm[master]: Removing libvirt_configure from pre-start and ask for manual...

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: Removing libvirt_configure from pre-start and ask for manual run
......................................................................


Patch Set 2:

(2 comments)

....................................................
File init/sysvinit/vdsmd.init.in
Line 137
Line 138
Line 139
Line 140
Line 141
To say nothing about the usage of the global RETVAL. In a future patch, I hope.


....................................................
File init/vdsmd_init_common.sh.in
Line 205: case "$1" in
Line 206:     --pre-start)
Line 207:         run_tasks " \
Line 208:             run_init_hooks gencerts check_libvirt_configure \
Line 209:             reconfigure_sanlock syslog_available nwfilter dummybr \
Yeah, it has to be moved to 'reconfigure' which is called by host-deploy.
Line 210:             load_needed_modules tune_system mkdirs test_space test_lo \
Line 211:             test_conflicting_conf"
Line 212:         ;;
Line 213:     --post-stop)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Icf70a749454ea341d5b52220e16e9567b90431a0
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: Alon Bar-Lev <alonbl(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes

Change in vdsm[master]: Removing libvirt_configure from pre-start and ask for manual...

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: Removing libvirt_configure from pre-start and ask for manual run
......................................................................


Patch Set 3: Code-Review+1

Awaiting a follow-up patch for sanlock configuration.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Icf70a749454ea341d5b52220e16e9567b90431a0
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: Alon Bar-Lev <alonbl(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

Change in vdsm[master]: Removing libvirt_configure from pre-start and ask for manual...

[Alon Bar-Lev]

Alon Bar-Lev has posted comments on this change.

Change subject: Removing libvirt_configure from pre-start and ask for manual run
......................................................................


Patch Set 2:

(4 comments)

....................................................
File init/sysvinit/vdsmd.init.in
Line 132:     shutdown_conflicting_srv "${CONFLICTING_SERVICES}"
Line 133: 
Line 134:     start_needed_srv "${NEEDED_SERVICES}"
Line 135: 
Line 136:     "${VDSMD_INIT_COMMON}" --pre-start || return 1
please do not return in middle of function... avoid spaghetti if possible.
Line 137: 
Line 138:     echo $"Starting up vdsm daemon: "
Line 139:     NICELEVEL="${NICE_LOWEST}" daemon --user=vdsm
"@VDSMDIR@/daemonAdapter" \
Line 140:         -0 /dev/null -1 /dev/null -2 /dev/null --syslog
"@VDSMDIR@/respawn" --minlifetime 10 \


....................................................
File init/vdsmd_init_common.sh.in
Line 57: task_check_libvirt_configure() {
Line 58:     if ! "$VDSM_TOOL" libvirt-is-configured; then
Line 59:         echo "Perform 'vdsm-tool libvirt-configure' before starting
vdsmd"
Line 60:         return 1
Line 61:     fi
add explicit return
Line 62: }
Line 63: 
Line 64: 
Line 65: task_syslog_available() {


Line 207:         run_tasks " \
Line 208:             run_init_hooks gencerts check_libvirt_configure \
Line 209:             reconfigure_sanlock syslog_available nwfilter dummybr \
Line 210:             load_needed_modules tune_system mkdirs test_space test_lo \
Line 211:             test_conflicting_conf"
I suggest to move this into one line per task to avoid such ugly patches.
Line 212:         ;;
Line 213:     --post-stop)
Line 214:         run_tasks "run_final_hooks"
Line 215:         ;;


....................................................
File lib/vdsm/tool/libvirt_configure.sh.in
Line 157:        echo "libvirt is already configured for vdsm"
Line 158:        return 0
Line 159:     fi
Line 160:     return 1
Line 161: }
should be:

 local rc=0
 while [ -n "$1" ]; do
     if not grep -q "${BY_VDSM_VERS}" "$1"; then
         rc=1
         break
     fi
     shift
 done
 [ ${rc} = 0 ] && echo "libvirt is already configured for vdsm"
 return ${rc}
Line 162: 
Line 163: configure() {
Line 164:     # if SysV init file and Upstart job file both exist, prefer Upstart
Line 165:     libvirtd_sysv2upstart || return $?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Icf70a749454ea341d5b52220e16e9567b90431a0
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: Alon Bar-Lev <alonbl(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes