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 <mtayer(a)redhat.com>
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 <ybronhei(a)redhat.com>
Gerrit-Reviewer: Alon Bar-Lev <alonbl(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: mooli tayer <mtayer(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes