Nir Soffer has posted comments on this change. Change subject: iscsi: checking image size against VG free space ......................................................................
Patch Set 6: (5 comments) You should not guess and estimate. Let the user choose the device showing the raw device size. Then let the user set the image size based on the available free space. Users will learn that there is some overhead when creating an an image, we don't promised what we will this overhead. http://gerrit.ovirt.org/#/c/34940/6/src/ovirt_hosted_engine_setup/constants.py File src/ovirt_hosted_engine_setup/constants.py: Line 269: Line 270: @util.export Line 271: @util.codegen Line 272: class Const(object): Line 273: MINIMUM_SPACE_STORAGEDOMAIN_MB = 20480 This value needs explanation. Line 274: FIRST_HOST_ID = 1 Line 275: HA_AGENT_SERVICE = 'ovirt-ha-agent' Line 276: HA_BROCKER_SERVICE = 'ovirt-ha-broker' Line 277: HOSTED_ENGINE_VM_NAME = 'HostedEngine' Line 274: FIRST_HOST_ID = 1 Line 275: HA_AGENT_SERVICE = 'ovirt-ha-agent' Line 276: HA_BROCKER_SERVICE = 'ovirt-ha-broker' Line 277: HOSTED_ENGINE_VM_NAME = 'HostedEngine' Line 278: OHD_OVERHEAD_GIB = 1 This is not clear - what is OHD, and what is the mysterious overhead? Needs a better name and couple lines of comments explaining this the next developer hacking in this area. Line 279: METADATA_CHUNK_SIZE = 4096 Line 280: MAX_HOST_ID = 250 Line 281: HA_NOTIF_SMTP_SERVER = 'smtp-server' Line 282: HA_NOTIF_SMTP_PORT = 'smtp-port' http://gerrit.ovirt.org/#/c/34940/6/src/plugins/ovirt-hosted-engine-setup/storage/iscsi.py File src/plugins/ovirt-hosted-engine-setup/storage/iscsi.py: Line 335: ) Line 336: ) Line 337: self.environment[ Line 338: ohostedcons.StorageEnv.BDEVICE_SIZE_GB Line 339: ] = size_mb / pow(2, 10) Use: size_mb / MB_PER_GB Or: size_mb / 1024 Or: size_mb / 2**10 Line 340: if self.environment[ Line 341: ohostedcons.StorageEnv.VG_UUID Line 342: ] is not None: Line 343: if device['vgUUID'] != self.environment[ http://gerrit.ovirt.org/#/c/34940/6/src/plugins/ovirt-hosted-engine-setup/vm/image.py File src/plugins/ovirt-hosted-engine-setup/vm/image.py: Line 94: # could be gathered from vginfo but at this point Line 95: # it still has to be created Line 96: available_gb = int(self.environment[ Line 97: ohostedcons.StorageEnv.BDEVICE_SIZE_GB Line 98: ]) - int(ohostedcons.Const.OHD_OVERHEAD_GIB) This is not available_gb, you don't know the availale gb at this point. The only thing you can show the device size in GiB - which you already have in BDEVICE_SIZE_GB. If you want to estimate the overhead of the vg, you can hardcode the overhead we currently use, which is 348MiB. But I would not do this. When you know the actual size available, you can display it, without any guesswork. Line 99: Line 100: valid = False Line 101: while not valid: Line 102: if interactive: Line 196: self.logger.debug(vginfo) Line 197: if vginfo['status']['code'] != 0: Line 198: raise RuntimeError(vginfo['status']['message']) Line 199: vgfree = int(vginfo['info']['vgfree']) Line 200: available_gb = vgfree / pow(2, 30) Use 2**30, or a constant - using pow() is the worst possible way to do this. Line 201: if self.environment[ Line 202: ohostedcons.StorageEnv.IMAGE_SIZE_GB Line 203: ] > available_gb: Line 204: raise ohosteddomains.InsufficientSpaceError( -- To view, visit http://gerrit.ovirt.org/34940 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I17f861a053f6ac38983967ffb07ecca9ff9b8de1 Gerrit-PatchSet: 6 Gerrit-Project: ovirt-hosted-engine-setup Gerrit-Branch: master Gerrit-Owner: Simone Tiraboschi <stira...@redhat.com> Gerrit-Reviewer: Lev Veyde <lve...@redhat.com> Gerrit-Reviewer: Nir Soffer <nsof...@redhat.com> Gerrit-Reviewer: Sandro Bonazzola <sbona...@redhat.com> Gerrit-Reviewer: Simone Tiraboschi <stira...@redhat.com> Gerrit-Reviewer: Yedidyah Bar David <d...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
