Change in vdsm[master]: Introducing configurator package in vdsm-tool
Yaniv Bronhaim has posted comments on this change. Change subject: Introducing configurator package in vdsm-tool .. Patch Set 32: Verified+1 -- To view, visit http://gerrit.ovirt.org/20100 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242 Gerrit-PatchSet: 32 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: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Sandro Bonazzola sbona...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshz...@linux.vnet.ibm.com Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Introducing configurator package in vdsm-tool
Yaniv Bronhaim has posted comments on this change. Change subject: Introducing configurator package in vdsm-tool .. Patch Set 32: (1 comment) File debian/vdsm-python.install Line 17: ./usr/lib/python2.7/dist-packages/vdsm/tool/load_needed_modules.py Line 18: ./usr/lib/python2.7/dist-packages/vdsm/tool/nwfilter.py Line 19: ./usr/lib/python2.7/dist-packages/vdsm/tool/passwd.py Line 20: ./usr/lib/python2.7/dist-packages/vdsm/tool/restore_nets.py Line 21: Introducing configurator package in vdsm-tool ah?! can't be its a lie. Line 22: ./usr/lib/python2.7/dist-packages/vdsm/tool/seboolsetup.py Line 23: ./usr/lib/python2.7/dist-packages/vdsm/tool/service.py Line 24: ./usr/lib/python2.7/dist-packages/vdsm/tool/transient.py Line 25: ./usr/lib/python2.7/dist-packages/vdsm/tool/validate_ovirt_certs.py -- To view, visit http://gerrit.ovirt.org/20100 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242 Gerrit-PatchSet: 32 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: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Sandro Bonazzola sbona...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshz...@linux.vnet.ibm.com Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Introducing configurator package in vdsm-tool
Dan Kenigsberg has posted comments on this change. Change subject: Introducing configurator package in vdsm-tool .. Patch Set 33: Code-Review+2 -- To view, visit http://gerrit.ovirt.org/20100 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242 Gerrit-PatchSet: 33 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: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Sandro Bonazzola sbona...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshz...@linux.vnet.ibm.com Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Introducing configurator package in vdsm-tool
Dan Kenigsberg has posted comments on this change. Change subject: Introducing configurator package in vdsm-tool .. Patch Set 32: Code-Review-1 (1 comment) File debian/vdsm-python.install Line 17: ./usr/lib/python2.7/dist-packages/vdsm/tool/load_needed_modules.py Line 18: ./usr/lib/python2.7/dist-packages/vdsm/tool/nwfilter.py Line 19: ./usr/lib/python2.7/dist-packages/vdsm/tool/passwd.py Line 20: ./usr/lib/python2.7/dist-packages/vdsm/tool/restore_nets.py Line 21: Introducing configurator package in vdsm-tool oh dear. Line 22: ./usr/lib/python2.7/dist-packages/vdsm/tool/seboolsetup.py Line 23: ./usr/lib/python2.7/dist-packages/vdsm/tool/service.py Line 24: ./usr/lib/python2.7/dist-packages/vdsm/tool/transient.py Line 25: ./usr/lib/python2.7/dist-packages/vdsm/tool/validate_ovirt_certs.py -- To view, visit http://gerrit.ovirt.org/20100 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242 Gerrit-PatchSet: 32 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: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Sandro Bonazzola sbona...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshz...@linux.vnet.ibm.com Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Introducing configurator package in vdsm-tool
Yaniv Bronhaim has posted comments on this change. Change subject: Introducing configurator package in vdsm-tool .. Patch Set 33: Verified+1 All honey :) -- To view, visit http://gerrit.ovirt.org/20100 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242 Gerrit-PatchSet: 33 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: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Sandro Bonazzola sbona...@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]: Introducing configurator package in vdsm-tool
oVirt Jenkins CI Server has posted comments on this change. Change subject: Introducing configurator package in vdsm-tool .. Patch Set 33: Verified-1 Build Failed http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/4520/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/5320/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/5398/ : FAILURE -- To view, visit http://gerrit.ovirt.org/20100 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242 Gerrit-PatchSet: 33 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: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Sandro Bonazzola sbona...@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]: Introducing configurator package in vdsm-tool
Dan Kenigsberg has submitted this change and it was merged. Change subject: Introducing configurator package in vdsm-tool .. Introducing configurator package in vdsm-tool This package union libvirt and sanlock configuration operations for now. The package defines ModuleConfigure interface and API for modules configuration tool. The format for the configure command is modified to: vdsm-tool configure --module libvirt --force This module also provides is-configured and validate-config verbs with the same syntax. --force flag means the configure flow will stop all related services if needed. Without force, if one of the related services is running, the configure will fail. For reuse and easier additional configurers for other services, all the developer needs to add is an object that inherit ModuleConfigure and implements the interface for test, dry and configure, then adding the class to configurers. Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242 Signed-off-by: Yaniv Bronhaim ybron...@redhat.com Reviewed-on: http://gerrit.ovirt.org/20100 Reviewed-by: Dan Kenigsberg dan...@redhat.com --- M debian/vdsm-python.install M init/systemd/systemd-vdsmd.in M init/sysvinit/vdsmd.init.in M init/vdsmd_init_common.sh.in M lib/vdsm/tool/Makefile.am A lib/vdsm/tool/configurator.py D lib/vdsm/tool/libvirt_configure.py M lib/vdsm/tool/libvirt_configure.sh.in D lib/vdsm/tool/sanlock.py M vdsm.spec.in 10 files changed, 316 insertions(+), 218 deletions(-) Approvals: Yaniv Bronhaim: Verified Dan Kenigsberg: Looks good to me, approved -- To view, visit http://gerrit.ovirt.org/20100 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: merged Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242 Gerrit-PatchSet: 34 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: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Sandro Bonazzola sbona...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshz...@linux.vnet.ibm.com ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Introducing configurator package in vdsm-tool
Dan Kenigsberg has posted comments on this change. Change subject: Introducing configurator package in vdsm-tool .. Patch Set 31: Code-Review+2 -- To view, visit http://gerrit.ovirt.org/20100 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242 Gerrit-PatchSet: 31 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: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Sandro Bonazzola sbona...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshz...@linux.vnet.ibm.com Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Introducing configurator package in vdsm-tool
oVirt Jenkins CI Server has posted comments on this change. Change subject: Introducing configurator package in vdsm-tool .. Patch Set 32: Verified-1 Build Failed http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/4515/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/5315/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/5393/ : FAILURE -- To view, visit http://gerrit.ovirt.org/20100 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242 Gerrit-PatchSet: 32 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: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Sandro Bonazzola sbona...@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]: Introducing configurator package in vdsm-tool
Yaniv Bronhaim has posted comments on this change. Change subject: Introducing configurator package in vdsm-tool .. Patch Set 28: Verified+1 -- To view, visit http://gerrit.ovirt.org/20100 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242 Gerrit-PatchSet: 28 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: Federico Simoncelli fsimo...@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]: Introducing configurator package in vdsm-tool
Dan Kenigsberg has posted comments on this change.
Change subject: Introducing configurator package in vdsm-tool
..
Patch Set 27:
(1 comment)
File lib/vdsm/tool/libvirt_configure.sh.in
Line 103: # so continue to use system default init mechanism
Line 104: return 0
Line 105: fi
Line 106:
Line 107: @CHKCONFIG_PATH@ libvirtd off
If indeed this code now runs only when you must configure (check), then you can
keep the change.
Line 108:
Line 109: if ! diff -q ${packaged} ${target} /dev/null 21; then
Line 110: /bin/cp -p ${packaged} ${target} || return 1
Line 111: /sbin/initctl reload-configuration
--
To view, visit http://gerrit.ovirt.org/20100
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242
Gerrit-PatchSet: 27
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: Federico Simoncelli fsimo...@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: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Introducing configurator package in vdsm-tool
Sandro Bonazzola has posted comments on this change. Change subject: Introducing configurator package in vdsm-tool .. Patch Set 28: Verified-1 vdsm-tool configure --force lead to vdsmd inactive dead for me. -- To view, visit http://gerrit.ovirt.org/20100 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242 Gerrit-PatchSet: 28 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: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Sandro Bonazzola sbona...@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]: Introducing configurator package in vdsm-tool
Yaniv Bronhaim has posted comments on this change. Change subject: Introducing configurator package in vdsm-tool .. Patch Set 28: -Verified Thanks. checking .. -- To view, visit http://gerrit.ovirt.org/20100 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242 Gerrit-PatchSet: 28 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: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Sandro Bonazzola sbona...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshz...@linux.vnet.ibm.com Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Introducing configurator package in vdsm-tool
Yaniv Bronhaim has posted comments on this change.
Change subject: Introducing configurator package in vdsm-tool
..
Patch Set 27:
(1 comment)
File lib/vdsm/tool/libvirt_configure.sh.in
Line 103: # so continue to use system default init mechanism
Line 104: return 0
Line 105: fi
Line 106:
Line 107: @CHKCONFIG_PATH@ libvirtd off
no its not. if you use --force flag it can be run even if configured.
Line 108:
Line 109: if ! diff -q ${packaged} ${target} /dev/null 21; then
Line 110: /bin/cp -p ${packaged} ${target} || return 1
Line 111: /sbin/initctl reload-configuration
--
To view, visit http://gerrit.ovirt.org/20100
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242
Gerrit-PatchSet: 27
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: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Sandro Bonazzola sbona...@redhat.com
Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com
Gerrit-Reviewer: Zhou Zheng Sheng zhshz...@linux.vnet.ibm.com
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Introducing configurator package in vdsm-tool
Yaniv Bronhaim has posted comments on this change. Change subject: Introducing configurator package in vdsm-tool .. Patch Set 29: Verified+1 fedora 19 - checked rhel 6.4 - checked reverify me will be appreciated very much thanks -- To view, visit http://gerrit.ovirt.org/20100 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242 Gerrit-PatchSet: 29 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: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Sandro Bonazzola sbona...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshz...@linux.vnet.ibm.com Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Introducing configurator package in vdsm-tool
oVirt Jenkins CI Server has posted comments on this change. Change subject: Introducing configurator package in vdsm-tool .. Patch Set 29: Verified-1 Build Failed http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/4453/ : FAILURE http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/5255/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/5333/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/20100 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242 Gerrit-PatchSet: 29 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: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Sandro Bonazzola sbona...@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]: Introducing configurator package in vdsm-tool
Alon Bar-Lev has posted comments on this change. Change subject: Introducing configurator package in vdsm-tool .. Patch Set 29: Code-Review+1 -- To view, visit http://gerrit.ovirt.org/20100 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242 Gerrit-PatchSet: 29 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: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Sandro Bonazzola sbona...@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]: Introducing configurator package in vdsm-tool
Sandro Bonazzola has posted comments on this change. Change subject: Introducing configurator package in vdsm-tool .. Patch Set 29: Code-Review+1 -- To view, visit http://gerrit.ovirt.org/20100 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242 Gerrit-PatchSet: 29 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: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Sandro Bonazzola sbona...@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]: Introducing configurator package in vdsm-tool
Yaniv Bronhaim has posted comments on this change. Change subject: Introducing configurator package in vdsm-tool .. Patch Set 29: Thanks for the acks! squashing to this patch the following once, no reason to separate them -- To view, visit http://gerrit.ovirt.org/20100 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242 Gerrit-PatchSet: 29 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: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Sandro Bonazzola sbona...@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]: Introducing configurator package in vdsm-tool
oVirt Jenkins CI Server has posted comments on this change. Change subject: Introducing configurator package in vdsm-tool .. Patch Set 30: Verified-1 Build Failed http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/4455/ : FAILURE http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/5257/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/5335/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/20100 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242 Gerrit-PatchSet: 30 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: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Sandro Bonazzola sbona...@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]: Introducing configurator package in vdsm-tool
Yaniv Bronhaim has posted comments on this change.
Change subject: Introducing configurator package in vdsm-tool
..
Patch Set 30:
(1 comment)
File vdsm.spec.in
Line 733: supervdsmd_start_required='yes'
Line 734: fi
Line 735:
Line 736: %{_bindir}/vdsm-tool configure --force /dev/null 21
Line 737:
oh.. this is wrong .. it shouldn't force to reconfigure if already configured..
it should force restarting of related services, but not if already configured
which both are swallowed in the --force flag.
Line 738: if [ ${supervdsmd_start_required} = 'yes' ]; then
Line 739: %{_bindir}/vdsm-tool service-start supervdsmd /dev/null 21
Line 740: fi
Line 741: if [ ${vdsmd_start_required} = 'yes' ]; then
--
To view, visit http://gerrit.ovirt.org/20100
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242
Gerrit-PatchSet: 30
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: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Sandro Bonazzola sbona...@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: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Introducing configurator package in vdsm-tool
Alon Bar-Lev has posted comments on this change.
Change subject: Introducing configurator package in vdsm-tool
..
Patch Set 30:
(2 comments)
File init/sysvinit/vdsmd.init.in
Line 208: reconfigure $@
Line 209: RETVAL=$?
Line 210: ;;
Line 211: *)
Line 212: echo Usage: $0
{start|stop|status|restart|force-reload|try-restart|condrestart|reconfigure
[force]}
this is not supported now, so reconfigure should not be exposed, it is backward
compatibility internal usage now.
Line 213: RETVAL=2
Line 214: esac
Line 215:
File vdsm.spec.in
Line 732: %{_bindir}/vdsm-tool service-stop supervdsmd /dev/null 21
Line 733: supervdsmd_start_required='yes'
Line 734: fi
Line 735:
Line 736: %{_bindir}/vdsm-tool configure --force /dev/null 21
this will not show errors if exist... how can we solve issues?
Line 737:
Line 738: if [ ${supervdsmd_start_required} = 'yes' ]; then
Line 739: %{_bindir}/vdsm-tool service-start supervdsmd /dev/null 21
Line 740: fi
--
To view, visit http://gerrit.ovirt.org/20100
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242
Gerrit-PatchSet: 30
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: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Sandro Bonazzola sbona...@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: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Introducing configurator package in vdsm-tool
Yaniv Bronhaim has posted comments on this change.
Change subject: Introducing configurator package in vdsm-tool
..
Patch Set 30:
(3 comments)
File init/sysvinit/vdsmd.init.in
Line 208: reconfigure $@
Line 209: RETVAL=$?
Line 210: ;;
Line 211: *)
Line 212: echo Usage: $0
{start|stop|status|restart|force-reload|try-restart|condrestart|reconfigure
[force]}
right
Line 213: RETVAL=2
Line 214: esac
Line 215:
File vdsm.spec.in
Line 732: %{_bindir}/vdsm-tool service-stop supervdsmd /dev/null 21
Line 733: supervdsmd_start_required='yes'
Line 734: fi
Line 735:
Line 736: %{_bindir}/vdsm-tool configure --force /dev/null 21
dan commented before that we need to swallow stderr in spec operations. we
can't solve it
Line 737:
Line 738: if [ ${supervdsmd_start_required} = 'yes' ]; then
Line 739: %{_bindir}/vdsm-tool service-start supervdsmd /dev/null 21
Line 740: fi
Line 733: supervdsmd_start_required='yes'
Line 734: fi
Line 735:
Line 736: %{_bindir}/vdsm-tool configure --force /dev/null 21
Line 737:
and.. only libvirt configure is relevant during upgrade. we don't want to force
sanlock restart
Line 738: if [ ${supervdsmd_start_required} = 'yes' ]; then
Line 739: %{_bindir}/vdsm-tool service-start supervdsmd /dev/null 21
Line 740: fi
Line 741: if [ ${vdsmd_start_required} = 'yes' ]; then
--
To view, visit http://gerrit.ovirt.org/20100
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242
Gerrit-PatchSet: 30
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: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Sandro Bonazzola sbona...@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: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Introducing configurator package in vdsm-tool
oVirt Jenkins CI Server has posted comments on this change. Change subject: Introducing configurator package in vdsm-tool .. Patch Set 31: Verified-1 Build Failed http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/4459/ : FAILURE http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/5261/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/5339/ : FAILURE -- To view, visit http://gerrit.ovirt.org/20100 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242 Gerrit-PatchSet: 31 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: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Sandro Bonazzola sbona...@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]: Introducing configurator package in vdsm-tool
Yaniv Bronhaim has posted comments on this change. Change subject: Introducing configurator package in vdsm-tool .. Patch Set 31: Verified+1 -- To view, visit http://gerrit.ovirt.org/20100 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242 Gerrit-PatchSet: 31 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: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Sandro Bonazzola sbona...@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]: Introducing configurator package in vdsm-tool
oVirt Jenkins CI Server has posted comments on this change. Change subject: Introducing configurator package in vdsm-tool .. Patch Set 25: Build Successful http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/4425/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/5229/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/5305/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/20100 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242 Gerrit-PatchSet: 25 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: Federico Simoncelli fsimo...@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]: Introducing configurator package in vdsm-tool
Alon Bar-Lev has posted comments on this change.
Change subject: Introducing configurator package in vdsm-tool
..
Patch Set 24:
(1 comment)
File vdsm.spec.in
Line 630: /usr/sbin/useradd -r -u 36 -g %{vdsm_group} -d /var/lib/vdsm \
Line 631: -s /sbin/nologin -c Node Virtualization Manager %{vdsm_user}
Line 632: /usr/sbin/usermod -a -G %{qemu_group},%{snlk_group} %{vdsm_user}
Line 633: /usr/sbin/usermod -a -G %{qemu_group},%{vdsm_group} %{snlk_user}
Line 634:
don't you need to call vdsm-tool configure anywhere here?
Line 635: %post
Line 636: %{_bindir}/vdsm-tool sebool-config || :
Line 637: # set the vdsm secret password for libvirt
Line 638: %{_bindir}/vdsm-tool set-saslpasswd
--
To view, visit http://gerrit.ovirt.org/20100
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242
Gerrit-PatchSet: 24
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: Federico Simoncelli fsimo...@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: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Introducing configurator package in vdsm-tool
Yaniv Bronhaim has posted comments on this change. Change subject: Introducing configurator package in vdsm-tool .. Patch Set 25: (2 comments) File lib/vdsm/tool/configurator.py Line 122: raise UserWarning(Must run as root) Line 123: Line 124: rc, out, err = utils.execCmd( Line 125: ( Line 126: '/usr/bin/usermod', yes, same for both as it was in spec it just should be in sbin and not bin.. my bad. and why not the full path? Line 127: '-a', Line 128: '-G', Line 129: 'qemu,kvm', Line 130: 'sanlock' Line 193: Line 194: service_to_start = [] Line 195: for c in configurer_to_trigger: Line 196: for s in c.getServices(): Line 197: if not service.service_status(s): service_status returns 0 if run and 1 if not exists or stopped which is the opposite of python's way. I'll explicit check for 0 or 1 instead Line 198: if not args.force: Line 199: raise RuntimeError( Line 200: Cannot configure while %s is running % s Line 201: ) -- To view, visit http://gerrit.ovirt.org/20100 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242 Gerrit-PatchSet: 25 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: Federico Simoncelli fsimo...@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: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Introducing configurator package in vdsm-tool
Alon Bar-Lev has posted comments on this change. Change subject: Introducing configurator package in vdsm-tool .. Patch Set 25: (2 comments) File lib/vdsm/tool/configurator.py Line 122: raise UserWarning(Must run as root) Line 123: Line 124: rc, out, err = utils.execCmd( Line 125: ( Line 126: '/usr/bin/usermod', because you do not control where files are at, they can be at /usr/local or any other place. but I know people here likes full paths so ignore me. Line 127: '-a', Line 128: '-G', Line 129: 'qemu,kvm', Line 130: 'sanlock' Line 193: Line 194: service_to_start = [] Line 195: for c in configurer_to_trigger: Line 196: for s in c.getServices(): Line 197: if not service.service_status(s): so fix this function... Line 198: if not args.force: Line 199: raise RuntimeError( Line 200: Cannot configure while %s is running % s Line 201: ) -- To view, visit http://gerrit.ovirt.org/20100 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242 Gerrit-PatchSet: 25 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: Federico Simoncelli fsimo...@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: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Introducing configurator package in vdsm-tool
Alon Bar-Lev has posted comments on this change. Change subject: Introducing configurator package in vdsm-tool .. Patch Set 26: (1 comment) File vdsm.spec.in Line 629 Line 630 Line 631 Line 632 Line 633 this should be replaced with vdsm-tool configure at %post? -- To view, visit http://gerrit.ovirt.org/20100 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242 Gerrit-PatchSet: 26 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: Federico Simoncelli fsimo...@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: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Introducing configurator package in vdsm-tool
oVirt Jenkins CI Server has posted comments on this change. Change subject: Introducing configurator package in vdsm-tool .. Patch Set 26: Build Successful http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/4426/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/5230/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/5306/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/20100 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242 Gerrit-PatchSet: 26 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: Federico Simoncelli fsimo...@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]: Introducing configurator package in vdsm-tool
Alon Bar-Lev has posted comments on this change.
Change subject: Introducing configurator package in vdsm-tool
..
Patch Set 26:
(1 comment)
File lib/vdsm/tool/configurator.py
Line 277: parser = argparse.ArgumentParser('vdsm-tool configure')
Line 278: parser.add_argument(
Line 279: '--module',
Line 280: dest='modules',
Line 281: default=[n.getName() for n in __configurers],
I just learned that we can put choices here...
choices=[n.getName() for n in __configurers],
..,
help=Specify module to %(choices)s,
also... can you please check if append does not append over the default which
is all?
Line 282: metavar='STRING',
Line 283: action='append',
Line 284: help=(
Line 285: 'Specify the module to configure '
--
To view, visit http://gerrit.ovirt.org/20100
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242
Gerrit-PatchSet: 26
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: Federico Simoncelli fsimo...@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: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Introducing configurator package in vdsm-tool
Yaniv Bronhaim has posted comments on this change.
Change subject: Introducing configurator package in vdsm-tool
..
Patch Set 26:
(2 comments)
File lib/vdsm/tool/configurator.py
Line 277: parser = argparse.ArgumentParser('vdsm-tool configure')
Line 278: parser.add_argument(
Line 279: '--module',
Line 280: dest='modules',
Line 281: default=[n.getName() for n in __configurers],
cool, I'll add it. thanks.
Line 282: metavar='STRING',
Line 283: action='append',
Line 284: help=(
Line 285: 'Specify the module to configure '
File vdsm.spec.in
Line 629
Line 630
Line 631
Line 632
Line 633
replaced? we make the configure as part of the deploy or manually . we don't
run it as part of the spec. we can though, but we don't.
--
To view, visit http://gerrit.ovirt.org/20100
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242
Gerrit-PatchSet: 26
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: Federico Simoncelli fsimo...@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: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Introducing configurator package in vdsm-tool
Yaniv Bronhaim has posted comments on this change. Change subject: Introducing configurator package in vdsm-tool .. Patch Set 27: Verified+1 -- To view, visit http://gerrit.ovirt.org/20100 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242 Gerrit-PatchSet: 27 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: Federico Simoncelli fsimo...@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]: Introducing configurator package in vdsm-tool
Alon Bar-Lev has posted comments on this change. Change subject: Introducing configurator package in vdsm-tool .. Patch Set 27: Code-Review+1 (1 comment) File lib/vdsm/tool/configurator.py Line 271: 'If non is specified, operation will run for ' Line 272: 'all related modules.' Line 273: ), Line 274: ) Line 275: if action == configure: this is not something I like... but oh... well... Line 276: parser.add_argument( Line 277: '--force', Line 278: dest='force', Line 279: default=False, -- To view, visit http://gerrit.ovirt.org/20100 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242 Gerrit-PatchSet: 27 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: Federico Simoncelli fsimo...@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: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Introducing configurator package in vdsm-tool
Dan Kenigsberg has posted comments on this change.
Change subject: Introducing configurator package in vdsm-tool
..
Patch Set 27:
(1 comment)
File lib/vdsm/tool/libvirt_configure.sh.in
Line 103: # so continue to use system default init mechanism
Line 104: return 0
Line 105: fi
Line 106:
Line 107: @CHKCONFIG_PATH@ libvirtd off
could you explain the reasoning for this change? Why do you want to run
chkconfig even after configuration is done? how is it related to the rest of
this patch? (I can guess why you've dropped `libvirtd stop` from here, but why
drop the condition and the comment?)
Line 108:
Line 109: if ! diff -q ${packaged} ${target} /dev/null 21; then
Line 110: /bin/cp -p ${packaged} ${target} || return 1
Line 111: /sbin/initctl reload-configuration
--
To view, visit http://gerrit.ovirt.org/20100
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242
Gerrit-PatchSet: 27
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: Federico Simoncelli fsimo...@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: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Introducing configurator package in vdsm-tool
Alon Bar-Lev has posted comments on this change.
Change subject: Introducing configurator package in vdsm-tool
..
Patch Set 27:
(1 comment)
File lib/vdsm/tool/libvirt_configure.sh.in
Line 103: # so continue to use system default init mechanism
Line 104: return 0
Line 105: fi
Line 106:
Line 107: @CHKCONFIG_PATH@ libvirtd off
I guess this is to play safe... always stop/remove from boot the init.d and
always have the upstrart alive.
Please keep in mind that this script will now not be called unless
configuration should be applied.
Line 108:
Line 109: if ! diff -q ${packaged} ${target} /dev/null 21; then
Line 110: /bin/cp -p ${packaged} ${target} || return 1
Line 111: /sbin/initctl reload-configuration
--
To view, visit http://gerrit.ovirt.org/20100
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242
Gerrit-PatchSet: 27
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: Federico Simoncelli fsimo...@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: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Introducing configurator package in vdsm-tool
Yaniv Bronhaim has posted comments on this change.
Change subject: Introducing configurator package in vdsm-tool
..
Patch Set 27:
(2 comments)
File lib/vdsm/tool/configurator.py
Line 271: 'If non is specified, operation will run for '
Line 272: 'all related modules.'
Line 273: ),
Line 274: )
Line 275: if action == configure:
its brilliant :)
Line 276: parser.add_argument(
Line 277: '--force',
Line 278: dest='force',
Line 279: default=False,
File lib/vdsm/tool/libvirt_configure.sh.in
Line 103: # so continue to use system default init mechanism
Line 104: return 0
Line 105: fi
Line 106:
Line 107: @CHKCONFIG_PATH@ libvirtd off
ok ok dan .. sorry, most of the time I don't do such things , but alonbl gave
the arrangement spirit and I couldn't hold this motion inside me to delete
those lines. it is not related, only the stop part. I'll get those back, you're
totaly right
Line 108:
Line 109: if ! diff -q ${packaged} ${target} /dev/null 21; then
Line 110: /bin/cp -p ${packaged} ${target} || return 1
Line 111: /sbin/initctl reload-configuration
--
To view, visit http://gerrit.ovirt.org/20100
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242
Gerrit-PatchSet: 27
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: Federico Simoncelli fsimo...@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: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Introducing configurator package in vdsm-tool
oVirt Jenkins CI Server has posted comments on this change. Change subject: Introducing configurator package in vdsm-tool .. Patch Set 28: Verified-1 Build Failed http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/4439/ : FAILURE http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/5243/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/5320/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/20100 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242 Gerrit-PatchSet: 28 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: Federico Simoncelli fsimo...@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]: Introducing configurator package in vdsm-tool
Yaniv Bronhaim has posted comments on this change. Change subject: Introducing configurator package in vdsm-tool .. Patch Set 23: Alon, lets continue with the review until I'll understand all the material about sanlock configure. I won't forget about your request to modify it. Is the patch accepted by you as it is? -- To view, visit http://gerrit.ovirt.org/20100 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242 Gerrit-PatchSet: 23 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: Federico Simoncelli fsimo...@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]: Introducing configurator package in vdsm-tool
Alon Bar-Lev has posted comments on this change. Change subject: Introducing configurator package in vdsm-tool .. Patch Set 23: I have no more comments except that we need to move the sanlock configuration here to have sane solution. -- To view, visit http://gerrit.ovirt.org/20100 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242 Gerrit-PatchSet: 23 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: Federico Simoncelli fsimo...@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]: Introducing configurator package in vdsm-tool
oVirt Jenkins CI Server has posted comments on this change. Change subject: Introducing configurator package in vdsm-tool .. Patch Set 24: Build Successful http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/4394/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/5198/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/5274/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/20100 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242 Gerrit-PatchSet: 24 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: Federico Simoncelli fsimo...@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]: Introducing configurator package in vdsm-tool
Yaniv Bronhaim has posted comments on this change. Change subject: Introducing configurator package in vdsm-tool .. Patch Set 24: Verified+1 Verified. although there is still libvirtd stop in libvirtd_sysv2upstart which I intend to remove, and I still check why in http://gerrit.ovirt.org/#/c/16742/ the sanlock check was called reconfigure, which confused me.. any more comments? please ack if not. -- To view, visit http://gerrit.ovirt.org/20100 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242 Gerrit-PatchSet: 24 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: Federico Simoncelli fsimo...@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]: Introducing configurator package in vdsm-tool
Alon Bar-Lev has posted comments on this change. Change subject: Introducing configurator package in vdsm-tool .. Patch Set 24: No more comments, just the sanlock should not be abnormality. -- To view, visit http://gerrit.ovirt.org/20100 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242 Gerrit-PatchSet: 24 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: Federico Simoncelli fsimo...@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]: Introducing configurator package in vdsm-tool
Alon Bar-Lev has posted comments on this change. Change subject: Introducing configurator package in vdsm-tool .. Patch Set 24: I still do not follow how restart of sanlock cause it to run under different account. The only possibility is that it is not 'configure' but 'upgrade', for example old sanlock ran under root, and newer under some other account and we want to restart the services to force upgrade... then we should explicitly comment this, including which version was the first to work correctly. -- To view, visit http://gerrit.ovirt.org/20100 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242 Gerrit-PatchSet: 24 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: Federico Simoncelli fsimo...@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]: Introducing configurator package in vdsm-tool
Yaniv Bronhaim has posted comments on this change. Change subject: Introducing configurator package in vdsm-tool .. Patch Set 23: not hard, just need that someone will explain me the way it is. why do we call this function configure sanlock and why don't we add qemu gid to sanlock group manually ? must be a reason for the way this function implemented . Federico, can you share what's behind that logic? It's quite new this part.. -- To view, visit http://gerrit.ovirt.org/20100 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242 Gerrit-PatchSet: 23 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: Federico Simoncelli fsimo...@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]: Introducing configurator package in vdsm-tool
Yaniv Bronhaim has posted comments on this change. Change subject: Introducing configurator package in vdsm-tool .. Patch Set 22: This part should be in pre_start tasks, I will check why it was added as configuration . but currently this patch should stable vdsm-tool api and not focus on the sanlock part imo -- To view, visit http://gerrit.ovirt.org/20100 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242 Gerrit-PatchSet: 22 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]: Introducing configurator package in vdsm-tool
oVirt Jenkins CI Server has posted comments on this change. Change subject: Introducing configurator package in vdsm-tool .. Patch Set 23: Build Successful http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/4373/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/5177/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/5253/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/20100 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242 Gerrit-PatchSet: 23 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]: Introducing configurator package in vdsm-tool
Alon Bar-Lev has posted comments on this change. Change subject: Introducing configurator package in vdsm-tool .. Patch Set 23: but currently this patch should stable vdsm-tool api and not focus on the sanlock part imo I do not think it is that hard to make it complete (libvirt and sanlock) -- To view, visit http://gerrit.ovirt.org/20100 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242 Gerrit-PatchSet: 23 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]: Introducing configurator package in vdsm-tool
Alon Bar-Lev has posted comments on this change.
Change subject: Introducing configurator package in vdsm-tool
..
Patch Set 19:
(2 comments)
File init/systemd/systemd-vdsmd.in
Line 25:
Line 26: [ $1 != reconfigure ] usage_exit
Line 27: [ -n $2 -a $2 != force ] usage_exit
Line 28:
Line 29: @BINDIR@/vdsm-tool configure ${2:+--force}
because legacy logic behavior is different.
legacy:
if --force or not configured:
vdsm-tool configure --force
vdsm-tool:
if not configured and not --force:
die if one of the service is up
configure
File lib/vdsm/tool/configurator.py
Line 159:
Line 160: sanlock = SanlockModuleConfigure()
Line 161: c[sanlock.getName()] = sanlock
Line 162:
Line 163: __configurers = setConfigurers()
I prefer the third one...
for c in __configurers:
if c.getName() in modules:
now... I know this has the side effect of ignoring unsupported modules, but I
am fine with this as backward/forward compatibility is easier this way.
Line 164:
Line 165:
Line 166: @vdsm.tool.expose(configure)
Line 167: def configure():
--
To view, visit http://gerrit.ovirt.org/20100
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242
Gerrit-PatchSet: 19
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: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Introducing configurator package in vdsm-tool
Yaniv Bronhaim has posted comments on this change. Change subject: Introducing configurator package in vdsm-tool .. Patch Set 19: (1 comment) File lib/vdsm/tool/configurator.py Line 172: services_to_stop = [] Line 173: for m in args.modules: Line 174: if m.getName() not in __configurers: Line 175: raise RuntimeError(Module %s does not exists) Line 176: if not __configurers[m].isconfigured(): but this means that if the validation doesn't pass you won't be able to configure for good. it'll always fail in this stage Line 177: services_to_stop.append(m.getServiceName()) Line 178: services_to_stop.append(m.getDependedServicesList()) Line 179: Line 180: service_to_start = [] -- To view, visit http://gerrit.ovirt.org/20100 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242 Gerrit-PatchSet: 19 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: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Introducing configurator package in vdsm-tool
Alon Bar-Lev has posted comments on this change. Change subject: Introducing configurator package in vdsm-tool .. Patch Set 19: (1 comment) File lib/vdsm/tool/configurator.py Line 172: services_to_stop = [] Line 173: for m in args.modules: Line 174: if m.getName() not in __configurers: Line 175: raise RuntimeError(Module %s does not exists) Line 176: if not __configurers[m].isconfigured(): so fail if not --force? I do not exactly understand what is the difference between unconfigured and unvalidated. Line 177: services_to_stop.append(m.getServiceName()) Line 178: services_to_stop.append(m.getDependedServicesList()) Line 179: Line 180: service_to_start = [] -- To view, visit http://gerrit.ovirt.org/20100 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242 Gerrit-PatchSet: 19 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: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Introducing configurator package in vdsm-tool
oVirt Jenkins CI Server has posted comments on this change. Change subject: Introducing configurator package in vdsm-tool .. Patch Set 20: Build Successful http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/4362/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/5166/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/5242/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/20100 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242 Gerrit-PatchSet: 20 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]: Introducing configurator package in vdsm-tool
oVirt Jenkins CI Server has posted comments on this change. Change subject: Introducing configurator package in vdsm-tool .. Patch Set 21: Build Successful http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/4364/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/5168/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/5244/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/20100 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242 Gerrit-PatchSet: 21 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]: Introducing configurator package in vdsm-tool
Alon Bar-Lev has posted comments on this change. Change subject: Introducing configurator package in vdsm-tool .. Patch Set 21: Great. You told me something about simplifying the libvirt script? We need to move here the sanlock configuration from wherever it is currently done. -- To view, visit http://gerrit.ovirt.org/20100 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242 Gerrit-PatchSet: 21 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]: Introducing configurator package in vdsm-tool
oVirt Jenkins CI Server has posted comments on this change. Change subject: Introducing configurator package in vdsm-tool .. Patch Set 22: Build Successful http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/4365/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/5169/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/5245/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/20100 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242 Gerrit-PatchSet: 22 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]: Introducing configurator package in vdsm-tool
Alon Bar-Lev has posted comments on this change. Change subject: Introducing configurator package in vdsm-tool .. Patch Set 22: (2 comments) still missing the sanlock actual configuration... Commit Message Line 9: This package union libvirt and sanlock configuration operations for now. Line 10: The package defines ModuleConfigure interface and API for modules configuration Line 11: tool. The format for the configure command is modified to: Line 12: Line 13: vdsm-tool configure --modules libvirt,sanlock --force --autoreset this is not true Line 14: Line 15: This module also provides is-configured and validate-config verbs with Line 16: the same syntax. Line 17: File lib/vdsm/tool/configurator.py Line 57: Line 58: def getServices(self): Line 59: return [supervdsmd, vdsmd, libvirtd] Line 60: Line 61: def _exec_libvirt_configure(self, action, *args): no need for *args, you do not use it Line 62: Line 63: Invoke libvirt_configure.sh script Line 64: Line 65: if os.getuid() != 0: -- To view, visit http://gerrit.ovirt.org/20100 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242 Gerrit-PatchSet: 22 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: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Introducing configurator package in vdsm-tool
oVirt Jenkins CI Server has posted comments on this change. Change subject: Introducing configurator package in vdsm-tool .. Patch Set 19: Build Successful http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/5131/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/4327/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/5205/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/20100 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242 Gerrit-PatchSet: 19 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]: Introducing configurator package in vdsm-tool
Alon Bar-Lev has posted comments on this change.
Change subject: Introducing configurator package in vdsm-tool
..
Patch Set 19:
(9 comments)
Thanks! I start to be able to follow the code!!!
Some comments.
File init/systemd/systemd-vdsmd.in
Line 25:
Line 26: [ $1 != reconfigure ] usage_exit
Line 27: [ -n $2 -a $2 != force ] usage_exit
Line 28:
Line 29: @BINDIR@/vdsm-tool configure ${2:+--force}
this is incorrect, the --force of the legacy init system just told if to
reconfigure even if configured or not... it always restarted services when
reconfiguring.
File init/sysvinit/vdsmd.init.in
Line 110:
Line 111: reconfigure() {
Line 112: local force
Line 113: [ ${1} = force ] force=--force
Line 114: $VDSM_TOOL configure ${force}
this is incorrect, the --force of the legacy init system just told if to
reconfigure even if configured or not... it always restarted services when
reconfiguring.
Line 115: }
Line 116:
Line 117: start() {
Line 118: test_already_running return 0
File lib/vdsm/tool/configurator.py
Line 44: def validate(self):
Line 45: return True
Line 46:
Line 47: def configure(self):
Line 48: return True
we do not need to return anything at configure... it is void... should be pass
here.
Line 49:
Line 50: def isconfigured(self):
Line 51: return True
Line 52:
Line 151: return True
Line 152:
Line 153:
Line 154: # List all available configurers
Line 155: def setConfigurers():
private?
Line 156: c = {}
Line 157: libvirt = LibvirtModuleConfigure()
Line 158: c[libvirt.getName()] = libvirt
Line 159:
Line 152:
Line 153:
Line 154: # List all available configurers
Line 155: def setConfigurers():
Line 156: c = {}
why dict?
Line 157: libvirt = LibvirtModuleConfigure()
Line 158: c[libvirt.getName()] = libvirt
Line 159:
Line 160: sanlock = SanlockModuleConfigure()
Line 159:
Line 160: sanlock = SanlockModuleConfigure()
Line 161: c[sanlock.getName()] = sanlock
Line 162:
Line 163: __configurers = setConfigurers()
why not:
__configurers = [
LibvirtModuleConfigure(),
SanlockModuleConfigure(),
]
Line 164:
Line 165:
Line 166: @vdsm.tool.expose(configure)
Line 167: def configure():
Line 172: services_to_stop = []
Line 173: for m in args.modules:
Line 174: if m.getName() not in __configurers:
Line 175: raise RuntimeError(Module %s does not exists)
Line 176: if not __configurers[m].isconfigured():
from what I see you should call validate and raise error if not validated, no?
Line 177: services_to_stop.append(m.getServiceName())
Line 178: services_to_stop.append(m.getDependedServicesList())
Line 179:
Line 180: service_to_start = []
Line 174: if m.getName() not in __configurers:
Line 175: raise RuntimeError(Module %s does not exists)
Line 176: if not __configurers[m].isconfigured():
Line 177: services_to_stop.append(m.getServiceName())
Line 178: services_to_stop.append(m.getDependedServicesList())
extend or += ?
and why do we need getServiceName?
we can always return [dependency, dependency, service] out of getServies(), no?
Line 179:
Line 180: service_to_start = []
Line 181: for s in services_to_stop:
Line 182: if service.service_status(s):
Line 235: 'all related modules')
Line 236: return parser.parse_args(sys.argv[2:])
Line 237:
Line 238:
Line 239: def parse_configure_args():
these functions are private, should begin with _, no?
also, I do not understand why function that is used one time is needed... but
not that important.
Line 240: parser = argparse.ArgumentParser('vdsm-tool configure')
Line 241: parser.add_argument('--module', dest='modules', default=None,
Line 242: metavar='STRING', action='append',
Line 243: help='Specify the module to configure '
--
To view, visit http://gerrit.ovirt.org/20100
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242
Gerrit-PatchSet: 19
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: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Introducing configurator package in vdsm-tool
Yaniv Bronhaim has posted comments on this change.
Change subject: Introducing configurator package in vdsm-tool
..
Patch Set 19:
(4 comments)
File init/systemd/systemd-vdsmd.in
Line 25:
Line 26: [ $1 != reconfigure ] usage_exit
Line 27: [ -n $2 -a $2 != force ] usage_exit
Line 28:
Line 29: @BINDIR@/vdsm-tool configure ${2:+--force}
it restarts libvirtd, but not supervdsmd . how is it relevant ? we still need
the force to stop the related services before the configure, and now we won't
send --force to the libvirt_configure script, which means we won't configure if
already configured, which is fine
I don't understand why is it incorrect?
File init/sysvinit/vdsmd.init.in
Line 110:
Line 111: reconfigure() {
Line 112: local force
Line 113: [ ${1} = force ] force=--force
Line 114: $VDSM_TOOL configure ${force}
again... what's wrong with that? it'll restart libvirtd twice? why is it matter?
Line 115: }
Line 116:
Line 117: start() {
Line 118: test_already_running return 0
File lib/vdsm/tool/configurator.py
Line 159:
Line 160: sanlock = SanlockModuleConfigure()
Line 161: c[sanlock.getName()] = sanlock
Line 162:
Line 163: __configurers = setConfigurers()
to allow getting the specific object by name without looping on it each time
(as I did in previous patchsets)
Line 164:
Line 165:
Line 166: @vdsm.tool.expose(configure)
Line 167: def configure():
Line 174: if m.getName() not in __configurers:
Line 175: raise RuntimeError(Module %s does not exists)
Line 176: if not __configurers[m].isconfigured():
Line 177: services_to_stop.append(m.getServiceName())
Line 178: services_to_stop.append(m.getDependedServicesList())
what's wrong with having 2 verbs? easier to have getServiceName method
Line 179:
Line 180: service_to_start = []
Line 181: for s in services_to_stop:
Line 182: if service.service_status(s):
--
To view, visit http://gerrit.ovirt.org/20100
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242
Gerrit-PatchSet: 19
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: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Introducing configurator package in vdsm-tool
Alon Bar-Lev has posted comments on this change.
Change subject: Introducing configurator package in vdsm-tool
..
Patch Set 19:
(3 comments)
File init/systemd/systemd-vdsmd.in
Line 25:
Line 26: [ $1 != reconfigure ] usage_exit
Line 27: [ -n $2 -a $2 != force ] usage_exit
Line 28:
Line 29: @BINDIR@/vdsm-tool configure ${2:+--force}
no... the legacy logic as much as I can see is:
if --force or not configured:
vdsm-tool configure --force
File lib/vdsm/tool/configurator.py
Line 159:
Line 160: sanlock = SanlockModuleConfigure()
Line 161: c[sanlock.getName()] = sanlock
Line 162:
Line 163: __configurers = setConfigurers()
why do you need to get specific object?
Line 164:
Line 165:
Line 166: @vdsm.tool.expose(configure)
Line 167: def configure():
Line 174: if m.getName() not in __configurers:
Line 175: raise RuntimeError(Module %s does not exists)
Line 176: if not __configurers[m].isconfigured():
Line 177: services_to_stop.append(m.getServiceName())
Line 178: services_to_stop.append(m.getDependedServicesList())
because you do not need to know what is the service and what are the
dependencies all you need to know is the effected services and the order.
Line 179:
Line 180: service_to_start = []
Line 181: for s in services_to_stop:
Line 182: if service.service_status(s):
--
To view, visit http://gerrit.ovirt.org/20100
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242
Gerrit-PatchSet: 19
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: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Introducing configurator package in vdsm-tool
Yaniv Bronhaim has posted comments on this change. Change subject: Introducing configurator package in vdsm-tool .. Patch Set 16: (1 comment) File lib/vdsm/tool/configurator.py Line 24: Line 25: from vdsm import utils Line 26: import vdsm.tool Line 27: from vdsm.tool import service Line 28: from vdsm.constants import P_VDSM_EXEC, DISKIMAGE_GROUP so if not, I accept --force will also restart the modules if needed, and I'll remove the --force from the libvirt-configure script Line 29: Line 30: Line 31: class _ModuleConfigure(object): Line 32: def __init__(self): -- To view, visit http://gerrit.ovirt.org/20100 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242 Gerrit-PatchSet: 16 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: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Introducing configurator package in vdsm-tool
oVirt Jenkins CI Server has posted comments on this change. Change subject: Introducing configurator package in vdsm-tool .. Patch Set 18: Build Successful http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/5112/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/4308/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/5186/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/20100 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242 Gerrit-PatchSet: 18 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]: Introducing configurator package in vdsm-tool
Yaniv Bronhaim has posted comments on this change. Change subject: Introducing configurator package in vdsm-tool .. Patch Set 18: Should be WIP label.. maybe next time I'll change it Anyhow, the argument about the --force flag vs two verbs need to be settled. Danken, if you still want to raise your voice, please do as soon as I move on with that direction. Thanks. -- To view, visit http://gerrit.ovirt.org/20100 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242 Gerrit-PatchSet: 18 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]: Introducing configurator package in vdsm-tool
Alon Bar-Lev has posted comments on this change. Change subject: Introducing configurator package in vdsm-tool .. Patch Set 18: (4 comments) File lib/vdsm/tool/configurator.py Line 175: services_to_configure = [] Line 176: for m in args.modules: Line 177: if m.getName() not in __configurers: Line 178: raise RuntimeError(Module %s does not exists) Line 179: else: no need else Line 180: if not __configurers[m].isconfigured(): Line 181: services_to_configure.append(m.getServiceName()) Line 182: Line 183: for s in services_to_configure: Line 177: if m.getName() not in __configurers: Line 178: raise RuntimeError(Module %s does not exists) Line 179: else: Line 180: if not __configurers[m].isconfigured(): Line 181: services_to_configure.append(m.getServiceName()) it is services_that_should_be_stopped... you configure package... Line 182: Line 183: for s in services_to_configure: Line 184: if service.service_status(s) and not args.force: Line 185: raise RuntimeError(Cannot configure while %s is running % Line 184: if service.service_status(s) and not args.force: Line 185: raise RuntimeError(Cannot configure while %s is running % Line 186:s) Line 187: service_to_start = [] Line 188: for s in services_to_configure: why not add this at the previous loop? you already ask for each service status. Line 189: if service.service_status(s): Line 190: service_to_start.append(s) Line 191: service.service_stop(s) Line 192: Line 193: for m in args.modules: Line 194: for ds in m.getDependedServicesList(): Line 195: if service.service_status(ds): Line 196: service_to_start.append(ds) Line 197: service.service_stop(ds) please finish with all service handling before you configure anything. Line 198: m.configure() Line 199: Line 200: for s in reversed(service_to_start): Line 201: service.service_start(s) -- To view, visit http://gerrit.ovirt.org/20100 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242 Gerrit-PatchSet: 18 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: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Introducing configurator package in vdsm-tool
Yaniv Bronhaim has posted comments on this change. Change subject: Introducing configurator package in vdsm-tool .. Patch Set 16: (1 comment) File lib/vdsm/tool/configurator.py Line 24: Line 25: from vdsm import utils Line 26: import vdsm.tool Line 27: from vdsm.tool import service Line 28: from vdsm.constants import P_VDSM_EXEC, DISKIMAGE_GROUP I tend to agree with alon. although we might need the --force as it was, to reconfigure even if was configured.. can you think about such case? I dont.. Line 29: Line 30: Line 31: class _ModuleConfigure(object): Line 32: def __init__(self): -- To view, visit http://gerrit.ovirt.org/20100 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242 Gerrit-PatchSet: 16 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: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Introducing configurator package in vdsm-tool
oVirt Jenkins CI Server has posted comments on this change. Change subject: Introducing configurator package in vdsm-tool .. Patch Set 17: Build Successful http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/5098/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/4294/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/5172/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/20100 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242 Gerrit-PatchSet: 17 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]: Introducing configurator package in vdsm-tool
Alon Bar-Lev has posted comments on this change. Change subject: Introducing configurator package in vdsm-tool .. Patch Set 17: Still ignoring my major comment... You cannot configure module by module without checking if all modules are stopped before starting configuration. -- To view, visit http://gerrit.ovirt.org/20100 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242 Gerrit-PatchSet: 17 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]: Introducing configurator package in vdsm-tool
oVirt Jenkins CI Server has posted comments on this change. Change subject: Introducing configurator package in vdsm-tool .. Patch Set 16: Build Successful http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/5005/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/4201/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/5079/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/20100 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242 Gerrit-PatchSet: 16 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]: Introducing configurator package in vdsm-tool
Dan Kenigsberg has posted comments on this change.
Change subject: Introducing configurator package in vdsm-tool
..
Patch Set 16:
(4 comments)
File init/systemd/systemd-vdsmd.in
Line 25:
Line 26: [ $1 != reconfigure ] usage_exit
Line 27: [ -n $2 -a $2 != force ] usage_exit
Line 28:
Line 29: @BINDIR@/vdsm-tool configure-all ${2:+--force}
use new vdsm-too verbs.
File lib/vdsm/tool/configurator.py
Line 24:
Line 25: from vdsm import utils
Line 26: import vdsm.tool
Line 27: from vdsm.tool import service
Line 28: from vdsm.constants import P_VDSM_EXEC, DISKIMAGE_GROUP
in my opinion, this is an over-complicated semantics for a configure verb.
I think that configure should do only editing of configuration files. As an
output, it should print which modules need to be restarted.
Then, another verb (say, configure_and_restart) should call configure and
restart the needed modules.
Line 29:
Line 30:
Line 31: class _ModuleConfigure(object):
Line 32: def __init__(self):
Line 188: c.configure(force=args.force)
Line 189: return 0
Line 190:
Line 191:
Line 192: @vdsm.tool.expose(isconfigured)
is-configured is a bit more readable.
Line 193: def isconfigured():
Line 194:
Line 195: Determine if module is configured
Line 196:
Line 205:
Line 206: Determine if configuration is valid for module
Line 207:
Line 208: args = parse_configure_actions_args('validate_config')
Line 209: for c in __configurers and (not args.module or c.getName() in
args.module):
args.modules
Line 210: c.validate()
Line 211: return 0
Line 212:
Line 213:
--
To view, visit http://gerrit.ovirt.org/20100
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242
Gerrit-PatchSet: 16
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: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Introducing configurator package in vdsm-tool
Alon Bar-Lev has posted comments on this change. Change subject: Introducing configurator package in vdsm-tool .. Patch Set 16: (2 comments) File lib/vdsm/tool/configurator.py Line 24: Line 25: from vdsm import utils Line 26: import vdsm.tool Line 27: from vdsm.tool import service Line 28: from vdsm.constants import P_VDSM_EXEC, DISKIMAGE_GROUP I think this only makes it more complex... I expect to have argument to do the restart and not execute this 1+ N times. Line 29: Line 30: Line 31: class _ModuleConfigure(object): Line 32: def __init__(self): Line 184: Line 185: args = parse_configure_args() Line 186: for c in __configurers and (not args.module or c.getName() in args.module): Line 187: sys.stdout.write(Configuring %s\n % (c.getName())) Line 188: c.configure(force=args.force) still my previous comment about atomic was not taken into account. Line 189: return 0 Line 190: Line 191: Line 192: @vdsm.tool.expose(isconfigured) -- To view, visit http://gerrit.ovirt.org/20100 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242 Gerrit-PatchSet: 16 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: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Introducing configurator package in vdsm-tool
Alon Bar-Lev has posted comments on this change.
Change subject: Introducing configurator package in vdsm-tool
..
Patch Set 14:
(10 comments)
File lib/vdsm/tool/configurator.py
Line 32: def __init__(self):
Line 33: pass
Line 34:
Line 35: def validate(self):
Line 36: return 0
return boolean?
Line 37:
Line 38: def configure(self, force, autoreset):
Line 39: return 0
Line 40:
Line 41: def isconfigured(self):
Line 42: return 0
Line 43:
Line 44: def getName(self):
Line 45: return ''
return None?
Line 46:
Line 47:
Line 48: class LibvirtModuleConfigure(_ModuleConfigure):
Line 49: def getName():
Line 44: def getName(self):
Line 45: return ''
Line 46:
Line 47:
Line 48: class LibvirtModuleConfigure(_ModuleConfigure):
call constructor?
def __init__(self):
super(LibvirtModuleConfigure, self).__init__()
Line 49: def getName():
Line 50: return 'libvirt'
Line 51:
Line 52: def _exec_libvirt_configure(self, action, *args):
Line 82: service_should_start.append(srv)
Line 83:
Line 84: args = ''
Line 85: if force:
Line 86: args = '--force'
consider:
args = '--force' if force else ''
Line 87:
Line 88: self._exec_libvirt_configure(reconfigure, args)
Line 89:
Line 90: for srv in reversed(service_should_start):
Line 84: args = ''
Line 85: if force:
Line 86: args = '--force'
Line 87:
Line 88: self._exec_libvirt_configure(reconfigure, args)
or consider:
self._exec_libvirt_configure(
reconfigure,
'--force' if force else '',
)
but I thought the --force is to restart services...
I thought I've commented the algorithm in previous comment.
you check if you should configure, if not you do nothing, if yes and services
is up and no force you exit with error, then if force you stop services,
configure, start stopped services.
this should be applied to all modules as a *SINGLE* transaction.
Line 89:
Line 90: for srv in reversed(service_should_start):
Line 91: service.service_start(srv)
Line 92:
Line 87:
Line 88: self._exec_libvirt_configure(reconfigure, args)
Line 89:
Line 90: for srv in reversed(service_should_start):
Line 91: service.service_start(srv)
missing return
Line 92:
Line 93: def validate(self):
Line 94:
Line 95: Validate conflict in configured files
Line 155: return 0
Line 156:
Line 157:
Line 158: # List all available configurers
Line 159: configurers = [
underscore, as it is module private?
Line 160: LibvirtModuleConfigure(),
Line 161: SanlockModuleConfigure()
Line 162: ]
Line 163:
Line 157:
Line 158: # List all available configurers
Line 159: configurers = [
Line 160: LibvirtModuleConfigure(),
Line 161: SanlockModuleConfigure()
comma
Line 162: ]
Line 163:
Line 164:
Line 165: @vdsm.tool.expose(configure)
Line 158: # List all available configurers
Line 159: configurers = [
Line 160: LibvirtModuleConfigure(),
Line 161: SanlockModuleConfigure()
Line 162: ]
I think this can be tupple
Line 163:
Line 164:
Line 165: @vdsm.tool.expose(configure)
Line 166: def configure():
Line 216: help='Specify the module to configure '
Line 217: '(e.g libvirt,sanlock ..).\n'
Line 218: 'If non is specified, operation will run
for '
Line 219: 'all related modules')
Line 220: parser.add_argument('--dry', dest='dry', default=False,
all these dry and test, we discussed, need to find something cleaner.
Line 221: action='store_true',
Line 222: help='Dry run, prints if configure is
required')
Line 223: parser.add_argument('--test', dest='test', default=False,
Line 224: action='store_true',
--
To view, visit http://gerrit.ovirt.org/20100
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242
Gerrit-PatchSet: 14
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: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Introducing configurator package in vdsm-tool
oVirt Jenkins CI Server has posted comments on this change. Change subject: Introducing configurator package in vdsm-tool .. Patch Set 14: Verified-1 Build Failed http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/5061/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/4987/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/4177/ : FAILURE -- To view, visit http://gerrit.ovirt.org/20100 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242 Gerrit-PatchSet: 14 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]: Introducing configurator package in vdsm-tool
oVirt Jenkins CI Server has posted comments on this change. Change subject: Introducing configurator package in vdsm-tool .. Patch Set 14: Build Failed http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/5061/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/4987/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/4179/ : FAILURE -- To view, visit http://gerrit.ovirt.org/20100 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242 Gerrit-PatchSet: 14 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]: Introducing configurator package in vdsm-tool
Alon Bar-Lev has posted comments on this change. Change subject: Introducing configurator package in vdsm-tool .. Patch Set 15: (4 comments) File lib/vdsm/tool/configurator.py Line 41: sys.stdout.write(%s is already configured\n % self.getName()) Line 42: elif service.service_status(self.getName()) and not force: Line 43: raise RuntimeError(Module %s is up. To force reconfiguration use Line 44:--force flag) Line 45: else: I suggest to use boolean and not numbers I suggest the following structure: if not self.isconfigured(): if service_status and not fource: raise ... return 0 notes: 1. if already configured, I do not expect failure. 2. please do not create dependency between module name and service name. Line 46: ret = 0 Line 47: return ret Line 48: Line 49: def isconfigured(self): Line 46: ret = 0 Line 47: return ret Line 48: Line 49: def isconfigured(self): Line 50: return 0 boolean please Line 51: Line 52: def getName(self): Line 53: return None Line 54: Line 57: def __init__(self): Line 58: super(LibvirtModuleConfigure, self).__init__() Line 59: Line 60: def getName(): Line 61: return 'libvirtd' usage friendly not service name? Line 62: Line 63: def _exec_libvirt_configure(self, action, *args): Line 64: Line 65: Invoke libvirt_configure.sh script Line 83: Line 84: libvirt configuration (--force to force reconfigure also when Line 85: configured) Line 86: Line 87: if super(LibvirtModuleConfigure, self).configure(force): again... this should be for the whole transaction. so if we have sanlock and libvirt to be configured we first as for all then perform all. moving this deeper into the code just make it more complex. Line 88: libvirt_related_services = [supervdsmd, libvirtd] Line 89: service_should_start = [] Line 90: for srv in libvirt_related_services: Line 91: if service.service_status(srv): -- To view, visit http://gerrit.ovirt.org/20100 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242 Gerrit-PatchSet: 15 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: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Introducing configurator package in vdsm-tool
oVirt Jenkins CI Server has posted comments on this change. Change subject: Introducing configurator package in vdsm-tool .. Patch Set 15: Verified-1 Build Failed http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/5069/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/4996/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/4190/ : FAILURE -- To view, visit http://gerrit.ovirt.org/20100 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242 Gerrit-PatchSet: 15 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]: Introducing configurator package in vdsm-tool
Yaniv Bronhaim has posted comments on this change. Change subject: Introducing configurator package in vdsm-tool .. Patch Set 15: (1 comment) File lib/vdsm/tool/configurator.py Line 83: Line 84: libvirt configuration (--force to force reconfigure also when Line 85: configured) Line 86: Line 87: if super(LibvirtModuleConfigure, self).configure(force): didn't understand the sentence. you mean having this operation on upper class is wrong and you prefer having the code in both configure functions, right? Line 88: libvirt_related_services = [supervdsmd, libvirtd] Line 89: service_should_start = [] Line 90: for srv in libvirt_related_services: Line 91: if service.service_status(srv): -- To view, visit http://gerrit.ovirt.org/20100 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242 Gerrit-PatchSet: 15 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: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Introducing configurator package in vdsm-tool
Alon Bar-Lev has posted comments on this change. Change subject: Introducing configurator package in vdsm-tool .. Patch Set 15: (1 comment) File lib/vdsm/tool/configurator.py Line 83: Line 84: libvirt configuration (--force to force reconfigure also when Line 85: configured) Line 86: Line 87: if super(LibvirtModuleConfigure, self).configure(force): see comment at patch#9: lets refine it... something like: services = [] for module in modules: if not module.configured: services += module.get_services for service in services: if is_running(service) and not force: raise 'cannot configure while services are running' to_start = [] for service in services: if is_running(service): to_start += service stop(service) for module in modules: module.configure for service in reversed(to_start): start_service(service) Line 88: libvirt_related_services = [supervdsmd, libvirtd] Line 89: service_should_start = [] Line 90: for srv in libvirt_related_services: Line 91: if service.service_status(srv): -- To view, visit http://gerrit.ovirt.org/20100 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242 Gerrit-PatchSet: 15 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: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Introducing configurator package in vdsm-tool
Zhou Zheng Sheng has posted comments on this change. Change subject: Introducing configurator package in vdsm-tool .. Patch Set 15: (4 comments) Commit Message Line 5: CommitDate: 2013-10-21 16:11:52 +0300 Line 6: Line 7: Introducing configurator package in vdsm-tool Line 8: Line 9: This package union libvirt and sanlock configuration operations for now. I think union is a noun. I guess you wanted to write unites libvirt and sanlock, or This package is a union of libvirt and sanlock . And what does for now means exactly? Do we have another plan in future? I guess you meant unite libvirt and sanlock configuration from present implementations. Line 10: The package defines ModuleConfigure interface and API for modules configuration Line 11: tool. The format for the configure command is modified to: Line 12: Line 13: vdsm-tool configure --modules libvirt,sanlock --force --autoreset Line 11: tool. The format for the configure command is modified to: Line 12: Line 13: vdsm-tool configure --modules libvirt,sanlock --force --autoreset Line 14: Line 15: --dry option is available to check if configure is required --dry usually means it does not touch the actual files and let the use examine the result. Using --dry to test if it is configured is not a straight forward use, kindly like a side effect. For me, better names are --probe/--test for configuration detection, and --verify/--validate for checking conflicts. Line 16: and --test to verify current configuration's conflicts. Line 17: Line 18: For reuse and easier additional configurers for other services, all the Line 19: developer needs to add is an object that inherit ModuleConfigure and File lib/vdsm/tool/configurator.py Line 43: raise RuntimeError(Module %s is up. To force reconfiguration use Line 44:--force flag) Line 45: else: Line 46: ret = 0 Line 47: return ret This return value is not used by any caller. If you are using exception here, I'd suggest the following style: when configure fails, raises an exception, when it succeeds, returns nothing. Line 48: Line 49: def isconfigured(self): Line 50: return 0 Line 51: Line 46: ret = 0 Line 47: return ret Line 48: Line 49: def isconfigured(self): Line 50: return 0 +1 Line 51: Line 52: def getName(self): Line 53: return None Line 54: -- To view, visit http://gerrit.ovirt.org/20100 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242 Gerrit-PatchSet: 15 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: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Introducing configurator package in vdsm-tool
oVirt Jenkins CI Server has posted comments on this change. Change subject: Introducing configurator package in vdsm-tool .. Patch Set 12: Verified-1 Build Failed http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/5049/ : ABORTED http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/4975/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/4165/ : FAILURE -- To view, visit http://gerrit.ovirt.org/20100 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242 Gerrit-PatchSet: 12 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: 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]: Introducing configurator package in vdsm-tool
Yaniv Bronhaim has posted comments on this change. Change subject: Introducing configurator package in vdsm-tool .. Patch Set 9: (1 comment) File lib/vdsm/tool/configurator.py Line 123: @vdsm.tool.expose(configure-all) Line 124: def configure_and_restart_all(*args): Line 125: Line 126: Configure related services for vdsm. --force flag is set for Line 127: forcing restart of related services after configure Do we still want it to not reconfigure while running in our new implementation and flags ? If yes, please recomment it there. Thanks. Line 128: Line 129: libvirt_related_services = [supervdsmd, libvirtd] Line 130: service_should_start = [] Line 131: -- To view, visit http://gerrit.ovirt.org/20100 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242 Gerrit-PatchSet: 9 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: 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]: Introducing configurator package in vdsm-tool
Yaniv Bronhaim has posted comments on this change.
Change subject: Introducing configurator package in vdsm-tool
..
Patch Set 13:
(1 comment)
File init/sysvinit/vdsmd.init.in
Line 110:
Line 111: reconfigure() {
Line 112: local force
Line 113: [ ${1} = force ] force=--force
Line 114: $VDSM_TOOL configure-all ${force}
after stabilizing I'll modify it also here
Line 115: }
Line 116:
Line 117: start() {
Line 118: test_already_running return 0
--
To view, visit http://gerrit.ovirt.org/20100
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242
Gerrit-PatchSet: 13
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: 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]: Introducing configurator package in vdsm-tool
oVirt Jenkins CI Server has posted comments on this change. Change subject: Introducing configurator package in vdsm-tool .. Patch Set 13: Verified-1 Build Failed http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/5053/ : ABORTED http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/4979/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/4169/ : FAILURE -- To view, visit http://gerrit.ovirt.org/20100 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242 Gerrit-PatchSet: 13 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: 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]: Introducing configurator package in vdsm-tool
Yaniv Bronhaim has posted comments on this change. Change subject: Introducing configurator package in vdsm-tool .. Patch Set 7: (1 comment) File lib/vdsm/tool/configurator.py Line 120: if diskimage_gid not in groups: Line 121: sys.stdout.write(sanlock service requires restart\n) Line 122: else: Line 123: sys.stdout.write(sanlock service is already configured\n) Line 124: retval = 0 procedural, as all of this file Line 125: Line 126: return retval Line 127: Line 128: -- To view, visit http://gerrit.ovirt.org/20100 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242 Gerrit-PatchSet: 7 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: 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]: Introducing configurator package in vdsm-tool
Alon Bar-Lev has posted comments on this change. Change subject: Introducing configurator package in vdsm-tool .. Patch Set 7: (1 comment) File lib/vdsm/tool/configurator.py Line 120: if diskimage_gid not in groups: Line 121: sys.stdout.write(sanlock service requires restart\n) Line 122: else: Line 123: sys.stdout.write(sanlock service is already configured\n) Line 124: retval = 0 the fact that you raise IOError and RuntimeError above, means that you work exceptional. Line 125: Line 126: return retval Line 127: Line 128: -- To view, visit http://gerrit.ovirt.org/20100 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242 Gerrit-PatchSet: 7 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: 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]: Introducing configurator package in vdsm-tool
oVirt Jenkins CI Server has posted comments on this change. Change subject: Introducing configurator package in vdsm-tool .. Patch Set 8: Build Successful http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/4990/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/4104/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/4914/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/20100 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242 Gerrit-PatchSet: 8 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: 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]: Introducing configurator package in vdsm-tool
Alon Bar-Lev has posted comments on this change. Change subject: Introducing configurator package in vdsm-tool .. Patch Set 9: (8 comments) File lib/vdsm/tool/configurator.py Line 95: Line 96: try: Line 97: with open(sanlock_pid_file, r) as f: Line 98: sanlock_pid = f.readline().strip() Line 99: with open(proc_status_path % sanlock_pid, r) as sanlock_status: os.path.join? Line 100: for status_line in sanlock_status: Line 101: if status_line.startswith(proc_status_group_prefix): Line 102: groups = [int(x) for x in Line 103: status_line[len(proc_status_group_prefix):]. Line 110: if e.errno == os.errno.ENOENT: Line 111: raise RuntimeError(sanlock service is not running) Line 112: raise Line 113: Line 114: diskimage_gid = grp.getgrnam(DISKIMAGE_GROUP)[2] why do you need this temp variable? Line 115: if diskimage_gid not in groups: Line 116: raise RuntimeError(sanlock service requires restart) Line 117: else: Line 118: sys.stdout.write(sanlock service is already configured\n) Line 111: raise RuntimeError(sanlock service is not running) Line 112: raise Line 113: Line 114: diskimage_gid = grp.getgrnam(DISKIMAGE_GROUP)[2] Line 115: if diskimage_gid not in groups: should always be: if fail: raise exception continue as usual Line 116: raise RuntimeError(sanlock service requires restart) Line 117: else: Line 118: sys.stdout.write(sanlock service is already configured\n) Line 119: Line 123: @vdsm.tool.expose(configure-all) Line 124: def configure_and_restart_all(*args): Line 125: Line 126: Configure related services for vdsm. --force flag is set for Line 127: forcing restart of related services after configure well... I expect that if no --force is given and we need to perform change in configuration and there is relevant service up to fail... what do you think? Line 128: Line 129: libvirt_related_services = [supervdsmd, libvirtd] Line 130: service_should_start = [] Line 131: Line 130: service_should_start = [] Line 131: Line 132: force = False Line 133: if '--force' in args: Line 134: force = True consider: force = '--force' in args or better to use python opt parser? Line 135: Line 136: # libvirt configuration Line 137: if force: Line 138: for srv in libvirt_related_services: Line 134: force = True Line 135: Line 136: # libvirt configuration Line 137: if force: Line 138: for srv in libvirt_related_services: consider: for srv in libvirt_related_services + ['sanlock']: Line 139: if service.service_status(srv): Line 140: service_should_start.append(srv) Line 141: Line 142: configure_libvirt(*args) Line 136: # libvirt configuration Line 137: if force: Line 138: for srv in libvirt_related_services: Line 139: if service.service_status(srv): Line 140: service_should_start.append(srv) where is the stop? Line 141: Line 142: configure_libvirt(*args) Line 143: for srv in reversed(service_should_start): Line 144: service.service_start(srv) Line 144: service.service_start(srv) Line 145: Line 146: # sanlock configuration Line 147: if sanlock_check_service(*args) and force: Line 148: service.service_restart(sanlock) remove in favor of above? Line 149: -- To view, visit http://gerrit.ovirt.org/20100 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242 Gerrit-PatchSet: 9 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: 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]: Introducing configurator package in vdsm-tool
oVirt Jenkins CI Server has posted comments on this change. Change subject: Introducing configurator package in vdsm-tool .. Patch Set 9: Verified-1 Build Failed http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/5000/ : FAILURE http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/4113/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/4923/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/20100 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242 Gerrit-PatchSet: 9 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: 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]: Introducing configurator package in vdsm-tool
Yaniv Bronhaim has posted comments on this change. Change subject: Introducing configurator package in vdsm-tool .. Patch Set 9: (7 comments) File lib/vdsm/tool/configurator.py Line 95: Line 96: try: Line 97: with open(sanlock_pid_file, r) as f: Line 98: sanlock_pid = f.readline().strip() Line 99: with open(proc_status_path % sanlock_pid, r) as sanlock_status: The return value is the concatenation of path1, and optionally path2, etc., .. this adds the sanlock_pid after /proc/.. , join doesn't help much Line 100: for status_line in sanlock_status: Line 101: if status_line.startswith(proc_status_group_prefix): Line 102: groups = [int(x) for x in Line 103: status_line[len(proc_status_group_prefix):]. Line 110: if e.errno == os.errno.ENOENT: Line 111: raise RuntimeError(sanlock service is not running) Line 112: raise Line 113: Line 114: diskimage_gid = grp.getgrnam(DISKIMAGE_GROUP)[2] why not? if grp.getgrnam(DISKIMAGE_GROUP)[2] not in groups -- is too bahh in my opinion. don't you think? what is it in [2]? the variable name saves us for commenting here Line 115: if diskimage_gid not in groups: Line 116: raise RuntimeError(sanlock service requires restart) Line 117: else: Line 118: sys.stdout.write(sanlock service is already configured\n) Line 123: @vdsm.tool.expose(configure-all) Line 124: def configure_and_restart_all(*args): Line 125: Line 126: Configure related services for vdsm. --force flag is set for Line 127: forcing restart of related services after configure too many manual operation to do ... don't you prefer to make the user's life a bit more easy.. configure will do a bit more then just call configure... otherwise this verb is redundant. And, sanlock_check_service doesn't do much, there is not really a configuration process here.. it just tells us if restart is required. so actually what you ask can be part of the verb itself, like configure_libvirt will fail if the service is up. --force will stop it before (as I suggest here and forgot to do :) will add it) Line 128: Line 129: libvirt_related_services = [supervdsmd, libvirtd] Line 130: service_should_start = [] Line 131: Line 130: service_should_start = [] Line 131: Line 132: force = False Line 133: if '--force' in args: Line 134: force = True right. for one parameter force = '--force' in args is enough and maybe later we'll refactor this verb anyway. Line 135: Line 136: # libvirt configuration Line 137: if force: Line 138: for srv in libvirt_related_services: Line 134: force = True Line 135: Line 136: # libvirt configuration Line 137: if force: Line 138: for srv in libvirt_related_services: but its different with sanlock .. the configure for sanlock tells us if it needs to be restarted. Line 139: if service.service_status(srv): Line 140: service_should_start.append(srv) Line 141: Line 142: configure_libvirt(*args) Line 136: # libvirt configuration Line 137: if force: Line 138: for srv in libvirt_related_services: Line 139: if service.service_status(srv): Line 140: service_should_start.append(srv) sorry, missed that Line 141: Line 142: configure_libvirt(*args) Line 143: for srv in reversed(service_should_start): Line 144: service.service_start(srv) Line 144: service.service_start(srv) Line 145: Line 146: # sanlock configuration Line 147: if sanlock_check_service(*args) and force: Line 148: service.service_restart(sanlock) sanlock configure should be separate part in my opinion . Line 149: -- To view, visit http://gerrit.ovirt.org/20100 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242 Gerrit-PatchSet: 9 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: 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]: Introducing configurator package in vdsm-tool
Alon Bar-Lev has posted comments on this change.
Change subject: Introducing configurator package in vdsm-tool
..
Patch Set 9:
(3 comments)
File lib/vdsm/tool/configurator.py
Line 95:
Line 96: try:
Line 97: with open(sanlock_pid_file, r) as f:
Line 98: sanlock_pid = f.readline().strip()
Line 99: with open(proc_status_path % sanlock_pid, r) as
sanlock_status:
I do not follow... I mean:
os.path.join('/proc', sanlock_pid, 'status')
Line 100: for status_line in sanlock_status:
Line 101: if status_line.startswith(proc_status_group_prefix):
Line 102: groups = [int(x) for x in
Line 103:
status_line[len(proc_status_group_prefix):].
Line 110: if e.errno == os.errno.ENOENT:
Line 111: raise RuntimeError(sanlock service is not running)
Line 112: raise
Line 113:
Line 114: diskimage_gid = grp.getgrnam(DISKIMAGE_GROUP)[2]
it is quite obvious... but how you prefer...
if grp.getgrnam(DISKIMAGE_GROUP)[2] not in groups:
Line 115: if diskimage_gid not in groups:
Line 116: raise RuntimeError(sanlock service requires restart)
Line 117: else:
Line 118: sys.stdout.write(sanlock service is already configured\n)
Line 123: @vdsm.tool.expose(configure-all)
Line 124: def configure_and_restart_all(*args):
Line 125:
Line 126: Configure related services for vdsm. --force flag is set for
Line 127: forcing restart of related services after configure
I prefer not to damage system... this means that we cannot configure components
that are still running... if we could, we could skip the restart...
so logic should be:
libvirt_configured = check...
sanlock_configured = check...
if not force:
if not libvirt_configured and libvirt running:
raise cannot configure while running
if not sanlock_configured and sanlock running:
raise cannot configure while running
if not libvirt_configured:
if running libvirt: stop+mark
configure libvirt
if not sanlock_configured:
if running sanlock: stop+mark
configure sanlock
optionally start stopped services
Line 128:
Line 129: libvirt_related_services = [supervdsmd, libvirtd]
Line 130: service_should_start = []
Line 131:
--
To view, visit http://gerrit.ovirt.org/20100
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242
Gerrit-PatchSet: 9
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: 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]: Introducing configurator package in vdsm-tool
oVirt Jenkins CI Server has posted comments on this change. Change subject: Introducing configurator package in vdsm-tool .. Patch Set 10: Build Successful http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/5003/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/4116/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/4926/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/20100 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242 Gerrit-PatchSet: 10 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: 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]: Introducing configurator package in vdsm-tool
oVirt Jenkins CI Server has posted comments on this change. Change subject: Introducing configurator package in vdsm-tool .. Patch Set 11: Verified-1 Build Failed http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/5006/ : FAILURE http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/4119/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/4929/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/20100 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242 Gerrit-PatchSet: 11 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: 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]: Introducing configurator package in vdsm-tool
oVirt Jenkins CI Server has posted comments on this change. Change subject: Introducing configurator package in vdsm-tool .. Patch Set 3: Verified-1 Build Failed http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/4946/ : ABORTED http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/4060/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/4870/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/20100 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242 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: 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]: Introducing configurator package in vdsm-tool
Alon Bar-Lev has posted comments on this change. Change subject: Introducing configurator package in vdsm-tool .. Patch Set 4: (1 comment) File lib/vdsm/tool/configurator.py Line 145: if service.service_status(srv): Line 146: service_should_start.append(srv) Line 147: Line 148: configure_libvirt(*args) Line 149: for srv in service_should_start: I believe that best to do the start in reverse order. Line 150: service.service_start(srv) Line 151: Line 152: # sanlock configuration Line 153: if sanlock_check_service(*args) and force: -- To view, visit http://gerrit.ovirt.org/20100 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242 Gerrit-PatchSet: 4 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: 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]: Introducing configurator package in vdsm-tool
oVirt Jenkins CI Server has posted comments on this change. Change subject: Introducing configurator package in vdsm-tool .. Patch Set 4: Build Successful http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/4948/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/4062/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/4872/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/20100 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242 Gerrit-PatchSet: 4 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: 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]: Introducing configurator package in vdsm-tool
Alon Bar-Lev has posted comments on this change. Change subject: Introducing configurator package in vdsm-tool .. Patch Set 7: (1 comment) File lib/vdsm/tool/configurator.py Line 120: if diskimage_gid not in groups: Line 121: sys.stdout.write(sanlock service requires restart\n) Line 122: else: Line 123: sys.stdout.write(sanlock service is already configured\n) Line 124: retval = 0 please decide if you writing procedural or exceptional. if you are writing exceptional, you can always throw an exception if there is error and avoid nesting. retval above will always be not equal to zero, so condition is void Line 125: Line 126: return retval Line 127: Line 128: -- To view, visit http://gerrit.ovirt.org/20100 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242 Gerrit-PatchSet: 7 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: 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]: Introducing configurator package in vdsm-tool
oVirt Jenkins CI Server has posted comments on this change. Change subject: Introducing configurator package in vdsm-tool .. Patch Set 5: Verified-1 Build Failed http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/4962/ : FAILURE http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/4076/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/4886/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/20100 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242 Gerrit-PatchSet: 5 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: 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]: Introducing configurator package in vdsm-tool
oVirt Jenkins CI Server has posted comments on this change. Change subject: Introducing configurator package in vdsm-tool .. Patch Set 6: Verified-1 Build Failed http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/4967/ : ABORTED http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/4081/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/4891/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/20100 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242 Gerrit-PatchSet: 6 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: 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]: Introducing configurator package in vdsm-tool
oVirt Jenkins CI Server has posted comments on this change. Change subject: Introducing configurator package in vdsm-tool .. Patch Set 7: Verified-1 Build Failed http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/4968/ : FAILURE http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/4082/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/4892/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/20100 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242 Gerrit-PatchSet: 7 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: 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]: Introducing configurator package in vdsm-tool
Yaniv Bronhaim has uploaded a new change for review.
Change subject: Introducing configurator package in vdsm-tool
..
Introducing configurator package in vdsm-tool
This package union libvirt and sanlock configuration operations.
The patch also adds a usage of new configure-and-restart-all vdsm-tool
verb that takes care of configuring all related services to vdsm
(currently sanlock and libvirtd).
Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242
Signed-off-by: Yaniv Bronhaim ybron...@redhat.com
---
M debian/vdsm-python.install
M init/systemd/systemd-vdsmd.in
M init/sysvinit/vdsmd.init.in
M lib/vdsm/tool/Makefile.am
A lib/vdsm/tool/configurator.py
D lib/vdsm/tool/libvirt_configure.py
D lib/vdsm/tool/sanlock.py
M vdsm.spec.in
8 files changed, 151 insertions(+), 154 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/00/20100/1
diff --git a/debian/vdsm-python.install b/debian/vdsm-python.install
index 0e73bfb..a639663 100644
--- a/debian/vdsm-python.install
+++ b/debian/vdsm-python.install
@@ -12,11 +12,10 @@
./usr/lib/python2.7/dist-packages/vdsm/qemuImg.py
./usr/lib/python2.7/dist-packages/vdsm/tool/__init__.py
./usr/lib/python2.7/dist-packages/vdsm/tool/dummybr.py
-./usr/lib/python2.7/dist-packages/vdsm/tool/libvirt_configure.py
+./usr/lib/python2.7/dist-packages/vdsm/tool/configurator.py
./usr/lib/python2.7/dist-packages/vdsm/tool/load_needed_modules.py
./usr/lib/python2.7/dist-packages/vdsm/tool/nwfilter.py
./usr/lib/python2.7/dist-packages/vdsm/tool/passwd.py
-./usr/lib/python2.7/dist-packages/vdsm/tool/sanlock.py
./usr/lib/python2.7/dist-packages/vdsm/tool/seboolsetup.py
./usr/lib/python2.7/dist-packages/vdsm/tool/service.py
./usr/lib/python2.7/dist-packages/vdsm/tool/validate_ovirt_certs.py
diff --git a/init/systemd/systemd-vdsmd.in b/init/systemd/systemd-vdsmd.in
index 692e5eb..3049793 100644
--- a/init/systemd/systemd-vdsmd.in
+++ b/init/systemd/systemd-vdsmd.in
@@ -26,5 +26,4 @@
[ $1 != reconfigure ] usage_exit
[ -n $2 -a $2 != force ] usage_exit
-@BINDIR@/vdsm-tool libvirt-configure ${2:+--force}
-@BINDIR@/vdsm-tool libvirt-configure-services-restart
+@BINDIR@/vdsm-tool configure-and-restart-all ${2:+--force}
diff --git a/init/sysvinit/vdsmd.init.in b/init/sysvinit/vdsmd.init.in
index 4a4fc7d..5a0d75a 100755
--- a/init/sysvinit/vdsmd.init.in
+++ b/init/sysvinit/vdsmd.init.in
@@ -108,11 +108,10 @@
return 1
}
-reconfigure_libvirt() {
+reconfigure() {
local force
[ ${1} = force ] force=--force
-$VDSM_TOOL libvirt-configure ${force}
-$VDSM_TOOL libvirt-configure-services-restart
+$VDSM_TOOL configure-and-restart-all ${force}
}
start() {
@@ -206,7 +205,7 @@
reconfigure)
# Jump over 'reconfigure'
shift 1
-reconfigure_libvirt $@
+reconfigure $@
RETVAL=$?
;;
*)
diff --git a/lib/vdsm/tool/Makefile.am b/lib/vdsm/tool/Makefile.am
index 9f00984..18e2b5f 100644
--- a/lib/vdsm/tool/Makefile.am
+++ b/lib/vdsm/tool/Makefile.am
@@ -38,9 +38,8 @@
__init__.py \
dummybr.py \
nwfilter.py \
- libvirt_configure.py \
+ configurator.py \
passwd.py \
- sanlock.py \
seboolsetup.py \
service.py \
vdsm-id.py \
diff --git a/lib/vdsm/tool/configurator.py b/lib/vdsm/tool/configurator.py
new file mode 100644
index 000..0854cb7
--- /dev/null
+++ b/lib/vdsm/tool/configurator.py
@@ -0,0 +1,144 @@
+# Copyright 2013 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+#
+# Refer to the README and COPYING files for full details of the license
+#
+
+import os
+import sys
+import grp
+
+from vdsm import utils
+import vdsm.tool
+from vdsm.tool import service
+from vdsm.constants import P_VDSM_EXEC, DISKIMAGE_GROUP
+
+
+def exec_libvirt_configure(action, *args):
+
+Invoke libvirt_configure.sh script
+
+if os.getuid() != 0:
+raise RuntimeError(Must run as root)
+
+rc, out, err = utils.execCmd([os.path.join(
+ P_VDSM_EXEC,
+ 'libvirt_configure.sh'), action] +
+ list(args),
+ raw=True)
+
+sys.stdout.write(out)
+
