Sandro Bonazzola has posted comments on this change.

Change subject: iscsi: add HE disk to the engine as direct lun
......................................................................


Patch Set 2:

(3 comments)

http://gerrit.ovirt.org/#/c/34783/2/src/plugins/ovirt-hosted-engine-setup/engine/add_disk.py
File src/plugins/ovirt-hosted-engine-setup/engine/add_disk.py:

Line 71:             
username=self.environment[ohostedcons.StorageEnv.ISCSI_USER],
Line 72:             
password=self.environment[ohostedcons.StorageEnv.ISCSI_PASSWORD],
Line 73:         )
Line 74:         disk = ovirtsdk.xml.params.Disk(
Line 75:             alias='direct_lun',
> If it is visible in the gui, should be configurable imo.
Make sense, yes, it's visible in the UI. My only concern is about how to 
identify it for making it clear to the engine that this is the disk of the 
hosted engine VM.
Any hint from storage guys?
Line 76:             
description=self.environment[ohostedcons.StorageEnv.IMAGE_DESC],
Line 77:             interface='virtio_scsi',
Line 78:             sgio='unfiltered',
Line 79:             format='raw',


Line 85:             ),
Line 86:         )
Line 87:         try:
Line 88:             self.logger.debug('Connecting to the Engine')
Line 89:             engine_api = ovirtsdk.api.API(
> We already connect to the engine in add_host, perhaps better keep this conn
Not sure it's wise to keep the connection in the env. 
It will introduce another substage dependency in closeup stage just for closing 
the connection cleanly.
Line 90:                 url='https://{fqdn}/ovirt-engine/api'.format(
Line 91:                     fqdn=self.environment[
Line 92:                         ohostedcons.NetworkEnv.OVIRT_HOSTED_ENGINE_FQDN
Line 93:                     ],


Line 99:                 ca_file=self.environment[
Line 100:                     ohostedcons.EngineEnv.TEMPORARY_CERT_FILE
Line 101:                 ],
Line 102:             )
Line 103:             engine_api.disks.add(disk)
> What if the disk already exists? No idea if that's possible, but might be -
Good point, better to handle the case or at least check what happens. I'll do.
Line 104:         except ovirtsdk.infrastructure.errors.RequestError:
Line 105:             self.logger.debug(
Line 106:                 'Cannot add the Hosted Engine VM Disk to the engine',
Line 107:                 exc_info=True,


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I9c5640bde713fc4d09cf74a7037c62e833495934
Gerrit-PatchSet: 2
Gerrit-Project: ovirt-hosted-engine-setup
Gerrit-Branch: master
Gerrit-Owner: Sandro Bonazzola <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Doron Fediuck <[email protected]>
Gerrit-Reviewer: Jiří Moskovčák <[email protected]>
Gerrit-Reviewer: Juan Hernandez <[email protected]>
Gerrit-Reviewer: Lev Veyde <[email protected]>
Gerrit-Reviewer: Martin Sivák <[email protected]>
Gerrit-Reviewer: Nir Soffer <[email protected]>
Gerrit-Reviewer: Sandro Bonazzola <[email protected]>
Gerrit-Reviewer: Simone Tiraboschi <[email protected]>
Gerrit-Reviewer: Tal Nisan <[email protected]>
Gerrit-Reviewer: Yedidyah Bar David <[email protected]>
Gerrit-Reviewer: [email protected]
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to