Change in vdsm[master]: Removing unnecessary subshell call

2013-10-10 Thread ybronhei 
Yaniv Bronhaim has uploaded a new change for review.

Change subject: Removing unnecessary subshell call
..

Removing unnecessary subshell call

Change-Id: If938a322836a82090671483f47a6a0c49878556a
Signed-off-by: Yaniv Bronhaim ybron...@redhat.com
---
M init/sysvinit/vdsmd.init.in
1 file changed, 2 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/61/20061/1

diff --git a/init/sysvinit/vdsmd.init.in b/init/sysvinit/vdsmd.init.in
index efd9c35..e10cca0 100755
--- a/init/sysvinit/vdsmd.init.in
+++ b/init/sysvinit/vdsmd.init.in
@@ -68,8 +68,8 @@
initctl stop ${srv} || : # stop fails when already down
initctl status ${srv} | grep -q stop/waiting
elif [ -x /etc/init.d/${srv} ]; then
-   (! service ${srv} status /dev/null 21) ||
-   service ${srv} stop
+! service ${srv} status /dev/null 21 ||
+service ${srv} stop
else
true
fi


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: If938a322836a82090671483f47a6a0c49878556a
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim ybron...@redhat.com
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: Removing unnecessary subshell call

2013-10-10 Thread oVirt Jenkins CI Server 
oVirt Jenkins CI Server has posted comments on this change.

Change subject: Removing unnecessary subshell call
..


Patch Set 1:

Build Successful 

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

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

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: If938a322836a82090671483f47a6a0c49878556a
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim ybron...@redhat.com
Gerrit-Reviewer: Alon Bar-Lev alo...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Zhou Zheng Sheng zhshz...@linux.vnet.ibm.com
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: Removing unnecessary subshell call

2013-10-10 Thread zhshzhou 
Zhou Zheng Sheng has posted comments on this change.

Change subject: Removing unnecessary subshell call
..


Patch Set 1:

(1 comment)


File init/sysvinit/vdsmd.init.in
Line 68:initctl stop ${srv} || : # stop fails when already 
down
Line 69:initctl status ${srv} | grep -q stop/waiting
Line 70:elif [ -x /etc/init.d/${srv} ]; then
Line 71: ! service ${srv} status /dev/null 21 ||
Line 72: service ${srv} stop
It seem there are five spaces of the indentation for both line 71 72.  I think 
the popular indentation is four spaces.
Line 73:else
Line 74:true
Line 75:fi
Line 76: fi


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

Gerrit-MessageType: comment
Gerrit-Change-Id: If938a322836a82090671483f47a6a0c49878556a
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim ybron...@redhat.com
Gerrit-Reviewer: Alon Bar-Lev alo...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Zhou Zheng Sheng zhshz...@linux.vnet.ibm.com
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: Removing unnecessary subshell call

2013-10-10 Thread danken 
Dan Kenigsberg has posted comments on this change.

Change subject: Removing unnecessary subshell call
..


Patch Set 1:

(1 comment)


File init/sysvinit/vdsmd.init.in
Line 68:initctl stop ${srv} || : # stop fails when already 
down
Line 69:initctl status ${srv} | grep -q stop/waiting
Line 70:elif [ -x /etc/init.d/${srv} ]; then
Line 71: ! service ${srv} status /dev/null 21 ||
Line 72: service ${srv} stop
negative logic confuses me. Any reason not to have

 service ${srv} status /dev/null 21  service ${srv} stop

?
Line 73:else
Line 74:true
Line 75:fi
Line 76: fi


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

Gerrit-MessageType: comment
Gerrit-Change-Id: If938a322836a82090671483f47a6a0c49878556a
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim ybron...@redhat.com
Gerrit-Reviewer: Alon Bar-Lev alo...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Zhou Zheng Sheng zhshz...@linux.vnet.ibm.com
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: Removing unnecessary subshell call

2013-10-10 Thread Alon Bar-Lev 
Alon Bar-Lev has posted comments on this change.

Change subject: Removing unnecessary subshell call
..


Patch Set 1:

(1 comment)


File init/sysvinit/vdsmd.init.in
Line 68:initctl stop ${srv} || : # stop fails when already 
down
Line 69:initctl status ${srv} | grep -q stop/waiting
Line 70:elif [ -x /etc/init.d/${srv} ]; then
Line 71: ! service ${srv} status /dev/null 21 ||
Line 72: service ${srv} stop
I already commented that...

we need to exit with rc=0 if service is already stopped.

the right statement will be:

 if service ${srv} status; then
 service ${srv} stop
 fi
Line 73:else
Line 74:true
Line 75:fi
Line 76: fi


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

Gerrit-MessageType: comment
Gerrit-Change-Id: If938a322836a82090671483f47a6a0c49878556a
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim ybron...@redhat.com
Gerrit-Reviewer: Alon Bar-Lev alo...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Zhou Zheng Sheng zhshz...@linux.vnet.ibm.com
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: Removing unnecessary subshell call

2013-10-10 Thread zhshzhou 
Zhou Zheng Sheng has posted comments on this change.

Change subject: Removing unnecessary subshell call
..


Patch Set 1:

(1 comment)


File init/sysvinit/vdsmd.init.in
Line 68:initctl stop ${srv} || : # stop fails when already 
down
Line 69:initctl status ${srv} | grep -q stop/waiting
Line 70:elif [ -x /etc/init.d/${srv} ]; then
Line 71: ! service ${srv} status /dev/null 21 ||
Line 72: service ${srv} stop
I prefer the form ! X || Y because I think if a service is not existing or is 
not running, the overall stop conflicting service operation should be 
considered successful. X  Y is not equal to this result. For simplicity, 
the form suggested by Alon is better.
Line 73:else
Line 74:true
Line 75:fi
Line 76: fi


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

Gerrit-MessageType: comment
Gerrit-Change-Id: If938a322836a82090671483f47a6a0c49878556a
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim ybron...@redhat.com
Gerrit-Reviewer: Alon Bar-Lev alo...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Zhou Zheng Sheng zhshz...@linux.vnet.ibm.com
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: Removing unnecessary subshell call

2013-10-10 Thread oVirt Jenkins CI Server 
oVirt Jenkins CI Server has posted comments on this change.

Change subject: Removing unnecessary subshell call
..


Patch Set 2:

Build Successful 

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

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

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: If938a322836a82090671483f47a6a0c49878556a
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim ybron...@redhat.com
Gerrit-Reviewer: Alon Bar-Lev alo...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Zhou Zheng Sheng zhshz...@linux.vnet.ibm.com
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: Removing unnecessary subshell call

2013-10-10 Thread ybronhei 
Yaniv Bronhaim has posted comments on this change.

Change subject: Removing unnecessary subshell call
..


Patch Set 2: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: If938a322836a82090671483f47a6a0c49878556a
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim ybron...@redhat.com
Gerrit-Reviewer: Alon Bar-Lev alo...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com
Gerrit-Reviewer: Zhou Zheng Sheng zhshz...@linux.vnet.ibm.com
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: Removing unnecessary subshell call

2013-10-10 Thread danken 
Dan Kenigsberg has submitted this change and it was merged.

Change subject: Removing unnecessary subshell call
..


Removing unnecessary subshell call

Change-Id: If938a322836a82090671483f47a6a0c49878556a
Signed-off-by: Yaniv Bronhaim ybron...@redhat.com
Reviewed-on: http://gerrit.ovirt.org/20061
Reviewed-by: Dan Kenigsberg dan...@redhat.com
---
M init/sysvinit/vdsmd.init.in
1 file changed, 17 insertions(+), 16 deletions(-)

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



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

Gerrit-MessageType: merged
Gerrit-Change-Id: If938a322836a82090671483f47a6a0c49878556a
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim ybron...@redhat.com
Gerrit-Reviewer: Alon Bar-Lev alo...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com
Gerrit-Reviewer: Zhou Zheng Sheng zhshz...@linux.vnet.ibm.com
Gerrit-Reviewer: oVirt Jenkins CI Server
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: Removing unnecessary subshell call

2013-10-10 Thread danken 
Dan Kenigsberg has posted comments on this change.

Change subject: Removing unnecessary subshell call
..


Patch Set 2: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: If938a322836a82090671483f47a6a0c49878556a
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim ybron...@redhat.com
Gerrit-Reviewer: Alon Bar-Lev alo...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com
Gerrit-Reviewer: Zhou Zheng Sheng zhshz...@linux.vnet.ibm.com
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches