Change in vdsm[master]: Adding remove/disable verbs to vdsm-tool for admin usages

mtayer at redhat.com mtayer at redhat.com
Thu Mar 6 10:09:02 UTC 2014


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


More information about the vdsm-patches mailing list