Change in vdsm[master]: Extend vdsm-tool: moving configure libvirt to external shell...

Alon Bar-Lev alonbl at redhat.com
Thu May 30 23:12:24 UTC 2013


Alon Bar-Lev has posted comments on this change.

Change subject: Extend vdsm-tool: moving configure libvirt to external shell script
......................................................................


Patch Set 1: (5 inline comments)

....................................................
File lib/vdsm/tool/libvirt_configure.py
Line 26: def libvirt_configure():
Line 27:     """
Line 28:     Configure libvirt conf files
Line 29:     """
Line 30:     rc, out, err = utils.execCmd(["./libvirt_configure.sh"], sudo=True)
are you sure the script set working directory to its base?
Line 31: 
Line 32:     if rc != 0:
Line 33:         msg = "Failed to configure libvirt: %s" % err
Line 34:         raise RuntimeError(msg)


Line 31: 
Line 32:     if rc != 0:
Line 33:         msg = "Failed to configure libvirt: %s" % err
Line 34:         raise RuntimeError(msg)
Line 35:     else:
No need else in exceptional programming.

 if failure:
     raise exception
 continue as usual
Line 36:         for msg in out:
Line 37:             sys.stdout.write(msg + '\n')


....................................................
File lib/vdsm/tool/libvirt_configure.sh.in
Line 10: 
Line 11: . @LIBEXECDIR@/ovirt_functions.sh
Line 12: GETCONFITEM=@VDSMDIR@/get-conf-item
Line 13: CONF_FILE=@CONFDIR@/vdsm.conf
Line 14: LCONF=/etc/libvirt/libvirtd.conf
shouldn't it be @sysconfdir@?
Line 15: QCONF=/etc/libvirt/qemu.conf
Line 16: LDCONF=/etc/sysconfig/libvirtd
Line 17: QLCONF=/etc/libvirt/qemu-sanlock.conf
Line 18: 


Line 108:     qconf="$3"
Line 109:     ldconf="$4"
Line 110:     qlconf="$5"
Line 111: 
Line 112:     # do not configure ovirt nodes before registration
I need to understand this comment better... not related to this change...
Line 113:     if isOvirt
Line 114:     then
Line 115:         if [ ! -f /etc/pki/vdsm/certs/vdsmcert.pem ]
Line 116:         then


Line 116:         then
Line 117: 	    echo "$prog: Missing certificates, $prog not registered" 1>&2
Line 118: 	    exit 6
Line 119: 	fi
Line 120: 	    /usr/bin/vdsm-tool validate-ovirt-certs
remove tabs, or add tabs to the whole scripts.
Line 121:     fi
Line 122: 
Line 123:     #
Line 124:     # reconfigure if:


--
To view, visit http://gerrit.ovirt.org/15216
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Id58b129afbf141a47a85b421961bf5b1776b41e4
Gerrit-PatchSet: 1
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: Zhou Zheng Sheng <zhshzhou at linux.vnet.ibm.com>
Gerrit-Reviewer: oVirt Jenkins CI Server


More information about the vdsm-patches mailing list