Nir Soffer has posted comments on this change.

Change subject: iscsi: checking image size against VG free space
......................................................................


Patch Set 6:

(2 comments)

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)
> > When you know the actual size available, you can display it, without any 
You cannot know and validate the vg size until you create it. Anything else is 
a guess that break any time when the underlying code changes.

Vdsm cannot not promise anything about the available space expect the vgfree 
value after you create the vg.

If you cannot customize the installation before creating the vg, you cannot 
offer the user to customize the image size.
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)
> This is the built-in pow function witch, as specified in the python docs, i
It does not matter if this is the built-in function. The code is less readable 
this way. The pow function is needed only when you deal with variables, not 
when you work with constant values.
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

Reply via email to