Alon Bar-Lev has posted comments on this change.
Change subject: ovirt-live: migrate ovirt live plugin to otopi
......................................................................
Patch Set 1: (15 inline comments)
why do you need empty ovirt_live.py file?
Please don't use capital letters in module name, OLPlugin->olive or something.
Took me a while to understand what OL is about.
....................................................
File fedora/kickstart/ovirt.ks
Line 159: yum localinstall -y /home/oVirtuser/oVirtLiveFiles/rpms/*.rpm
Line 160:
Line 161: # Updating patched files
Line 162: mkdir
/usr/share/ovirt-engine/setup/plugins/ovirt-engine-setup/ovirtlive
Line 163: cp /home/oVirtuser/oVirtLiveFiles/OLPlugin/*.py
/usr/share/ovirt-engine/setup/plugins/ovirt-engine-setup/ovirtlive
file structure within git repository should match exactly the file structure at
destination filesystem.
a simple copy recursive should be established to copy all files.
Line 164:
Line 165:
Line 166: # Manipulate fqdn validation, so that it is possible to setup with
answer file
Line 167: sed -i 's/raise Exception(output_messages.ERR_EXP_VALIDATE_PARAM %
param.getKey("CONF_NAME"))/logging.debug("Failed to validate %s with value
%s",param,paramValue)/g' /usr/share/ovirt-engine/scripts/engine-setup.py
....................................................
File fedora/oVirtLiveFiles/OLPlugin/core.py
Line 28: from otopi import util
Line 29: from otopi import plugin
Line 30: from otopi import filetransaction
Line 31: from otopi import constants as otopicons
Line 32: import os
first python system, then package specific move this os, shutil, glob above
gettext
Line 33: import shutil
Line 34: import glob
Line 35: import ovirtsdk.api
Line 36: import ovirtsdk.xml as params
Line 31: from otopi import constants as otopicons
Line 32: import os
Line 33: import shutil
Line 34: import glob
Line 35: import ovirtsdk.api
should go after setup block
Line 36: import ovirtsdk.xml as params
Line 37:
Line 38: from ovirt_engine_setup import constants as osetupcons
Line 39: from ovirt_engine_setup import dialog
Line 33: import shutil
Line 34: import glob
Line 35: import ovirtsdk.api
Line 36: import ovirtsdk.xml as params
Line 37:
two spaces
Line 38: from ovirt_engine_setup import constants as osetupcons
Line 39: from ovirt_engine_setup import dialog
Line 40:
Line 41:
Line 56: self.environment.setdefault(
Line 57: osetupcons.OLEnv.ENABLE,
Line 58: True
Line 59: )
Line 60:
one space, please run pyflakes, pep8
Line 61:
Line 62: @plugin.event(
Line 63: stage=plugin.Stages.STAGE_VALIDATION,
Line 64: condition=lambda self: self.environment[
Line 64: condition=lambda self: self.environment[
Line 65: osetupcons.OLEnv.CONFIGURE
Line 66: ],
Line 67: )
Line 68: def _validation(self):
please order based on execution order, this should go after setup
Line 69: import ovirtsdk.api
Line 70: import ovirtsdk.xml
Line 71: self._ovirtsdk_api = ovirtsdk.api
Line 72: self._ovirtsdk_xml = ovirtsdk.xml
Line 85: stage=plugin.Stages.STAGE_EARLY_MISC,
Line 86: condition=lambda self: self._enabled,
Line 87: name=osetupcons.Stages.OL_CONFIG_STORAGE,
Line 88: before=[
Line 89: osetupcons.Stages.OL_COPY_ISO
have you patched osetupcons? this these should be in your own class.
Line 90: ]
Line 91: )
Line 92: def _createstorage(self):
Line 93: engine_api = self._ovirtsdk_api.API(
Line 92: def _createstorage(self):
Line 93: engine_api = self._ovirtsdk_api.API(
Line 94: url='https://{fqdn}:{port}/api'.format(
Line 95: fqdn=self.environment[osetupcons.ConfigEnv.FQDN],
Line 96: port=self.environment[osetupcons.ConfigEnv.HTTPS_PORT],
PUBLIC_HTTPS_PORT I think
Line 97: ),
Line 98: username='{user}@{domain}'.format(
Line 99: user=osetupcons.Const.USER_ADMIN,
Line 100: domain=osetupcons.Const.DOMAIN_INTERNAL,
Line 121: )
Line 122:
Line 123: def _copyiso(self):
Line 124: self.logger.debug('Copying Iso Files')
Line 125: isoPattern = "/home/oVirtuser/oVirtLiveFiles/iso/*.iso"
please put in constants FileLocations (your constants)
Line 126: fileList = glob.glob(isoPattern)
Line 127: targetPath =
os.path.join(self.environment[osetupcons.ConfigEnv.ISO_DOMAIN_DEFAULT_NFS_MOUNT_POINT],
self.environment[osetupcons.ConfigEnv.ISO_DOMAIN_SD_UUID], "images",
"11111111-1111-1111-1111-111111111111")
Line 128: CONST_VDSM_UID = 36
Line 129: for filename in fileList:
Line 128: CONST_VDSM_UID = 36
Line 129: for filename in fileList:
Line 130: shutil.move(filename, targetPath)
Line 131: file = os.path.join(targetPath,filename)
Line 132: os.chown(file,36,36)
doesn't the location constant, so you can put it during image preparation?
Line 133:
Line 134: @plugin.event(
Line 135: stage=plugin.Stages.STAGE_EARLY_MISC,
Line 136: condition=lambda self: self._enabled,
Line 138: after=[
Line 139: osetupcons.Stages.OL_COPY_ISO,
Line 140: ],
Line 141: )
Line 142:
remove space
Line 143: def _createvm(self):
Line 144: self.logger.debug("Creating VM")
Line 145:
Line 146: engine_api = self._ovirtsdk_api.API(
Line 157: )
Line 158: engine_version = self._ovirtsdk_xml.params.Version(
Line 159: major=self._version[0],
Line 160: minor=self._version[1],
Line 161: )
are you sure we need to do this at misc and not after aio is setup?
Line 162:
Line 163:
Line 164: # Defins OS param for the boot option
Line 165: os=params.OperatingSystem(type_='unassigned',
boot=[params.Boot(dev='cdrom'), params.Boot(dev='hd')])
Line 178: status=None,
Line 179: interface='virtio',
Line 180: format='cow',
Line 181: sparse=True,
Line 182: bootable=True))
the above format is invalid, please sync with reset of setup format.
Line 183: self.logger.debug("HD Created")
Line 184:
Line 185:
Line 186:
Line 185:
Line 186:
Line 187:
Line 188:
Line 189:
two spaces
Line 190:
Line 191:
Line 192:
....................................................
File fedora/oVirtLiveFiles/OLPlugin/__init__.py
Line 41: super_user.Plugin(context=context)
Line 42: vdsm.Plugin(context=context)
Line 43: storage.Plugin(context=context)
Line 44: firewall.Plugin(context=context)
Line 45:
two spaces
--
To view, visit http://gerrit.ovirt.org/17518
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I703f64dc1183a6fe176d9d0352f93de381d906bb
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-live
Gerrit-Branch: master
Gerrit-Owner: Ohad Basan <[email protected]>
Gerrit-Reviewer: Alex Lourie <[email protected]>
Gerrit-Reviewer: Alon Bar-Lev <[email protected]>
Gerrit-Reviewer: Eyal Edri <[email protected]>
Gerrit-Reviewer: Moran Goldboim <[email protected]>
Gerrit-Reviewer: Sandro Bonazzola <[email protected]>
Gerrit-Reviewer: Yedidyah Bar David <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
