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_XXXX 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 <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