Change in vdsm[master]: replace configure_libvirt.py with python code.
Nir Soffer has posted comments on this change. Change subject: replace configure_libvirt.py with python code. .. Patch Set 28: (1 comment) http://gerrit.ovirt.org/#/c/27298/28/lib/vdsm/tool/configurator.py File lib/vdsm/tool/configurator.py: Line 20: import os Line 21: import sys Line 22: import grp Line 23: import argparse Line 24: import pwd Please add a patch *before* this patch, sorting existing imports. This trivial patch can be merged quickly *without* this patch, saving your reviewers a lot of spam from gerrit. Line 25: import filecmp Line 26: import rpm Line 27: import shutil Line 28: import traceback -- To view, visit http://gerrit.ovirt.org/27298 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I74bfe05bb4b5f5d09021f21b324f9b7d5d0fdaab Gerrit-PatchSet: 28 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: mooli tayer mta...@redhat.com Gerrit-Reviewer: Alon Bar-Lev alo...@redhat.com Gerrit-Reviewer: Dima Kuznetsov dkuzn...@redhat.com Gerrit-Reviewer: Douglas Schilling Landgraf dougsl...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshz...@linux.vnet.ibm.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer mta...@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]: replace configure_libvirt.py with python code.
mooli tayer has posted comments on this change. Change subject: replace configure_libvirt.py with python code. .. Patch Set 28: Verified+1 -- To view, visit http://gerrit.ovirt.org/27298 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I74bfe05bb4b5f5d09021f21b324f9b7d5d0fdaab Gerrit-PatchSet: 28 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: mooli tayer mta...@redhat.com Gerrit-Reviewer: Alon Bar-Lev alo...@redhat.com Gerrit-Reviewer: Dima Kuznetsov dkuzn...@redhat.com Gerrit-Reviewer: Douglas Schilling Landgraf dougsl...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshz...@linux.vnet.ibm.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer mta...@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]: replace configure_libvirt.py with python code.
mooli tayer has posted comments on this change. Change subject: replace configure_libvirt.py with python code. .. Patch Set 30: Verified+1 -- To view, visit http://gerrit.ovirt.org/27298 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I74bfe05bb4b5f5d09021f21b324f9b7d5d0fdaab Gerrit-PatchSet: 30 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: mooli tayer mta...@redhat.com Gerrit-Reviewer: Alon Bar-Lev alo...@redhat.com Gerrit-Reviewer: Dima Kuznetsov dkuzn...@redhat.com Gerrit-Reviewer: Douglas Schilling Landgraf dougsl...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshz...@linux.vnet.ibm.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer mta...@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]: replace configure_libvirt.py with python code.
Yaniv Bronhaim has posted comments on this change. Change subject: replace configure_libvirt.py with python code. .. Patch Set 30: Code-Review+1 -- To view, visit http://gerrit.ovirt.org/27298 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I74bfe05bb4b5f5d09021f21b324f9b7d5d0fdaab Gerrit-PatchSet: 30 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: mooli tayer mta...@redhat.com Gerrit-Reviewer: Alon Bar-Lev alo...@redhat.com Gerrit-Reviewer: Dima Kuznetsov dkuzn...@redhat.com Gerrit-Reviewer: Douglas Schilling Landgraf dougsl...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshz...@linux.vnet.ibm.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer mta...@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]: replace configure_libvirt.py with python code.
Dan Kenigsberg has submitted this change and it was merged.
Change subject: replace configure_libvirt.py with python code.
..
replace configure_libvirt.py with python code.
The port is to behave as similar as possible to the old
sh code. python should allow easier testing (tests added)
and better overall readability.
A testing matrix [1] documents my tests of this feature.
The new code uses temp files and replace instead of line by line
editing for durability.
removeConf verb has been added and used in the spec file.
known behavior changes.
0.)order of configuration values added to config files has
changed. This does not seem to cause any issues.
1.)isConfigured() now also tests libvirt's
logrotate file for vdsm configuration section.
2.)configuration version is now added to /etc/logrotate.d/libvirt
like all the other files. (used to be '## beginning of ...' with
out version). this will allow versioning of this file.
No errors in removing old configuration.
3.)removeConf (now a verb and not done in vdsm.spec.in)
now calls ovirt_store_config when running on ovirt node.
(I guess before after removing vdsm rpms and restart
the vdsm configuration would re appear)
4.) log_filters inside libvirtd.conf used to be:
3:virobject 3:virfile 2:virnetlink 3:cgroup...
That was due to the following sh line:
set_if_default ${lconf} log_filters \3:virobject 3:virfile 2:virnetlink \
3:cgroup 3:event 3:json 1:libvirt 1:util 1:qemu\
now it is
3:virobject 3:virfile 2:virnetlink 3:cgroup...
both work fine.
[1] http://www.ovirt.org/Configure_libvirt_testing_matrix
Change-Id: I74bfe05bb4b5f5d09021f21b324f9b7d5d0fdaab
Signed-off-by: Mooli Tayer mta...@redhat.com
Reviewed-on: http://gerrit.ovirt.org/27298
Reviewed-by: Yaniv Bronhaim ybron...@redhat.com
Reviewed-by: Dan Kenigsberg dan...@redhat.com
---
M .gitignore
M configure.ac
M debian/vdsm-python.install
M lib/vdsm/constants.py.in
M lib/vdsm/tool/Makefile.am
M lib/vdsm/tool/configurator.py
D lib/vdsm/tool/libvirt_configure.sh.in
A lib/vdsm/tool/libvirtd.logrotate
M lib/vdsm/tool/passwd.py
M tests/Makefile.am
M tests/toolTests.py
A tests/toolTests_empty.conf
A tests/toolTests_lconf_ssl.conf
A tests/toolTests_libvirt_logrotate.conf
A tests/toolTests_libvirtd.conf
A tests/toolTests_qemu_sanlock.conf
A tests/toolTests_qemu_ssl.conf
A tests/toolTests_vdsm_no_ssl.conf
A tests/toolTests_vdsm_ssl.conf
M vdsm.spec.in
20 files changed, 776 insertions(+), 566 deletions(-)
Approvals:
Yaniv Bronhaim: Looks good to me, but someone else must approve
mooli tayer: Verified
Dan Kenigsberg: Looks good to me, approved
--
To view, visit http://gerrit.ovirt.org/27298
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I74bfe05bb4b5f5d09021f21b324f9b7d5d0fdaab
Gerrit-PatchSet: 31
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: mooli tayer mta...@redhat.com
Gerrit-Reviewer: Alon Bar-Lev alo...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Dima Kuznetsov dkuzn...@redhat.com
Gerrit-Reviewer: Douglas Schilling Landgraf dougsl...@redhat.com
Gerrit-Reviewer: Nir Soffer nsof...@redhat.com
Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com
Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com
Gerrit-Reviewer: Zhou Zheng Sheng zhshz...@linux.vnet.ibm.com
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: mooli tayer mta...@redhat.com
Gerrit-Reviewer: oVirt Jenkins CI Server
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: replace configure_libvirt.py with python code.
Dan Kenigsberg has posted comments on this change. Change subject: replace configure_libvirt.py with python code. .. Patch Set 30: Code-Review+2 Let's break stuff! -- To view, visit http://gerrit.ovirt.org/27298 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I74bfe05bb4b5f5d09021f21b324f9b7d5d0fdaab Gerrit-PatchSet: 30 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: mooli tayer mta...@redhat.com Gerrit-Reviewer: Alon Bar-Lev alo...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Dima Kuznetsov dkuzn...@redhat.com Gerrit-Reviewer: Douglas Schilling Landgraf dougsl...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshz...@linux.vnet.ibm.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer mta...@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]: replace configure_libvirt.py with python code.
oVirt Jenkins CI Server has posted comments on this change. Change subject: replace configure_libvirt.py with python code. .. Patch Set 31: Build Successful http://jenkins.ovirt.org/job/vdsm_master_install_rpm_sanity_gerrit/739/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_create-rpms_merged/1437/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/27298 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I74bfe05bb4b5f5d09021f21b324f9b7d5d0fdaab Gerrit-PatchSet: 31 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: mooli tayer mta...@redhat.com Gerrit-Reviewer: Alon Bar-Lev alo...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Dima Kuznetsov dkuzn...@redhat.com Gerrit-Reviewer: Douglas Schilling Landgraf dougsl...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshz...@linux.vnet.ibm.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer mta...@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]: replace configure_libvirt.py with python code.
mooli tayer has posted comments on this change. Change subject: replace configure_libvirt.py with python code. .. Patch Set 29: Verified+1 -- To view, visit http://gerrit.ovirt.org/27298 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I74bfe05bb4b5f5d09021f21b324f9b7d5d0fdaab Gerrit-PatchSet: 29 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: mooli tayer mta...@redhat.com Gerrit-Reviewer: Alon Bar-Lev alo...@redhat.com Gerrit-Reviewer: Dima Kuznetsov dkuzn...@redhat.com Gerrit-Reviewer: Douglas Schilling Landgraf dougsl...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshz...@linux.vnet.ibm.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer mta...@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]: replace configure_libvirt.py with python code.
oVirt Jenkins CI Server has posted comments on this change. Change subject: replace configure_libvirt.py with python code. .. Patch Set 29: Code-Review-1 Verified-1 Build Failed http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/9170/ : FAILURE http://jenkins.ovirt.org/job/vdsm_master_install_rpm_sanity_gerrit/736/ : FAILURE http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/9954/ : UNSTABLE http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/10109/ : FAILURE http://jenkins.ovirt.org/job/vdsm_master_verify-error-codes_merged/5036/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit-tests_merged/3193/ : FAILURE -- To view, visit http://gerrit.ovirt.org/27298 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I74bfe05bb4b5f5d09021f21b324f9b7d5d0fdaab Gerrit-PatchSet: 29 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: mooli tayer mta...@redhat.com Gerrit-Reviewer: Alon Bar-Lev alo...@redhat.com Gerrit-Reviewer: Dima Kuznetsov dkuzn...@redhat.com Gerrit-Reviewer: Douglas Schilling Landgraf dougsl...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshz...@linux.vnet.ibm.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer mta...@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]: replace configure_libvirt.py with python code.
oVirt Jenkins CI Server has posted comments on this change. Change subject: replace configure_libvirt.py with python code. .. Patch Set 24: Build Successful http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/9801/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/8863/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_install_rpm_sanity_gerrit/691/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/9648/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/27298 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I74bfe05bb4b5f5d09021f21b324f9b7d5d0fdaab Gerrit-PatchSet: 24 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: mooli tayer mta...@redhat.com Gerrit-Reviewer: Alon Bar-Lev alo...@redhat.com Gerrit-Reviewer: Dima Kuznetsov dkuzn...@redhat.com Gerrit-Reviewer: Douglas Schilling Landgraf dougsl...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshz...@linux.vnet.ibm.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer mta...@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]: replace configure_libvirt.py with python code.
mooli tayer has posted comments on this change. Change subject: replace configure_libvirt.py with python code. .. Patch Set 24: (3 comments) http://gerrit.ovirt.org/#/c/27298/24/lib/vdsm/tool/configurator.py File lib/vdsm/tool/configurator.py: Line 42: P_VDSM Line 43: from vdsm.config import config Line 44: Line 45: if utils.isOvirtNode(): Line 46: from ovirt.node.utils.fs import Config as NodeCfg node Line 47: Line 48: Line 49: class InvalidConfig(UsageError): Line 50: raise when invalid configuration passed Line 204: if os.path.isfile(TARGET): Line 205: oldmod = os.stat(TARGET).st_mode Line 206: Line 207: if utils.isOvirtNode(): Line 208: NodeCfg().unpersist(TARGET) node Line 209: shutil.copyfile(packaged, TARGET) Line 210: if utils.isOvirtNode(): Line 211: NodeCfg().persist(TARGET) Line 212: Line 309: Line 310: delete a file if it exists. Line 311: Line 312: utils.rmFile(content['path']) Line 313: if utils.isOvirtNode(): node Line 314: NodeCfg().unpersist(content['path']) Line 315: Line 316: def _unprefixAndRemoveSection(self, path): Line 317: -- To view, visit http://gerrit.ovirt.org/27298 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I74bfe05bb4b5f5d09021f21b324f9b7d5d0fdaab Gerrit-PatchSet: 24 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: mooli tayer mta...@redhat.com Gerrit-Reviewer: Alon Bar-Lev alo...@redhat.com Gerrit-Reviewer: Dima Kuznetsov dkuzn...@redhat.com Gerrit-Reviewer: Douglas Schilling Landgraf dougsl...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshz...@linux.vnet.ibm.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer mta...@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]: replace configure_libvirt.py with python code.
oVirt Jenkins CI Server has posted comments on this change. Change subject: replace configure_libvirt.py with python code. .. Patch Set 25: Build Successful http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/8910/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_install_rpm_sanity_gerrit/696/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/9694/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/9849/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/27298 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I74bfe05bb4b5f5d09021f21b324f9b7d5d0fdaab Gerrit-PatchSet: 25 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: mooli tayer mta...@redhat.com Gerrit-Reviewer: Alon Bar-Lev alo...@redhat.com Gerrit-Reviewer: Dima Kuznetsov dkuzn...@redhat.com Gerrit-Reviewer: Douglas Schilling Landgraf dougsl...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshz...@linux.vnet.ibm.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer mta...@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]: replace configure_libvirt.py with python code.
Yaniv Bronhaim has posted comments on this change. Change subject: replace configure_libvirt.py with python code. .. Patch Set 23: Code-Review+1 (1 comment) hope i haven't miss more changes . but expect the naming, it seems alright http://gerrit.ovirt.org/#/c/27298/23/lib/vdsm/tool/configurator.py File lib/vdsm/tool/configurator.py: Line 36: SANLOCK_ENABLED, LIBVIRT_SELINUX, P_VDSM Line 37: from vdsm.config import config Line 38: Line 39: if utils.isOvirtNode: Line 40: from ovirt.node.utils.fs import Config import as nodeCfg will be more easier to understand imo Line 41: Line 42: Line 43: class InvalidConfig(UsageError): Line 44: raise when invalid configuration passed -- To view, visit http://gerrit.ovirt.org/27298 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I74bfe05bb4b5f5d09021f21b324f9b7d5d0fdaab Gerrit-PatchSet: 23 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: mooli tayer mta...@redhat.com Gerrit-Reviewer: Alon Bar-Lev alo...@redhat.com Gerrit-Reviewer: Dima Kuznetsov dkuzn...@redhat.com Gerrit-Reviewer: Douglas Schilling Landgraf dougsl...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshz...@linux.vnet.ibm.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer mta...@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]: replace configure_libvirt.py with python code.
mooli tayer has posted comments on this change. Change subject: replace configure_libvirt.py with python code. .. Patch Set 23: (4 comments) http://gerrit.ovirt.org/#/c/27298/23/lib/vdsm/tool/configurator.py File lib/vdsm/tool/configurator.py: Line 36: SANLOCK_ENABLED, LIBVIRT_SELINUX, P_VDSM Line 37: from vdsm.config import config Line 38: Line 39: if utils.isOvirtNode: Line 40: from ovirt.node.utils.fs import Config Douglas: looks right? Line 41: Line 42: Line 43: class InvalidConfig(UsageError): Line 44: raise when invalid configuration passed Line 36: SANLOCK_ENABLED, LIBVIRT_SELINUX, P_VDSM Line 37: from vdsm.config import config Line 38: Line 39: if utils.isOvirtNode: Line 40: from ovirt.node.utils.fs import Config import as nodeCfg will be more easier to understand imo ybronhei: OK. (waiting for comments from Douglas) Line 41: Line 42: Line 43: class InvalidConfig(UsageError): Line 44: raise when invalid configuration passed Line 189: if os.path.isfile(TARGET): Line 190: oldmod = os.stat(TARGET).st_mode Line 191: Line 192: if utils.isOvirtNode(): Line 193: Config().unpersist(TARGET) Douglas: looks right? Line 194: shutil.copyfile(packaged, TARGET) Line 195: if utils.isOvirtNode(): Line 196: Config().persist(TARGET) Line 197: Line 301: delete a file if it exists. Line 302: Line 303: utils.rmFile(content['path']) Line 304: if utils.isOvirtNode(): Line 305: Config().unpersist(content['path']) Douglas: looks right? Line 306: Line 307: def _unprefixAndRemoveSection(self, path): Line 308: Line 309: undo changes done by _prefixAndPrepend. -- To view, visit http://gerrit.ovirt.org/27298 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I74bfe05bb4b5f5d09021f21b324f9b7d5d0fdaab Gerrit-PatchSet: 23 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: mooli tayer mta...@redhat.com Gerrit-Reviewer: Alon Bar-Lev alo...@redhat.com Gerrit-Reviewer: Dima Kuznetsov dkuzn...@redhat.com Gerrit-Reviewer: Douglas Schilling Landgraf dougsl...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshz...@linux.vnet.ibm.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer mta...@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]: replace configure_libvirt.py with python code.
oVirt Jenkins CI Server has posted comments on this change. Change subject: replace configure_libvirt.py with python code. .. Patch Set 23: Build Failed http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/9776/ : FAILURE http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/8838/ : FAILURE http://jenkins.ovirt.org/job/vdsm_master_install_rpm_sanity_gerrit/690/ : FAILURE http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/9624/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/27298 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I74bfe05bb4b5f5d09021f21b324f9b7d5d0fdaab Gerrit-PatchSet: 23 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: mooli tayer mta...@redhat.com Gerrit-Reviewer: Alon Bar-Lev alo...@redhat.com Gerrit-Reviewer: Dima Kuznetsov dkuzn...@redhat.com Gerrit-Reviewer: Douglas Schilling Landgraf dougsl...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshz...@linux.vnet.ibm.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer mta...@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]: replace configure_libvirt.py with python code.
Saggi Mizrahi has posted comments on this change.
Change subject: replace configure_libvirt.py with python code.
..
Patch Set 23:
(8 comments)
http://gerrit.ovirt.org/#/c/27298/23//COMMIT_MSG
Commit Message:
Line 6:
Line 7: replace configure_libvirt.py with python code.
Line 8:
Line 9: The port is to behave as similar as possible to the old
Line 10: sh code. python should allow easier testing (tests added
Seems like ) got moved around
Line 11: ) and better overall readability.
Line 12:
Line 13: A testing matrix [1] documents my tests of this feature.
Line 14:
http://gerrit.ovirt.org/#/c/27298/23/lib/vdsm/tool/configurator.py
File lib/vdsm/tool/configurator.py:
Line 30: from .. import utils
Line 31: from . import service, expose, validate_ovirt_certs
Line 32: from . import NotRootError, UsageError
Line 33: from configfile import ConfigFile, ParserWrapper
Line 34: from ..constants import QEMU_PROCESS_GROUP, \
If you are already working on this line please change it to
from .. constants import \
QEMU_PROCESS_GROUP,
SANLOCK_USER,
.
So that future diffs are easier to read.
Line 35: SANLOCK_USER, VDSM_GROUP, SYSCONF_PATH, P_VDSM_CERT, \
Line 36: SANLOCK_ENABLED, LIBVIRT_SELINUX, P_VDSM
Line 37: from vdsm.config import config
Line 38:
Line 35: SANLOCK_USER, VDSM_GROUP, SYSCONF_PATH, P_VDSM_CERT, \
Line 36: SANLOCK_ENABLED, LIBVIRT_SELINUX, P_VDSM
Line 37: from vdsm.config import config
Line 38:
Line 39: if utils.isOvirtNode:
Again, I think it's a function.
Line 40: from ovirt.node.utils.fs import Config
Line 41:
Line 42:
Line 43: class InvalidConfig(UsageError):
Line 117: # write configuration
Line 118: for cfile, content in self.FILES.items():
Line 119: content['configure'](self, content, vdsmConfiguration)
Line 120:
Line 121: sys.stdout.write(Reconfiguration of libvirt is done.)
end with \n?
Why write to stdout directly?
Line 122:
Line 123: def validate(self):
Line 124:
Line 125: Validate conflict in configured files
Line 179: LIBVIRTD_UPSTART
os.path.basename(fname) == LIBVIRTD_UPSTART
is much more accurate representation of what you actually trying to do.
Line 183: if os.path.isfile(packaged):
Line 184: if not os.path.isfile(TARGET):
Line 185: service.service_stop('libvirtd')
Line 186: if (not os.path.isfile(TARGET) or
Line 187: not filecmp.cmp(packaged, TARGET)):
if not os.path.isfile(TARGET):
service.service_stop('libvirtd')
if not filecmp.cmp(packaged, TARGET):
Line 188: oldmod = None
Line 189: if os.path.isfile(TARGET):
Line 190: oldmod = os.stat(TARGET).st_mode
Line 191:
Line 197:
Line 198: if (oldmod is not None and
Line 199: oldmod != os.stat(TARGET).st_mode):
Line 200: os.chmod(TARGET, oldmod)
Line 201: rc, out, err = utils.execCmd((INITCTL,
Factor out to a proper function
Line 202:
reload-configuration))
Line 203: if rc != 0:
Line 204: sys.stdout.write(out)
Line 205: sys.stderr.write(err)
Line 215: ssl = config.getboolean('vars', 'ssl')
Line 216:
Line 217: lconf_p = ParserWrapper({
Line 218: 'listen_tcp': '0',
Line 219: 'auth_tcp': 'sasl'
Add comma at EOL
Line 220: })
Line 221: lconf_p.read(self._getFile('LCONF'))
Line 222: listen_tcp = lconf_p.getint('listen_tcp')
Line 223: auth_tcp = lconf_p.get('auth_tcp')
--
To view, visit http://gerrit.ovirt.org/27298
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I74bfe05bb4b5f5d09021f21b324f9b7d5d0fdaab
Gerrit-PatchSet: 23
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: mooli tayer mta...@redhat.com
Gerrit-Reviewer: Alon Bar-Lev alo...@redhat.com
Gerrit-Reviewer: Dima Kuznetsov dkuzn...@redhat.com
Gerrit-Reviewer: Douglas Schilling Landgraf dougsl...@redhat.com
Gerrit-Reviewer: Nir Soffer nsof...@redhat.com
Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com
Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com
Gerrit-Reviewer: Zhou Zheng Sheng zhshz...@linux.vnet.ibm.com
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: mooli tayer mta...@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]: replace configure_libvirt.py with python code.
mooli tayer has posted comments on this change.
Change subject: replace configure_libvirt.py with python code.
..
Patch Set 23:
(8 comments)
http://gerrit.ovirt.org/#/c/27298/23//COMMIT_MSG
Commit Message:
Line 6:
Line 7: replace configure_libvirt.py with python code.
Line 8:
Line 9: The port is to behave as similar as possible to the old
Line 10: sh code. python should allow easier testing (tests added
Seems like ) got moved around
fixed
Line 11: ) and better overall readability.
Line 12:
Line 13: A testing matrix [1] documents my tests of this feature.
Line 14:
http://gerrit.ovirt.org/#/c/27298/23/lib/vdsm/tool/configurator.py
File lib/vdsm/tool/configurator.py:
Line 30: from .. import utils
Line 31: from . import service, expose, validate_ovirt_certs
Line 32: from . import NotRootError, UsageError
Line 33: from configfile import ConfigFile, ParserWrapper
Line 34: from ..constants import QEMU_PROCESS_GROUP, \
If you are already working on this line please change it to
from .. constants import \
QEMU_PROCESS_GROUP, \
SANLOCK_USER, \
...
?
Line 35: SANLOCK_USER, VDSM_GROUP, SYSCONF_PATH, P_VDSM_CERT, \
Line 36: SANLOCK_ENABLED, LIBVIRT_SELINUX, P_VDSM
Line 37: from vdsm.config import config
Line 38:
Line 35: SANLOCK_USER, VDSM_GROUP, SYSCONF_PATH, P_VDSM_CERT, \
Line 36: SANLOCK_ENABLED, LIBVIRT_SELINUX, P_VDSM
Line 37: from vdsm.config import config
Line 38:
Line 39: if utils.isOvirtNode:
Again, I think it's a function.
changed.
Line 40: from ovirt.node.utils.fs import Config
Line 41:
Line 42:
Line 43: class InvalidConfig(UsageError):
Line 117: # write configuration
Line 118: for cfile, content in self.FILES.items():
Line 119: content['configure'](self, content, vdsmConfiguration)
Line 120:
Line 121: sys.stdout.write(Reconfiguration of libvirt is done.)
end with \n?
Added \n, also notice this is moved to verb level here:
http://gerrit.ovirt.org/#/c/27841/
Line 122:
Line 123: def validate(self):
Line 124:
Line 125: Validate conflict in configured files
Line 179: LIBVIRTD_UPSTART
os.path.basename(fname) == LIBVIRTD_UPSTART
Done.
Line 183: if os.path.isfile(packaged):
Line 184: if not os.path.isfile(TARGET):
Line 185: service.service_stop('libvirtd')
Line 186: if (not os.path.isfile(TARGET) or
Line 187: not filecmp.cmp(packaged, TARGET)):
if not os.path.isfile(TARGET):
iiuc what you are saying would be true if it was:
not os.path.isfile(TARGET) and not filecmp.cmp(packaged, TARGET)
I think
if a():
x()
if a or b:
y()
is not the same as
if a:
x()
if b:
y()
In the case where a = False and b = True.
Line 188: oldmod = None
Line 189: if os.path.isfile(TARGET):
Line 190: oldmod = os.stat(TARGET).st_mode
Line 191:
Line 197:
Line 198: if (oldmod is not None and
Line 199: oldmod != os.stat(TARGET).st_mode):
Line 200: os.chmod(TARGET, oldmod)
Line 201: rc, out, err = utils.execCmd((INITCTL,
Factor out to a proper function
Done
Line 202:
reload-configuration))
Line 203: if rc != 0:
Line 204: sys.stdout.write(out)
Line 205: sys.stderr.write(err)
Line 215: ssl = config.getboolean('vars', 'ssl')
Line 216:
Line 217: lconf_p = ParserWrapper({
Line 218: 'listen_tcp': '0',
Line 219: 'auth_tcp': 'sasl'
Add comma at EOL
Done
Line 220: })
Line 221: lconf_p.read(self._getFile('LCONF'))
Line 222: listen_tcp = lconf_p.getint('listen_tcp')
Line 223: auth_tcp = lconf_p.get('auth_tcp')
--
To view, visit http://gerrit.ovirt.org/27298
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I74bfe05bb4b5f5d09021f21b324f9b7d5d0fdaab
Gerrit-PatchSet: 23
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: mooli tayer mta...@redhat.com
Gerrit-Reviewer: Alon Bar-Lev alo...@redhat.com
Gerrit-Reviewer: Dima Kuznetsov dkuzn...@redhat.com
Gerrit-Reviewer: Douglas Schilling Landgraf dougsl...@redhat.com
Gerrit-Reviewer: Nir Soffer nsof...@redhat.com
Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com
Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com
Gerrit-Reviewer: Zhou Zheng Sheng zhshz...@linux.vnet.ibm.com
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: mooli tayer mta...@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]: replace configure_libvirt.py with python code.
Yaniv Bronhaim has posted comments on this change. Change subject: replace configure_libvirt.py with python code. .. Patch Set 22: Code-Review+1 -- To view, visit http://gerrit.ovirt.org/27298 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I74bfe05bb4b5f5d09021f21b324f9b7d5d0fdaab Gerrit-PatchSet: 22 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: mooli tayer mta...@redhat.com Gerrit-Reviewer: Alon Bar-Lev alo...@redhat.com Gerrit-Reviewer: Dima Kuznetsov dkuzn...@redhat.com Gerrit-Reviewer: Douglas Schilling Landgraf dougsl...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshz...@linux.vnet.ibm.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer mta...@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]: replace configure_libvirt.py with python code.
Yaniv Bronhaim has posted comments on this change. Change subject: replace configure_libvirt.py with python code. .. Patch Set 21: (1 comment) http://gerrit.ovirt.org/#/c/27298/21/lib/vdsm/tool/configurator.py File lib/vdsm/tool/configurator.py: Line 597: ) Line 598: sys.stdout.write(out) Line 599: sys.stderr.write(err) Line 600: if rc != 0: Line 601: raise RuntimeError(Failed to perform sanlock config.) Should this not be invalidRun too? yes.. but you don't want to change anything that doesn't relate to your change. so leave it for now Line 602: Line 603: def isconfigured(self): Line 604: Line 605: True if sanlock service is configured, False if sanlock service -- To view, visit http://gerrit.ovirt.org/27298 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I74bfe05bb4b5f5d09021f21b324f9b7d5d0fdaab Gerrit-PatchSet: 21 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: mooli tayer mta...@redhat.com Gerrit-Reviewer: Alon Bar-Lev alo...@redhat.com Gerrit-Reviewer: Dima Kuznetsov dkuzn...@redhat.com Gerrit-Reviewer: Douglas Schilling Landgraf dougsl...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshz...@linux.vnet.ibm.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer mta...@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]: replace configure_libvirt.py with python code.
Yaniv Bronhaim has posted comments on this change. Change subject: replace configure_libvirt.py with python code. .. Patch Set 22: just verify again. the code looks complete from my prospective -- To view, visit http://gerrit.ovirt.org/27298 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I74bfe05bb4b5f5d09021f21b324f9b7d5d0fdaab Gerrit-PatchSet: 22 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: mooli tayer mta...@redhat.com Gerrit-Reviewer: Alon Bar-Lev alo...@redhat.com Gerrit-Reviewer: Dima Kuznetsov dkuzn...@redhat.com Gerrit-Reviewer: Douglas Schilling Landgraf dougsl...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshz...@linux.vnet.ibm.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer mta...@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]: replace configure_libvirt.py with python code.
mooli tayer has posted comments on this change. Change subject: replace configure_libvirt.py with python code. .. Patch Set 21: (3 comments) http://gerrit.ovirt.org/#/c/27298/21/lib/vdsm/tool/configurator.py File lib/vdsm/tool/configurator.py: Line 92: Line 93: if utils.isOvirtNode(): Line 94: if not os.path.exists(P_VDSM_CERT): Line 95: raise RuntimeError( Line 96: vdsm: Missing certificate, vdsm not registered) raise InvalidRun instead, http://gerrit.ovirt.org/26565 was merged. Done. thanks Line 97: validate_ovirt_certs.validate_ovirt_certs() Line 98: Line 99: # Remove a previous configuration (if present) Line 100: self.removeConf() Line 203: reload-configuration)) Line 204: if rc != 0: Line 205: sys.stdout.write(out) Line 206: sys.stderr.write(err) Line 207: raise RuntimeError( same Done Line 208: Failed to reload upstart configuration.) Line 209: Line 210: def _isSslConflict(self): Line 211: Line 597: ) Line 598: sys.stdout.write(out) Line 599: sys.stderr.write(err) Line 600: if rc != 0: Line 601: raise RuntimeError(Failed to perform sanlock config.) Should this not be invalidRun too? I can do in a different patch Line 602: Line 603: def isconfigured(self): Line 604: Line 605: True if sanlock service is configured, False if sanlock service -- To view, visit http://gerrit.ovirt.org/27298 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I74bfe05bb4b5f5d09021f21b324f9b7d5d0fdaab Gerrit-PatchSet: 21 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: mooli tayer mta...@redhat.com Gerrit-Reviewer: Alon Bar-Lev alo...@redhat.com Gerrit-Reviewer: Dima Kuznetsov dkuzn...@redhat.com Gerrit-Reviewer: Douglas Schilling Landgraf dougsl...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshz...@linux.vnet.ibm.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer mta...@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]: replace configure_libvirt.py with python code.
oVirt Jenkins CI Server has posted comments on this change. Change subject: replace configure_libvirt.py with python code. .. Patch Set 22: Build Failed http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/9650/ : FAILURE http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/8718/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_install_rpm_sanity_gerrit/673/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/9504/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_storage_functional_tests_localfs_gerrit/1043/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/27298 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I74bfe05bb4b5f5d09021f21b324f9b7d5d0fdaab Gerrit-PatchSet: 22 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: mooli tayer mta...@redhat.com Gerrit-Reviewer: Alon Bar-Lev alo...@redhat.com Gerrit-Reviewer: Dima Kuznetsov dkuzn...@redhat.com Gerrit-Reviewer: Douglas Schilling Landgraf dougsl...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshz...@linux.vnet.ibm.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer mta...@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]: replace configure_libvirt.py with python code.
Yaniv Bronhaim has posted comments on this change.
Change subject: replace configure_libvirt.py with python code.
..
Patch Set 20: Code-Review-1
(15 comments)
http://gerrit.ovirt.org/#/c/27298/20/lib/vdsm/tool/configurator.py
File lib/vdsm/tool/configurator.py:
Line 78:
Line 79: def getName(self):
Line 80: return 'libvirt'
Line 81:
Line 82: def getFile(self, fname):
should be private
Line 83: return self.FILES[fname]['path']
Line 84:
Line 85: def getServices(self):
Line 86: return [supervdsmd, vdsmd, libvirtd]
Line 137: ret = False
Line 138: if not ret:
Line 139: sys.stdout.write(libvirt is not configured for vdsm
yet\n)
Line 140: else:
Line 141: sys.stdout.write(libvirt is already configured for
vdsm\n)
same as the print of configure, the verb prints it, so here its redundant.
please remove
Line 142: return ret
Line 143:
Line 144: def removeConf(self):
Line 145: for cfile, content in LibvirtModuleConfigure.FILES.items():
Line 163:
Line 164: def _sysvToUpstart(self):
Line 165:
Line 166: On RHEL 6, libvirtd can be started by either SysV init or
Upstart.
Line 167: We prefer upstart because it respawns libvirtd if when
libvirtd
remove the if or the when
Line 168: crashed.
Line 169:
Line 170: def iterateLibvirtFiles():
Line 171: ts = rpm.TransactionSet()
Line 173: for matches in ts.dbMatch('name', name):
Line 174: for filename in matches[rpm.RPMTAG_FILENAMES]:
Line 175: yield filename
Line 176:
Line 177: INITCTL = '/sbin/initctl'
we have declaration for this in service.py (_INITCTL), make it accessible
Line 178: LIBVIRTD_UPSTART = 'libvirtd.upstart'
Line 179: TARGET = os.path.join(SYSCONF_PATH, init/libvirtd.conf)
Line 180:
Line 181: if os.path.isfile(INITCTL) and os.access(INITCTL, os.X_OK):
Line 200: if (oldmod is not None and
Line 201: oldmod != os.stat(TARGET).st_mode):
Line 202: os.chmod(TARGET, oldmod)
Line 203: rc, out, err = utils.execCmd((INITCTL,
Line 204:
reload-configuration))
can't we use service.py for that?
Line 205: if rc != 0:
Line 206: sys.stdout.write(out)
Line 207: sys.stderr.write(err)
Line 208: raise RuntimeError(
Line 209: Failed to reload upstart configuration.)
Line 210:
Line 211: def _isSslConflict(self):
Line 212:
Line 213: return True iff libvirt files ssl configuration match
vdsm.conf
return True if libvirt configuration files match ssl configuration of vdsm.conf
Line 214:
Line 215: config.read(self.getFile('VDSM_CONF'))
Line 216: ssl = config.getboolean('vars', 'ssl')
Line 217:
Line 257: return ret
Line 258:
Line 259: def _isApplicable(self, fragment, vdsmConfiguration):
Line 260:
Line 261: Return true iff this fragment should be included for
current
return True if fragment ..
Line 262: configuration. An applicaple fragment is a fragment who's
list
Line 263: of conditions are met according to vdsmConfiguration.
Line 264:
Line 265: applyFragment = True
Line 305: try:
Line 306: os.remove(content['path'])
Line 307: except OSError as e:
Line 308: if e.errno != errno.ENOENT:
Line 309: raise
use utils.rmFile
Line 310:
Line 311: # On removeConf functions
Line 312: def _unprefixAndRemoveSection(self, path):
Line 313:
Line 307: except OSError as e:
Line 308: if e.errno != errno.ENOENT:
Line 309: raise
Line 310:
Line 311: # On removeConf functions
I don't understand those comments (On removeConf functions, On configure
functions). I think its redundant . I'll read configure and see what it uses
Line 312: def _unprefixAndRemoveSection(self, path):
Line 313:
Line 314: undo changes done by _prefixAndPrepend.
Line 315:
Line 329:
Line 330: # version != PACKAGE_VERSION since we do not want to update
configuration
Line 331: # on every update. see 'configuration versioning:' at
ConfigFile.py for
Line 332: # details.
Line 333: CONF_VERSION = '4.13.0'
configfile.py ? I don't see more details there..
Line 334:
Line 335: PKI_DIR = os.path.join(SYSCONF_PATH, 'pki/vdsm')
Line 336: CA_FILE = os.path.join(PKI_DIR, 'certs/cacert.pem')
Line 337: CERT_FILE = os.path.join(PKI_DIR, 'certs/vdsmcert.pem')
Line 352: 'removeConf': lambda x, y: True,
Line 353:
Change in vdsm[master]: replace configure_libvirt.py with python code.
mooli tayer has posted comments on this change.
Change subject: replace configure_libvirt.py with python code.
..
Patch Set 20:
(15 comments)
http://gerrit.ovirt.org/#/c/27298/20/lib/vdsm/tool/configurator.py
File lib/vdsm/tool/configurator.py:
Line 78:
Line 79: def getName(self):
Line 80: return 'libvirt'
Line 81:
Line 82: def getFile(self, fname):
should be private
Done
Line 83: return self.FILES[fname]['path']
Line 84:
Line 85: def getServices(self):
Line 86: return [supervdsmd, vdsmd, libvirtd]
Line 137: ret = False
Line 138: if not ret:
Line 139: sys.stdout.write(libvirt is not configured for vdsm
yet\n)
Line 140: else:
Line 141: sys.stdout.write(libvirt is already configured for
vdsm\n)
same as the print of configure, the verb prints it, so here its redundant
currently we have modules print + verb print.
example of current output:
libvirt is not configured for vdsm yet\n
sanlock service is already configured\n
Modules libvirt are not configured\n
I do exactly the same so the output will not change
after this patch. If you want to fix it it should be done
in a different patch (and sanlock print should be removed too)
Line 142: return ret
Line 143:
Line 144: def removeConf(self):
Line 145: for cfile, content in LibvirtModuleConfigure.FILES.items():
Line 163:
Line 164: def _sysvToUpstart(self):
Line 165:
Line 166: On RHEL 6, libvirtd can be started by either SysV init or
Upstart.
Line 167: We prefer upstart because it respawns libvirtd if when
libvirtd
remove the if or the when
Done
Line 168: crashed.
Line 169:
Line 170: def iterateLibvirtFiles():
Line 171: ts = rpm.TransactionSet()
Line 173: for matches in ts.dbMatch('name', name):
Line 174: for filename in matches[rpm.RPMTAG_FILENAMES]:
Line 175: yield filename
Line 176:
Line 177: INITCTL = '/sbin/initctl'
we have declaration for this in service.py (_INITCTL), make it accessible
Yes I actually did it that way at first but I do not agree.
service.py exposes only stuff in a way that is init system
agnostic. now we could start exposing a way to only start
a specific init system there, or only the path for init
systems but it kind of breaks encapsulation.
It is much better to simply have initctl here
too. if this bugs you too much we can expose init systems
path in a place like constants.py.in.
Line 178: LIBVIRTD_UPSTART = 'libvirtd.upstart'
Line 179: TARGET = os.path.join(SYSCONF_PATH, init/libvirtd.conf)
Line 180:
Line 181: if os.path.isfile(INITCTL) and os.access(INITCTL, os.X_OK):
Line 200: if (oldmod is not None and
Line 201: oldmod != os.stat(TARGET).st_mode):
Line 202: os.chmod(TARGET, oldmod)
Line 203: rc, out, err = utils.execCmd((INITCTL,
Line 204:
reload-configuration))
can't we use service.py for that?
Please see previews comment.
I believe calling initctl explicitly is best here.
Line 205: if rc != 0:
Line 206: sys.stdout.write(out)
Line 207: sys.stderr.write(err)
Line 208: raise RuntimeError(
Line 209: Failed to reload upstart configuration.)
Line 210:
Line 211: def _isSslConflict(self):
Line 212:
Line 213: return True iff libvirt files ssl configuration match
vdsm.conf
return True if libvirt configuration files match ssl configuration of vdsm.
Done. iff means 'if and only if' but maybe that's unclear...
Line 214:
Line 215: config.read(self.getFile('VDSM_CONF'))
Line 216: ssl = config.getboolean('vars', 'ssl')
Line 217:
Line 257: return ret
Line 258:
Line 259: def _isApplicable(self, fragment, vdsmConfiguration):
Line 260:
Line 261: Return true iff this fragment should be included for
current
return True if fragment ..
Done
Line 262: configuration. An applicaple fragment is a fragment who's
list
Line 263: of conditions are met according to vdsmConfiguration.
Line 264:
Line 265: applyFragment = True
Line 305: try:
Line 306: os.remove(content['path'])
Line 307: except OSError as e:
Line 308: if e.errno != errno.ENOENT:
Line 309: raise
use utils.rmFile
Done
Line 310:
Line 311: # On removeConf functions
Line 312: def _unprefixAndRemoveSection(self, path):
Line 313:
Line 307: except OSError as e:
Line 308: if e.errno != errno.ENOENT:
Line 309: raise
Line
Change in vdsm[master]: replace configure_libvirt.py with python code.
mooli tayer has posted comments on this change. Change subject: replace configure_libvirt.py with python code. .. Patch Set 20: Verified-1 Removing until ovirt-node verification can be done. -- To view, visit http://gerrit.ovirt.org/27298 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I74bfe05bb4b5f5d09021f21b324f9b7d5d0fdaab Gerrit-PatchSet: 20 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: mooli tayer mta...@redhat.com Gerrit-Reviewer: Alon Bar-Lev alo...@redhat.com Gerrit-Reviewer: Dima Kuznetsov dkuzn...@redhat.com Gerrit-Reviewer: Douglas Schilling Landgraf dougsl...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshz...@linux.vnet.ibm.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer mta...@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]: replace configure_libvirt.py with python code.
Yaniv Bronhaim has posted comments on this change.
Change subject: replace configure_libvirt.py with python code.
..
Patch Set 20:
(4 comments)
that looks complete. only nits and the verification with node after restart (to
see if persist works as expected)
http://gerrit.ovirt.org/#/c/27298/20/lib/vdsm/tool/configurator.py
File lib/vdsm/tool/configurator.py:
Line 137: ret = False
Line 138: if not ret:
Line 139: sys.stdout.write(libvirt is not configured for vdsm
yet\n)
Line 140: else:
Line 141: sys.stdout.write(libvirt is already configured for
vdsm\n)
currently we have modules print + verb print.
ok
Line 142: return ret
Line 143:
Line 144: def removeConf(self):
Line 145: for cfile, content in LibvirtModuleConfigure.FILES.items():
Line 173: for matches in ts.dbMatch('name', name):
Line 174: for filename in matches[rpm.RPMTAG_FILENAMES]:
Line 175: yield filename
Line 176:
Line 177: INITCTL = '/sbin/initctl'
Yes I actually did it that way at first but I do not agree.
leave it as is
Line 178: LIBVIRTD_UPSTART = 'libvirtd.upstart'
Line 179: TARGET = os.path.join(SYSCONF_PATH, init/libvirtd.conf)
Line 180:
Line 181: if os.path.isfile(INITCTL) and os.access(INITCTL, os.X_OK):
Line 200: if (oldmod is not None and
Line 201: oldmod != os.stat(TARGET).st_mode):
Line 202: os.chmod(TARGET, oldmod)
Line 203: rc, out, err = utils.execCmd((INITCTL,
Line 204:
reload-configuration))
Please see previews comment.
ok
Line 205: if rc != 0:
Line 206: sys.stdout.write(out)
Line 207: sys.stderr.write(err)
Line 208: raise RuntimeError(
Line 329:
Line 330: # version != PACKAGE_VERSION since we do not want to update
configuration
Line 331: # on every update. see 'configuration versioning:' at
ConfigFile.py for
Line 332: # details.
Line 333: CONF_VERSION = '4.13.0'
Please search for 'configuration versioning' there.
ok
Line 334:
Line 335: PKI_DIR = os.path.join(SYSCONF_PATH, 'pki/vdsm')
Line 336: CA_FILE = os.path.join(PKI_DIR, 'certs/cacert.pem')
Line 337: CERT_FILE = os.path.join(PKI_DIR, 'certs/vdsmcert.pem')
--
To view, visit http://gerrit.ovirt.org/27298
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I74bfe05bb4b5f5d09021f21b324f9b7d5d0fdaab
Gerrit-PatchSet: 20
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: mooli tayer mta...@redhat.com
Gerrit-Reviewer: Alon Bar-Lev alo...@redhat.com
Gerrit-Reviewer: Dima Kuznetsov dkuzn...@redhat.com
Gerrit-Reviewer: Douglas Schilling Landgraf dougsl...@redhat.com
Gerrit-Reviewer: Nir Soffer nsof...@redhat.com
Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com
Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com
Gerrit-Reviewer: Zhou Zheng Sheng zhshz...@linux.vnet.ibm.com
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: mooli tayer mta...@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]: replace configure_libvirt.py with python code.
oVirt Jenkins CI Server has posted comments on this change. Change subject: replace configure_libvirt.py with python code. .. Patch Set 21: Build Failed http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/9630/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/8698/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_install_rpm_sanity_gerrit/667/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/9484/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_storage_functional_tests_localfs_gerrit/1028/ : FAILURE -- To view, visit http://gerrit.ovirt.org/27298 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I74bfe05bb4b5f5d09021f21b324f9b7d5d0fdaab Gerrit-PatchSet: 21 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: mooli tayer mta...@redhat.com Gerrit-Reviewer: Alon Bar-Lev alo...@redhat.com Gerrit-Reviewer: Dima Kuznetsov dkuzn...@redhat.com Gerrit-Reviewer: Douglas Schilling Landgraf dougsl...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshz...@linux.vnet.ibm.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer mta...@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]: replace configure_libvirt.py with python code.
Yaniv Bronhaim has posted comments on this change. Change subject: replace configure_libvirt.py with python code. .. Patch Set 21: (2 comments) http://gerrit.ovirt.org/#/c/27298/21/lib/vdsm/tool/configurator.py File lib/vdsm/tool/configurator.py: Line 92: Line 93: if utils.isOvirtNode(): Line 94: if not os.path.exists(P_VDSM_CERT): Line 95: raise RuntimeError( Line 96: vdsm: Missing certificate, vdsm not registered) raise InvalidRun instead, http://gerrit.ovirt.org/26565 was merged. sorry to miss that last time Line 97: validate_ovirt_certs.validate_ovirt_certs() Line 98: Line 99: # Remove a previous configuration (if present) Line 100: self.removeConf() Line 203: reload-configuration)) Line 204: if rc != 0: Line 205: sys.stdout.write(out) Line 206: sys.stderr.write(err) Line 207: raise RuntimeError( same Line 208: Failed to reload upstart configuration.) Line 209: Line 210: def _isSslConflict(self): Line 211: -- To view, visit http://gerrit.ovirt.org/27298 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I74bfe05bb4b5f5d09021f21b324f9b7d5d0fdaab Gerrit-PatchSet: 21 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: mooli tayer mta...@redhat.com Gerrit-Reviewer: Alon Bar-Lev alo...@redhat.com Gerrit-Reviewer: Dima Kuznetsov dkuzn...@redhat.com Gerrit-Reviewer: Douglas Schilling Landgraf dougsl...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshz...@linux.vnet.ibm.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer mta...@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]: replace configure_libvirt.py with python code.
oVirt Jenkins CI Server has posted comments on this change. Change subject: replace configure_libvirt.py with python code. .. Patch Set 18: Build Failed http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/9594/ : FAILURE http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/8662/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_install_rpm_sanity_gerrit/662/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/9448/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_storage_functional_tests_localfs_gerrit/1009/ : FAILURE -- To view, visit http://gerrit.ovirt.org/27298 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I74bfe05bb4b5f5d09021f21b324f9b7d5d0fdaab Gerrit-PatchSet: 18 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: mooli tayer mta...@redhat.com Gerrit-Reviewer: Alon Bar-Lev alo...@redhat.com Gerrit-Reviewer: Dima Kuznetsov dkuzn...@redhat.com Gerrit-Reviewer: Douglas Schilling Landgraf dougsl...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshz...@linux.vnet.ibm.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer mta...@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]: replace configure_libvirt.py with python code.
oVirt Jenkins CI Server has posted comments on this change. Change subject: replace configure_libvirt.py with python code. .. Patch Set 19: Build Failed http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/9601/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/8669/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_install_rpm_sanity_gerrit/663/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/9455/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_storage_functional_tests_localfs_gerrit/1013/ : FAILURE -- To view, visit http://gerrit.ovirt.org/27298 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I74bfe05bb4b5f5d09021f21b324f9b7d5d0fdaab Gerrit-PatchSet: 19 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: mooli tayer mta...@redhat.com Gerrit-Reviewer: Alon Bar-Lev alo...@redhat.com Gerrit-Reviewer: Dima Kuznetsov dkuzn...@redhat.com Gerrit-Reviewer: Douglas Schilling Landgraf dougsl...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshz...@linux.vnet.ibm.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer mta...@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]: replace configure_libvirt.py with python code.
mooli tayer has posted comments on this change. Change subject: replace configure_libvirt.py with python code. .. Patch Set 19: Verified+1 -- To view, visit http://gerrit.ovirt.org/27298 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I74bfe05bb4b5f5d09021f21b324f9b7d5d0fdaab Gerrit-PatchSet: 19 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: mooli tayer mta...@redhat.com Gerrit-Reviewer: Alon Bar-Lev alo...@redhat.com Gerrit-Reviewer: Dima Kuznetsov dkuzn...@redhat.com Gerrit-Reviewer: Douglas Schilling Landgraf dougsl...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshz...@linux.vnet.ibm.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer mta...@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]: replace configure_libvirt.py with python code.
mooli tayer has posted comments on this change. Change subject: replace configure_libvirt.py with python code. .. Patch Set 20: Verified+1 -- To view, visit http://gerrit.ovirt.org/27298 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I74bfe05bb4b5f5d09021f21b324f9b7d5d0fdaab Gerrit-PatchSet: 20 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: mooli tayer mta...@redhat.com Gerrit-Reviewer: Alon Bar-Lev alo...@redhat.com Gerrit-Reviewer: Dima Kuznetsov dkuzn...@redhat.com Gerrit-Reviewer: Douglas Schilling Landgraf dougsl...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshz...@linux.vnet.ibm.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer mta...@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]: replace configure_libvirt.py with python code.
oVirt Jenkins CI Server has posted comments on this change. Change subject: replace configure_libvirt.py with python code. .. Patch Set 20: Build Successful http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/9608/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/8676/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_install_rpm_sanity_gerrit/664/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/9462/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_storage_functional_tests_localfs_gerrit/1014/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/27298 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I74bfe05bb4b5f5d09021f21b324f9b7d5d0fdaab Gerrit-PatchSet: 20 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: mooli tayer mta...@redhat.com Gerrit-Reviewer: Alon Bar-Lev alo...@redhat.com Gerrit-Reviewer: Dima Kuznetsov dkuzn...@redhat.com Gerrit-Reviewer: Douglas Schilling Landgraf dougsl...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshz...@linux.vnet.ibm.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer mta...@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]: replace configure_libvirt.py with python code.
Douglas Schilling Landgraf has posted comments on this change. Change subject: replace configure_libvirt.py with python code. .. Patch Set 20: Mooli, as we are testing in ovirt-node, please click in verified just after the validation. -- To view, visit http://gerrit.ovirt.org/27298 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I74bfe05bb4b5f5d09021f21b324f9b7d5d0fdaab Gerrit-PatchSet: 20 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: mooli tayer mta...@redhat.com Gerrit-Reviewer: Alon Bar-Lev alo...@redhat.com Gerrit-Reviewer: Dima Kuznetsov dkuzn...@redhat.com Gerrit-Reviewer: Douglas Schilling Landgraf dougsl...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshz...@linux.vnet.ibm.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer mta...@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]: replace configure_libvirt.py with python code.
mooli tayer has posted comments on this change. Change subject: replace configure_libvirt.py with python code. .. Patch Set 17: (1 comment) http://gerrit.ovirt.org/#/c/27298/17/lib/vdsm/tool/configurator.py File lib/vdsm/tool/configurator.py: Line 193: shutil.copyfile(packaged, TARGET) Line 194: if (oldmod is not None and Line 195: oldmod != os.stat(TARGET).st_mode): Line 196: os.chmod(TARGET, oldmod) Line 197: os.chmod(TARGET, oldmod) Well this is a mistake, Will fix! Line 198: rc, out, err = utils.execCmd((INITCTL, Line 199: reload-configuration)) Line 200: if rc != 0: Line 201: sys.stdout.write(out) -- To view, visit http://gerrit.ovirt.org/27298 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I74bfe05bb4b5f5d09021f21b324f9b7d5d0fdaab Gerrit-PatchSet: 17 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: mooli tayer mta...@redhat.com Gerrit-Reviewer: Alon Bar-Lev alo...@redhat.com Gerrit-Reviewer: Dima Kuznetsov dkuzn...@redhat.com Gerrit-Reviewer: Douglas Schilling Landgraf dougsl...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshz...@linux.vnet.ibm.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer mta...@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]: replace configure_libvirt.py with python code.
oVirt Jenkins CI Server has posted comments on this change. Change subject: replace configure_libvirt.py with python code. .. Patch Set 17: Build Failed http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/9286/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/9429/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/8498/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_install_rpm_sanity_gerrit/643/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_storage_functional_tests_localfs_gerrit/917/ : FAILURE -- To view, visit http://gerrit.ovirt.org/27298 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I74bfe05bb4b5f5d09021f21b324f9b7d5d0fdaab Gerrit-PatchSet: 17 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: mooli tayer mta...@redhat.com Gerrit-Reviewer: Alon Bar-Lev alo...@redhat.com Gerrit-Reviewer: Dima Kuznetsov dkuzn...@redhat.com Gerrit-Reviewer: Douglas Schilling Landgraf dougsl...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshz...@linux.vnet.ibm.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer mta...@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]: replace configure_libvirt.py with python code.
mooli tayer has posted comments on this change.
Change subject: replace configure_libvirt.py with python code.
..
Patch Set 14:
(11 comments)
http://gerrit.ovirt.org/#/c/27298/14//COMMIT_MSG
Commit Message:
Line 5: CommitDate: 2014-05-19 13:56:56 +0300
Line 6:
Line 7: replace configure_libvirt.py with python code.
Line 8:
Line 9: The port is to behave as simillar as possible and will be tested
minor typo: similar
Done
Line 10: to configure/remove configuration created by configure_libvirt.sh.
Line 11:
Line 12: The new code uses temp files and replace instead of line by line
Line 13: editing for durability.
http://gerrit.ovirt.org/#/c/27298/14/lib/vdsm/tool/configurator.py
File lib/vdsm/tool/configurator.py:
Line 20: import errno
Line 21: import os
Line 22: import sys
Line 23: import grp
Line 24: import argparse
imports alphabetically?
done here: http://gerrit.ovirt.org/#/c/27783/3
Line 25: import filecmp
Line 26: import rpm
Line 27: import shutil
Line 28: import traceback
Line 125:
Line 126: for path in (self._getPersistedFiles()):
Line 127: if not self._openConfig(path).hasConf():
Line 128: return False
Line 129: return True
In _isSslConflict() I see you only using one return statement, what about u
Very well.
Line 130:
Line 131: def removeConf(self):
Line 132: for cfile, content in LibvirtModuleConfigure.FILES.items():
Line 133: content['removeConf'](self, content['path'])
Line 160: # this could happen in Ubuntu or other distro,
Line 161: # so continue to use system default init mechanism
Line 162: for filename in iterateLibvirtFiles():
Line 163: if LIBVIRTD_UPSTART in filename:
Line 164: packeged = filename
Is it a typo for packaged ?
yes fixed
Line 165: break
Line 166:
Line 167: if packeged is not None and os.path.isfile(packeged):
Line 168: if not os.path.isfile(TARGET):
Line 163: if LIBVIRTD_UPSTART in filename:
Line 164: packeged = filename
Line 165: break
Line 166:
Line 167: if packeged is not None and os.path.isfile(packeged):
if packeged is not assigned for some reason in the above for/if I don't thi
good catch. fixed.
Line 168: if not os.path.isfile(TARGET):
Line 169: service.service_stop('libvirtd')
Line 170: if (not os.path.isfile(TARGET) or
Line 171: not filecmp.cmp(packeged, TARGET)):
Line 183: sys.stderr.write(err)
Line 184: raise RuntimeError(
Line 185: Failed to reload upstart configuration.)
Line 186:
Line 187: def _isSslConflict(self):
I see you used docstring to others methods, is it possible to add docstring
Done
Line 188: config.read(self.getFile('VDSM_CONF'))
Line 189: ssl = config.getboolean('vars', 'ssl')
Line 190:
Line 191: lconf_p = ParserWrapper({
Line 243:
Line 244: def _openConfig(self, path):
Line 245: return ConfigFile(path, self.CONF_VERSION)
Line 246:
Line 247: # On configure functions
Should this comment be part of docstring?
It is a title for the 3 methods bellow.
Line 248: def _addSection(self, content, vdsmConfiguration):
Line 249:
Line 250: Add a 'configuration section by vdsm' part to a config file.
Line 251: This section contains only keys not originally defined
Line 280: except OSError as e:
Line 281: if e.errno != errno.ENOENT:
Line 282: raise
Line 283:
Line 284: # On removeConf functions
Should this comment be part of the below docstring?
It is a title for the methods bellow.
Line 285: def _unprefixAndRemoveSection(self, path):
Line 286:
Line 287: undo changes done by _prefixAndPrepend.
Line 288:
Line 304: # on every update. see 'configuration versioning:' at
ConfigFile.py for
Line 305: # details.
Line 306: CONF_VERSION = '4.13.0'
Line 307:
Line 308: PKI = os.path.join(SYSCONF_PATH, 'pki/vdsm')
following the naming schema, would be PKI_DIR?
Done
Line 309: CA_FILE = os.path.join(PKI, 'certs/cacert.pem')
Line 310: CERT_FILE = os.path.join(PKI, 'certs/vdsmcert.pem')
Line 311: KEY_FILE = os.path.join(PKI, 'keys/vdsmkey.pem')
Line 312: LS_CERT_DIR = os.path.join(PKI, 'libvirt-spice')
Line 339: {
Line 340: 'conditions': {},
Line 341: 'content': {
Line 342: 'listen_addr': '0.0.0.0',
Line 343: 'unix_sock_group': 'qemu',
Is it possible to use QEMU_PROCESS_GROUP constant here?
I think it should be ok. fixed.
Line 344: 'unix_sock_rw_perms': '0770',
Change in vdsm[master]: replace configure_libvirt.py with python code.
oVirt Jenkins CI Server has posted comments on this change. Change subject: replace configure_libvirt.py with python code. .. Patch Set 16: Build Failed http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/9241/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/9384/ : FAILURE http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/8453/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_install_rpm_sanity_gerrit/638/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/27298 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I74bfe05bb4b5f5d09021f21b324f9b7d5d0fdaab Gerrit-PatchSet: 16 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: mooli tayer mta...@redhat.com Gerrit-Reviewer: Alon Bar-Lev alo...@redhat.com Gerrit-Reviewer: Dima Kuznetsov dkuzn...@redhat.com Gerrit-Reviewer: Douglas Schilling Landgraf dougsl...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshz...@linux.vnet.ibm.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer mta...@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]: replace configure_libvirt.py with python code.
Yaniv Bronhaim has posted comments on this change.
Change subject: replace configure_libvirt.py with python code.
..
Patch Set 12: Code-Review-1
(3 comments)
http://gerrit.ovirt.org/#/c/27298/12/lib/vdsm/tool/configurator.py
File lib/vdsm/tool/configurator.py:
Line 121: self._getPersistedFiles()
Line 122: ):
Line 123: ovirtfunctions.ovirt_store_config(path)
Line 124:
Line 125: sys.stdout.write(Reconfiguration of libvirt is done.)
remove this print and update the print def configure() with modules' names
Line 126:
Line 127: def validate(self):
Line 128:
Line 129: Validate conflict in configured files
Line 311: with self._openConfig(path) as conff:
Line 312: conff.removeConf()
Line 313:
Line 314: FILES = {
Line 315:
I would put the CONF_VERSION aside the configuration and state that
modification in defaults must follow VERSION update
Line 316: # Vdsm configuration
Line 317:
Line 318: 'VDSM_CONF': {
Line 319: 'path': os.path.join(
http://gerrit.ovirt.org/#/c/27298/12/vdsm.spec.in
File vdsm.spec.in:
Line 749: removeConf
it's called remove-config
--
To view, visit http://gerrit.ovirt.org/27298
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I74bfe05bb4b5f5d09021f21b324f9b7d5d0fdaab
Gerrit-PatchSet: 12
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: mooli tayer mta...@redhat.com
Gerrit-Reviewer: Alon Bar-Lev alo...@redhat.com
Gerrit-Reviewer: Douglas Schilling Landgraf dougsl...@redhat.com
Gerrit-Reviewer: Nir Soffer nsof...@redhat.com
Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com
Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com
Gerrit-Reviewer: Zhou Zheng Sheng zhshz...@linux.vnet.ibm.com
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: mooli tayer mta...@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]: replace configure_libvirt.py with python code.
mooli tayer has posted comments on this change.
Change subject: replace configure_libvirt.py with python code.
..
Patch Set 12:
(3 comments)
http://gerrit.ovirt.org/#/c/27298/12/lib/vdsm/tool/configurator.py
File lib/vdsm/tool/configurator.py:
Line 121: self._getPersistedFiles()
Line 122: ):
Line 123: ovirtfunctions.ovirt_store_config(path)
Line 124:
Line 125: sys.stdout.write(Reconfiguration of libvirt is done.)
remove this print and update the print def configure() with modules' name
ok.
here:
http://gerrit.ovirt.org/#/c/27841/
Line 126:
Line 127: def validate(self):
Line 128:
Line 129: Validate conflict in configured files
Line 311: with self._openConfig(path) as conff:
Line 312: conff.removeConf()
Line 313:
Line 314: FILES = {
Line 315:
I would put the CONF_VERSION aside the configuration and state that modific
Done
Line 316: # Vdsm configuration
Line 317:
Line 318: 'VDSM_CONF': {
Line 319: 'path': os.path.join(
http://gerrit.ovirt.org/#/c/27298/12/vdsm.spec.in
File vdsm.spec.in:
Line 749: removeConf
it's called remove-config
Thanks. done
--
To view, visit http://gerrit.ovirt.org/27298
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I74bfe05bb4b5f5d09021f21b324f9b7d5d0fdaab
Gerrit-PatchSet: 12
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: mooli tayer mta...@redhat.com
Gerrit-Reviewer: Alon Bar-Lev alo...@redhat.com
Gerrit-Reviewer: Douglas Schilling Landgraf dougsl...@redhat.com
Gerrit-Reviewer: Nir Soffer nsof...@redhat.com
Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com
Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com
Gerrit-Reviewer: Zhou Zheng Sheng zhshz...@linux.vnet.ibm.com
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: mooli tayer mta...@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]: replace configure_libvirt.py with python code.
oVirt Jenkins CI Server has posted comments on this change. Change subject: replace configure_libvirt.py with python code. .. Patch Set 13: Build Failed http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/8991/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/9132/ : FAILURE http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/8203/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_install_rpm_sanity_gerrit/614/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/27298 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I74bfe05bb4b5f5d09021f21b324f9b7d5d0fdaab Gerrit-PatchSet: 13 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: mooli tayer mta...@redhat.com Gerrit-Reviewer: Alon Bar-Lev alo...@redhat.com Gerrit-Reviewer: Douglas Schilling Landgraf dougsl...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshz...@linux.vnet.ibm.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer mta...@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]: replace configure_libvirt.py with python code.
oVirt Jenkins CI Server has posted comments on this change. Change subject: replace configure_libvirt.py with python code. .. Patch Set 14: Build Failed http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/8995/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/9136/ : FAILURE http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/8207/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_install_rpm_sanity_gerrit/615/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/27298 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I74bfe05bb4b5f5d09021f21b324f9b7d5d0fdaab Gerrit-PatchSet: 14 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: mooli tayer mta...@redhat.com Gerrit-Reviewer: Alon Bar-Lev alo...@redhat.com Gerrit-Reviewer: Douglas Schilling Landgraf dougsl...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshz...@linux.vnet.ibm.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer mta...@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]: replace configure_libvirt.py with python code.
Douglas Schilling Landgraf has posted comments on this change.
Change subject: replace configure_libvirt.py with python code.
..
Patch Set 14:
(11 comments)
http://gerrit.ovirt.org/#/c/27298/14//COMMIT_MSG
Commit Message:
Line 5: CommitDate: 2014-05-19 13:56:56 +0300
Line 6:
Line 7: replace configure_libvirt.py with python code.
Line 8:
Line 9: The port is to behave as simillar as possible and will be tested
minor typo: similar
Line 10: to configure/remove configuration created by configure_libvirt.sh.
Line 11:
Line 12: The new code uses temp files and replace instead of line by line
Line 13: editing for durability.
http://gerrit.ovirt.org/#/c/27298/14/lib/vdsm/tool/configurator.py
File lib/vdsm/tool/configurator.py:
Line 20: import errno
Line 21: import os
Line 22: import sys
Line 23: import grp
Line 24: import argparse
imports alphabetically?
Line 25: import filecmp
Line 26: import rpm
Line 27: import shutil
Line 28: import traceback
Line 125:
Line 126: for path in (self._getPersistedFiles()):
Line 127: if not self._openConfig(path).hasConf():
Line 128: return False
Line 129: return True
In _isSslConflict() I see you only using one return statement, what about use
it here as well?
Line 130:
Line 131: def removeConf(self):
Line 132: for cfile, content in LibvirtModuleConfigure.FILES.items():
Line 133: content['removeConf'](self, content['path'])
Line 160: # this could happen in Ubuntu or other distro,
Line 161: # so continue to use system default init mechanism
Line 162: for filename in iterateLibvirtFiles():
Line 163: if LIBVIRTD_UPSTART in filename:
Line 164: packeged = filename
Is it a typo for packaged ?
Line 165: break
Line 166:
Line 167: if packeged is not None and os.path.isfile(packeged):
Line 168: if not os.path.isfile(TARGET):
Line 163: if LIBVIRTD_UPSTART in filename:
Line 164: packeged = filename
Line 165: break
Line 166:
Line 167: if packeged is not None and os.path.isfile(packeged):
if packeged is not assigned for some reason in the above for/if I don't think
it will be None and even exist in the stack.
Line 168: if not os.path.isfile(TARGET):
Line 169: service.service_stop('libvirtd')
Line 170: if (not os.path.isfile(TARGET) or
Line 171: not filecmp.cmp(packeged, TARGET)):
Line 183: sys.stderr.write(err)
Line 184: raise RuntimeError(
Line 185: Failed to reload upstart configuration.)
Line 186:
Line 187: def _isSslConflict(self):
I see you used docstring to others methods, is it possible to add docstring
here as well? Please consider to removeConf() and _getPersistedFiles() too.
Line 188: config.read(self.getFile('VDSM_CONF'))
Line 189: ssl = config.getboolean('vars', 'ssl')
Line 190:
Line 191: lconf_p = ParserWrapper({
Line 243:
Line 244: def _openConfig(self, path):
Line 245: return ConfigFile(path, self.CONF_VERSION)
Line 246:
Line 247: # On configure functions
Should this comment be part of docstring?
Line 248: def _addSection(self, content, vdsmConfiguration):
Line 249:
Line 250: Add a 'configuration section by vdsm' part to a config file.
Line 251: This section contains only keys not originally defined
Line 280: except OSError as e:
Line 281: if e.errno != errno.ENOENT:
Line 282: raise
Line 283:
Line 284: # On removeConf functions
Should this comment be part of the below docstring?
Line 285: def _unprefixAndRemoveSection(self, path):
Line 286:
Line 287: undo changes done by _prefixAndPrepend.
Line 288:
Line 304: # on every update. see 'configuration versioning:' at
ConfigFile.py for
Line 305: # details.
Line 306: CONF_VERSION = '4.13.0'
Line 307:
Line 308: PKI = os.path.join(SYSCONF_PATH, 'pki/vdsm')
following the naming schema, would be PKI_DIR?
Line 309: CA_FILE = os.path.join(PKI, 'certs/cacert.pem')
Line 310: CERT_FILE = os.path.join(PKI, 'certs/vdsmcert.pem')
Line 311: KEY_FILE = os.path.join(PKI, 'keys/vdsmkey.pem')
Line 312: LS_CERT_DIR = os.path.join(PKI, 'libvirt-spice')
Line 339: {
Line 340: 'conditions': {},
Line 341: 'content': {
Line 342: 'listen_addr': '0.0.0.0',
Line 343: 'unix_sock_group': 'qemu',
Is it possible to use QEMU_PROCESS_GROUP constant here?
Line 344: 'unix_sock_rw_perms': '0770',
Line 345: 'auth_unix_rw': 'sasl',
Line
Change in vdsm[master]: replace configure_libvirt.py with python code.
oVirt Jenkins CI Server has posted comments on this change. Change subject: replace configure_libvirt.py with python code. .. Patch Set 11: Build Failed http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/8952/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/9093/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/8164/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_install_rpm_sanity_gerrit/610/ : FAILURE -- To view, visit http://gerrit.ovirt.org/27298 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I74bfe05bb4b5f5d09021f21b324f9b7d5d0fdaab Gerrit-PatchSet: 11 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: mooli tayer mta...@redhat.com Gerrit-Reviewer: Alon Bar-Lev alo...@redhat.com Gerrit-Reviewer: Douglas Schilling Landgraf dougsl...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshz...@linux.vnet.ibm.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer mta...@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]: replace configure_libvirt.py with python code.
oVirt Jenkins CI Server has posted comments on this change. Change subject: replace configure_libvirt.py with python code. .. Patch Set 12: Build Failed http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/8963/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/9104/ : FAILURE http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/8175/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_install_rpm_sanity_gerrit/611/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/27298 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I74bfe05bb4b5f5d09021f21b324f9b7d5d0fdaab Gerrit-PatchSet: 12 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: mooli tayer mta...@redhat.com Gerrit-Reviewer: Alon Bar-Lev alo...@redhat.com Gerrit-Reviewer: Douglas Schilling Landgraf dougsl...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshz...@linux.vnet.ibm.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer mta...@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]: replace configure_libvirt.py with python code.
mooli tayer has posted comments on this change. Change subject: replace configure_libvirt.py with python code. .. Patch Set 9: (5 comments) http://gerrit.ovirt.org/#/c/27298/9/lib/vdsm/tool/configurator.py File lib/vdsm/tool/configurator.py: Line 25: import filecmp Line 26: import rpm Line 27: import shutil Line 28: import traceback Line 29: import uuid Import should be sorted. it wasn't sorted originally, added patch to sort the whole thing Line 30: Line 31: from .. import utils Line 32: from . import service, expose, validate_ovirt_certs Line 33: from configfile import ConfigFile, ParserWrapper Line 109: conf.prefixLines(self.PREFIX) Line 110: with open( Line 111: self.getFile(content['prependFile']), Line 112: 'r' Line 113: ) as src_conf: It is hard to read when you format this in such a non-standard way. Please Agree about removing the lines. Not really to the temporary variable. But this isn't too long. Line 114: conf.prependSection(src_conf.read()) Line 115: Line 116: def _removeFile(self, content, context): Line 117: Line 122: except OSError as e: Line 123: if e.errno != errno.ENOENT: Line 124: raise Line 125: Line 126: # On removeConf functions Missing empty line. Where? is it a pep8 thing? Line 127: def _unprefixAndRemoveSection(self, path): Line 128: Line 129: undo changes done by _prefixAndPrepend. Line 130: Line 135: Line 136: def _removeSection(self, path): Line 137: Line 138: remove entire 'configuration section by vdsm' section. Line 139: section is remove regardless of it's version. remove - removed Done. Line 140: Line 141: if os.path.exists(path): Line 142: with self._openConfig(path) as conff: Line 143: conff.removeConf() Line 389: applyFragment = True Line 390: for key, val in fragment['conditions'].items(): Line 391: if context[key] != val: Line 392: applyFragment = False Line 393: return applyFragment One would expect that this function does apply the fragment to something, b 1.) Will rename to isApplicable. in English what it should mean is determine if the this fragment should be included according to current configuration 2.) doing ret=False for ...: if ... ret=True return ret Is a conscious decision. Alon convinced me saying functional code should never return mid clause. I could add a if ... ret=True break but since this code checks 0-4 conditions I am not concerned about speed. 3.) A fragment should be applied if one of its conditions are not met according to context. But that's not what the code does. Maybe you mean: A fragment should be applied UNLESS one of its conditions are not met according to context. I think the description says what it does. if there are no conditions it is a vacuous truth. 4.) I will change value to booleanValue I will also rename context to vdsmConfiguration. 5.) Good idea! will change order. -- interface functions -- private functions -- private # On configure/removeConf (function called from huge configuration) -- configuration Line 394: Line 395: def _openConfig(self, path): Line 396: return ConfigFile(path, **self.VDSM_CONF_SECTION) Line 397: -- To view, visit http://gerrit.ovirt.org/27298 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I74bfe05bb4b5f5d09021f21b324f9b7d5d0fdaab Gerrit-PatchSet: 9 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: mooli tayer mta...@redhat.com Gerrit-Reviewer: Alon Bar-Lev alo...@redhat.com Gerrit-Reviewer: Douglas Schilling Landgraf dougsl...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshz...@linux.vnet.ibm.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer mta...@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]: replace configure_libvirt.py with python code.
oVirt Jenkins CI Server has posted comments on this change. Change subject: replace configure_libvirt.py with python code. .. Patch Set 10: Build Failed http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/8922/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/9060/ : FAILURE http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/8132/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_install_rpm_sanity_gerrit/609/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/27298 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I74bfe05bb4b5f5d09021f21b324f9b7d5d0fdaab Gerrit-PatchSet: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: mooli tayer mta...@redhat.com Gerrit-Reviewer: Alon Bar-Lev alo...@redhat.com Gerrit-Reviewer: Douglas Schilling Landgraf dougsl...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshz...@linux.vnet.ibm.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer mta...@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]: replace configure_libvirt.py with python code.
mooli tayer has posted comments on this change. Change subject: replace configure_libvirt.py with python code. .. Patch Set 8: (1 comment) ok, than I would suggest to: 1. Rename allTrue to needsUpdate - because it seems that this is what it means. How about 'applyFragment'? 2. When you check for inequality use != instead of not: if context[key] != val: needsUpdate = True OK 3. Even cleaner: factor out the check to separate function def needsUpdate(a, b): for key, val in b.iteritems(): if a[key] != val: return True return False OK And then your loop becomes: for fragment in content['fragments']: if self.needsUpdate(fragment['conditions'], context): configuration.update(fragment['content']) OK 4. And docstring telling what this method does cannot hurt - as you see it fooled me. Very well 5. The method name is confusing - this is not about adding sections but about updating them, right? What it ultimatly does is add one section in a configuration file that look something like: ## beginning of configuration section by vdsm-4.13.0 key=val ... ## end of configuration section by vdsm-4.13.0 That section is created by updating maps but the importent line is: conff.addEntry(key, val) these entries create the new section http://gerrit.ovirt.org/#/c/27298/8//COMMIT_MSG Commit Message: Line 3: AuthorDate: 2014-04-30 14:29:36 +0300 Line 4: Commit: Mooli Tayer mta...@redhat.com Line 5: CommitDate: 2014-05-14 14:12:24 +0300 Line 6: Line 7: replace configure_libvirt.py with python code. It would be helpful to describe here why this change is needed, and if this OK i'm writing a very descriptive (hope so) one. Line 8: Line 9: Change-Id: I74bfe05bb4b5f5d09021f21b324f9b7d5d0fdaab -- To view, visit http://gerrit.ovirt.org/27298 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I74bfe05bb4b5f5d09021f21b324f9b7d5d0fdaab Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: mooli tayer mta...@redhat.com Gerrit-Reviewer: Alon Bar-Lev alo...@redhat.com Gerrit-Reviewer: Douglas Schilling Landgraf dougsl...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer mta...@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]: replace configure_libvirt.py with python code.
oVirt Jenkins CI Server has posted comments on this change. Change subject: replace configure_libvirt.py with python code. .. Patch Set 9: Build Failed http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/8893/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/9029/ : FAILURE http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/8103/ : FAILURE http://jenkins.ovirt.org/job/vdsm_master_install_rpm_sanity_gerrit/600/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/27298 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I74bfe05bb4b5f5d09021f21b324f9b7d5d0fdaab Gerrit-PatchSet: 9 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: mooli tayer mta...@redhat.com Gerrit-Reviewer: Alon Bar-Lev alo...@redhat.com Gerrit-Reviewer: Douglas Schilling Landgraf dougsl...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshz...@linux.vnet.ibm.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer mta...@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]: replace configure_libvirt.py with python code.
Nir Soffer has posted comments on this change. Change subject: replace configure_libvirt.py with python code. .. Patch Set 9: (5 comments) http://gerrit.ovirt.org/#/c/27298/9/lib/vdsm/tool/configurator.py File lib/vdsm/tool/configurator.py: Line 25: import filecmp Line 26: import rpm Line 27: import shutil Line 28: import traceback Line 29: import uuid Import should be sorted. Line 30: Line 31: from .. import utils Line 32: from . import service, expose, validate_ovirt_certs Line 33: from configfile import ConfigFile, ParserWrapper Line 109: conf.prefixLines(self.PREFIX) Line 110: with open( Line 111: self.getFile(content['prependFile']), Line 112: 'r' Line 113: ) as src_conf: It is hard to read when you format this in such a non-standard way. Please do this like this: with open(self.getFile(content['prependFile']) as src_conf: ... The 'r' parameter is not needed, everyone knows that open(path) use readonly mode. If it does not fit in one line - use a temporary. It is much easier to read short lines. prepend_file = self.getFile(content['prependFile']) with open(prepend_file) as src_conf: ... Line 114: conf.prependSection(src_conf.read()) Line 115: Line 116: def _removeFile(self, content, context): Line 117: Line 135: Line 136: def _removeSection(self, path): Line 137: Line 138: remove entire 'configuration section by vdsm' section. Line 139: section is remove regardless of it's version. remove - removed Line 140: Line 141: if os.path.exists(path): Line 142: with self._openConfig(path) as conff: Line 143: conff.removeConf() Line 389: applyFragment = True Line 390: for key, val in fragment['conditions'].items(): Line 391: if context[key] != val: Line 392: applyFragment = False Line 393: return applyFragment One would expect that this function does apply the fragment to something, but this is function answer the question, if this fragment should be applied. Renaming this to shouldApplyFragment or fragmentNeedsUpdate will much more clear. Now this function seem to return false if one of the context values is different from the fragment values. So there is no need to iterate over all keys. As soon as you find one key that does not match, you can return. This is not only faster, but much more clear. When you look at the code now, you wonder why you are comparing all keys. Finally, the docstring is confusing, because it says: An applicaple fragment is a fragment who's list of conditions are met according to context. But the code care only about one different condition. So either the code is not correct, or the docstring should say: A fragment should be applied if one of its conditions are not met according to context. So this would be more clear: def shouldApplyFragment(self, fragment, context): for key, val in fragment['conditions'].items(): if context[key] != val: return False return True Finally, using the generic key and value make this function less clear - what is value? can you find a more descriptive name? One more thing - having this method at the bottom, where the method that calls it at the top make it harder to navigate. Why not put all the methods at the top, and the huge configuration dict at the bottom? Line 394: Line 395: def _openConfig(self, path): Line 396: return ConfigFile(path, **self.VDSM_CONF_SECTION) Line 397: Line 446: def _getPersistedFiles(self): Line 447: return [ Line 448: cfile['path'] for cfile in self.FILES.values() Line 449: if cfile['persisted'] Line 450: ] The standard way to indent this kind of code: return [cfile['path'] for cfile in self.FILES.values() if cfile['persisted']] Using standard formatting helps others to understand and work with your code. Line 451: Line 452: def _sysvToUpstart(self): Line 453: Line 454: On RHEL 6, libvirtd can be started by either SysV init or Upstart. -- To view, visit http://gerrit.ovirt.org/27298 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I74bfe05bb4b5f5d09021f21b324f9b7d5d0fdaab Gerrit-PatchSet: 9 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: mooli tayer mta...@redhat.com Gerrit-Reviewer: Alon Bar-Lev alo...@redhat.com Gerrit-Reviewer: Douglas Schilling Landgraf dougsl...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshz...@linux.vnet.ibm.com Gerrit-Reviewer: automat...@ovirt.org
Change in vdsm[master]: replace configure_libvirt.py with python code.
Nir Soffer has posted comments on this change. Change subject: replace configure_libvirt.py with python code. .. Patch Set 9: (2 comments) http://gerrit.ovirt.org/#/c/27298/9/lib/vdsm/tool/configurator.py File lib/vdsm/tool/configurator.py: Line 122: except OSError as e: Line 123: if e.errno != errno.ENOENT: Line 124: raise Line 125: Line 126: # On removeConf functions Missing empty line. Line 127: def _unprefixAndRemoveSection(self, path): Line 128: Line 129: undo changes done by _prefixAndPrepend. Line 130: Line 446: def _getPersistedFiles(self): Line 447: return [ Line 448: cfile['path'] for cfile in self.FILES.values() Line 449: if cfile['persisted'] Line 450: ] I must!! You have good points, but I value more the look of the code. Code is mostly read and rarely modified, so I'm ready to pay for this. Line 451: Line 452: def _sysvToUpstart(self): Line 453: Line 454: On RHEL 6, libvirtd can be started by either SysV init or Upstart. -- To view, visit http://gerrit.ovirt.org/27298 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I74bfe05bb4b5f5d09021f21b324f9b7d5d0fdaab Gerrit-PatchSet: 9 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: mooli tayer mta...@redhat.com Gerrit-Reviewer: Alon Bar-Lev alo...@redhat.com Gerrit-Reviewer: Douglas Schilling Landgraf dougsl...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshz...@linux.vnet.ibm.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer mta...@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]: replace configure_libvirt.py with python code.
Alon Bar-Lev has posted comments on this change. Change subject: replace configure_libvirt.py with python code. .. Patch Set 9: (1 comment) http://gerrit.ovirt.org/#/c/27298/9/lib/vdsm/tool/configurator.py File lib/vdsm/tool/configurator.py: Line 446: def _getPersistedFiles(self): Line 447: return [ Line 448: cfile['path'] for cfile in self.FILES.values() Line 449: if cfile['persisted'] Line 450: ] You have good points, but I value more the look of the code. Code is mostly try out this convention for a while and you never be able to read different code. you just used to X so you think there is a value in it, so you pay for something you do not know any cheaper consistent alternative. Line 451: Line 452: def _sysvToUpstart(self): Line 453: Line 454: On RHEL 6, libvirtd can be started by either SysV init or Upstart. -- To view, visit http://gerrit.ovirt.org/27298 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I74bfe05bb4b5f5d09021f21b324f9b7d5d0fdaab Gerrit-PatchSet: 9 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: mooli tayer mta...@redhat.com Gerrit-Reviewer: Alon Bar-Lev alo...@redhat.com Gerrit-Reviewer: Douglas Schilling Landgraf dougsl...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshz...@linux.vnet.ibm.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer mta...@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]: replace configure_libvirt.py with python code.
mooli tayer has posted comments on this change.
Change subject: replace configure_libvirt.py with python code.
..
Patch Set 7:
(5 comments)
http://gerrit.ovirt.org/#/c/27298/7/lib/vdsm/tool/configurator.py
File lib/vdsm/tool/configurator.py:
Line 83: def removeSection(self, path):
Line 84: if os.path.exists(path):
Line 85: with ConfigFile(
Line 86: path,
Line 87: **LibvirtModuleConfigure.VDSM_CONF_SECTION
Use self
Done
Line 88: ) as conff:
Line 89: conff.removeConf()
Line 90:
Line 91: def addSection(self, content, context):
Line 95: for key, val in fragment['conditions'].items():
Line 96: if not context[key] == val:
Line 97: allTrue = False
Line 98: if allTrue:
Line 99: configuration.update(fragment['content'])
It looks like you are trying to check if there is some key with diffrent va
fragment['conditions'] is a subset of context.
Line 100: if configuration:
Line 101: with ConfigFile(
Line 102: content['path'],
Line 103: **LibvirtModuleConfigure.VDSM_CONF_SECTION
Line 100: if configuration:
Line 101: with ConfigFile(
Line 102: content['path'],
Line 103: **LibvirtModuleConfigure.VDSM_CONF_SECTION
Line 104: ) as conff:
This apears 3 times (maybe more?) and is too long to be easy to read. You c
Done.
Line 105: for key, val in configuration.items():
Line 106: conff.addEntry(key, val)
Line 107:
Line 108: def prefixAndPrepend(self, content, context):
Line 117: conf.prependSection(src_conf.read())
Line 118:
Line 119: def removeFile(self, content, context):
Line 120: if os.path.exists(content['path']):
Line 121: os.remove(content['path'])
This is racy. It is better to use this pattern:
Done!
Line 122:
Line 123: FILES = {
Line 124:
Line 125: # Vdsm configuration
Line 400: def getPersistedFiles(self):
Line 401: return [
Line 402: cfile['path'] for cfile in self.FILES.values()
Line 403: if cfile['configure'].__name__ == 'prefixAndPrepend' or
Line 404: cfile['configure'].__name__ == 'addSection'
I would have added boolean property instead of this complex condition.
OK.
Line 405: ]
Line 406:
Line 407: def _sysvToUpstart(self):
Line 408:
--
To view, visit http://gerrit.ovirt.org/27298
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I74bfe05bb4b5f5d09021f21b324f9b7d5d0fdaab
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: mooli tayer mta...@redhat.com
Gerrit-Reviewer: Alon Bar-Lev alo...@redhat.com
Gerrit-Reviewer: Nir Soffer nsof...@redhat.com
Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com
Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: mooli tayer mta...@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]: replace configure_libvirt.py with python code.
Nir Soffer has posted comments on this change. Change subject: replace configure_libvirt.py with python code. .. Patch Set 7: (1 comment) http://gerrit.ovirt.org/#/c/27298/7/lib/vdsm/tool/configurator.py File lib/vdsm/tool/configurator.py: Line 95: for key, val in fragment['conditions'].items(): Line 96: if not context[key] == val: Line 97: allTrue = False Line 98: if allTrue: Line 99: configuration.update(fragment['content']) fragment['conditions'] is a subset of context. ok, than I would suggest to: 1. Rename allTrue to needsUpdate - because it seems that this is what it means. 2. When you check for inequality use != instead of not: if context[key] != val: needsUpdate = True 3. Even cleaner: factor out the check to separate function def needsUpdate(a, b): for key, val in b.iteritems(): if a[key] != val: return True return False And then your loop becomes: for fragment in content['fragments']: if self.needsUpdate(fragment['conditions'], context): configuration.update(fragment['content']) 4. And docstring telling what this method does cannot hurt - as you see it fooled me. 5. The method name is confusing - this is not about adding sections but about updating them, right? Line 100: if configuration: Line 101: with ConfigFile( Line 102: content['path'], Line 103: **LibvirtModuleConfigure.VDSM_CONF_SECTION -- To view, visit http://gerrit.ovirt.org/27298 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I74bfe05bb4b5f5d09021f21b324f9b7d5d0fdaab Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: mooli tayer mta...@redhat.com Gerrit-Reviewer: Alon Bar-Lev alo...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer mta...@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]: replace configure_libvirt.py with python code.
Nir Soffer has posted comments on this change. Change subject: replace configure_libvirt.py with python code. .. Patch Set 8: (1 comment) http://gerrit.ovirt.org/#/c/27298/8//COMMIT_MSG Commit Message: Line 3: AuthorDate: 2014-04-30 14:29:36 +0300 Line 4: Commit: Mooli Tayer mta...@redhat.com Line 5: CommitDate: 2014-05-14 14:12:24 +0300 Line 6: Line 7: replace configure_libvirt.py with python code. It would be helpful to describe here why this change is needed, and if this is only a port of the current code to Python, or there is also some behavior changes. Line 8: Line 9: Change-Id: I74bfe05bb4b5f5d09021f21b324f9b7d5d0fdaab -- To view, visit http://gerrit.ovirt.org/27298 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I74bfe05bb4b5f5d09021f21b324f9b7d5d0fdaab Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: mooli tayer mta...@redhat.com Gerrit-Reviewer: Alon Bar-Lev alo...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer mta...@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]: replace configure_libvirt.py with python code.
oVirt Jenkins CI Server has posted comments on this change. Change subject: replace configure_libvirt.py with python code. .. Patch Set 8: Build Failed http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/8816/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/8952/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/8026/ : FAILURE http://jenkins.ovirt.org/job/vdsm_master_install_rpm_sanity_gerrit/592/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/27298 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I74bfe05bb4b5f5d09021f21b324f9b7d5d0fdaab Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: mooli tayer mta...@redhat.com Gerrit-Reviewer: Alon Bar-Lev alo...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer mta...@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]: replace configure_libvirt.py with python code.
mooli tayer has posted comments on this change.
Change subject: replace configure_libvirt.py with python code.
..
Patch Set 6:
(7 comments)
http://gerrit.ovirt.org/#/c/27298/6/lib/vdsm/tool/configurator.py
File lib/vdsm/tool/configurator.py:
Line 98: 'path': os.path.join(
Line 99: SYSCONF_PATH,
Line 100: 'libvirt/libvirtd.conf'
Line 101: ),
Line 102: 'configureFunc': 'addSection',
why do you care if function is above :)
Done
Line 103: 'removeConfFunc': 'removeSection',
Line 104: 'fragments': [
Line 105: {
Line 106: 'conditions': {},
Line 311: }
Line 312: }
Line 313:
Line 314: def __init__(self):
Line 315: super(LibvirtModuleConfigure, self).__init__()
This __init__ is useless - if you remove it, the __init__() method you inhe
checked and done.
Line 316:
Line 317: def getName(self):
Line 318: return 'libvirt'
Line 319:
Line 317: def getName(self):
Line 318: return 'libvirt'
Line 319:
Line 320: def getFile(self, fname):
Line 321: return LibvirtModuleConfigure.FILES[fname]['path']
Use self.FILES - class attributes are available to an instance in Python.
Done
Line 322:
Line 323: def getServices(self):
Line 324: return [supervdsmd, vdsmd, libvirtd]
Line 325:
Line 373: context = {
Line 374: 'certs_exist': all(os.path.isfile(f) for f in [
Line 375: LibvirtModuleConfigure.CA_FILE,
Line 376: LibvirtModuleConfigure.CERT_FILE,
Line 377: LibvirtModuleConfigure.KEY_FILE
self would be nicer.
Done
Line 378: ]),
Line 379: 'ssl_enabled': config.getboolean('vars', 'ssl'),
Line 380: 'sanlock_enabled': SANLOCK_ENABLED,
Line 381: 'libvirt_selinux': LIBVIRT_SELINUX
Line 403: lambda cfile: (
Line 404: cfile['configureFunc'] == 'prefixAndPrepend' or
Line 405: cfile['configureFunc'] == 'addSection'),
Line 406: LibvirtModuleConfigure.FILES.values()
Line 407: )
It is not clear what you are trying to do here; While Python support this f
Done +
alon's comment about using functions and not function names.
Line 408: )
Line 409:
Line 410: def _sysvToUpstart(self):
Line 411:
Line 416: INITCTL = '/sbin/initctl'
Line 417: LIBVIRTD_UPSTART = 'libvirtd.upstart'
Line 418: TARGET = os.path.join(SYSCONF_PATH, init/libvirtd.conf)
Line 419:
Line 420: if os.path.isfile(INITCTL) and os.access(INITCTL, os.X_OK):
os.access uses the real uid/gid, so if someone run this code using sudo, th
This code always runs as root so I think this is enough here.
Line 421: ts = rpm.TransactionSet()
Line 422: mi = itertools.chain(*[ts.dbMatch('name', name)
Line 423:for name in ['libvirt',
'libvirt-daemon']])
Line 424: # libvirtd package does not provide libvirtd.upstart,
Line 425: # this could happen in Ubuntu or other distro,
Line 426: # so continue to use system default init mechanism
Line 427:
Line 428: for filename in itertools.chain(*[h[rpm.RPMTAG_FILENAMES]
Line 429: for h in mi]):
Two chains with these cryptic variable names make it too hard to understand
Yes it is nicer.
Line 430: if LIBVIRTD_UPSTART in filename:
Line 431: packeged = filename
Line 432: break
Line 433:
--
To view, visit http://gerrit.ovirt.org/27298
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I74bfe05bb4b5f5d09021f21b324f9b7d5d0fdaab
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: mooli tayer mta...@redhat.com
Gerrit-Reviewer: Alon Bar-Lev alo...@redhat.com
Gerrit-Reviewer: Nir Soffer nsof...@redhat.com
Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com
Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: mooli tayer mta...@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]: replace configure_libvirt.py with python code.
oVirt Jenkins CI Server has posted comments on this change. Change subject: replace configure_libvirt.py with python code. .. Patch Set 5: Build Failed http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/8729/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/8865/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/7939/ : FAILURE http://jenkins.ovirt.org/job/vdsm_master_install_rpm_sanity_gerrit/579/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/27298 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I74bfe05bb4b5f5d09021f21b324f9b7d5d0fdaab Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: mooli tayer mta...@redhat.com Gerrit-Reviewer: Alon Bar-Lev alo...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer mta...@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]: replace configure_libvirt.py with python code.
oVirt Jenkins CI Server has posted comments on this change. Change subject: replace configure_libvirt.py with python code. .. Patch Set 6: Code-Review-1 Verified-1 Build Failed http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/8767/ : UNSTABLE http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/8903/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/7977/ : FAILURE http://jenkins.ovirt.org/job/vdsm_master_install_rpm_sanity_gerrit/586/ : FAILURE -- To view, visit http://gerrit.ovirt.org/27298 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I74bfe05bb4b5f5d09021f21b324f9b7d5d0fdaab Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: mooli tayer mta...@redhat.com Gerrit-Reviewer: Alon Bar-Lev alo...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer mta...@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]: replace configure_libvirt.py with python code.
oVirt Jenkins CI Server has posted comments on this change. Change subject: replace configure_libvirt.py with python code. .. Patch Set 7: Build Failed http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/8781/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/8917/ : FAILURE http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/7991/ : FAILURE http://jenkins.ovirt.org/job/vdsm_master_install_rpm_sanity_gerrit/588/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/27298 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I74bfe05bb4b5f5d09021f21b324f9b7d5d0fdaab Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: mooli tayer mta...@redhat.com Gerrit-Reviewer: Alon Bar-Lev alo...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer mta...@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]: replace configure_libvirt.py with python code.
Alon Bar-Lev has posted comments on this change. Change subject: replace configure_libvirt.py with python code. .. Patch Set 7: (1 comment) ok, I am done, looks ok, I am sure there are more cleanups that can be done, but I do think it is enough for now. http://gerrit.ovirt.org/#/c/27298/7/lib/vdsm/tool/configurator.py File lib/vdsm/tool/configurator.py: Line 400: def getPersistedFiles(self): Line 401: return [ Line 402: cfile['path'] for cfile in self.FILES.values() Line 403: if cfile['configure'].__name__ == 'prefixAndPrepend' or Line 404: cfile['configure'].__name__ == 'addSection' I would have added boolean property instead of this complex condition. Line 405: ] Line 406: Line 407: def _sysvToUpstart(self): Line 408: -- To view, visit http://gerrit.ovirt.org/27298 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I74bfe05bb4b5f5d09021f21b324f9b7d5d0fdaab Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: mooli tayer mta...@redhat.com Gerrit-Reviewer: Alon Bar-Lev alo...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer mta...@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]: replace configure_libvirt.py with python code.
Nir Soffer has posted comments on this change.
Change subject: replace configure_libvirt.py with python code.
..
Patch Set 7:
(4 comments)
http://gerrit.ovirt.org/#/c/27298/7/lib/vdsm/tool/configurator.py
File lib/vdsm/tool/configurator.py:
Line 83: def removeSection(self, path):
Line 84: if os.path.exists(path):
Line 85: with ConfigFile(
Line 86: path,
Line 87: **LibvirtModuleConfigure.VDSM_CONF_SECTION
Use self
Line 88: ) as conff:
Line 89: conff.removeConf()
Line 90:
Line 91: def addSection(self, content, context):
Line 95: for key, val in fragment['conditions'].items():
Line 96: if not context[key] == val:
Line 97: allTrue = False
Line 98: if allTrue:
Line 99: configuration.update(fragment['content'])
It looks like you are trying to check if there is some key with diffrent value
in two dictionaries. If this is your intent, then you can do:
if fragment['conditions'] != context:
configuration.update(fragment['content'])
Line 100: if configuration:
Line 101: with ConfigFile(
Line 102: content['path'],
Line 103: **LibvirtModuleConfigure.VDSM_CONF_SECTION
Line 100: if configuration:
Line 101: with ConfigFile(
Line 102: content['path'],
Line 103: **LibvirtModuleConfigure.VDSM_CONF_SECTION
Line 104: ) as conff:
This apears 3 times (maybe more?) and is too long to be easy to read. You can
move this to a helper:
def _openConfig(self, content):
return ConfigFile(content['path'], **self.VDSM_CONF_SECTION)
And then in the code use:
with self._openConfig(content) as conf:
conf.dostuff()
Line 105: for key, val in configuration.items():
Line 106: conff.addEntry(key, val)
Line 107:
Line 108: def prefixAndPrepend(self, content, context):
Line 117: conf.prependSection(src_conf.read())
Line 118:
Line 119: def removeFile(self, content, context):
Line 120: if os.path.exists(content['path']):
Line 121: os.remove(content['path'])
This is racy. It is better to use this pattern:
try:
os.remove(path)
except OSError as e:
if e.errno != errno.ENOENT:
raise
Line 122:
Line 123: FILES = {
Line 124:
Line 125: # Vdsm configuration
--
To view, visit http://gerrit.ovirt.org/27298
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I74bfe05bb4b5f5d09021f21b324f9b7d5d0fdaab
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: mooli tayer mta...@redhat.com
Gerrit-Reviewer: Alon Bar-Lev alo...@redhat.com
Gerrit-Reviewer: Nir Soffer nsof...@redhat.com
Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com
Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: mooli tayer mta...@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]: replace configure_libvirt.py with python code.
Yaniv Bronhaim has posted comments on this change.
Change subject: replace configure_libvirt.py with python code.
..
Patch Set 5:
(5 comments)
http://gerrit.ovirt.org/#/c/27298/5/lib/vdsm/tool/configurator.py
File lib/vdsm/tool/configurator.py:
Line 41: VDSM_CONF_SECTION = {
Line 42: 'sectionStart': '## beginning of configuration section by vdsm',
Line 43: 'sectionEnd': '## end of configuration section by vdsm',
Line 44: 'version': '4.13.0'
Line 45: }
you removed the long and important comment:
please add it here -
# The PACKAGE_VERSION define is not used here because we do not want to
# update the libvirt configure file every time we change vdsm package
# version. In fact the configure generated here is almost unrelated to the
# package version, so anything meaningful can be used here. Since a hard
# coded version string has been already used, for compatibility we will
# continue to use this string.
Line 46:
Line 47:
Line 48: class _ModuleConfigure(object):
Line 49: def __init__(self):
Line 83:
Line 84: # Libvirt daemon configuration
Line 85: {
Line 86: 'condition': lambda context:
Line 87: True,
use variable. True doesn't mean anything
Line 88: 'conf': LCONF,
Line 89: 'content': {
Line 90: 'listen_addr': '0.0.0.0',
Line 91: 'unix_sock_group': 'qemu',
Line 137:
Line 138: # qemu configuration
Line 139: {
Line 140: 'condition': lambda context:
Line 141: True,
use variable
Line 142:
Line 143: 'conf': QCONF,
Line 144: 'content': {
Line 145: 'dynamic_ownership': 0,
Line 198:
Line 199: # libvirt sysconfig file
Line 200: {
Line 201: 'condition': lambda x:
Line 202: True,
why x ? put more meaning to the variables names
Line 203:
Line 204: 'conf': LDCONF,
Line 205: 'content': {
Line 206: 'LIBVIRTD_ARGS': '--listen',
Line 251: 'QLCONF': 'libvirt/qemu-sanlock.conf',
Line 252: 'LRCONF': 'logrotate.d/libvirtd',
Line 253: 'QNETWORK':
'libvirt/qemu/networks/autostart/default.xml'
Line 254: }[fname]
Line 255: )
what happened to the indentation ?
Line 256:
Line 257: def getServices(self):
Line 258: return [supervdsmd, vdsmd, libvirtd]
Line 259:
--
To view, visit http://gerrit.ovirt.org/27298
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I74bfe05bb4b5f5d09021f21b324f9b7d5d0fdaab
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: mooli tayer mta...@redhat.com
Gerrit-Reviewer: Alon Bar-Lev alo...@redhat.com
Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com
Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: mooli tayer mta...@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]: replace configure_libvirt.py with python code.
Alon Bar-Lev has posted comments on this change.
Change subject: replace configure_libvirt.py with python code.
..
Patch Set 5:
(10 comments)
http://gerrit.ovirt.org/#/c/27298/5/lib/vdsm/tool/configurator.py
File lib/vdsm/tool/configurator.py:
Line 37: SANLOCK_ENABLED, LIBVIRT_SELINUX
Line 38: from vdsm.config import config
Line 39:
Line 40:
Line 41: VDSM_CONF_SECTION = {
when do you decide what to put at class level and what as global?
I prefer class level if possible :)
Line 42: 'sectionStart': '## beginning of configuration section by vdsm',
Line 43: 'sectionEnd': '## end of configuration section by vdsm',
Line 44: 'version': '4.13.0'
Line 45: }
Line 83:
Line 84: # Libvirt daemon configuration
Line 85: {
Line 86: 'condition': lambda context:
Line 87: True,
use variable. True doesn't mean anything
I guess this what he meant, to execute it always.
Line 88: 'conf': LCONF,
Line 89: 'content': {
Line 90: 'listen_addr': '0.0.0.0',
Line 91: 'unix_sock_group': 'qemu',
Line 210:
Line 211: # sanlock configuration file
Line 212: {
Line 213: 'condition': lambda x:
Line 214: SANLOCK_ENABLED,
I suggest that for consistency you move all into context or as I see now the
condition list which is much clearer, you can use dictionary key instead of
lambda... as there is no logic.
conditions = {
'sanlock_enabled': True,
...
}
{
condition_key = '',
Line 215:
Line 216: 'conf': QLCONF,
Line 217: 'content': {
Line 218: 'auto_disk_leases': 0,
Line 292: for item in self.CONF:
Line 293: condition, conf, content = (
Line 294: item['condition'], item['conf'], item['content'])
Line 295: if condition(c):
Line 296: config_map[conf].update(content)
any reason to use extensive temp vars?
why not as simple as:
for item in self.CONF:
if item['condition'](c):
config_map[item['conf']],update(item['content'])
Line 297:
Line 298: for name, configuration in config_map.items():
Line 299: with ConfigFile(self.envGet(name), **VDSM_CONF_SECTION)
as conff:
Line 300: for key, val in configuration.items():
Line 301: conff.addEntry(key, val)
Line 302:
Line 303: qnetwork = self.envGet('QNETWORK')
Line 304: if os.path.exists(qnetwork):
Line 305: os.remove(qnetwork)
why is this an exception and cannot be done for all?
Cant we have some key within the dictionary instructing to remove so we have no
exceptions in logic?
Line 306:
Line 307: # libvirt log rotate configuration
Line 308: # Todo: make idempotent
Line 309: with ConfigFile(
Line 308: # Todo: make idempotent
Line 309: with ConfigFile(
Line 310: self.envGet('LRCONF'), **VDSM_CONF_SECTION) as conf:
Line 311: conf.prefixLines('# VDSM backup')
Line 312: conf.prependSection(self.LLOGR_CONF)
why is this exception? can we have dictionary entry of 'type' to prepend or
addEntry and remove the explicit logic?
Line 313:
Line 314: if utils.isOvirtNode():
Line 315: from ovirtnode import ovirtfunctions
Line 316:
Line 320: self.envGet('LDCONF'),
Line 321: self.envGet('QLCONF'),
Line 322: self.envGet('LRCONF')
Line 323: ):
Line 324: ovirtfunctions.ovirt_store_config(fname)
please have a list/set of all files that were updated and iterate through that
list and not explicit hardcode this
Line 325:
Line 326: sys.stdout.write(Reconfiguration of libvirt is done.)
Line 327:
Line 328: def _sysvToUpstart(self):
Line 353: if not os.path.isfile(TARGET):
Line 354: service.service_stop('libvirtd')
Line 355: if not os.path.isfile(TARGET) or \
Line 356: not filecmp.cmp(packeged, TARGET):
Line 357: shutil.copyfile(packeged, TARGET)
when you copy a file you need chmod as well.
Line 358: rc, out, err = utils.execCmd((INITCTL,
Line 359:
reload-configuration))
Line 360: if rc != 0:
Line 361: sys.stdout.write(out)
Line 391: libvirtd.conf: listen_tcp=0, auth_tcp=\sasl\,
\n
Line 392: qemu.conf: spice_tls=1.\n
Line 393: )
Line 394: return False
Line 395: else:
if you return at middle of function (look above) why do you need the else?
please consider avoiding returning at middle, you can set ret = instead return
and all will be super.
Line 396: if listen_tcp == 1 and
Change in vdsm[master]: replace configure_libvirt.py with python code.
mooli tayer has posted comments on this change.
Change subject: replace configure_libvirt.py with python code.
..
Patch Set 5:
(13 comments)
The way I see things now, I think 'type' belongs on file and not on
a config section.
http://gerrit.ovirt.org/#/c/27298/5/lib/vdsm/tool/configurator.py
File lib/vdsm/tool/configurator.py:
Line 37: SANLOCK_ENABLED, LIBVIRT_SELINUX
Line 38: from vdsm.config import config
Line 39:
Line 40:
Line 41: VDSM_CONF_SECTION = {
when do you decide what to put at class level and what as global?
I have another patch that moves sanlock file stuff to SanlockModuleConfigure
and then I will need this for both.
for now I will move it to LibvirtModuleConfigure.
Line 42: 'sectionStart': '## beginning of configuration section by vdsm',
Line 43: 'sectionEnd': '## end of configuration section by vdsm',
Line 44: 'version': '4.13.0'
Line 45: }
Line 41: VDSM_CONF_SECTION = {
Line 42: 'sectionStart': '## beginning of configuration section by vdsm',
Line 43: 'sectionEnd': '## end of configuration section by vdsm',
Line 44: 'version': '4.13.0'
Line 45: }
you removed the long and important comment:
I've edit it a little bit to be more informative.
see next version. (there is also info about it in configfile.py)
Line 46:
Line 47:
Line 48: class _ModuleConfigure(object):
Line 49: def __init__(self):
Line 137:
Line 138: # qemu configuration
Line 139: {
Line 140: 'condition': lambda context:
Line 141: True,
use variable
same as above
Line 142:
Line 143: 'conf': QCONF,
Line 144: 'content': {
Line 145: 'dynamic_ownership': 0,
Line 198:
Line 199: # libvirt sysconfig file
Line 200: {
Line 201: 'condition': lambda x:
Line 202: True,
why x ? put more meaning to the variables names
I will rename all to context
Line 203:
Line 204: 'conf': LDCONF,
Line 205: 'content': {
Line 206: 'LIBVIRTD_ARGS': '--listen',
Line 210:
Line 211: # sanlock configuration file
Line 212: {
Line 213: 'condition': lambda x:
Line 214: SANLOCK_ENABLED,
I suggest that for consistency you move all into context or as I see now th
Done
Line 215:
Line 216: 'conf': QLCONF,
Line 217: 'content': {
Line 218: 'auto_disk_leases': 0,
Line 251: 'QLCONF': 'libvirt/qemu-sanlock.conf',
Line 252: 'LRCONF': 'logrotate.d/libvirtd',
Line 253: 'QNETWORK':
'libvirt/qemu/networks/autostart/default.xml'
Line 254: }[fname]
Line 255: )
what happened to the indentation ?
Done
Line 256:
Line 257: def getServices(self):
Line 258: return [supervdsmd, vdsmd, libvirtd]
Line 259:
Line 292: for item in self.CONF:
Line 293: condition, conf, content = (
Line 294: item['condition'], item['conf'], item['content'])
Line 295: if condition(c):
Line 296: config_map[conf].update(content)
any reason to use extensive temp vars?
OK
Line 297:
Line 298: for name, configuration in config_map.items():
Line 299: with ConfigFile(self.envGet(name), **VDSM_CONF_SECTION)
as conff:
Line 300: for key, val in configuration.items():
Line 301: conff.addEntry(key, val)
Line 302:
Line 303: qnetwork = self.envGet('QNETWORK')
Line 304: if os.path.exists(qnetwork):
Line 305: os.remove(qnetwork)
why is this an exception and cannot be done for all?
Done
Line 306:
Line 307: # libvirt log rotate configuration
Line 308: # Todo: make idempotent
Line 309: with ConfigFile(
Line 308: # Todo: make idempotent
Line 309: with ConfigFile(
Line 310: self.envGet('LRCONF'), **VDSM_CONF_SECTION) as conf:
Line 311: conf.prefixLines('# VDSM backup')
Line 312: conf.prependSection(self.LLOGR_CONF)
why is this exception? can we have dictionary entry of 'type' to prepend or
Done
Line 313:
Line 314: if utils.isOvirtNode():
Line 315: from ovirtnode import ovirtfunctions
Line 316:
Line 320: self.envGet('LDCONF'),
Line 321: self.envGet('QLCONF'),
Line 322: self.envGet('LRCONF')
Line 323: ):
Line 324: ovirtfunctions.ovirt_store_config(fname)
please have a list/set of all files that were updated and iterate through t
Ok
Line 325:
Line 326: sys.stdout.write(Reconfiguration of libvirt is done.)
Line 327:
Line 328: def _sysvToUpstart(self):
Line 353: if not os.path.isfile(TARGET):
Line 354: service.service_stop('libvirtd')
Line 355: if not
Change in vdsm[master]: replace configure_libvirt.py with python code.
Alon Bar-Lev has posted comments on this change.
Change subject: replace configure_libvirt.py with python code.
..
Patch Set 5:
(1 comment)
http://gerrit.ovirt.org/#/c/27298/5/lib/vdsm/tool/configurator.py
File lib/vdsm/tool/configurator.py:
Line 353: if not os.path.isfile(TARGET):
Line 354: service.service_stop('libvirtd')
Line 355: if not os.path.isfile(TARGET) or \
Line 356: not filecmp.cmp(packeged, TARGET):
Line 357: shutil.copyfile(packeged, TARGET)
Do I need the mode of the packaged file or that of the old TARGET?
you need to keep the destination mode, you can either save it and restore after
copy or apply after copy
Line 358: rc, out, err = utils.execCmd((INITCTL,
Line 359:
reload-configuration))
Line 360: if rc != 0:
Line 361: sys.stdout.write(out)
--
To view, visit http://gerrit.ovirt.org/27298
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I74bfe05bb4b5f5d09021f21b324f9b7d5d0fdaab
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: mooli tayer mta...@redhat.com
Gerrit-Reviewer: Alon Bar-Lev alo...@redhat.com
Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com
Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: mooli tayer mta...@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]: replace configure_libvirt.py with python code.
mooli tayer has posted comments on this change.
Change subject: replace configure_libvirt.py with python code.
..
Patch Set 5: Verified+1
(1 comment)
http://gerrit.ovirt.org/#/c/27298/5/lib/vdsm/tool/configurator.py
File lib/vdsm/tool/configurator.py:
Line 353: if not os.path.isfile(TARGET):
Line 354: service.service_stop('libvirtd')
Line 355: if not os.path.isfile(TARGET) or \
Line 356: not filecmp.cmp(packeged, TARGET):
Line 357: shutil.copyfile(packeged, TARGET)
you need to keep the destination mode, you can either save it and restore a
Done
Line 358: rc, out, err = utils.execCmd((INITCTL,
Line 359:
reload-configuration))
Line 360: if rc != 0:
Line 361: sys.stdout.write(out)
--
To view, visit http://gerrit.ovirt.org/27298
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I74bfe05bb4b5f5d09021f21b324f9b7d5d0fdaab
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: mooli tayer mta...@redhat.com
Gerrit-Reviewer: Alon Bar-Lev alo...@redhat.com
Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com
Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: mooli tayer mta...@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]: replace configure_libvirt.py with python code.
Alon Bar-Lev has posted comments on this change.
Change subject: replace configure_libvirt.py with python code.
..
Patch Set 6:
(1 comment)
nice! I think it is much better than what we had.
http://gerrit.ovirt.org/#/c/27298/6/lib/vdsm/tool/configurator.py
File lib/vdsm/tool/configurator.py:
Line 98: 'path': os.path.join(
Line 99: SYSCONF_PATH,
Line 100: 'libvirt/libvirtd.conf'
Line 101: ),
Line 102: 'configureFunc': 'addSection',
why not just put here the function? not a string the function, you can call it
directly.
addSection
or
LibvirtModuleConfigure.addSection
should work
also, if you do not want to have function... just put
lambda x, y, z: True
Line 103: 'removeConfFunc': 'removeSection',
Line 104: 'fragments': [
Line 105: {
Line 106: 'conditions': {},
--
To view, visit http://gerrit.ovirt.org/27298
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I74bfe05bb4b5f5d09021f21b324f9b7d5d0fdaab
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: mooli tayer mta...@redhat.com
Gerrit-Reviewer: Alon Bar-Lev alo...@redhat.com
Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com
Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: mooli tayer mta...@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]: replace configure_libvirt.py with python code.
mooli tayer has posted comments on this change.
Change subject: replace configure_libvirt.py with python code.
..
Patch Set 6:
(1 comment)
http://gerrit.ovirt.org/#/c/27298/6/lib/vdsm/tool/configurator.py
File lib/vdsm/tool/configurator.py:
Line 98: 'path': os.path.join(
Line 99: SYSCONF_PATH,
Line 100: 'libvirt/libvirtd.conf'
Line 101: ),
Line 102: 'configureFunc': 'addSection',
why not just put here the function? not a string the function, you can call
This only works if the functions are defined above FILES. maybe I should move
those functions to a new class?
Line 103: 'removeConfFunc': 'removeSection',
Line 104: 'fragments': [
Line 105: {
Line 106: 'conditions': {},
--
To view, visit http://gerrit.ovirt.org/27298
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I74bfe05bb4b5f5d09021f21b324f9b7d5d0fdaab
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: mooli tayer mta...@redhat.com
Gerrit-Reviewer: Alon Bar-Lev alo...@redhat.com
Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com
Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: mooli tayer mta...@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]: replace configure_libvirt.py with python code.
Alon Bar-Lev has posted comments on this change.
Change subject: replace configure_libvirt.py with python code.
..
Patch Set 6:
(1 comment)
http://gerrit.ovirt.org/#/c/27298/6/lib/vdsm/tool/configurator.py
File lib/vdsm/tool/configurator.py:
Line 98: 'path': os.path.join(
Line 99: SYSCONF_PATH,
Line 100: 'libvirt/libvirtd.conf'
Line 101: ),
Line 102: 'configureFunc': 'addSection',
This only works if the functions are defined above FILES. maybe I should mo
why do you care if function is above :)
you can also write:
def xxx(*args):
self._xxx(*args)
so you have only two lines above.
Line 103: 'removeConfFunc': 'removeSection',
Line 104: 'fragments': [
Line 105: {
Line 106: 'conditions': {},
--
To view, visit http://gerrit.ovirt.org/27298
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I74bfe05bb4b5f5d09021f21b324f9b7d5d0fdaab
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: mooli tayer mta...@redhat.com
Gerrit-Reviewer: Alon Bar-Lev alo...@redhat.com
Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com
Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: mooli tayer mta...@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]: replace configure_libvirt.py with python code.
Nir Soffer has posted comments on this change.
Change subject: replace configure_libvirt.py with python code.
..
Patch Set 6:
(6 comments)
Partial review.
http://gerrit.ovirt.org/#/c/27298/6/lib/vdsm/tool/configurator.py
File lib/vdsm/tool/configurator.py:
Line 311: }
Line 312: }
Line 313:
Line 314: def __init__(self):
Line 315: super(LibvirtModuleConfigure, self).__init__()
This __init__ is useless - if you remove it, the __init__() method you inherit
from the super class will run anyway.
Line 316:
Line 317: def getName(self):
Line 318: return 'libvirt'
Line 319:
Line 317: def getName(self):
Line 318: return 'libvirt'
Line 319:
Line 320: def getFile(self, fname):
Line 321: return LibvirtModuleConfigure.FILES[fname]['path']
Use self.FILES - class attributes are available to an instance in Python.
Line 322:
Line 323: def getServices(self):
Line 324: return [supervdsmd, vdsmd, libvirtd]
Line 325:
Line 373: context = {
Line 374: 'certs_exist': all(os.path.isfile(f) for f in [
Line 375: LibvirtModuleConfigure.CA_FILE,
Line 376: LibvirtModuleConfigure.CERT_FILE,
Line 377: LibvirtModuleConfigure.KEY_FILE
self would be nicer.
Line 378: ]),
Line 379: 'ssl_enabled': config.getboolean('vars', 'ssl'),
Line 380: 'sanlock_enabled': SANLOCK_ENABLED,
Line 381: 'libvirt_selinux': LIBVIRT_SELINUX
Line 403: lambda cfile: (
Line 404: cfile['configureFunc'] == 'prefixAndPrepend' or
Line 405: cfile['configureFunc'] == 'addSection'),
Line 406: LibvirtModuleConfigure.FILES.values()
Line 407: )
It is not clear what you are trying to do here; While Python support this
funcional style, it is usually much clear to use list comprehension for these
tasks:
return [file['path'] for file in self.FILES.values()
if (file['configureFunc'] == 'prefixAndPrepend' or
file['configureFunc'] == 'addSection')]
Hopefully I got this right :-)
Line 408: )
Line 409:
Line 410: def _sysvToUpstart(self):
Line 411:
Line 416: INITCTL = '/sbin/initctl'
Line 417: LIBVIRTD_UPSTART = 'libvirtd.upstart'
Line 418: TARGET = os.path.join(SYSCONF_PATH, init/libvirtd.conf)
Line 419:
Line 420: if os.path.isfile(INITCTL) and os.access(INITCTL, os.X_OK):
os.access uses the real uid/gid, so if someone run this code using sudo, the
check may fail when it should not.
See https://docs.python.org/2/library/os.html#os.access
If you want to check if a file is executable by some user, check the mode and
owner.
Line 421: ts = rpm.TransactionSet()
Line 422: mi = itertools.chain(*[ts.dbMatch('name', name)
Line 423:for name in ['libvirt',
'libvirt-daemon']])
Line 424: # libvirtd package does not provide libvirtd.upstart,
Line 425: # this could happen in Ubuntu or other distro,
Line 426: # so continue to use system default init mechanism
Line 427:
Line 428: for filename in itertools.chain(*[h[rpm.RPMTAG_FILENAMES]
Line 429: for h in mi]):
Two chains with these cryptic variable names make it too hard to understand.
Using a symple iterator function is more clear:
def iterateLibvirtFiles(self):
ts = rpm.TransactionSet()
for name in ['libvirt', 'libvirt-daemon']:
for matches in ts.dbMatch('name', name):
for filename in matches[rpm.RPMTAG_FILENAMES]:
yield filename
Hopefully I got this right, I had to guess what is mi and h.
Line 430: if LIBVIRTD_UPSTART in filename:
Line 431: packeged = filename
Line 432: break
Line 433:
--
To view, visit http://gerrit.ovirt.org/27298
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I74bfe05bb4b5f5d09021f21b324f9b7d5d0fdaab
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: mooli tayer mta...@redhat.com
Gerrit-Reviewer: Alon Bar-Lev alo...@redhat.com
Gerrit-Reviewer: Nir Soffer nsof...@redhat.com
Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com
Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: mooli tayer mta...@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]: replace configure_libvirt.py with python code.
mooli tayer has posted comments on this change.
Change subject: replace configure_libvirt.py with python code.
..
Patch Set 4:
(1 comment)
http://gerrit.ovirt.org/#/c/27298/4/lib/vdsm/tool/configurator.py
File lib/vdsm/tool/configurator.py:
Line 149: 'require_lease_for_disks': 0,
Line 150: }
Line 151:
Line 152: # libvirt log rotate configuration
Line 153: LLOGR_CONF = (
I am unsure where vdsm packagers wants this but... at Makefile.am of tool y
Had some trouble with this issue so i'm moving it to it's own change. could be
squashed back together later.
Line 154: '/var/log/libvirt/libvirtd.log {'
Line 155: 'rotate 100'
Line 156: 'missingok'
Line 157: 'copytruncate'
--
To view, visit http://gerrit.ovirt.org/27298
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I74bfe05bb4b5f5d09021f21b324f9b7d5d0fdaab
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: mooli tayer mta...@redhat.com
Gerrit-Reviewer: Alon Bar-Lev alo...@redhat.com
Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com
Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: mooli tayer mta...@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]: replace configure_libvirt.py with python code.
mooli tayer has posted comments on this change.
Change subject: replace configure_libvirt.py with python code.
..
Patch Set 4:
(6 comments)
http://gerrit.ovirt.org/#/c/27298/4/lib/vdsm/tool/configurator.py
File lib/vdsm/tool/configurator.py:
Line 149: 'require_lease_for_disks': 0,
Line 150: }
Line 151:
Line 152: # libvirt log rotate configuration
Line 153: LLOGR_CONF = (
why don't you embed this as file at /usr/share instead of hardcode?
A hint on how to do this?
Line 154: '/var/log/libvirt/libvirtd.log {'
Line 155: 'rotate 100'
Line 156: 'missingok'
Line 157: 'copytruncate'
Line 158: 'size 15M'
Line 159: 'compress'
Line 160: 'compresscmd /usr/bin/xz'
Line 161: 'uncompresscmd /usr/bin/unxz'
Line 162: 'compressext .xz'
I hope we have xz dependency at spec...
Yes we do.
Line 163: '}'
Line 164: )
Line 165:
Line 166: def __init__(self):
Line 218: if SANLOCK_ENABLED:
Line 219: qconf_map.update(self.QCONF_SANLOCK)
Line 220: qlconf_map.update(self.QLCONF_SANLOCK)
Line 221: if not LIBVIRT_SELINUX:
Line 222: qconf_map.update(self.QCONF_NO_SELINUX)
I do not see any reason to put one time used constants...
Ok I'm giving this a try.
Line 223:
Line 224: # write configuration
Line 225: for file_name, configuration in [
Line 226: (self.envGet('LCONF'), lconf_map),
Line 240: self.envGet('LRCONF'), **VDSM_CONF_SECTION) as conf:
Line 241: conf.prefixLines('# VDSM backup')
Line 242: conf.prependSection(self.LLOGR_CONF)
Line 243:
Line 244: if utils.isOvirtNode() and ovirtfunctions:
it cannot be that it is node and functions are missing.
Done
Line 245: for fname in (
Line 246: self.envGet('LCONF'),
Line 247: self.envGet('QCONF'),
Line 248: self.envGet('LDCONF'),
Line 260: crashed.
Line 261:
Line 262: INITCTL = '/sbin/initctl'
Line 263: LIBVIRTD_UPSTART = 'libvirtd.upstart'
Line 264: TARGET = /etc/init/libvirtd.conf
etc should be sysconfdir from autoconf
Done
Line 265: if os.path.isfile(INITCTL) and os.access(INITCTL, os.X_OK):
Line 266: ts = rpm.TransactionSet()
Line 267: mi = itertools.chain(*[ts.dbMatch('name', name)
Line 268:for name in ['libvirt',
'libvirt-daemon']])
http://gerrit.ovirt.org/#/c/27298/4/tests/toolTests.py
File tests/toolTests.py:
Line 56: keepalive_interval=-1\n
Line 57: log_outputs=\1:file:/var/log/libvirt/libvirtd.log\\n
Line 58: ca_file=\/etc/pki/vdsm/certs/cacert.pem\\n
Line 59: cert_file=\/etc/pki/vdsm/certs/vdsmcert.pem\\n
Line 60: ## end of configuration section by vdsm-4.13.0\n
I would have put all these in separate files out of the script
Done
Line 61: ),
Line 62:
Line 63: qconf_ssl: (
Line 64: ## beginning of configuration section by vdsm-4.13.0\n
--
To view, visit http://gerrit.ovirt.org/27298
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I74bfe05bb4b5f5d09021f21b324f9b7d5d0fdaab
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: mooli tayer mta...@redhat.com
Gerrit-Reviewer: Alon Bar-Lev alo...@redhat.com
Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com
Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: mooli tayer mta...@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]: replace configure_libvirt.py with python code.
Alon Bar-Lev has posted comments on this change.
Change subject: replace configure_libvirt.py with python code.
..
Patch Set 4:
(1 comment)
http://gerrit.ovirt.org/#/c/27298/4/lib/vdsm/tool/configurator.py
File lib/vdsm/tool/configurator.py:
Line 149: 'require_lease_for_disks': 0,
Line 150: }
Line 151:
Line 152: # libvirt log rotate configuration
Line 153: LLOGR_CONF = (
A hint on how to do this?
I am unsure where vdsm packagers wants this but... at Makefile.am of tool you
can put:
toolfilesdir=$(pkgdatadir)/tool-files
dist_toolfilesdir_DATA=list of files
these files will be installed at /usr/share/vdsm/tool-files
Line 154: '/var/log/libvirt/libvirtd.log {'
Line 155: 'rotate 100'
Line 156: 'missingok'
Line 157: 'copytruncate'
--
To view, visit http://gerrit.ovirt.org/27298
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I74bfe05bb4b5f5d09021f21b324f9b7d5d0fdaab
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: mooli tayer mta...@redhat.com
Gerrit-Reviewer: Alon Bar-Lev alo...@redhat.com
Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com
Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: mooli tayer mta...@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]: replace configure_libvirt.py with python code.
Yaniv Bronhaim has posted comments on this change.
Change subject: replace configure_libvirt.py with python code.
..
Patch Set 3: Code-Review-1
(1 comment)
http://gerrit.ovirt.org/#/c/27298/3/tests/toolTests.py
File tests/toolTests.py:
Line 166: ('QCONF', 'empty'),
Line 167: )
Line 168: self.assertFalse(libvirtConfigure.isconfigured())
Line 169:
Line 170:
you need to add much more tests in this patch scope. then we'll be able to see
if the segfault was vanished or not
Line 171: class ConfigFileTests(TestCase):
Line 172: def setUp(self):
Line 173: fd, self.tname = tempfile.mkstemp()
Line 174: os.close(fd)
--
To view, visit http://gerrit.ovirt.org/27298
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I74bfe05bb4b5f5d09021f21b324f9b7d5d0fdaab
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: mooli tayer mta...@redhat.com
Gerrit-Reviewer: Alon Bar-Lev alo...@redhat.com
Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com
Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com
Gerrit-Reviewer: automat...@ovirt.org
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]: replace configure_libvirt.py with python code.
mooli tayer has posted comments on this change.
Change subject: replace configure_libvirt.py with python code.
..
Patch Set 3:
(9 comments)
http://gerrit.ovirt.org/#/c/27298/3/lib/vdsm/tool/Makefile.am
File lib/vdsm/tool/Makefile.am:
Line 31
Line 32
Line 33
Line 34
Line 35
remove all section
Done
http://gerrit.ovirt.org/#/c/27298/3/lib/vdsm/tool/configurator.py
File lib/vdsm/tool/configurator.py:
Line 176: VMCONF
leave it VDSMCONF
ok VDSM_CONF.
Line 195: raise RuntimeError(
Line 196: vdsm: Missing certificate, vdsm not registered)
Line 197: validate_ovirt_certs.validate_ovirt_certs()
Line 198: # Remove a previous configuration (if present)
Line 199: self.removeConf()
for backward compatibility .. but im not sure we want to remove the conf if
I'm not sure.
Anyway does not belong in this patch.
Line 200: lconf_maps = [self.LCONF_GENERAL]
Line 201: qconf_maps = [self.QCONF_GENERAL]
Line 202: ldconf_maps = [self.LDCONF_GENERAL]
Line 203: qlconf_maps = []
Line 209:[self.CA_FILE, self.CERT_FILE, self.KEY_FILE]):
Line 210: lconf_maps.append(self.LCONF_SSL)
Line 211: qconf_maps.append(self.QCONF_SSL_CERTS)
Line 212: else:
Line 213: lconf_maps.append(self.LCONF_NO_SSL)
this will introduce conflict ^, you must have the certs if ssl=True, no ?
Same behaviour as libvirt_configure.sh.
see line 258 there.
Line 214: else:
Line 215: qconf_maps.append(self.QCONF_NO_SSL)
Line 216: lconf_maps.append(self.LCONF_NO_SSL)
Line 217: if SANLOCK_ENABLED:
Line 237:
Line 238: with ConfigFile(
Line 239: self.envGet('LRCONF'), **VDSM_CONF_SECTION) as conf:
Line 240: conf.prefixLines('# VDSM backup')
Line 241: conf.prependSection(self.LLOGR_CONF)
maybe pass boolean if backup is required or not. it can lead to big conf fi
This backup is the same as done currently by configure_libvirt.sh.
(line 313)
Line 242:
Line 243: for fname in (
Line 244: self.envGet('LCONF'),
Line 245: self.envGet('QCONF'),
Line 247: self.envGet('QLCONF'),
Line 248: self.envGet('LRCONF')
Line 249: ):
Line 250: if utils.isOvirtNode() and ovirtfunctions:
Line 251: ovirtfunctions.ovirt_store_config(fname)
put the condition before the for
Done
Line 252: sys.stdout.write(Reconfiguration of libvirt is done.)
Line 253:
Line 254: def sysvToUpstart(self):
Line 255:
Line 258: crashed.
Line 259:
Line 260: INITCTL = '/sbin/initctl'
Line 261: LIBVIRTD_UPSTART = 'libvirtd.upstart'
Line 262: TARGET = /etc/init/libvirtd.conf
private. add underscore ^
Done
Line 263: if os.path.isfile(INITCTL) and os.access(INITCTL, os.X_OK):
Line 264: ts = rpm.TransactionSet()
Line 265: mi = itertools.chain(*[ts.dbMatch('name', name)
Line 266:for name in ['libvirt',
'libvirt-daemon']])
Line 296: 'listen_tcp': '0',
Line 297: 'auth_tcp': 'none'
Line 298: })
Line 299: lconf_p.read(self.envGet('LCONF'))
Line 300: listen_tcp = lconf_p.getboolean('listen_tcp')
is getboolean really needed? you check if its 1 or 0
will change to get int
Line 301: auth_tcp = lconf_p.get('auth_tcp')
Line 302: qconf_p = ParserWrapper({'spice_tls': '0'})
Line 303: qconf_p.read(self.envGet('QCONF'))
Line 304: spice_tls = qconf_p.getboolean('spice_tls')
http://gerrit.ovirt.org/#/c/27298/3/tests/toolTests.py
File tests/toolTests.py:
Line 166: ('QCONF', 'empty'),
Line 167: )
Line 168: self.assertFalse(libvirtConfigure.isconfigured())
Line 169:
Line 170:
you need to add much more tests in this patch scope. then we'll be able to
I plan more tests to test configure verb.(instead of those removed) also I am
still testing manually.
Line 171: class ConfigFileTests(TestCase):
Line 172: def setUp(self):
Line 173: fd, self.tname = tempfile.mkstemp()
Line 174: os.close(fd)
--
To view, visit http://gerrit.ovirt.org/27298
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I74bfe05bb4b5f5d09021f21b324f9b7d5d0fdaab
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: mooli tayer mta...@redhat.com
Gerrit-Reviewer: Alon Bar-Lev alo...@redhat.com
Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com
Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: mooli tayer mta...@redhat.com
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing
Change in vdsm[master]: replace configure_libvirt.py with python code.
mooli tayer has posted comments on this change.
Change subject: replace configure_libvirt.py with python code.
..
Patch Set 3:
(1 comment)
http://gerrit.ovirt.org/#/c/27298/3/tests/toolTests.py
File tests/toolTests.py:
Line 166: ('QCONF', 'empty'),
Line 167: )
Line 168: self.assertFalse(libvirtConfigure.isconfigured())
Line 169:
Line 170:
I plan more tests to test configure verb.(instead of those removed) also I
Done
Line 171: class ConfigFileTests(TestCase):
Line 172: def setUp(self):
Line 173: fd, self.tname = tempfile.mkstemp()
Line 174: os.close(fd)
--
To view, visit http://gerrit.ovirt.org/27298
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I74bfe05bb4b5f5d09021f21b324f9b7d5d0fdaab
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: mooli tayer mta...@redhat.com
Gerrit-Reviewer: Alon Bar-Lev alo...@redhat.com
Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com
Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: mooli tayer mta...@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]: replace configure_libvirt.py with python code.
oVirt Jenkins CI Server has posted comments on this change. Change subject: replace configure_libvirt.py with python code. .. Patch Set 4: Build Failed http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/8630/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/7840/ : FAILURE http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/8758/ : FAILURE http://jenkins.ovirt.org/job/vdsm_master_install_rpm_sanity_gerrit/567/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/27298 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I74bfe05bb4b5f5d09021f21b324f9b7d5d0fdaab Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: mooli tayer mta...@redhat.com Gerrit-Reviewer: Alon Bar-Lev alo...@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer mta...@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]: replace configure_libvirt.py with python code.
Alon Bar-Lev has posted comments on this change.
Change subject: replace configure_libvirt.py with python code.
..
Patch Set 4:
(6 comments)
these tend to be a mess... I think it can be simpler, however it is not that
important.
the only important note is to move all content of files to separate files
instead of hardcode them, it will enable much easier management instead of
touching the code.
http://gerrit.ovirt.org/#/c/27298/4/lib/vdsm/tool/configurator.py
File lib/vdsm/tool/configurator.py:
Line 149: 'require_lease_for_disks': 0,
Line 150: }
Line 151:
Line 152: # libvirt log rotate configuration
Line 153: LLOGR_CONF = (
why don't you embed this as file at /usr/share instead of hardcode?
also please notice that /var should be gotten from autoconf localstatedir
Line 154: '/var/log/libvirt/libvirtd.log {'
Line 155: 'rotate 100'
Line 156: 'missingok'
Line 157: 'copytruncate'
Line 158: 'size 15M'
Line 159: 'compress'
Line 160: 'compresscmd /usr/bin/xz'
Line 161: 'uncompresscmd /usr/bin/unxz'
Line 162: 'compressext .xz'
I hope we have xz dependency at spec...
Line 163: '}'
Line 164: )
Line 165:
Line 166: def __init__(self):
Line 218: if SANLOCK_ENABLED:
Line 219: qconf_map.update(self.QCONF_SANLOCK)
Line 220: qlconf_map.update(self.QLCONF_SANLOCK)
Line 221: if not LIBVIRT_SELINUX:
Line 222: qconf_map.update(self.QCONF_NO_SELINUX)
I do not see any reason to put one time used constants...
much better to get these map using designated function for each subject having
all required information at that function.
also, if these conditions are trivial... then you can even add lambda for each
key...
[
{
condition=lambda x,
conf=xxx,
content={
}
},
...
]
then you go entry by entry executing lambda and add the content...
you have less code more metadata... which is what I like.
but not that important... just a different pattern.
Line 223:
Line 224: # write configuration
Line 225: for file_name, configuration in [
Line 226: (self.envGet('LCONF'), lconf_map),
Line 240: self.envGet('LRCONF'), **VDSM_CONF_SECTION) as conf:
Line 241: conf.prefixLines('# VDSM backup')
Line 242: conf.prependSection(self.LLOGR_CONF)
Line 243:
Line 244: if utils.isOvirtNode() and ovirtfunctions:
it cannot be that it is node and functions are missing.
I suggest to import it here.
Line 245: for fname in (
Line 246: self.envGet('LCONF'),
Line 247: self.envGet('QCONF'),
Line 248: self.envGet('LDCONF'),
Line 260: crashed.
Line 261:
Line 262: INITCTL = '/sbin/initctl'
Line 263: LIBVIRTD_UPSTART = 'libvirtd.upstart'
Line 264: TARGET = /etc/init/libvirtd.conf
etc should be sysconfdir from autoconf
Line 265: if os.path.isfile(INITCTL) and os.access(INITCTL, os.X_OK):
Line 266: ts = rpm.TransactionSet()
Line 267: mi = itertools.chain(*[ts.dbMatch('name', name)
Line 268:for name in ['libvirt',
'libvirt-daemon']])
http://gerrit.ovirt.org/#/c/27298/4/tests/toolTests.py
File tests/toolTests.py:
Line 56: keepalive_interval=-1\n
Line 57: log_outputs=\1:file:/var/log/libvirt/libvirtd.log\\n
Line 58: ca_file=\/etc/pki/vdsm/certs/cacert.pem\\n
Line 59: cert_file=\/etc/pki/vdsm/certs/vdsmcert.pem\\n
Line 60: ## end of configuration section by vdsm-4.13.0\n
I would have put all these in separate files out of the script
Line 61: ),
Line 62:
Line 63: qconf_ssl: (
Line 64: ## beginning of configuration section by vdsm-4.13.0\n
--
To view, visit http://gerrit.ovirt.org/27298
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I74bfe05bb4b5f5d09021f21b324f9b7d5d0fdaab
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: mooli tayer mta...@redhat.com
Gerrit-Reviewer: Alon Bar-Lev alo...@redhat.com
Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com
Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: mooli tayer mta...@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]: replace configure_libvirt.py with python code.
mooli tayer has posted comments on this change. Change subject: replace configure_libvirt.py with python code. .. Patch Set 4: (1 comment) http://gerrit.ovirt.org/#/c/27298/4/lib/vdsm/tool/libvirt_configure.sh.in File lib/vdsm/tool/libvirt_configure.sh.in: Line 43 Line 44 Line 45 Line 46 Line 47 Alonbl: please notice I removed the above marker file behavior since I did not see it used. however I am not sure about it. -- To view, visit http://gerrit.ovirt.org/27298 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I74bfe05bb4b5f5d09021f21b324f9b7d5d0fdaab Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: mooli tayer mta...@redhat.com Gerrit-Reviewer: Alon Bar-Lev alo...@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer mta...@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]: replace configure_libvirt.py with python code.
Alon Bar-Lev has posted comments on this change. Change subject: replace configure_libvirt.py with python code. .. Patch Set 4: (1 comment) http://gerrit.ovirt.org/#/c/27298/4/lib/vdsm/tool/libvirt_configure.sh.in File lib/vdsm/tool/libvirt_configure.sh.in: Line 43 Line 44 Line 45 Line 46 Line 47 Alonbl: please notice I removed the above marker file it is used by ovirt-host-deploy-1.0 (ovirt-engine-3.2) to force configuration during start of service, but if not used it will fallback to the old methods. so safe to remove. -- To view, visit http://gerrit.ovirt.org/27298 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I74bfe05bb4b5f5d09021f21b324f9b7d5d0fdaab Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: mooli tayer mta...@redhat.com Gerrit-Reviewer: Alon Bar-Lev alo...@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer mta...@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]: replace configure_libvirt.py with python code.
Yaniv Bronhaim has posted comments on this change.
Change subject: replace configure_libvirt.py with python code.
..
Patch Set 3:
(8 comments)
love it ! thanks!! just few comments. please have more reviews now. i think its
almost fully ready
http://gerrit.ovirt.org/#/c/27298/3/lib/vdsm/tool/Makefile.am
File lib/vdsm/tool/Makefile.am:
Line 31
Line 32
Line 33
Line 34
Line 35
remove all section
http://gerrit.ovirt.org/#/c/27298/3/lib/vdsm/tool/configurator.py
File lib/vdsm/tool/configurator.py:
Line 176: VMCONF
leave it VDSMCONF
Line 195: raise RuntimeError(
Line 196: vdsm: Missing certificate, vdsm not registered)
Line 197: validate_ovirt_certs.validate_ovirt_certs()
Line 198: # Remove a previous configuration (if present)
Line 199: self.removeConf()
for backward compatibility .. but im not sure we want to remove the conf if
exist... maybe on force
Line 200: lconf_maps = [self.LCONF_GENERAL]
Line 201: qconf_maps = [self.QCONF_GENERAL]
Line 202: ldconf_maps = [self.LDCONF_GENERAL]
Line 203: qlconf_maps = []
Line 209:[self.CA_FILE, self.CERT_FILE, self.KEY_FILE]):
Line 210: lconf_maps.append(self.LCONF_SSL)
Line 211: qconf_maps.append(self.QCONF_SSL_CERTS)
Line 212: else:
Line 213: lconf_maps.append(self.LCONF_NO_SSL)
this will introduce conflict ^, you must have the certs if ssl=True, no ?
Line 214: else:
Line 215: qconf_maps.append(self.QCONF_NO_SSL)
Line 216: lconf_maps.append(self.LCONF_NO_SSL)
Line 217: if SANLOCK_ENABLED:
Line 237:
Line 238: with ConfigFile(
Line 239: self.envGet('LRCONF'), **VDSM_CONF_SECTION) as conf:
Line 240: conf.prefixLines('# VDSM backup')
Line 241: conf.prependSection(self.LLOGR_CONF)
maybe pass boolean if backup is required or not. it can lead to big conf files
and might cause a regression
Line 242:
Line 243: for fname in (
Line 244: self.envGet('LCONF'),
Line 245: self.envGet('QCONF'),
Line 247: self.envGet('QLCONF'),
Line 248: self.envGet('LRCONF')
Line 249: ):
Line 250: if utils.isOvirtNode() and ovirtfunctions:
Line 251: ovirtfunctions.ovirt_store_config(fname)
put the condition before the for
Line 252: sys.stdout.write(Reconfiguration of libvirt is done.)
Line 253:
Line 254: def sysvToUpstart(self):
Line 255:
Line 258: crashed.
Line 259:
Line 260: INITCTL = '/sbin/initctl'
Line 261: LIBVIRTD_UPSTART = 'libvirtd.upstart'
Line 262: TARGET = /etc/init/libvirtd.conf
private. add underscore ^
Line 263: if os.path.isfile(INITCTL) and os.access(INITCTL, os.X_OK):
Line 264: ts = rpm.TransactionSet()
Line 265: mi = itertools.chain(*[ts.dbMatch('name', name)
Line 266:for name in ['libvirt',
'libvirt-daemon']])
Line 296: 'listen_tcp': '0',
Line 297: 'auth_tcp': 'none'
Line 298: })
Line 299: lconf_p.read(self.envGet('LCONF'))
Line 300: listen_tcp = lconf_p.getboolean('listen_tcp')
is getboolean really needed? you check if its 1 or 0
Line 301: auth_tcp = lconf_p.get('auth_tcp')
Line 302: qconf_p = ParserWrapper({'spice_tls': '0'})
Line 303: qconf_p.read(self.envGet('QCONF'))
Line 304: spice_tls = qconf_p.getboolean('spice_tls')
--
To view, visit http://gerrit.ovirt.org/27298
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I74bfe05bb4b5f5d09021f21b324f9b7d5d0fdaab
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: mooli tayer mta...@redhat.com
Gerrit-Reviewer: Alon Bar-Lev alo...@redhat.com
Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com
Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com
Gerrit-Reviewer: automat...@ovirt.org
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]: replace configure_libvirt.py with python code.
mooli tayer has uploaded a new change for review.
Change subject: replace configure_libvirt.py with python code.
..
replace configure_libvirt.py with python code.
Change-Id: I74bfe05bb4b5f5d09021f21b324f9b7d5d0fdaab
Signed-off-by: Mooli Tayer mta...@redhat.com
---
M .gitignore
M configure.ac
M debian/vdsm-python.install
M lib/vdsm/constants.py.in
M lib/vdsm/tool/Makefile.am
M lib/vdsm/tool/configurator.py
R lib/vdsm/tool/libvirt_configure.sh
M lib/vdsm/tool/passwd.py
M lib/vdsm/utils.py
M tests/toolTests.py
M vdsm.spec.in
11 files changed, 496 insertions(+), 200 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/98/27298/1
diff --git a/.gitignore b/.gitignore
index 6c41d2b..95dde41 100644
--- a/.gitignore
+++ b/.gitignore
@@ -34,7 +34,6 @@
init/vdsmd_init_common.sh
lib/vdsm/config.py
lib/vdsm/constants.py
-lib/vdsm/tool/libvirt_configure.sh
lib/vdsm/tool/load_needed_modules.py
lib/vdsm/tool/validate_ovirt_certs.py
lib/vdsm/vdscli.py
diff --git a/configure.ac b/configure.ac
index 12828be..097fac6 100644
--- a/configure.ac
+++ b/configure.ac
@@ -233,6 +233,7 @@
AC_PATH_PROG([RM_PATH], [rm], [/bin/rm])
AC_PATH_PROG([RPM_PATH], [rpm], [/bin/rpm])
AC_PATH_PROG([RSYNC_PATH], [rsync], [/usr/bin/rsync])
+AC_PATH_PROG([SASLPASSWD2_PATH], [saslpasswd2], [/sbin/saslpasswd2])
AC_PATH_PROG([SED_PATH], [sed], [/bin/sed])
AC_PATH_PROG([SERVICE_PATH], [service], [/sbin/service])
AC_PATH_PROG([SETSID_PATH], [setsid], [/usr/bin/setsid])
diff --git a/debian/vdsm-python.install b/debian/vdsm-python.install
index df873c6..73e5237 100644
--- a/debian/vdsm-python.install
+++ b/debian/vdsm-python.install
@@ -30,4 +30,3 @@
./usr/lib/python2.7/dist-packages/vdsm/tool/vdsm-id.py
./usr/lib/python2.7/dist-packages/vdsm/utils.py
./usr/lib/python2.7/dist-packages/vdsm/vdscli.py
-./usr/libexec/vdsm/libvirt_configure.sh
diff --git a/lib/vdsm/constants.py.in b/lib/vdsm/constants.py.in
index 4ddfe84..49c34ab 100644
--- a/lib/vdsm/constants.py.in
+++ b/lib/vdsm/constants.py.in
@@ -42,7 +42,12 @@
QEMU_PROCESS_GROUP = '@QEMUGROUP@'
# Sanlock definitions
+SANLOCK_ENABLED = '@ENABLE_LIBVIRT_SANLOCK@' == 'yes'
SANLOCK_USER = '@SNLKUSER@'
+
+# Libvirt selinux
+LIBVIRT_SELINUX = '@ENABLE_LIBVIRT_SELINUX@' == 'yes'
+
#
# The username of SASL authenticating for libvirt connection
@@ -75,12 +80,18 @@
P_VDSM_CONF = '@CONFDIR@/'
P_VDSM_KEYS = '/etc/pki/vdsm/keys/'
P_VDSM_LIBVIRT_PASSWD = P_VDSM_KEYS + 'libvirt_password'
+P_VDSM_CERT = '/etc/pki/vdsm/certs/vdsmcert.pem'
P_VDSM_CLIENT_LOG = '@VDSMRUNDIR@/client.log'
P_VDSM_LOG = '@VDSMLOGDIR@'
P_VDSM_NODE_ID = '/etc/vdsm/vdsm.id'
P_VDSM_EXEC = '@LIBEXECDIR@'
+
+#
+# Configuration file definitions
+#
+SYSCONF_PATH = '@sysconfdir@'
#
# External programs (sorted, please keep in order).
@@ -130,6 +141,8 @@
EXT_RSYNC = '@RSYNC_PATH@'
+EXT_SASLPASSWD2 = '@SASLPASSWD2_PATH@'
+
EXT_SERVICE = '@SERVICE_PATH@'
EXT_SETSID = '@SETSID_PATH@'
EXT_SH = '/bin/sh' # The shell path is invariable
diff --git a/lib/vdsm/tool/Makefile.am b/lib/vdsm/tool/Makefile.am
index c7c2175..e925c87 100644
--- a/lib/vdsm/tool/Makefile.am
+++ b/lib/vdsm/tool/Makefile.am
@@ -21,7 +21,6 @@
EXTRA_DIST = \
load_needed_modules.py.in \
- libvirt_configure.sh.in \
validate_ovirt_certs.py.in \
$(NULL)
diff --git a/lib/vdsm/tool/configurator.py b/lib/vdsm/tool/configurator.py
index f0944f3..e5c6a37 100644
--- a/lib/vdsm/tool/configurator.py
+++ b/lib/vdsm/tool/configurator.py
@@ -21,11 +21,32 @@
import sys
import grp
import argparse
+import filecmp
+import itertools
+import rpm
+import shutil
+import traceback
+import uuid
from .. import utils
-from . import service, expose
-from ..constants import P_VDSM_EXEC, QEMU_PROCESS_GROUP, \
-SANLOCK_USER, VDSM_GROUP
+from . import service, expose, validate_ovirt_certs
+from .configfile import ConfigFile, ParserWrapper
+from ..constants import QEMU_PROCESS_GROUP, \
+SANLOCK_USER, VDSM_GROUP, SYSCONF_PATH, P_VDSM_CERT, \
+SANLOCK_ENABLED, LIBVIRT_SELINUX
+from vdsm.config import config
+
+try:
+from ovirtnode import ovirtfunctions
+except ImportError:
+pass
+
+
+VDSM_CONF_SECTION = {
+'sectionStart': '## beginning of configuration section by vdsm',
+'sectionEnd': '## end of configuration section by vdsm',
+'version': '4.13.0'
+}
class _ModuleConfigure(object):
@@ -50,71 +71,301 @@
def reconfigureOnForce(self):
return True
+def removeConf(self):
+pass
+
class LibvirtModuleConfigure(_ModuleConfigure):
-def __init__(self, env_override=None):
+
+PKI = os.path.join(SYSCONF_PATH, 'pki/vdsm')
+CA_FILE = os.path.join(PKI, 'certs/cacert.pem')
+CERT_FILE = os.path.join(PKI, 'certs/vdsmcert.pem')
+KEY_FILE = os.path.join(PKI, 'keys/vdsmkey.pem')
+LS_CERT_DIR = os.path.join(PKI, 'libvirt-spice')
+
+# Libvirt daemon configuration
+
Change in vdsm[master]: replace configure_libvirt.py with python code.
oVirt Jenkins CI Server has posted comments on this change. Change subject: replace configure_libvirt.py with python code. .. Patch Set 1: Code-Review-1 Verified-1 Build Failed http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/8468/ : UNSTABLE http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/7678/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/8589/ : FAILURE http://jenkins.ovirt.org/job/vdsm_master_install_rpm_sanity_gerrit/539/ : FAILURE -- To view, visit http://gerrit.ovirt.org/27298 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I74bfe05bb4b5f5d09021f21b324f9b7d5d0fdaab Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: mooli tayer mta...@redhat.com Gerrit-Reviewer: automat...@ovirt.org 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]: replace configure_libvirt.py with python code.
oVirt Jenkins CI Server has posted comments on this change. Change subject: replace configure_libvirt.py with python code. .. Patch Set 2: Code-Review-1 Verified-1 Build Failed http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/8472/ : UNSTABLE http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/7682/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/8593/ : FAILURE http://jenkins.ovirt.org/job/vdsm_master_install_rpm_sanity_gerrit/540/ : FAILURE -- To view, visit http://gerrit.ovirt.org/27298 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I74bfe05bb4b5f5d09021f21b324f9b7d5d0fdaab Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: mooli tayer mta...@redhat.com Gerrit-Reviewer: Alon Bar-Lev alo...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: automat...@ovirt.org 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]: replace configure_libvirt.py with python code.
oVirt Jenkins CI Server has posted comments on this change. Change subject: replace configure_libvirt.py with python code. .. Patch Set 3: Build Failed http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/8473/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/7683/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/8594/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_install_rpm_sanity_gerrit/541/ : FAILURE -- To view, visit http://gerrit.ovirt.org/27298 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I74bfe05bb4b5f5d09021f21b324f9b7d5d0fdaab Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: mooli tayer mta...@redhat.com Gerrit-Reviewer: Alon Bar-Lev alo...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: automat...@ovirt.org 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
