Change in vdsm[master]: Adding remove/disable verbs to vdsm-tool for admin usages
Yaniv Bronhaim has abandoned this change. Change subject: Adding remove/disable verbs to vdsm-tool for admin usages .. Abandoned -- To view, visit http://gerrit.ovirt.org/21772 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: abandon Gerrit-Change-Id: Ie7f2c031436a6d202f856c24d9c9420c8bfdf6df Gerrit-PatchSet: 16 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim Gerrit-Reviewer: Alon Bar-Lev Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer 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]: Adding remove/disable verbs to vdsm-tool for admin usages
Yaniv Bronhaim has posted comments on this change. Change subject: Adding remove/disable verbs to vdsm-tool for admin usages .. Patch Set 16: Code-Review-1 fix to my previous comment: the configurator.py changes can be part of http://gerrit.ovirt.org/#/c/27130/2 and the passwd.py can stay here. -- To view, visit http://gerrit.ovirt.org/21772 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7f2c031436a6d202f856c24d9c9420c8bfdf6df Gerrit-PatchSet: 16 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim Gerrit-Reviewer: Alon Bar-Lev Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer 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]: Adding remove/disable verbs to vdsm-tool for admin usages
Yaniv Bronhaim has posted comments on this change. Change subject: Adding remove/disable verbs to vdsm-tool for admin usages .. Patch Set 16: the configurator.py changes can be part of http://gerrit.ovirt.org/#/c/27130/2 and the calls to this verb can be separately -- To view, visit http://gerrit.ovirt.org/21772 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7f2c031436a6d202f856c24d9c9420c8bfdf6df Gerrit-PatchSet: 16 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim Gerrit-Reviewer: Alon Bar-Lev Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer 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]: Adding remove/disable verbs to vdsm-tool for admin usages
oVirt Jenkins CI Server has posted comments on this change. Change subject: Adding remove/disable verbs to vdsm-tool for admin usages .. Patch Set 16: Build Successful http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/8406/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/7616/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/8526/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/21772 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7f2c031436a6d202f856c24d9c9420c8bfdf6df Gerrit-PatchSet: 16 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim Gerrit-Reviewer: Alon Bar-Lev Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer 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]: Adding remove/disable verbs to vdsm-tool for admin usages
mooli tayer has posted comments on this change.
Change subject: Adding remove/disable verbs to vdsm-tool for admin usages
..
Patch Set 15:
(2 comments)
http://gerrit.ovirt.org/#/c/21772/15/lib/vdsm/tool/passwd.py
File lib/vdsm/tool/passwd.py:
Line 53: Remove vdsm password for libvirt connection
Line 54: """
Line 55: rc, out, err = utils.execCmd(
Line 56: (constants.EXT_SASLPASSWD2, '-p', '-a', 'libvirt',
Line 57: '-d', constants.SASL_USERNAME,),
> please consider to indent:
Yes it is better.
Line 58: )
Line 59: if rc != 0:
Line 56: (constants.EXT_SASLPASSWD2, '-p', '-a', 'libvirt',
Line 57: '-d', constants.SASL_USERNAME,),
Line 58: )
Line 59: if rc != 0:
Line 60: raise RuntimeError("Remove password failed: %s" % (err,))
> % err is sufficient.
I believe vdsm style usually uses tuple in such cases in
preparation to other variables.
--
To view, visit http://gerrit.ovirt.org/21772
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie7f2c031436a6d202f856c24d9c9420c8bfdf6df
Gerrit-PatchSet: 15
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim
Gerrit-Reviewer: Alon Bar-Lev
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Yaniv Bronhaim
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: mooli tayer
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]: Adding remove/disable verbs to vdsm-tool for admin usages
Alon Bar-Lev has posted comments on this change.
Change subject: Adding remove/disable verbs to vdsm-tool for admin usages
..
Patch Set 15:
(2 comments)
why don't you merge this? what should it vest in gerrit?
http://gerrit.ovirt.org/#/c/21772/15/lib/vdsm/tool/passwd.py
File lib/vdsm/tool/passwd.py:
Line 53: Remove vdsm password for libvirt connection
Line 54: """
Line 55: rc, out, err = utils.execCmd(
Line 56: (constants.EXT_SASLPASSWD2, '-p', '-a', 'libvirt',
Line 57: '-d', constants.SASL_USERNAME,),
please consider to indent:
rc, out, err = utils.execCmd(
(
constants.EXT_SASLPASSWD2,
'-p',
'-a', 'libvirt',
'-d', constants.SASL_USERNAME,
),
)
Line 58: )
Line 59: if rc != 0:
Line 56: (constants.EXT_SASLPASSWD2, '-p', '-a', 'libvirt',
Line 57: '-d', constants.SASL_USERNAME,),
Line 58: )
Line 59: if rc != 0:
Line 60: raise RuntimeError("Remove password failed: %s" % (err,))
% err is sufficient.
--
To view, visit http://gerrit.ovirt.org/21772
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie7f2c031436a6d202f856c24d9c9420c8bfdf6df
Gerrit-PatchSet: 15
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim
Gerrit-Reviewer: Alon Bar-Lev
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Yaniv Bronhaim
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: mooli tayer
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]: Adding remove/disable verbs to vdsm-tool for admin usages
oVirt Jenkins CI Server has posted comments on this change. Change subject: Adding remove/disable verbs to vdsm-tool for admin usages .. Patch Set 15: Build Failed http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/8349/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/7559/ : FAILURE http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/8469/ : FAILURE -- To view, visit http://gerrit.ovirt.org/21772 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7f2c031436a6d202f856c24d9c9420c8bfdf6df Gerrit-PatchSet: 15 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim Gerrit-Reviewer: Alon Bar-Lev Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer 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]: Adding remove/disable verbs to vdsm-tool for admin usages
oVirt Jenkins CI Server has posted comments on this change. Change subject: Adding remove/disable verbs to vdsm-tool for admin usages .. Patch Set 14: Code-Review-1 Verified-1 Build Failed http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/8216/ : UNSTABLE http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/8329/ : FAILURE http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/7426/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/21772 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7f2c031436a6d202f856c24d9c9420c8bfdf6df Gerrit-PatchSet: 14 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim Gerrit-Reviewer: Alon Bar-Lev Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer 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]: Adding remove/disable verbs to vdsm-tool for admin usages
mooli tayer has posted comments on this change. Change subject: Adding remove/disable verbs to vdsm-tool for admin usages .. Patch Set 13: Code-Review-1 -- To view, visit http://gerrit.ovirt.org/21772 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7f2c031436a6d202f856c24d9c9420c8bfdf6df Gerrit-PatchSet: 13 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim Gerrit-Reviewer: Alon Bar-Lev Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer 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]: Adding remove/disable verbs to vdsm-tool for admin usages
oVirt Jenkins CI Server has posted comments on this change. Change subject: Adding remove/disable verbs to vdsm-tool for admin usages .. Patch Set 13: Build Successful http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/6751/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/7541/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/7651/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/21772 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7f2c031436a6d202f856c24d9c9420c8bfdf6df Gerrit-PatchSet: 13 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim Gerrit-Reviewer: Alon Bar-Lev Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer 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]: Adding remove/disable verbs to vdsm-tool for admin usages
oVirt Jenkins CI Server has posted comments on this change. Change subject: Adding remove/disable verbs to vdsm-tool for admin usages .. Patch Set 12: Build Successful http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/6750/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/7540/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/7650/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/21772 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7f2c031436a6d202f856c24d9c9420c8bfdf6df Gerrit-PatchSet: 12 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim Gerrit-Reviewer: Alon Bar-Lev Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer 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]: Adding remove/disable verbs to vdsm-tool for admin usages
oVirt Jenkins CI Server has posted comments on this change. Change subject: Adding remove/disable verbs to vdsm-tool for admin usages .. Patch Set 11: Build Failed http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/6718/ : FAILURE http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/7508/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/7618/ : FAILURE -- To view, visit http://gerrit.ovirt.org/21772 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7f2c031436a6d202f856c24d9c9420c8bfdf6df Gerrit-PatchSet: 11 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim Gerrit-Reviewer: Alon Bar-Lev Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer 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]: Adding remove/disable verbs to vdsm-tool for admin usages
Yaniv Bronhaim has posted comments on this change. Change subject: Adding remove/disable verbs to vdsm-tool for admin usages .. Patch Set 10: Code-Review+1 -- To view, visit http://gerrit.ovirt.org/21772 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7f2c031436a6d202f856c24d9c9420c8bfdf6df Gerrit-PatchSet: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim Gerrit-Reviewer: Alon Bar-Lev Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer 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]: Adding remove/disable verbs to vdsm-tool for admin usages
oVirt Jenkins CI Server has posted comments on this change. Change subject: Adding remove/disable verbs to vdsm-tool for admin usages .. Patch Set 10: Build Successful http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/7482/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/6691/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/7592/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/21772 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7f2c031436a6d202f856c24d9c9420c8bfdf6df Gerrit-PatchSet: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim Gerrit-Reviewer: Alon Bar-Lev Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer 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]: Adding remove/disable verbs to vdsm-tool for admin usages
Yaniv Bronhaim has posted comments on this change. Change subject: Adding remove/disable verbs to vdsm-tool for admin usages .. Patch Set 9: (1 comment) http://gerrit.ovirt.org/#/c/21772/9/lib/vdsm/tool/configurator.py File lib/vdsm/tool/configurator.py: Line 31: Line 32: P_SYSTEMCTL_CONF = SYSCONF_PATH + '/sysctl.d/vdsm' Line 33: P_VDSM_LCONF = SYSCONF_PATH + '/libvirt/libvirtd.conf' Line 34: P_VDSM_LDCONF = SYSCONF_PATH + '/sysconfig/libvirtd' Line 35: P_VDSM_QCONF = SYSCONF_PATH + '/libvirt/qemu.conf' > sorry I mean sysconfdir defined in autogen. so if we already have it would didn't we talk about it ..? i thought we agreed already that you can leave it as is, just fix all jenkins errors Line 36: Line 37: def removeSectionFromFile(filename, beginField=None, endField=None): Line 38: ( Line 39: BEFORE, -- To view, visit http://gerrit.ovirt.org/21772 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7f2c031436a6d202f856c24d9c9420c8bfdf6df Gerrit-PatchSet: 9 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim Gerrit-Reviewer: Alon Bar-Lev Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer 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]: Adding remove/disable verbs to vdsm-tool for admin usages
mooli tayer has posted comments on this change. Change subject: Adding remove/disable verbs to vdsm-tool for admin usages .. Patch Set 9: I fixed pep8 issues long ago. I am waiting for you to respond before I upload next version. -- To view, visit http://gerrit.ovirt.org/21772 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7f2c031436a6d202f856c24d9c9420c8bfdf6df Gerrit-PatchSet: 9 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim Gerrit-Reviewer: Alon Bar-Lev Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer 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]: Adding remove/disable verbs to vdsm-tool for admin usages
Yaniv Bronhaim has posted comments on this change. Change subject: Adding remove/disable verbs to vdsm-tool for admin usages .. Patch Set 9: Code-Review-1 fix pep8 issues.. -- To view, visit http://gerrit.ovirt.org/21772 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7f2c031436a6d202f856c24d9c9420c8bfdf6df Gerrit-PatchSet: 9 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim Gerrit-Reviewer: Alon Bar-Lev Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer 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]: Adding remove/disable verbs to vdsm-tool for admin usages
Yaniv Bronhaim has posted comments on this change. Change subject: Adding remove/disable verbs to vdsm-tool for admin usages .. Patch Set 9: Code-Review+1 -- To view, visit http://gerrit.ovirt.org/21772 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7f2c031436a6d202f856c24d9c9420c8bfdf6df Gerrit-PatchSet: 9 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim Gerrit-Reviewer: Alon Bar-Lev Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer 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]: Adding remove/disable verbs to vdsm-tool for admin usages
mooli tayer has posted comments on this change. Change subject: Adding remove/disable verbs to vdsm-tool for admin usages .. Patch Set 9: (1 comment) http://gerrit.ovirt.org/#/c/21772/9/lib/vdsm/tool/configurator.py File lib/vdsm/tool/configurator.py: Line 31: Line 32: P_SYSTEMCTL_CONF = SYSCONF_PATH + '/sysctl.d/vdsm' Line 33: P_VDSM_LCONF = SYSCONF_PATH + '/libvirt/libvirtd.conf' Line 34: P_VDSM_LDCONF = SYSCONF_PATH + '/sysconfig/libvirtd' Line 35: P_VDSM_QCONF = SYSCONF_PATH + '/libvirt/qemu.conf' > you add it in this patch.. i say its redundant. i prefer to avoid more cons sorry I mean sysconfdir defined in autogen. so if we already have it would it not be better to use it? Line 36: Line 37: def removeSectionFromFile(filename, beginField=None, endField=None): Line 38: ( Line 39: BEFORE, -- To view, visit http://gerrit.ovirt.org/21772 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7f2c031436a6d202f856c24d9c9420c8bfdf6df Gerrit-PatchSet: 9 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim Gerrit-Reviewer: Alon Bar-Lev Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer 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]: Adding remove/disable verbs to vdsm-tool for admin usages
Yaniv Bronhaim has posted comments on this change. Change subject: Adding remove/disable verbs to vdsm-tool for admin usages .. Patch Set 9: (1 comment) http://gerrit.ovirt.org/#/c/21772/9/lib/vdsm/tool/configurator.py File lib/vdsm/tool/configurator.py: Line 31: Line 32: P_SYSTEMCTL_CONF = SYSCONF_PATH + '/sysctl.d/vdsm' Line 33: P_VDSM_LCONF = SYSCONF_PATH + '/libvirt/libvirtd.conf' Line 34: P_VDSM_LDCONF = SYSCONF_PATH + '/sysconfig/libvirtd' Line 35: P_VDSM_QCONF = SYSCONF_PATH + '/libvirt/qemu.conf' > Ok for moving, you add it in this patch.. i say its redundant. i prefer to avoid more constants Line 36: Line 37: def removeSectionFromFile(filename, beginField=None, endField=None): Line 38: ( Line 39: BEFORE, -- To view, visit http://gerrit.ovirt.org/21772 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7f2c031436a6d202f856c24d9c9420c8bfdf6df Gerrit-PatchSet: 9 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim Gerrit-Reviewer: Alon Bar-Lev Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer 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]: Adding remove/disable verbs to vdsm-tool for admin usages
mooli tayer has posted comments on this change. Change subject: Adding remove/disable verbs to vdsm-tool for admin usages .. Patch Set 9: (1 comment) http://gerrit.ovirt.org/#/c/21772/9/lib/vdsm/tool/configurator.py File lib/vdsm/tool/configurator.py: Line 31: Line 32: P_SYSTEMCTL_CONF = SYSCONF_PATH + '/sysctl.d/vdsm' Line 33: P_VDSM_LCONF = SYSCONF_PATH + '/libvirt/libvirtd.conf' Line 34: P_VDSM_LDCONF = SYSCONF_PATH + '/sysconfig/libvirtd' Line 35: P_VDSM_QCONF = SYSCONF_PATH + '/libvirt/qemu.conf' > its only used in LibvirtModuleConfigure, put it there. Ok for moving, as for naming the variables with /etc/ what's the point in that? itsn't it why we have SYSCONF_PATH for? Line 36: Line 37: def removeSectionFromFile(filename, beginField=None, endField=None): Line 38: ( Line 39: BEFORE, -- To view, visit http://gerrit.ovirt.org/21772 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7f2c031436a6d202f856c24d9c9420c8bfdf6df Gerrit-PatchSet: 9 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim Gerrit-Reviewer: Alon Bar-Lev Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer 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]: Adding remove/disable verbs to vdsm-tool for admin usages
Yaniv Bronhaim has posted comments on this change. Change subject: Adding remove/disable verbs to vdsm-tool for admin usages .. Patch Set 9: (1 comment) one last nit http://gerrit.ovirt.org/#/c/21772/9/lib/vdsm/tool/configurator.py File lib/vdsm/tool/configurator.py: Line 31: Line 32: P_SYSTEMCTL_CONF = SYSCONF_PATH + '/sysctl.d/vdsm' Line 33: P_VDSM_LCONF = SYSCONF_PATH + '/libvirt/libvirtd.conf' Line 34: P_VDSM_LDCONF = SYSCONF_PATH + '/sysconfig/libvirtd' Line 35: P_VDSM_QCONF = SYSCONF_PATH + '/libvirt/qemu.conf' its only used in LibvirtModuleConfigure, put it there. SYSCONF_PATH refers to /etc/ , I prefer to leave the constants.py file, abandon the SYCONF_PATH, and declare the full path here Line 36: Line 37: def removeSectionFromFile(filename, beginField=None, endField=None): Line 38: ( Line 39: BEFORE, -- To view, visit http://gerrit.ovirt.org/21772 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7f2c031436a6d202f856c24d9c9420c8bfdf6df Gerrit-PatchSet: 9 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim Gerrit-Reviewer: Alon Bar-Lev Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer 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]: Adding remove/disable verbs to vdsm-tool for admin usages
oVirt Jenkins CI Server has posted comments on this change. Change subject: Adding remove/disable verbs to vdsm-tool for admin usages .. Patch Set 9: Code-Review-1 Verified-1 Build Failed http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/7439/ : UNSTABLE http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/6646/ : FAILURE http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/7548/ : FAILURE -- To view, visit http://gerrit.ovirt.org/21772 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7f2c031436a6d202f856c24d9c9420c8bfdf6df Gerrit-PatchSet: 9 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim Gerrit-Reviewer: Alon Bar-Lev Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer 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]: Adding remove/disable verbs to vdsm-tool for admin usages
mooli tayer has posted comments on this change. Change subject: Adding remove/disable verbs to vdsm-tool for admin usages .. Patch Set 8: (1 comment) http://gerrit.ovirt.org/#/c/21772/8/lib/vdsm/constants.py.in File lib/vdsm/constants.py.in: Line 85: SYSCONF_PATH = '@sysconfdir@' Line 86: P_SYSTEMCTL_CONF = SYSCONF_PATH + '/sysctl.d/vdsm' Line 87: P_VDSM_LCONF = SYSCONF_PATH + '/libvirt/libvirtd.conf' Line 88: P_VDSM_LDCONF = SYSCONF_PATH + '/sysconfig/libvirtd' Line 89: P_VDSM_QCONF = SYSCONF_PATH + '/libvirt/qemu.conf' > these files also appear in the spec, is there any way to have them only in OK so I keep only SYSCONF_PATH here and remove the other 4 to configurator.py. Line 90: Line 91: # Line 92: # External programs (sorted, please keep in order). Line 93: # -- To view, visit http://gerrit.ovirt.org/21772 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7f2c031436a6d202f856c24d9c9420c8bfdf6df Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim Gerrit-Reviewer: Alon Bar-Lev Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer 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]: Adding remove/disable verbs to vdsm-tool for admin usages
mooli tayer has posted comments on this change. Change subject: Adding remove/disable verbs to vdsm-tool for admin usages .. Patch Set 8: (2 comments) http://gerrit.ovirt.org/#/c/21772/8/lib/vdsm/constants.py.in File lib/vdsm/constants.py.in: Line 42: QEMU_PROCESS_GROUP = '@QEMUGROUP@' Line 43: Line 44: # Line 45: # SASL definitions Line 46: # > why changing that? if comment, at least make it informative, like this is b My mistake, this was relevant for a previews commit. Will revert. Line 47: SASL_USERNAME = "vdsm@ovirt" Line 48: Line 49: # This is the domain version translation list Line 50: # DO NOT CHANGE OLD VALUES ONLY APPEND Line 85: SYSCONF_PATH = '@sysconfdir@' Line 86: P_SYSTEMCTL_CONF = SYSCONF_PATH + '/sysctl.d/vdsm' Line 87: P_VDSM_LCONF = SYSCONF_PATH + '/libvirt/libvirtd.conf' Line 88: P_VDSM_LDCONF = SYSCONF_PATH + '/sysconfig/libvirtd' Line 89: P_VDSM_QCONF = SYSCONF_PATH + '/libvirt/qemu.conf' > lets move those to configurator.py ? only there we use it.. maybe sysconf_p these files also appear in the spec, is there any way to have them only in one place or is the spec irrelevant to this? Also they appear in debain scripts. Line 90: Line 91: # Line 92: # External programs (sorted, please keep in order). Line 93: # -- To view, visit http://gerrit.ovirt.org/21772 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7f2c031436a6d202f856c24d9c9420c8bfdf6df Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim Gerrit-Reviewer: Alon Bar-Lev Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer 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]: Adding remove/disable verbs to vdsm-tool for admin usages
Yaniv Bronhaim has posted comments on this change. Change subject: Adding remove/disable verbs to vdsm-tool for admin usages .. Patch Set 8: (2 comments) thanks! only the files path in constants is annoying me. we use it only in libvirtConfigureModule so hold it there http://gerrit.ovirt.org/#/c/21772/8/lib/vdsm/constants.py.in File lib/vdsm/constants.py.in: Line 42: QEMU_PROCESS_GROUP = '@QEMUGROUP@' Line 43: Line 44: # Line 45: # SASL definitions Line 46: # why changing that? if comment, at least make it informative, like this is being used for libvirt communication Line 47: SASL_USERNAME = "vdsm@ovirt" Line 48: Line 49: # This is the domain version translation list Line 50: # DO NOT CHANGE OLD VALUES ONLY APPEND Line 85: SYSCONF_PATH = '@sysconfdir@' Line 86: P_SYSTEMCTL_CONF = SYSCONF_PATH + '/sysctl.d/vdsm' Line 87: P_VDSM_LCONF = SYSCONF_PATH + '/libvirt/libvirtd.conf' Line 88: P_VDSM_LDCONF = SYSCONF_PATH + '/sysconfig/libvirtd' Line 89: P_VDSM_QCONF = SYSCONF_PATH + '/libvirt/qemu.conf' lets move those to configurator.py ? only there we use it.. maybe sysconf_path can stay here Line 90: Line 91: # Line 92: # External programs (sorted, please keep in order). Line 93: # -- To view, visit http://gerrit.ovirt.org/21772 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7f2c031436a6d202f856c24d9c9420c8bfdf6df Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim Gerrit-Reviewer: Alon Bar-Lev Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer 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]: Adding remove/disable verbs to vdsm-tool for admin usages
oVirt Jenkins CI Server has posted comments on this change. Change subject: Adding remove/disable verbs to vdsm-tool for admin usages .. Patch Set 8: Code-Review-1 Verified-1 Build Failed http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/7429/ : UNSTABLE http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/6636/ : FAILURE http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/7538/ : FAILURE -- To view, visit http://gerrit.ovirt.org/21772 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7f2c031436a6d202f856c24d9c9420c8bfdf6df Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim Gerrit-Reviewer: Alon Bar-Lev Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer 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]: Adding remove/disable verbs to vdsm-tool for admin usages
mooli tayer has posted comments on this change. Change subject: Adding remove/disable verbs to vdsm-tool for admin usages .. Patch Set 7: I am talking about your new tests that I need to complete: http://gerrit.ovirt.org/#/c/25263/2 I will add the other patches soon -- To view, visit http://gerrit.ovirt.org/21772 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7f2c031436a6d202f856c24d9c9420c8bfdf6df Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim Gerrit-Reviewer: Alon Bar-Lev Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer 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]: Adding remove/disable verbs to vdsm-tool for admin usages
Yaniv Bronhaim has posted comments on this change. Change subject: Adding remove/disable verbs to vdsm-tool for admin usages .. Patch Set 7: what tests? please at the same patch calls to remove-conf verbs from vdsm.spec when needed. replace the inline scripts -- To view, visit http://gerrit.ovirt.org/21772 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7f2c031436a6d202f856c24d9c9420c8bfdf6df Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim Gerrit-Reviewer: Alon Bar-Lev Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer 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]: Adding remove/disable verbs to vdsm-tool for admin usages
mooli tayer has posted comments on this change. Change subject: Adding remove/disable verbs to vdsm-tool for admin usages .. Patch Set 7: Verified-1 I guess I better put -1 until all patches work... -- To view, visit http://gerrit.ovirt.org/21772 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7f2c031436a6d202f856c24d9c9420c8bfdf6df Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim Gerrit-Reviewer: Alon Bar-Lev Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer 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]: Adding remove/disable verbs to vdsm-tool for admin usages
Alon Bar-Lev has posted comments on this change. Change subject: Adding remove/disable verbs to vdsm-tool for admin usages .. Patch Set 7: Code-Review+1 -- To view, visit http://gerrit.ovirt.org/21772 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7f2c031436a6d202f856c24d9c9420c8bfdf6df Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim Gerrit-Reviewer: Alon Bar-Lev Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer 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]: Adding remove/disable verbs to vdsm-tool for admin usages
mooli tayer has posted comments on this change. Change subject: Adding remove/disable verbs to vdsm-tool for admin usages .. Patch Set 7: This patch now works while tests fail as they are not yet ready. -- To view, visit http://gerrit.ovirt.org/21772 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7f2c031436a6d202f856c24d9c9420c8bfdf6df Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim Gerrit-Reviewer: Alon Bar-Lev Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer 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]: Adding remove/disable verbs to vdsm-tool for admin usages
oVirt Jenkins CI Server has posted comments on this change. Change subject: Adding remove/disable verbs to vdsm-tool for admin usages .. Patch Set 7: Code-Review-1 Verified-1 Build Failed http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/7425/ : UNSTABLE http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/6632/ : FAILURE http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/7534/ : FAILURE -- To view, visit http://gerrit.ovirt.org/21772 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7f2c031436a6d202f856c24d9c9420c8bfdf6df Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim Gerrit-Reviewer: Alon Bar-Lev Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer 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]: Adding remove/disable verbs to vdsm-tool for admin usages
mooli tayer has posted comments on this change. Change subject: Adding remove/disable verbs to vdsm-tool for admin usages .. Patch Set 7: Verified+1 -- To view, visit http://gerrit.ovirt.org/21772 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7f2c031436a6d202f856c24d9c9420c8bfdf6df Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim Gerrit-Reviewer: Alon Bar-Lev Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer 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]: Adding remove/disable verbs to vdsm-tool for admin usages
mooli tayer has posted comments on this change. Change subject: Adding remove/disable verbs to vdsm-tool for admin usages .. Patch Set 6: Sending another one... -- To view, visit http://gerrit.ovirt.org/21772 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7f2c031436a6d202f856c24d9c9420c8bfdf6df Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim Gerrit-Reviewer: Alon Bar-Lev Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer 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]: Adding remove/disable verbs to vdsm-tool for admin usages
oVirt Jenkins CI Server has posted comments on this change. Change subject: Adding remove/disable verbs to vdsm-tool for admin usages .. Patch Set 6: Code-Review-1 Verified-1 Build Failed http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/7423/ : UNSTABLE http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/6630/ : FAILURE http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/7532/ : FAILURE -- To view, visit http://gerrit.ovirt.org/21772 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7f2c031436a6d202f856c24d9c9420c8bfdf6df Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim Gerrit-Reviewer: Alon Bar-Lev Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer 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]: Adding remove/disable verbs to vdsm-tool for admin usages
mooli tayer has posted comments on this change.
Change subject: Adding remove/disable verbs to vdsm-tool for admin usages
..
Patch Set 6:
(2 comments)
http://gerrit.ovirt.org/#/c/21772/6/lib/vdsm/tool/passwd.py
File lib/vdsm/tool/passwd.py:
Line 43: script, stdin=f, stdout=subprocess.PIPE,
Line 44: stderr=subprocess.PIPE, close_fds=True)
Line 45: output, err = p.communicate()
Line 46: if p.returncode != 0:
Line 47: raise Exception("Set password failed: %s" % err )
Whoops replaced these two. removing
Line 48:
Line 49:
Line 50: @expose("remove-saslpasswd")
Line 51: def remove_saslpasswd():
Line 55: rc, out, err = utils.execCmd(
Line 56: (constants.EXT_SASLPASSWD2, '-p', '-a', 'libvirt', '-d',
constants.SASL_USERNAME,),
Line 57: )
Line 58: if rc != 0:
Line 59: raise RuntimeError("Remove password failed: %s" % (err, ))
Whoops replaced these two. removing
--
To view, visit http://gerrit.ovirt.org/21772
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie7f2c031436a6d202f856c24d9c9420c8bfdf6df
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim
Gerrit-Reviewer: Alon Bar-Lev
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Yaniv Bronhaim
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: mooli tayer
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]: Adding remove/disable verbs to vdsm-tool for admin usages
Yaniv Bronhaim has posted comments on this change. Change subject: Adding remove/disable verbs to vdsm-tool for admin usages .. Patch Set 5: (1 comment) http://gerrit.ovirt.org/#/c/21772/5/lib/vdsm/tool/configurator.py File lib/vdsm/tool/configurator.py: Line 140: except RuntimeError: Line 141: return False Line 142: Line 143: def removeConf(self): Line 144: vdsm_ver = '-4.9.10' > Well after we talked, I removed the vdsm_ver completely. we don't. send your new version Line 145: conf_prefix = '## beginning of configuration section by vdsm' + \ Line 146: vdsm_ver Line 147: conf_suffix = '## end of configuration section by vdsm' + vdsm_ver Line 148: -- To view, visit http://gerrit.ovirt.org/21772 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7f2c031436a6d202f856c24d9c9420c8bfdf6df Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim Gerrit-Reviewer: Alon Bar-Lev Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer 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]: Adding remove/disable verbs to vdsm-tool for admin usages
mooli tayer has posted comments on this change.
Change subject: Adding remove/disable verbs to vdsm-tool for admin usages
..
Patch Set 5:
(5 comments)
http://gerrit.ovirt.org/#/c/21772/5/lib/vdsm/tool/configurator.py
File lib/vdsm/tool/configurator.py:
Line 27: from .. import utils
Line 28: from . import service, expose
Line 29: from ..constants import P_VDSM_EXEC, DISKIMAGE_GROUP,
QEMU_PROCESS_GROUP, \
Line 30: VDSM_GROUP, P_VDSM_LCONF, P_VDSM_QCONF, P_VDSM_LDCONF,
P_SYSTEMCTL_CONF
Line 31:
> odd the pep8 wasn't see it ... but also, not related in this scope. post sm
It was originally added in this change in a previews patch set
Line 32:
Line 33: def removeSectionFromFile(filename, beginField=None, endField=None):
Line 34: (
Line 35: BEFORE,
Line 140: except RuntimeError:
Line 141: return False
Line 142:
Line 143: def removeConf(self):
Line 144: vdsm_ver = '-4.9.10'
> so this version is stated twice now. once in libvirt_configure and one in l
Well after we talked, I removed the vdsm_ver completely.
As I understood from you there should always be only one section with whatever
number and that is what we want to remove so why do we need to distinguish
between versions(in removeConf)?
Line 145: conf_prefix = '## beginning of configuration section by vdsm'
+ \
Line 146: vdsm_ver
Line 147: conf_suffix = '## end of configuration section by vdsm' +
vdsm_ver
Line 148:
Line 152: P_VDSM_LDCONF,
Line 153: ):
Line 154: if os.path.exists(path):
Line 155: removeSectionFromFile(path, conf_prefix, conf_suffix)
Line 156: utils.rmFile(P_SYSTEMCTL_CONF)
> also systemctl conf part will be separated after arranging the libvirt_conf
Done
Line 157:
Line 158:
Line 159: class SanlockModuleConfigure(_ModuleConfigure):
Line 160:
http://gerrit.ovirt.org/#/c/21772/5/lib/vdsm/tool/passwd.py
File lib/vdsm/tool/passwd.py:
Line 43: script, stdin=f, stdout=subprocess.PIPE,
Line 44: stderr=subprocess.PIPE, close_fds=True)
Line 45: output, err = p.communicate()
Line 46: if p.returncode != 0:
Line 47: raise RuntimeError("Set password failed: %s" % (err, ))
> if you can do it patch later that also change the popen usage to execCmd it
Ok so two patches more.
Line 48:
Line 49:
Line 50: @expose("remove-saslpasswd")
Line 51: def remove_saslpasswd():
Line 55: rc, out, err = utils.execCmd(
Line 56: (constants.EXT_SASLPASSWD2, '-p', '-a', 'libvirt', '-d',
constants.SASL_USERNAME,),
Line 57: )
Line 58: if rc != 0:
Line 59: raise RuntimeError("Remove password failed: %s" % (err, ))
> just thought I revisit this... why do you need tuple for err?
Removing
--
To view, visit http://gerrit.ovirt.org/21772
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie7f2c031436a6d202f856c24d9c9420c8bfdf6df
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim
Gerrit-Reviewer: Alon Bar-Lev
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Yaniv Bronhaim
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: mooli tayer
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]: Adding remove/disable verbs to vdsm-tool for admin usages
Yaniv Bronhaim has posted comments on this change.
Change subject: Adding remove/disable verbs to vdsm-tool for admin usages
..
Patch Set 5:
(4 comments)
http://gerrit.ovirt.org/#/c/21772/5/lib/vdsm/tool/configurator.py
File lib/vdsm/tool/configurator.py:
Line 27: from .. import utils
Line 28: from . import service, expose
Line 29: from ..constants import P_VDSM_EXEC, DISKIMAGE_GROUP,
QEMU_PROCESS_GROUP, \
Line 30: VDSM_GROUP, P_VDSM_LCONF, P_VDSM_QCONF, P_VDSM_LDCONF,
P_SYSTEMCTL_CONF
Line 31:
> removing unused DISKIMAGE_GROUP
odd the pep8 wasn't see it ... but also, not related in this scope. post small
patches for such issues, it'll be accepted much faster upstream
Line 32:
Line 33: def removeSectionFromFile(filename, beginField=None, endField=None):
Line 34: (
Line 35: BEFORE,
Line 140: except RuntimeError:
Line 141: return False
Line 142:
Line 143: def removeConf(self):
Line 144: vdsm_ver = '-4.9.10'
so this version is stated twice now. once in libvirt_configure and one in
libvirt_configure script. that's an open to mistakes. I suggest that removeConf
won't count on version number, it'll search for vdsm stamp and remove all
vdsm's scope of the conf file. later on, when libvirt_configure will be part of
this code we'll be able to distinguish between the versions
Line 145: conf_prefix = '## beginning of configuration section by vdsm'
+ \
Line 146: vdsm_ver
Line 147: conf_suffix = '## end of configuration section by vdsm' +
vdsm_ver
Line 148:
Line 152: P_VDSM_LDCONF,
Line 153: ):
Line 154: if os.path.exists(path):
Line 155: removeSectionFromFile(path, conf_prefix, conf_suffix)
Line 156: utils.rmFile(P_SYSTEMCTL_CONF)
also systemctl conf part will be separated after arranging the
libvirt_configure script. so for now, lets leave it that way without the
version (anyway you check only if the line "startwith" the vdsm string)
Line 157:
Line 158:
Line 159: class SanlockModuleConfigure(_ModuleConfigure):
Line 160:
http://gerrit.ovirt.org/#/c/21772/5/lib/vdsm/tool/passwd.py
File lib/vdsm/tool/passwd.py:
Line 43: script, stdin=f, stdout=subprocess.PIPE,
Line 44: stderr=subprocess.PIPE, close_fds=True)
Line 45: output, err = p.communicate()
Line 46: if p.returncode != 0:
Line 47: raise RuntimeError("Set password failed: %s" % (err, ))
> why is this related to this patch?
if you can do it patch later that also change the popen usage to execCmd it'd
be great...
Line 48:
Line 49:
Line 50: @expose("remove-saslpasswd")
Line 51: def remove_saslpasswd():
--
To view, visit http://gerrit.ovirt.org/21772
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie7f2c031436a6d202f856c24d9c9420c8bfdf6df
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim
Gerrit-Reviewer: Alon Bar-Lev
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Yaniv Bronhaim
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: mooli tayer
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]: Adding remove/disable verbs to vdsm-tool for admin usages
Alon Bar-Lev has posted comments on this change.
Change subject: Adding remove/disable verbs to vdsm-tool for admin usages
..
Patch Set 5:
(2 comments)
http://gerrit.ovirt.org/#/c/21772/5/lib/vdsm/tool/passwd.py
File lib/vdsm/tool/passwd.py:
Line 43: script, stdin=f, stdout=subprocess.PIPE,
Line 44: stderr=subprocess.PIPE, close_fds=True)
Line 45: output, err = p.communicate()
Line 46: if p.returncode != 0:
Line 47: raise RuntimeError("Set password failed: %s" % (err, ))
why is this related to this patch?
Line 48:
Line 49:
Line 50: @expose("remove-saslpasswd")
Line 51: def remove_saslpasswd():
Line 55: rc, out, err = utils.execCmd(
Line 56: (constants.EXT_SASLPASSWD2, '-p', '-a', 'libvirt', '-d',
constants.SASL_USERNAME,),
Line 57: )
Line 58: if rc != 0:
Line 59: raise RuntimeError("Remove password failed: %s" % (err, ))
just thought I revisit this... why do you need tuple for err?
--
To view, visit http://gerrit.ovirt.org/21772
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie7f2c031436a6d202f856c24d9c9420c8bfdf6df
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim
Gerrit-Reviewer: Alon Bar-Lev
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Yaniv Bronhaim
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: mooli tayer
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]: Adding remove/disable verbs to vdsm-tool for admin usages
mooli tayer has posted comments on this change. Change subject: Adding remove/disable verbs to vdsm-tool for admin usages .. Patch Set 5: (1 comment) http://gerrit.ovirt.org/#/c/21772/5/lib/vdsm/tool/configurator.py File lib/vdsm/tool/configurator.py: Line 27: from .. import utils Line 28: from . import service, expose Line 29: from ..constants import P_VDSM_EXEC, DISKIMAGE_GROUP, QEMU_PROCESS_GROUP, \ Line 30: VDSM_GROUP, P_VDSM_LCONF, P_VDSM_QCONF, P_VDSM_LDCONF, P_SYSTEMCTL_CONF Line 31: removing unused DISKIMAGE_GROUP Line 32: Line 33: def removeSectionFromFile(filename, beginField=None, endField=None): Line 34: ( Line 35: BEFORE, -- To view, visit http://gerrit.ovirt.org/21772 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7f2c031436a6d202f856c24d9c9420c8bfdf6df Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim Gerrit-Reviewer: Alon Bar-Lev Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer 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]: Adding remove/disable verbs to vdsm-tool for admin usages
Alon Bar-Lev has posted comments on this change. Change subject: Adding remove/disable verbs to vdsm-tool for admin usages .. Patch Set 5: Code-Review+1 ok, I am good, now vdsm people will probably wake up :) -- To view, visit http://gerrit.ovirt.org/21772 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7f2c031436a6d202f856c24d9c9420c8bfdf6df Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim Gerrit-Reviewer: Alon Bar-Lev Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer 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]: Adding remove/disable verbs to vdsm-tool for admin usages
oVirt Jenkins CI Server has posted comments on this change. Change subject: Adding remove/disable verbs to vdsm-tool for admin usages .. Patch Set 5: Code-Review-1 Verified-1 Build Failed http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/7407/ : UNSTABLE http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/6614/ : FAILURE http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/7516/ : FAILURE -- To view, visit http://gerrit.ovirt.org/21772 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7f2c031436a6d202f856c24d9c9420c8bfdf6df Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim Gerrit-Reviewer: Alon Bar-Lev Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer 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]: Adding remove/disable verbs to vdsm-tool for admin usages
Alon Bar-Lev has posted comments on this change. Change subject: Adding remove/disable verbs to vdsm-tool for admin usages .. Patch Set 4: (1 comment) http://gerrit.ovirt.org/#/c/21772/4/lib/vdsm/tool/passwd.py File lib/vdsm/tool/passwd.py: Line 53: Remove vdsm password for libvirt connection Line 54: """ Line 55: rc, out, err = utils.execCmd( Line 56: (constants.P_EXEC_SASL, '-p', '-a', 'libvirt', '-d', constants.SASL_USERNAME,), Line 57: raw=True, > I saw that raw=False (default) splits output lines, and we don't need that. well, I tend to use default unless there is a good reason, not the other way around :) Line 58: ) Line 59: if rc != 0: -- To view, visit http://gerrit.ovirt.org/21772 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7f2c031436a6d202f856c24d9c9420c8bfdf6df Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim Gerrit-Reviewer: Alon Bar-Lev Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer 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]: Adding remove/disable verbs to vdsm-tool for admin usages
mooli tayer has posted comments on this change.
Change subject: Adding remove/disable verbs to vdsm-tool for admin usages
..
Patch Set 4:
(4 comments)
http://gerrit.ovirt.org/#/c/21772/4/lib/vdsm/constants.py.in
File lib/vdsm/constants.py.in:
Line 43:
Line 44: #
Line 45: # SASL definitions
Line 46: #
Line 47: P_EXEC_SASL = '/usr/sbin/saslpasswd2'
> I think it should be similar to other EXT_GREP = '@GREP_PATH@', and determi
Done
Line 48: SASL_USERNAME = "vdsm@ovirt"
Line 49:
Line 50: # This is the domain version translation list
Line 51: # DO NOT CHANGE OLD VALUES ONLY APPEND
http://gerrit.ovirt.org/#/c/21772/4/lib/vdsm/tool/configurator.py
File lib/vdsm/tool/configurator.py:
Line 354: for c in __configurers:
Line 355: if c.getName() in args.modules:
Line 356: try:
Line 357: c.removeConf()
Line 358: except:
> please do not swallow exceptions.
Done
Line 359: failed.append(c.getName)
Line 360: if failed:
Line 361: sys.stderr.write(
Line 362: "Could not remove configuration for modules %s\n" %
','.join(failed),
Line 359: failed.append(c.getName)
Line 360: if failed:
Line 361: sys.stderr.write(
Line 362: "Could not remove configuration for modules %s\n" %
','.join(failed),
Line 363: )
> I think this can be dropped in favour of boolean and print above.
Done
Line 364: raise RuntimeError("Remove configuration failed")
Line 365:
Line 366:
Line 367: def _parse_args(action):
http://gerrit.ovirt.org/#/c/21772/4/lib/vdsm/tool/passwd.py
File lib/vdsm/tool/passwd.py:
Line 53: Remove vdsm password for libvirt connection
Line 54: """
Line 55: rc, out, err = utils.execCmd(
Line 56: (constants.P_EXEC_SASL, '-p', '-a', 'libvirt', '-d',
constants.SASL_USERNAME,),
Line 57: raw=True,
> why raw?
I saw that raw=False (default) splits output lines, and we don't need that. I
not sure why its a part of the function?
Remove the raw?
Line 58: )
Line 59: if rc != 0:
--
To view, visit http://gerrit.ovirt.org/21772
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie7f2c031436a6d202f856c24d9c9420c8bfdf6df
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim
Gerrit-Reviewer: Alon Bar-Lev
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Yaniv Bronhaim
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: mooli tayer
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]: Adding remove/disable verbs to vdsm-tool for admin usages
Alon Bar-Lev has posted comments on this change.
Change subject: Adding remove/disable verbs to vdsm-tool for admin usages
..
Patch Set 4:
(5 comments)
http://gerrit.ovirt.org/#/c/21772/4/lib/vdsm/constants.py.in
File lib/vdsm/constants.py.in:
Line 43:
Line 44: #
Line 45: # SASL definitions
Line 46: #
Line 47: P_EXEC_SASL = '/usr/sbin/saslpasswd2'
I think it should be similar to other EXT_GREP = '@GREP_PATH@', and determined
by configure.ac
Line 48: SASL_USERNAME = "vdsm@ovirt"
Line 49:
Line 50: # This is the domain version translation list
Line 51: # DO NOT CHANGE OLD VALUES ONLY APPEND
http://gerrit.ovirt.org/#/c/21772/4/lib/vdsm/tool/configurator.py
File lib/vdsm/tool/configurator.py:
Line 354: for c in __configurers:
Line 355: if c.getName() in args.modules:
Line 356: try:
Line 357: c.removeConf()
Line 358: except:
please do not swallow exceptions.
worse case it should be except Exception to only catch these who are valid, but
you should send info to logger.
any reason why not:
except Exception:
sys.stderr.write('Cannot remove %s\n')
traceback.print_exc(file=sys.stderr)
Line 359: failed.append(c.getName)
Line 360: if failed:
Line 361: sys.stderr.write(
Line 362: "Could not remove configuration for modules %s\n" %
','.join(failed),
Line 355: if c.getName() in args.modules:
Line 356: try:
Line 357: c.removeConf()
Line 358: except:
Line 359: failed.append(c.getName)
getName->getName()
Line 360: if failed:
Line 361: sys.stderr.write(
Line 362: "Could not remove configuration for modules %s\n" %
','.join(failed),
Line 363: )
Line 359: failed.append(c.getName)
Line 360: if failed:
Line 361: sys.stderr.write(
Line 362: "Could not remove configuration for modules %s\n" %
','.join(failed),
Line 363: )
I think this can be dropped in favour of boolean and print above.
Line 364: raise RuntimeError("Remove configuration failed")
Line 365:
Line 366:
Line 367: def _parse_args(action):
http://gerrit.ovirt.org/#/c/21772/4/lib/vdsm/tool/passwd.py
File lib/vdsm/tool/passwd.py:
Line 53: Remove vdsm password for libvirt connection
Line 54: """
Line 55: rc, out, err = utils.execCmd(
Line 56: (constants.P_EXEC_SASL, '-p', '-a', 'libvirt', '-d',
constants.SASL_USERNAME,),
Line 57: raw=True,
why raw?
Line 58: )
Line 59: if rc != 0:
--
To view, visit http://gerrit.ovirt.org/21772
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie7f2c031436a6d202f856c24d9c9420c8bfdf6df
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim
Gerrit-Reviewer: Alon Bar-Lev
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Yaniv Bronhaim
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: mooli tayer
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]: Adding remove/disable verbs to vdsm-tool for admin usages
oVirt Jenkins CI Server has posted comments on this change. Change subject: Adding remove/disable verbs to vdsm-tool for admin usages .. Patch Set 4: Code-Review-1 Verified-1 Build Failed http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/7396/ : UNSTABLE http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/6603/ : FAILURE http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/7505/ : FAILURE -- To view, visit http://gerrit.ovirt.org/21772 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7f2c031436a6d202f856c24d9c9420c8bfdf6df Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim Gerrit-Reviewer: Alon Bar-Lev Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer 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]: Adding remove\disable verbs to vdsm-tool for admin usages
mooli tayer has posted comments on this change.
Change subject: Adding remove\disable verbs to vdsm-tool for admin usages
..
Patch Set 3:
(4 comments)
http://gerrit.ovirt.org/#/c/21772/3//COMMIT_MSG
Commit Message:
Line 3: AuthorDate: 2013-11-27 13:23:56 +0200
Line 4: Commit: Mooli Tayer
Line 5: CommitDate: 2014-03-02 20:10:47 +0200
Line 6:
Line 7: Adding remove\disable verbs to vdsm-tool for admin usages
> interesting ... what i wrote is what my english teacher thought me in highs
Done
Line 8:
Line 9: The spec will be modified separately to use vdsm-tool instead of
hard-coded
Line 10: operations
Line 11:
http://gerrit.ovirt.org/#/c/21772/3/lib/vdsm/tool/configurator.py
File lib/vdsm/tool/configurator.py:
Line 360: c.removeConf()
Line 361: except:
Line 362: failed.append(c.getName)
Line 363: if failed:
Line 364: sys.stdout.write(
> ah? no!
I changed this line as part of the next commit. I was talking about other lines
like 342 and others which are not related to this patch...
Line 365: "Could not remove configuration for modules %s\n" %
','.join(failed),
Line 366: )
Line 367: raise RuntimeError("Remove configuration failed")
Line 368:
http://gerrit.ovirt.org/#/c/21772/3/lib/vdsm/tool/passwd.py
File lib/vdsm/tool/passwd.py:
Line 50: def remove_saslpasswd():
Line 51: """
Line 52: Remove vdsm password for libvirt connection
Line 53: """
Line 54: p = subprocess.Popen(('/usr/sbin/saslpasswd2', '-p', '-a',
'libvirt', '-d', constants.SASL_USERNAME))
> yes
Done
Line 55: output, err = p.communicate()
Line 56: if p.returncode != 0:
Line 52: Remove vdsm password for libvirt connection
Line 53: """
Line 54: p = subprocess.Popen(('/usr/sbin/saslpasswd2', '-p', '-a',
'libvirt', '-d', constants.SASL_USERNAME))
Line 55: output, err = p.communicate()
Line 56: if p.returncode != 0:
> utils.execCmd instead of Popen
Done
--
To view, visit http://gerrit.ovirt.org/21772
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie7f2c031436a6d202f856c24d9c9420c8bfdf6df
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim
Gerrit-Reviewer: Alon Bar-Lev
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Yaniv Bronhaim
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: mooli tayer
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]: Adding remove\disable verbs to vdsm-tool for admin usages
Yaniv Bronhaim has posted comments on this change.
Change subject: Adding remove\disable verbs to vdsm-tool for admin usages
..
Patch Set 3:
(5 comments)
http://gerrit.ovirt.org/#/c/21772/3//COMMIT_MSG
Commit Message:
Line 3: AuthorDate: 2013-11-27 13:23:56 +0200
Line 4: Commit: Mooli Tayer
Line 5: CommitDate: 2014-03-02 20:10:47 +0200
Line 6:
Line 7: Adding remove\disable verbs to vdsm-tool for admin usages
> \ is called 'back slash' and should not be used as far as I know.
interesting ... what i wrote is what my english teacher thought me in
highschool. i never doubted about it :/
Line 8:
Line 9: The spec will be modified separately to use vdsm-tool instead of
hard-coded
Line 10: operations
Line 11:
http://gerrit.ovirt.org/#/c/21772/3/lib/vdsm/tool/configurator.py
File lib/vdsm/tool/configurator.py:
Line 139: except RuntimeError:
Line 140: return False
Line 141:
Line 142: def removeConf(self):
Line 143: vdsm_ver = '-4.9.10'
> this is the version of the change not the version of vdsm... like signature
this is how we know in which version of vdsm we introduce new configuration
change. only if this number is different we know if to override the
configuration with the old or new values. I don't understand why you need it
when removing the config.. it does not matter at all. please check first the
libvirt_configure.sh, read the code and see what it does. you just need to do
the opposite of that..
Line 144: conf_prefix = '## beginning of configuration section by vdsm'
+ \
Line 145: vdsm_ver
Line 146: conf_suffix = '## end of configuration section by vdsm' +
vdsm_ver
Line 147:
Line 360: c.removeConf()
Line 361: except:
Line 362: failed.append(c.getName)
Line 363: if failed:
Line 364: sys.stdout.write(
> yes, other patch if required fixing.
ah? no!
for errors use stderr, for regular output to user such as - start removing...
or removed successfully prints use stdout. do it rightly, we don't need to have
set of patches here with no reason.
Line 365: "Could not remove configuration for modules %s\n" %
','.join(failed),
Line 366: )
Line 367: raise RuntimeError("Remove configuration failed")
Line 368:
http://gerrit.ovirt.org/#/c/21772/3/lib/vdsm/tool/passwd.py
File lib/vdsm/tool/passwd.py:
Line 50: def remove_saslpasswd():
Line 51: """
Line 52: Remove vdsm password for libvirt connection
Line 53: """
Line 54: p = subprocess.Popen(('/usr/sbin/saslpasswd2', '-p', '-a',
'libvirt', '-d', constants.SASL_USERNAME))
> Are you saying /usr/sbin/saslpasswd2 should be in constants.py?
yes
Line 55: output, err = p.communicate()
Line 56: if p.returncode != 0:
Line 52: Remove vdsm password for libvirt connection
Line 53: """
Line 54: p = subprocess.Popen(('/usr/sbin/saslpasswd2', '-p', '-a',
'libvirt', '-d', constants.SASL_USERNAME))
Line 55: output, err = p.communicate()
Line 56: if p.returncode != 0:
> Yaniv?
utils.execCmd instead of Popen
--
To view, visit http://gerrit.ovirt.org/21772
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie7f2c031436a6d202f856c24d9c9420c8bfdf6df
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim
Gerrit-Reviewer: Alon Bar-Lev
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Yaniv Bronhaim
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: mooli tayer
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]: Adding remove\disable verbs to vdsm-tool for admin usages
Alon Bar-Lev has posted comments on this change.
Change subject: Adding remove\disable verbs to vdsm-tool for admin usages
..
Patch Set 3:
(2 comments)
http://gerrit.ovirt.org/#/c/21772/3/lib/vdsm/tool/configurator.py
File lib/vdsm/tool/configurator.py:
Line 139: except RuntimeError:
Line 140: return False
Line 141:
Line 142: def removeConf(self):
Line 143: vdsm_ver = '-4.9.10'
> This patch fails due to this line. Where from do I get the current version?
this is the version of the change not the version of vdsm... like signature...
the fact that the version of vdsm was used once and became the signature is
strange but this what we have.
Line 144: conf_prefix = '## beginning of configuration section by vdsm'
+ \
Line 145: vdsm_ver
Line 146: conf_suffix = '## end of configuration section by vdsm' +
vdsm_ver
Line 147:
Line 360: c.removeConf()
Line 361: except:
Line 362: failed.append(c.getName)
Line 363: if failed:
Line 364: sys.stdout.write(
> Done.
yes, other patch if required fixing.
Line 365: "Could not remove configuration for modules %s\n" %
','.join(failed),
Line 366: )
Line 367: raise RuntimeError("Remove configuration failed")
Line 368:
--
To view, visit http://gerrit.ovirt.org/21772
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie7f2c031436a6d202f856c24d9c9420c8bfdf6df
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim
Gerrit-Reviewer: Alon Bar-Lev
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Yaniv Bronhaim
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: mooli tayer
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]: Adding remove\disable verbs to vdsm-tool for admin usages
mooli tayer has posted comments on this change.
Change subject: Adding remove\disable verbs to vdsm-tool for admin usages
..
Patch Set 3:
(7 comments)
Also rebasing on top of new tests:
http://gerrit.ovirt.org/#/c/25263/
http://gerrit.ovirt.org/#/c/21772/3/lib/vdsm/tool/configurator.py
File lib/vdsm/tool/configurator.py:
Line 139: except RuntimeError:
Line 140: return False
Line 141:
Line 142: def removeConf(self):
Line 143: vdsm_ver = '-4.9.10'
This patch fails due to this line. Where from do I get the current version?
Yaniv?
Line 144: conf_prefix = '## beginning of configuration section by vdsm'
+ \
Line 145: vdsm_ver
Line 146: conf_suffix = '## end of configuration section by vdsm' +
vdsm_ver
Line 147:
Line 148: for path in [
Line 149: P_VDSM_LCONF,
Line 150: P_VDSM_QCONF,
Line 151: P_VDSM_LDCONF,
Line 152: ]:
> tuple and not list will be better.
Done
Line 153: if os.path.exists(path):
Line 154: removeSectionFromFile(path, conf_prefix, conf_suffix)
Line 155: utils.rmFile(P_SYSTEMCTL_CONF)
Line 156:
Line 151: P_VDSM_LDCONF,
Line 152: ]:
Line 153: if os.path.exists(path):
Line 154: removeSectionFromFile(path, conf_prefix, conf_suffix)
Line 155: utils.rmFile(P_SYSTEMCTL_CONF)
> yes, we need to split and arrange - sysctl conf to SystemctlModuleConfigure
So for now I leave this as is
Line 156:
Line 157:
Line 158: class SanlockModuleConfigure(_ModuleConfigure):
Line 159:
Line 168: def getServices(self):
Line 169: return ['sanlock']
Line 170:
Line 171: def removeConf(self):
Line 172: pass
> no need
Done
Line 173:
Line 174: def configure(self):
Line 175: """
Line 176: Configure sanlock process groups
Line 360: c.removeConf()
Line 361: except:
Line 362: failed.append(c.getName)
Line 363: if failed:
Line 364: sys.stdout.write(
> stderr?
Done.
We have the same problem in other modules.
Fix in a different patch set?
Line 365: "Could not remove configuration for modules %s\n" %
','.join(failed),
Line 366: )
Line 367: raise RuntimeError("Remove configuration failed")
Line 368:
http://gerrit.ovirt.org/#/c/21772/3/lib/vdsm/tool/passwd.py
File lib/vdsm/tool/passwd.py:
Line 50: def remove_saslpasswd():
Line 51: """
Line 52: Remove vdsm password for libvirt connection
Line 53: """
Line 54: p = subprocess.Popen(('/usr/sbin/saslpasswd2', '-p', '-a',
'libvirt', '-d', constants.SASL_USERNAME))
> should be in P_EXEC_
Are you saying /usr/sbin/saslpasswd2 should be in constants.py?
Line 55: output, err = p.communicate()
Line 56: if p.returncode != 0:
Line 52: Remove vdsm password for libvirt connection
Line 53: """
Line 54: p = subprocess.Popen(('/usr/sbin/saslpasswd2', '-p', '-a',
'libvirt', '-d', constants.SASL_USERNAME))
Line 55: output, err = p.communicate()
Line 56: if p.returncode != 0:
> please use vdsm utilities for process execution, I guess there are.
Yaniv?
--
To view, visit http://gerrit.ovirt.org/21772
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie7f2c031436a6d202f856c24d9c9420c8bfdf6df
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim
Gerrit-Reviewer: Alon Bar-Lev
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Yaniv Bronhaim
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: mooli tayer
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]: Adding remove\disable verbs to vdsm-tool for admin usages
Alon Bar-Lev has posted comments on this change. Change subject: Adding remove\disable verbs to vdsm-tool for admin usages .. Patch Set 3: (1 comment) http://gerrit.ovirt.org/#/c/21772/3//COMMIT_MSG Commit Message: Line 3: AuthorDate: 2013-11-27 13:23:56 +0200 Line 4: Commit: Mooli Tayer Line 5: CommitDate: 2014-03-02 20:10:47 +0200 Line 6: Line 7: Adding remove\disable verbs to vdsm-tool for admin usages > the slash for "or" (afaik) should lean to the opposite side of the language \ is called 'back slash' and should not be used as far as I know. http://www.englishclub.com/writing/punctuation-slash.htm Line 8: Line 9: The spec will be modified separately to use vdsm-tool instead of hard-coded Line 10: operations Line 11: -- To view, visit http://gerrit.ovirt.org/21772 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7f2c031436a6d202f856c24d9c9420c8bfdf6df Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim Gerrit-Reviewer: Alon Bar-Lev Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer 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]: Adding remove\disable verbs to vdsm-tool for admin usages
Yaniv Bronhaim has posted comments on this change. Change subject: Adding remove\disable verbs to vdsm-tool for admin usages .. Patch Set 3: (2 comments) http://gerrit.ovirt.org/#/c/21772/3//COMMIT_MSG Commit Message: Line 3: AuthorDate: 2013-11-27 13:23:56 +0200 Line 4: Commit: Mooli Tayer Line 5: CommitDate: 2014-03-02 20:10:47 +0200 Line 6: Line 7: Adding remove\disable verbs to vdsm-tool for admin usages > I must comment \ -> / :))) the slash for "or" (afaik) should lean to the opposite side of the language writing direction. in hebrew its /, english \ (from left to right) Line 8: Line 9: The spec will be modified separately to use vdsm-tool instead of hard-coded Line 10: operations Line 11: http://gerrit.ovirt.org/#/c/21772/3/lib/vdsm/tool/configurator.py File lib/vdsm/tool/configurator.py: Line 151: P_VDSM_LDCONF, Line 152: ]: Line 153: if os.path.exists(path): Line 154: removeSectionFromFile(path, conf_prefix, conf_suffix) Line 155: utils.rmFile(P_SYSTEMCTL_CONF) > is this one^ related to libvirt? yes, we need to split and arrange - sysctl conf to SystemctlModuleConfigure, move qemu part to QemuMeduleConfigure. but this later. in this phase lets just create the removal part, afterwards we'll arrange to modules (i plan also SelinuxModuleConfigure, VdsmModuleConfigure with flags (like having --ssl option for configure and inc) and more improvement for this tool to avoid such scriptish spec file) Line 156: Line 157: Line 158: class SanlockModuleConfigure(_ModuleConfigure): Line 159: -- To view, visit http://gerrit.ovirt.org/21772 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7f2c031436a6d202f856c24d9c9420c8bfdf6df Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim Gerrit-Reviewer: Alon Bar-Lev Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer 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]: Adding remove\disable verbs to vdsm-tool for admin usages
Alon Bar-Lev has posted comments on this change.
Change subject: Adding remove\disable verbs to vdsm-tool for admin usages
..
Patch Set 3:
(7 comments)
http://gerrit.ovirt.org/#/c/21772/3//COMMIT_MSG
Commit Message:
Line 3: AuthorDate: 2013-11-27 13:23:56 +0200
Line 4: Commit: Mooli Tayer
Line 5: CommitDate: 2014-03-02 20:10:47 +0200
Line 6:
Line 7: Adding remove\disable verbs to vdsm-tool for admin usages
I must comment \ -> / :)))
Line 8:
Line 9: The spec will be modified separately to use vdsm-tool instead of
hard-coded
Line 10: operations
Line 11:
http://gerrit.ovirt.org/#/c/21772/3/lib/vdsm/tool/configurator.py
File lib/vdsm/tool/configurator.py:
Line 148: for path in [
Line 149: P_VDSM_LCONF,
Line 150: P_VDSM_QCONF,
Line 151: P_VDSM_LDCONF,
Line 152: ]:
tuple and not list will be better.
Line 153: if os.path.exists(path):
Line 154: removeSectionFromFile(path, conf_prefix, conf_suffix)
Line 155: utils.rmFile(P_SYSTEMCTL_CONF)
Line 156:
Line 151: P_VDSM_LDCONF,
Line 152: ]:
Line 153: if os.path.exists(path):
Line 154: removeSectionFromFile(path, conf_prefix, conf_suffix)
Line 155: utils.rmFile(P_SYSTEMCTL_CONF)
is this one^ related to libvirt?
Line 156:
Line 157:
Line 158: class SanlockModuleConfigure(_ModuleConfigure):
Line 159:
Line 168: def getServices(self):
Line 169: return ['sanlock']
Line 170:
Line 171: def removeConf(self):
Line 172: pass
no need
Line 173:
Line 174: def configure(self):
Line 175: """
Line 176: Configure sanlock process groups
Line 360: c.removeConf()
Line 361: except:
Line 362: failed.append(c.getName)
Line 363: if failed:
Line 364: sys.stdout.write(
stderr?
Line 365: "Could not remove configuration for modules %s\n" %
','.join(failed),
Line 366: )
Line 367: raise RuntimeError("Remove configuration failed")
Line 368:
http://gerrit.ovirt.org/#/c/21772/3/lib/vdsm/tool/passwd.py
File lib/vdsm/tool/passwd.py:
Line 50: def remove_saslpasswd():
Line 51: """
Line 52: Remove vdsm password for libvirt connection
Line 53: """
Line 54: p = subprocess.Popen(('/usr/sbin/saslpasswd2', '-p', '-a',
'libvirt', '-d', constants.SASL_USERNAME))
should be in P_EXEC_
Line 55: output, err = p.communicate()
Line 56: if p.returncode != 0:
Line 52: Remove vdsm password for libvirt connection
Line 53: """
Line 54: p = subprocess.Popen(('/usr/sbin/saslpasswd2', '-p', '-a',
'libvirt', '-d', constants.SASL_USERNAME))
Line 55: output, err = p.communicate()
Line 56: if p.returncode != 0:
please use vdsm utilities for process execution, I guess there are.
--
To view, visit http://gerrit.ovirt.org/21772
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie7f2c031436a6d202f856c24d9c9420c8bfdf6df
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim
Gerrit-Reviewer: Alon Bar-Lev
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Yaniv Bronhaim
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: mooli tayer
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]: Adding remove\disable verbs to vdsm-tool for admin usages
oVirt Jenkins CI Server has posted comments on this change. Change subject: Adding remove\disable verbs to vdsm-tool for admin usages .. Patch Set 3: Code-Review-1 Verified-1 Build Failed http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/7383/ : UNSTABLE http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/6592/ : FAILURE http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/7494/ : FAILURE -- To view, visit http://gerrit.ovirt.org/21772 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7f2c031436a6d202f856c24d9c9420c8bfdf6df Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim Gerrit-Reviewer: Alon Bar-Lev Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer 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]: Adding remove\disable verbs to vdsm-tool for admin usages
mooli tayer has posted comments on this change. Change subject: Adding remove\disable verbs to vdsm-tool for admin usages .. Patch Set 2: (1 comment) http://gerrit.ovirt.org/#/c/21772/2/lib/vdsm/tool/configurator.py File lib/vdsm/tool/configurator.py: Line 142: except RuntimeError: Line 143: return False Line 144: Line 145: def removeConf(self): Line 146: vdsm_ver = '-4.9.10' This patch fails due to this line. Where do I need to get the correct version from? Line 147: conf_prefix = '## beginning of configuration section by vdsm' + \ Line 148: vdsm_ver Line 149: conf_suffix = '## end of configuration section by vdsm' + vdsm_ver Line 150: -- To view, visit http://gerrit.ovirt.org/21772 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7f2c031436a6d202f856c24d9c9420c8bfdf6df Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim Gerrit-Reviewer: Alon Bar-Lev Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer 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]: Adding remove\disable verbs to vdsm-tool for admin usages
Yaniv Bronhaim has posted comments on this change. Change subject: Adding remove\disable verbs to vdsm-tool for admin usages .. Patch Set 2: (1 comment) http://gerrit.ovirt.org/#/c/21772/2/lib/vdsm/tool/configurator.py File lib/vdsm/tool/configurator.py: Line 29: from ..constants import P_VDSM_EXEC, DISKIMAGE_GROUP, QEMU_PROCESS_GROUP, \ Line 30: VDSM_GROUP, P_VDSM_LCONF, P_VDSM_QCONF, P_VDSM_LDCONF, P_SYSTEMCTL_CONF Line 31: Line 32: Line 33: def removeSectionFromFile(filename, beginField=None, endField=None): > >> testing the exact lines we add instead of signature before/after we can change the way vdsm reads its configuration files (vdsm.conf) by creating vdsm.d dir and under it all specific conf files with num prefix. but here vdsm modifies libvirtd.conf and qemu.conf . we won't change their way of reading the config, currently those services use one file for configuration and vdsm part interferes in it Line 34: ( Line 35: BEFORE, Line 36: WITHIN, Line 37: AFTER, -- To view, visit http://gerrit.ovirt.org/21772 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7f2c031436a6d202f856c24d9c9420c8bfdf6df Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim Gerrit-Reviewer: Alon Bar-Lev Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer 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]: Adding remove\disable verbs to vdsm-tool for admin usages
mooli tayer has posted comments on this change. Change subject: Adding remove\disable verbs to vdsm-tool for admin usages .. Patch Set 2: (4 comments) http://gerrit.ovirt.org/#/c/21772/2/lib/vdsm/constants.py.in File lib/vdsm/constants.py.in: Line 86: P_SYSTEMCTL_CONF = SYSCONF_PATH + '/sysctl.d/vdsm' Line 87: P_VDSM_LCONF = SYSCONF_PATH + '/libvirt/libvirtd.conf' Line 88: P_VDSM_LDCONF = SYSCONF_PATH + '/sysconfig/libvirtd' Line 89: P_VDSM_QCONF = SYSCONF_PATH + '/libvirt/qemu.conf' Line 90: > no need for that I guess this is related to your comment on configurator.py in line 151? my response to you there is that i'm not sure what you want done in this patch instead? Line 91: # Line 92: # External programs (sorted, please keep in order). Line 93: # Line 94: EXT_BLKID = '@BLKID_PATH@' http://gerrit.ovirt.org/#/c/21772/2/lib/vdsm/tool/configurator.py File lib/vdsm/tool/configurator.py: Line 29: from ..constants import P_VDSM_EXEC, DISKIMAGE_GROUP, QEMU_PROCESS_GROUP, \ Line 30: VDSM_GROUP, P_VDSM_LCONF, P_VDSM_QCONF, P_VDSM_LDCONF, P_SYSTEMCTL_CONF Line 31: Line 32: Line 33: def removeSectionFromFile(filename, beginField=None, endField=None): > I do not think it is that complex, and it can be improved (testing the exac >> testing the exact lines we add instead of signature before/after I guess that a patch removing exact lines should also change the code that inserts them? not sure if it should be done now or or in a different patch. Also, I do not like the duplicated: '## beginning of configuration section by vdsm' in this patch. Just a thought: is there a way we can do a one line import inside libvirt configuration file/s to a new file containing our conf and then just remove that? Line 34: ( Line 35: BEFORE, Line 36: WITHIN, Line 37: AFTER, Line 147: conf_prefix = '## beginning of configuration section by vdsm' + \ Line 148: vdsm_ver Line 149: conf_suffix = '## end of configuration section by vdsm' + vdsm_ver Line 150: Line 151: conf_paths = [ > libvirt_configure.sh already keeps those files names in it, remove_conf ver I am not sure what you are suggesting to do in this patch instead? Line 152: P_VDSM_LCONF, Line 153: P_VDSM_QCONF, Line 154: P_VDSM_LDCONF, Line 155: ] Line 357: m = [ Line 358: c.getName() for c in __configurers Line 359: if c.getName() in args.modules and not c.removeConf() Line 360: ] Line 361: if m: > c.removeConf() Well I guess this whole section should change since c.removeConf() can never return false? Some other way should be used to detect errors. Line 362: sys.stdout.write( Line 363: "Could not remove configuration for modules %s\n" % ','.join(m), Line 364: ) Line 365: ret = False -- To view, visit http://gerrit.ovirt.org/21772 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7f2c031436a6d202f856c24d9c9420c8bfdf6df Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim Gerrit-Reviewer: Alon Bar-Lev Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer 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]: Adding remove\disable verbs to vdsm-tool for admin usages
Alon Bar-Lev has posted comments on this change. Change subject: Adding remove\disable verbs to vdsm-tool for admin usages .. Patch Set 2: (1 comment) http://gerrit.ovirt.org/#/c/21772/2/lib/vdsm/tool/configurator.py File lib/vdsm/tool/configurator.py: Line 29: from ..constants import P_VDSM_EXEC, DISKIMAGE_GROUP, QEMU_PROCESS_GROUP, \ Line 30: VDSM_GROUP, P_VDSM_LCONF, P_VDSM_QCONF, P_VDSM_LDCONF, P_SYSTEMCTL_CONF Line 31: Line 32: Line 33: def removeSectionFromFile(filename, beginField=None, endField=None): > yes I remember that, but after seeing below .. i asked for opinions ab I do not think it is that complex, and it can be improved (testing the exact lines we add instead of signature before/after). I would much like to see that shell script go away. Line 34: ( Line 35: BEFORE, Line 36: WITHIN, Line 37: AFTER, -- To view, visit http://gerrit.ovirt.org/21772 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7f2c031436a6d202f856c24d9c9420c8bfdf6df Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim Gerrit-Reviewer: Alon Bar-Lev Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer 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]: Adding remove\disable verbs to vdsm-tool for admin usages
Yaniv Bronhaim has posted comments on this change. Change subject: Adding remove\disable verbs to vdsm-tool for admin usages .. Patch Set 2: (1 comment) http://gerrit.ovirt.org/#/c/21772/2/lib/vdsm/tool/configurator.py File lib/vdsm/tool/configurator.py: Line 29: from ..constants import P_VDSM_EXEC, DISKIMAGE_GROUP, QEMU_PROCESS_GROUP, \ Line 30: VDSM_GROUP, P_VDSM_LCONF, P_VDSM_QCONF, P_VDSM_LDCONF, P_SYSTEMCTL_CONF Line 31: Line 32: Line 33: def removeSectionFromFile(filename, beginField=None, endField=None): > I thought we want to minimize the the shell script processing until total r yes I remember that, but after seeing below .. i asked for opinions about it. the current implementation seems to me way too much for removal a well defined scope of text. at least I prefer the same call to "ed -s" command by execCmd instead of this confusing loop Line 34: ( Line 35: BEFORE, Line 36: WITHIN, Line 37: AFTER, -- To view, visit http://gerrit.ovirt.org/21772 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7f2c031436a6d202f856c24d9c9420c8bfdf6df Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim Gerrit-Reviewer: Alon Bar-Lev Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer 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]: Adding remove\disable verbs to vdsm-tool for admin usages
Alon Bar-Lev has posted comments on this change. Change subject: Adding remove\disable verbs to vdsm-tool for admin usages .. Patch Set 2: (1 comment) http://gerrit.ovirt.org/#/c/21772/2/lib/vdsm/tool/configurator.py File lib/vdsm/tool/configurator.py: Line 29: from ..constants import P_VDSM_EXEC, DISKIMAGE_GROUP, QEMU_PROCESS_GROUP, \ Line 30: VDSM_GROUP, P_VDSM_LCONF, P_VDSM_QCONF, P_VDSM_LDCONF, P_SYSTEMCTL_CONF Line 31: Line 32: Line 33: def removeSectionFromFile(filename, beginField=None, endField=None): > are we sure that its better than just executing ed command as we do in libv I thought we want to minimize the the shell script processing until total removal. Line 34: ( Line 35: BEFORE, Line 36: WITHIN, Line 37: AFTER, -- To view, visit http://gerrit.ovirt.org/21772 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7f2c031436a6d202f856c24d9c9420c8bfdf6df Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim Gerrit-Reviewer: Alon Bar-Lev Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer 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]: Adding remove\disable verbs to vdsm-tool for admin usages
Yaniv Bronhaim has posted comments on this change.
Change subject: Adding remove\disable verbs to vdsm-tool for admin usages
..
Patch Set 2:
(3 comments)
I know we took over of existing patch, but this original implementation is
wrong in my opinion. there are much simpler ways to remove whatever vdsm
configs. in gerenal this patch should do the opposite\clean of vdsm-tool
sebool-config, vdsm-tool set-saslpasswd, vdsm-tool configure.
http://gerrit.ovirt.org/#/c/21772/2/lib/vdsm/constants.py.in
File lib/vdsm/constants.py.in:
Line 86: P_SYSTEMCTL_CONF = SYSCONF_PATH + '/sysctl.d/vdsm'
Line 87: P_VDSM_LCONF = SYSCONF_PATH + '/libvirt/libvirtd.conf'
Line 88: P_VDSM_LDCONF = SYSCONF_PATH + '/sysconfig/libvirtd'
Line 89: P_VDSM_QCONF = SYSCONF_PATH + '/libvirt/qemu.conf'
Line 90:
no need for that
Line 91: #
Line 92: # External programs (sorted, please keep in order).
Line 93: #
Line 94: EXT_BLKID = '@BLKID_PATH@'
http://gerrit.ovirt.org/#/c/21772/2/lib/vdsm/tool/configurator.py
File lib/vdsm/tool/configurator.py:
Line 33: removeSectionFromFile
are we sure that its better than just executing ed command as we do in
libvirt_configure.sh?
ed -s "${confFile}" >/dev/null 2>&1 &1 < Done
libvirt_configure.sh already keeps those files names in it, remove_conf verb
call is enough without all this bunch of code.
Line 152: P_VDSM_LCONF,
Line 153: P_VDSM_QCONF,
Line 154: P_VDSM_LDCONF,
Line 155: ]
--
To view, visit http://gerrit.ovirt.org/21772
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie7f2c031436a6d202f856c24d9c9420c8bfdf6df
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim
Gerrit-Reviewer: Alon Bar-Lev
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Yaniv Bronhaim
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: mooli tayer
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]: Adding remove\disable verbs to vdsm-tool for admin usages
Yaniv Bronhaim has posted comments on this change. Change subject: Adding remove\disable verbs to vdsm-tool for admin usages .. Patch Set 2: you can write ut that imports the removeSectionFromFile func, creates temporary file and cat in the vdsm section config, then check your code on that. shouldn't be complex. im working on testing for the configurator itself when vdsm.conf is changed, to verify that vdsm actually configures the right values for libvirt and qemu. but this doesn't need to block you from testing that removal funcs.. -- To view, visit http://gerrit.ovirt.org/21772 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7f2c031436a6d202f856c24d9c9420c8bfdf6df Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim Gerrit-Reviewer: Alon Bar-Lev Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer 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]: Adding remove\disable verbs to vdsm-tool for admin usages
Alon Bar-Lev has posted comments on this change. Change subject: Adding remove\disable verbs to vdsm-tool for admin usages .. Patch Set 2: (1 comment) http://gerrit.ovirt.org/#/c/21772/2/lib/vdsm/tool/configurator.py File lib/vdsm/tool/configurator.py: Line 357: m = [ Line 358: c.getName() for c in __configurers Line 359: if c.getName() in args.modules and not c.removeConf() Line 360: ] Line 361: if m: > I don't see understand. what do we change an object status here? c.removeConf() Line 362: sys.stdout.write( Line 363: "Could not remove configuration for modules %s\n" % ','.join(m), Line 364: ) Line 365: ret = False -- To view, visit http://gerrit.ovirt.org/21772 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7f2c031436a6d202f856c24d9c9420c8bfdf6df Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim Gerrit-Reviewer: Alon Bar-Lev Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer 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]: Adding remove\disable verbs to vdsm-tool for admin usages
mooli tayer has posted comments on this change.
Change subject: Adding remove\disable verbs to vdsm-tool for admin usages
..
Patch Set 2:
(6 comments)
I would like to write a unit test, at least for 'removeSectionFromFile'
Where do such a test belong?
Also if I want to test on a file should that file be a resource or
Should I write it on demand?
http://gerrit.ovirt.org/#/c/21772/2/lib/vdsm/tool/configurator.py
File lib/vdsm/tool/configurator.py:
Line 53: newcontent.append(line)
Line 54: else:
Line 55: raise RuntimeError("Internal Error")
Line 56: if content != newcontent:
Line 57: tmpfile = tempfile.NamedTemporaryFile(delete=False)
> you need to create temp file at same directory of original file... see temp
Done
Line 58: tname = tmpfile.name
Line 59: tmpfile.writelines(newcontent)
Line 60: tmpfile.close()
Line 61: shutil.move(tname, filename)
Line 56: if content != newcontent:
Line 57: tmpfile = tempfile.NamedTemporaryFile(delete=False)
Line 58: tname = tmpfile.name
Line 59: tmpfile.writelines(newcontent)
Line 60: tmpfile.close()
> you should never use close() but python with structure.
Done
Line 61: shutil.move(tname, filename)
Line 62:
Line 63:
Line 64: class _ModuleConfigure(object):
Line 147: conf_prefix = '## beginning of configuration section by vdsm'
+ \
Line 148: vdsm_ver
Line 149: conf_suffix = '## end of configuration section by vdsm' +
vdsm_ver
Line 150:
Line 151: conf_paths = [
> consider remove temporary one time used variable.
Done
Line 152: P_VDSM_LCONF,
Line 153: P_VDSM_QCONF,
Line 154: P_VDSM_LDCONF,
Line 155: ]
Line 156: for path in conf_paths:
Line 157: if os.path.exists(path):
Line 158: try:
Line 159: removeSectionFromFile(path, conf_prefix,
conf_suffix)
Line 160: except OSError, e:
> no need try/except now
If you are sure about it...
Line 161: if e.errno != os.errno.EEXIST:
Line 162: raise
Line 163:
Line 164: utils.rmFile(P_SYSTEMCTL_CONF)
Line 359: args
I don't see how c.removeConf() can return false.
Am I missing something?
Line 357: m = [
Line 358: c.getName() for c in __configurers
Line 359: if c.getName() in args.modules and not c.removeConf()
Line 360: ]
Line 361: if m:
> well, this is ugly... usually these methods of building lists should not ch
I don't see understand. what do we change an object status here?
Line 362: sys.stdout.write(
Line 363: "Could not remove configuration for modules %s\n" %
','.join(m),
Line 364: )
Line 365: ret = False
--
To view, visit http://gerrit.ovirt.org/21772
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie7f2c031436a6d202f856c24d9c9420c8bfdf6df
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim
Gerrit-Reviewer: Alon Bar-Lev
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Yaniv Bronhaim
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: mooli tayer
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]: Adding remove\disable verbs to vdsm-tool for admin usages
Alon Bar-Lev has posted comments on this change.
Change subject: Adding remove\disable verbs to vdsm-tool for admin usages
..
Patch Set 2:
(5 comments)
http://gerrit.ovirt.org/#/c/21772/2/lib/vdsm/tool/configurator.py
File lib/vdsm/tool/configurator.py:
Line 53: newcontent.append(line)
Line 54: else:
Line 55: raise RuntimeError("Internal Error")
Line 56: if content != newcontent:
Line 57: tmpfile = tempfile.NamedTemporaryFile(delete=False)
you need to create temp file at same directory of original file... see
tempfile.mkstemp, and then use os.rename for atomic.
Line 58: tname = tmpfile.name
Line 59: tmpfile.writelines(newcontent)
Line 60: tmpfile.close()
Line 61: shutil.move(tname, filename)
Line 56: if content != newcontent:
Line 57: tmpfile = tempfile.NamedTemporaryFile(delete=False)
Line 58: tname = tmpfile.name
Line 59: tmpfile.writelines(newcontent)
Line 60: tmpfile.close()
you should never use close() but python with structure.
Line 61: shutil.move(tname, filename)
Line 62:
Line 63:
Line 64: class _ModuleConfigure(object):
Line 147: conf_prefix = '## beginning of configuration section by vdsm'
+ \
Line 148: vdsm_ver
Line 149: conf_suffix = '## end of configuration section by vdsm' +
vdsm_ver
Line 150:
Line 151: conf_paths = [
consider remove temporary one time used variable.
for path in [
]:
Line 152: P_VDSM_LCONF,
Line 153: P_VDSM_QCONF,
Line 154: P_VDSM_LDCONF,
Line 155: ]
Line 156: for path in conf_paths:
Line 157: if os.path.exists(path):
Line 158: try:
Line 159: removeSectionFromFile(path, conf_prefix,
conf_suffix)
Line 160: except OSError, e:
no need try/except now
Line 161: if e.errno != os.errno.EEXIST:
Line 162: raise
Line 163:
Line 164: utils.rmFile(P_SYSTEMCTL_CONF)
Line 357: m = [
Line 358: c.getName() for c in __configurers
Line 359: if c.getName() in args.modules and not c.removeConf()
Line 360: ]
Line 361: if m:
well, this is ugly... usually these methods of building lists should not change
the object status...
Line 362: sys.stdout.write(
Line 363: "Could not remove configuration for modules %s\n" %
','.join(m),
Line 364: )
Line 365: ret = False
--
To view, visit http://gerrit.ovirt.org/21772
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie7f2c031436a6d202f856c24d9c9420c8bfdf6df
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim
Gerrit-Reviewer: Alon Bar-Lev
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Yaniv Bronhaim
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: mooli tayer
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]: Adding remove\disable verbs to vdsm-tool for admin usages
oVirt Jenkins CI Server has posted comments on this change. Change subject: Adding remove\disable verbs to vdsm-tool for admin usages .. Patch Set 2: Code-Review-1 Build Unstable http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/6517/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/7301/ : UNSTABLE http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/7419/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/21772 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7f2c031436a6d202f856c24d9c9420c8bfdf6df Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim Gerrit-Reviewer: Alon Bar-Lev Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer 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]: Adding remove\disable verbs to vdsm-tool for admin usages
mooli tayer has posted comments on this change. Change subject: Adding remove\disable verbs to vdsm-tool for admin usages .. Patch Set 2: (1 comment) http://gerrit.ovirt.org/#/c/21772/2/lib/vdsm/tool/configurator.py File lib/vdsm/tool/configurator.py: Line 401: ': Removed -- To view, visit http://gerrit.ovirt.org/21772 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7f2c031436a6d202f856c24d9c9420c8bfdf6df Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim Gerrit-Reviewer: Alon Bar-Lev Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer 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]: Adding remove\disable verbs to vdsm-tool for admin usages
Alon Bar-Lev has posted comments on this change. Change subject: Adding remove\disable verbs to vdsm-tool for admin usages .. Patch Set 1: (1 comment) http://gerrit.ovirt.org/#/c/21772/1/lib/vdsm/tool/configurator.py File lib/vdsm/tool/configurator.py: Line 106: conf_prefix, Line 107: conf_suffix) Line 108: except OSError, e: Line 109: if e.errno != os.errno.EEXIST: Line 110: raise > Not sure what you mean here. should be: if os.path.exists(xxx): super(...).removeConf() Line 111: Line 112: utils.rmFile(P_SYSTEMCTL_CONF) Line 113: Line 114: def _exec_libvirt_configure(self, action): -- To view, visit http://gerrit.ovirt.org/21772 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7f2c031436a6d202f856c24d9c9420c8bfdf6df Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim Gerrit-Reviewer: Alon Bar-Lev Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: mooli tayer 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]: Adding remove\disable verbs to vdsm-tool for admin usages
mooli tayer has posted comments on this change.
Change subject: Adding remove\disable verbs to vdsm-tool for admin usages
..
Patch Set 1:
(8 comments)
I am continuing Yaniv's work here.
http://gerrit.ovirt.org/#/c/21772/1/lib/vdsm/tool/configurator.py
File lib/vdsm/tool/configurator.py:
Line 48:
Line 49: def isconfigured(self):
Line 50: return True
Line 51:
Line 52: def removeConf(self, filename, beginField=None, endField=None):
> this is not removeConf in inheritance model, it is utility, the removeConf
Done
Line 53: newfile = []
Line 54: if not beginField and not endField:
Line 55: return
Line 56: else:
Line 52: def removeConf(self, filename, beginField=None, endField=None):
Line 53: newfile = []
Line 54: if not beginField and not endField:
Line 55: return
Line 56: else:
> so why else?
Done
Line 57: with open(filename, 'r') as f:
Line 58: skip = False
Line 59: for line in f.readlines():
Line 60: if beginField and endField:
Line 70: continue
Line 71: else:
Line 72: if line.endswith('%s\n' % endField):
Line 73: continue
Line 74: newfile.append(line)
> we are talked about this structure... it is spaghetti!
Done
Line 75: tmp_file = tempfile.NamedTemporaryFile(delete=False)
Line 76: tname = tmp_file.name
Line 77: tmp_file.writelines(newfile)
Line 78: tmp_file.close()
Line 103: for path in conf_paths:
Line 104: try:
Line 105: super(LibvirtModuleConfigure, self).removeConf(path,
Line 106:
conf_prefix,
Line 107:
conf_suffix)
> please avoid drawing code...
Replaced with utility method
Line 108: except OSError, e:
Line 109: if e.errno != os.errno.EEXIST:
Line 110: raise
Line 111:
Line 106:
conf_prefix,
Line 107:
conf_suffix)
Line 108: except OSError, e:
Line 109: if e.errno != os.errno.EEXIST:
Line 110: raise
> you should first check if file exist then attempt to modify, do not assume
Not sure what you mean here.
Line 111:
Line 112: utils.rmFile(P_SYSTEMCTL_CONF)
Line 113:
Line 114: def _exec_libvirt_configure(self, action):
Line 320: """
adding
ret = True
http://gerrit.ovirt.org/#/c/21772/1/lib/vdsm/tool/passwd.py
File lib/vdsm/tool/passwd.py:
Line 52: Remove vdsm password for libvirt connection
Line 53: """
Line 54: script = ['/usr/sbin/saslpasswd2', '-p', '-a', 'libvirt', '-d',
Line 55: constants.SASL_USERNAME]
Line 56: p = subprocess.Popen(script)
> why do you need this script temp variable? why isn't it tuple?
done
Line 57: output, err = p.communicate()
Line 58: if p.returncode != 0:
http://gerrit.ovirt.org/#/c/21772/1/lib/vdsm/tool/seboolsetup.py
File lib/vdsm/tool/seboolsetup.py:
Line 65: """Disable the required selinux booleans"""
Line 66: setup_booleans(False)
Line 67:
Line 68:
Line 69: @expose("clear-selinux-policy")
> sebool_unconfig does the same. I'll remove that
Removing section
Line 70: def clear_selinux_policy():
Line 71: """
Line 72: Clear the changes of selinux policy
Line 73: """
--
To view, visit http://gerrit.ovirt.org/21772
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie7f2c031436a6d202f856c24d9c9420c8bfdf6df
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim
Gerrit-Reviewer: Alon Bar-Lev
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Yaniv Bronhaim
Gerrit-Reviewer: mooli tayer
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]: Adding remove\disable verbs to vdsm-tool for admin usages
Alon Bar-Lev has posted comments on this change.
Change subject: Adding remove\disable verbs to vdsm-tool for admin usages
..
Patch Set 1:
(7 comments)
File lib/vdsm/tool/configurator.py
Line 48:
Line 49: def isconfigured(self):
Line 50: return True
Line 51:
Line 52: def removeConf(self, filename, beginField=None, endField=None):
this is not removeConf in inheritance model, it is utility, the removeConf in
interface should do nothing, while implementations can call a utility function.
Line 53: newfile = []
Line 54: if not beginField and not endField:
Line 55: return
Line 56: else:
Line 52: def removeConf(self, filename, beginField=None, endField=None):
Line 53: newfile = []
Line 54: if not beginField and not endField:
Line 55: return
Line 56: else:
so why else?
won't it better to just net bellow and have positive condition?
Line 57: with open(filename, 'r') as f:
Line 58: skip = False
Line 59: for line in f.readlines():
Line 60: if beginField and endField:
Line 70: continue
Line 71: else:
Line 72: if line.endswith('%s\n' % endField):
Line 73: continue
Line 74: newfile.append(line)
we are talked about this structure... it is spaghetti!
you need to use state machine like structure.
(
BEFORE,
WITHIN,
AFTER,
) = range(3)
newcontent = []
state = BEFORE if beginField else WITHIN
with open(filename, 'r') as f:
content = f.readlines()
for line in content:
if state == BEFORE:
if line.startswith(beginField):
state = WITHIN
else:
newcontent.append(line)
elif state == WITHIN:
if endField and line.startswith(endField):
state = AFTER
elif state == AFTER:
newcontent.append(line)
else:
raise RuntimeError("Internal error")
if content != newcontent:
# write temp, move
Line 75: tmp_file = tempfile.NamedTemporaryFile(delete=False)
Line 76: tname = tmp_file.name
Line 77: tmp_file.writelines(newfile)
Line 78: tmp_file.close()
Line 103: for path in conf_paths:
Line 104: try:
Line 105: super(LibvirtModuleConfigure, self).removeConf(path,
Line 106:
conf_prefix,
Line 107:
conf_suffix)
please avoid drawing code...
super(xxx, self).removeConf(
p1,
p2,
)
so that if name is modified you do not need to revisit the entire indentation.
saying that, and using super for this is completely wrong.
Line 108: except OSError, e:
Line 109: if e.errno != os.errno.EEXIST:
Line 110: raise
Line 111:
Line 106:
conf_prefix,
Line 107:
conf_suffix)
Line 108: except OSError, e:
Line 109: if e.errno != os.errno.EEXIST:
Line 110: raise
you should first check if file exist then attempt to modify, do not assume that
good follow has exceptions.
Line 111:
Line 112: utils.rmFile(P_SYSTEMCTL_CONF)
Line 113:
Line 114: def _exec_libvirt_configure(self, action):
File lib/vdsm/tool/passwd.py
Line 52: Remove vdsm password for libvirt connection
Line 53: """
Line 54: script = ['/usr/sbin/saslpasswd2', '-p', '-a', 'libvirt', '-d',
Line 55: constants.SASL_USERNAME]
Line 56: p = subprocess.Popen(script)
why do you need this script temp variable? why isn't it tuple?
Line 57: output, err = p.communicate()
Line 58: if p.returncode != 0:
File lib/vdsm/tool/seboolsetup.py
Line 70: def clear_selinux_policy():
Line 71: """
Line 72: Clear the changes of selinux policy
Line 73: """
Line 74: selinux_policys = [
why do you need this temp variable? why isn't it tuple?
Line 75: 'virt_use_nfs',
Line 76: 'virt_use_sanlock',
Line 77: 'sanlock_use_nfs',
Line 78: ]
--
To view, visit http://gerrit.ovirt.org/21772
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie7f2c031436a6d202f856c24d9c9420c8bfdf6df
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim
Gerrit-Reviewer: Alon Bar-Lev
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Yaniv Bronhaim
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
Change in vdsm[master]: Adding remove\disable verbs to vdsm-tool for admin usages
oVirt Jenkins CI Server has posted comments on this change. Change subject: Adding remove\disable verbs to vdsm-tool for admin usages .. Patch Set 1: Verified-1 Build Failed http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/4912/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/5712/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/5801/ : FAILURE -- To view, visit http://gerrit.ovirt.org/21772 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7f2c031436a6d202f856c24d9c9420c8bfdf6df Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim Gerrit-Reviewer: Alon Bar-Lev Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Yaniv Bronhaim 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]: Adding remove\disable verbs to vdsm-tool for admin usages
Yaniv Bronhaim has posted comments on this change.
Change subject: Adding remove\disable verbs to vdsm-tool for admin usages
..
Patch Set 1:
(2 comments)
File lib/vdsm/tool/configurator.py
Line 48:
Line 49: def isconfigured(self):
Line 50: return True
Line 51:
Line 52: def removeConf(self, filename, beginField=None, endField=None):
I can you use "sed -i" explictly if it's simpler to understand. let me know
Line 53: newfile = []
Line 54: if not beginField and not endField:
Line 55: return
Line 56: else:
File lib/vdsm/tool/seboolsetup.py
Line 65: """Disable the required selinux booleans"""
Line 66: setup_booleans(False)
Line 67:
Line 68:
Line 69: @expose("clear-selinux-policy")
sebool_unconfig does the same. I'll remove that
Line 70: def clear_selinux_policy():
Line 71: """
Line 72: Clear the changes of selinux policy
Line 73: """
--
To view, visit http://gerrit.ovirt.org/21772
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie7f2c031436a6d202f856c24d9c9420c8bfdf6df
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim
Gerrit-Reviewer: Yaniv Bronhaim
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Adding remove\disable verbs to vdsm-tool for admin usages
Yaniv Bronhaim has uploaded a new change for review.
Change subject: Adding remove\disable verbs to vdsm-tool for admin usages
..
Adding remove\disable verbs to vdsm-tool for admin usages
The spec will be modified separately to use vdsm-tool instead of hard-coded
operations
Change-Id: Ie7f2c031436a6d202f856c24d9c9420c8bfdf6df
Signed-off-by: Yaniv Bronhaim
---
M lib/vdsm/constants.py.in
M lib/vdsm/tool/configurator.py
M lib/vdsm/tool/passwd.py
M lib/vdsm/tool/seboolsetup.py
4 files changed, 118 insertions(+), 3 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/72/21772/1
diff --git a/lib/vdsm/constants.py.in b/lib/vdsm/constants.py.in
index 790a3a0..43e13da 100644
--- a/lib/vdsm/constants.py.in
+++ b/lib/vdsm/constants.py.in
@@ -80,6 +80,15 @@
P_VDSM_EXEC = '@LIBEXECDIR@'
#
+# Configuration file definitions
+#
+SYSCONF_PATH = '@sysconfdir@'
+P_SYSTEMCTL_CONF = SYSCONF_PATH + '/sysctl.d/vdsm'
+P_VDSM_LCONF = SYSCONF_PATH + '/libvirt/libvirtd.conf'
+P_VDSM_LDCONF = SYSCONF_PATH + '/sysconfig/libvirtd'
+P_VDSM_QCONF = SYSCONF_PATH + '/libvirt/qemu.conf'
+
+#
# External programs (sorted, please keep in order).
#
EXT_BLKID = '@BLKID_PATH@'
diff --git a/lib/vdsm/tool/configurator.py b/lib/vdsm/tool/configurator.py
index 51eda80..a30827a 100644
--- a/lib/vdsm/tool/configurator.py
+++ b/lib/vdsm/tool/configurator.py
@@ -21,11 +21,13 @@
import sys
import grp
import argparse
+import tempfile
+import shutil
from .. import utils
from . import service, expose
-from ..constants import P_VDSM_EXEC, DISKIMAGE_GROUP
-from ..constants import QEMU_PROCESS_GROUP, VDSM_GROUP
+from ..constants import P_VDSM_EXEC, DISKIMAGE_GROUP, QEMU_PROCESS_GROUP, \
+VDSM_GROUP, P_VDSM_LCONF, P_VDSM_QCONF, P_VDSM_LDCONF, P_SYSTEMCTL_CONF
class _ModuleConfigure(object):
@@ -47,6 +49,35 @@
def isconfigured(self):
return True
+def removeConf(self, filename, beginField=None, endField=None):
+newfile = []
+if not beginField and not endField:
+return
+else:
+with open(filename, 'r') as f:
+skip = False
+for line in f.readlines():
+if beginField and endField:
+if skip:
+if line.startswith(endField):
+skip = False
+continue
+if line.startswith(beginField):
+skip = True
+continue
+elif beginField:
+if line.startswith(beginField):
+continue
+else:
+if line.endswith('%s\n' % endField):
+continue
+newfile.append(line)
+tmp_file = tempfile.NamedTemporaryFile(delete=False)
+tname = tmp_file.name
+tmp_file.writelines(newfile)
+tmp_file.close()
+shutil.move(tname, filename)
+
class LibvirtModuleConfigure(_ModuleConfigure):
def __init__(self):
@@ -57,6 +88,28 @@
def getServices(self):
return ["supervdsmd", "vdsmd", "libvirtd"]
+
+def removeConf(self):
+vdsm_ver = '-4.9.10'
+conf_prefix = '## beginning of configuration section by vdsm' + \
+ vdsm_ver
+conf_suffix = '## end of configuration section by vdsm' + vdsm_ver
+
+conf_paths = [
+P_VDSM_LCONF,
+P_VDSM_QCONF,
+P_VDSM_LDCONF,
+]
+for path in conf_paths:
+try:
+super(LibvirtModuleConfigure, self).removeConf(path,
+ conf_prefix,
+ conf_suffix)
+except OSError, e:
+if e.errno != os.errno.EEXIST:
+raise
+
+utils.rmFile(P_SYSTEMCTL_CONF)
def _exec_libvirt_configure(self, action):
"""
@@ -113,6 +166,9 @@
def getServices(self):
return ['sanlock']
+
+def removeConf(self):
+pass
def configure(self):
"""
@@ -257,6 +313,25 @@
raise RuntimeError("Config is not valid. Check conf files")
+@expose("remove-config")
+def remove_config(*args):
+"""
+Remove vdsm configuration from conf files
+"""
+args = _parse_args('remove-config')
+m = [
+c.getName() for c in __configurers
+if c.getName() in args.modules and not c.removeConf()
+]
+if m:
+sys.stdout.write(
+"Could not remove configuration for modules %s\n" % ','.join(m),
+)
+ret = False
+if not ret:
+raise RuntimeError("Remove configuration failed")
+
+
def _parse_args(action):
parser = argparse.ArgumentParser('vdsm-tool %s' % (a
