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 <ybron...@redhat.com> Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com> Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com> Gerrit-Reviewer: Yaniv Bronhaim <ybron...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer <mta...@redhat.com> Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches