Change in vdsm[master]: Initial commit for vdsm-tool testing infrastructure

ybronhei at redhat.com ybronhei at redhat.com
Tue Mar 18 08:18:49 UTC 2014


Yaniv Bronhaim has posted comments on this change.

Change subject: Initial commit for vdsm-tool testing infrastructure
......................................................................


Patch Set 10:

(4 comments)

http://gerrit.ovirt.org/#/c/25263/10/.gitignore
File .gitignore:

Line 33: init/upstart/vdsmd.upstart
Line 34: init/vdsmd_init_common.sh
Line 35: lib/vdsm/config.py
Line 36: lib/vdsm/constants.py
Line 37: lib/vdsm/tool/tests/logrotate-libvirtd
> why are you creating these files in tree? you should run the test in a temp
oh, i changed it and forgot to remove those
Line 38: lib/vdsm/tool/tests/libvirtd.conf
Line 39: lib/vdsm/tool/tests/libvirtd
Line 40: lib/vdsm/tool/tests/qemu.conf
Line 41: lib/vdsm/tool/tests/qemu-sanlock.conf


http://gerrit.ovirt.org/#/c/25263/10/lib/vdsm/tool/configurator.py
File lib/vdsm/tool/configurator.py:

Line 131:         if not self.testenv and os.getuid() != 0:
Line 132:             raise UserWarning("Must run as root")
Line 133: 
Line 134:         rc, out, err = utils.execCmd(
Line 135:             [
> why not tupple?
with tupple the execCmd with sudo fails when it tries to add the sudo parameters to the command (TypeError: can only concatenate tuple (not "list") to tuple)

im leaving this change fix here.
Line 136:                 '/usr/sbin/usermod',
Line 137:                 '-a',
Line 138:                 '-G',
Line 139:                 ','.join(self.SANLOCK_GROUPS),


Line 139:                 ','.join(self.SANLOCK_GROUPS),
Line 140:                 'sanlock'
Line 141:             ],
Line 142:             raw=True,
Line 143:             sudo=True if self.testenv else False,
> Why run this configurator at all during tests? I find it a bit rude.
how will i test this part ? I just won't?.. any better ideas? but i agree with you, and i want this patch in asap to make it easier for others to add more tool tests, so until finding better way to test sanlock config part im removing it.
Line 144:         )
Line 145:         sys.stdout.write(out)
Line 146:         sys.stderr.write(err)
Line 147:         if rc != 0:


http://gerrit.ovirt.org/#/c/25263/10/lib/vdsm/tool/libvirt_configure.sh.in
File lib/vdsm/tool/libvirt_configure.sh.in:

Line 301:         rm -f "${lnetwork}"
Line 302:     fi
Line 303: 
Line 304:     if [ "${LIBVIRT_LOGROTATE}" = '' ]; then
Line 305:         local llogr=/etc/logrotate.d/libvirtd
> this is equivalent to
much better. thanks!
Line 306:     else
Line 307:         local llogr="${LIBVIRT_LOGROTATE}"
Line 308:     fi
Line 309: 


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iae0fec9b2057c5ba932f38ed5d34d654486043ef
Gerrit-PatchSet: 10
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap 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