Nir Soffer has posted comments on this change.

Change subject: storage: Initial size for thin provisioning disk
......................................................................


Patch Set 5:

(4 comments)

https://gerrit.ovirt.org/#/c/46417/5/client/vdsClient.py
File client/vdsClient.py:

Line 1062:         sdUUID = args[0]
Line 1063:         spUUID = args[1]
Line 1064:         imgUUID = args[2]
Line 1065:         diskSize = int(args[3])
Line 1066:         convertFactor = 2097152  # number of blocks in 1G
Wouldn't it be nicer as:

    BLOCKS_PER_GB = 2097152

And later:

    size = diskSize * BLOCK_PER_GB

This is still confusing, but the constant explain itself whenever you use it.
Line 1067:         size = diskSize * convertFactor
Line 1068:         volFormat = int(args[4])
Line 1069:         preallocate = int(args[5])
Line 1070:         diskType = int(args[6])


Line 1080:             srcVolUUID = BLANK_UUID
Line 1081:         params = [sdUUID, spUUID, imgUUID, size, volFormat, 
preallocate,
Line 1082:                   diskType, newVol, descr, srcImgUUID, srcVolUUID]
Line 1083:         if len(args) > 11:
Line 1084:             params.append(int(args[11]) * convertFactor)  # 
initialSize
It would be more clear and consistent with the other code like this:

    initialSize = int(args[11]) * convertFactor
    params.append(initialSize)

And now the comment is not needed.
Line 1085: 
Line 1086:         image = self.s.createVolume(*params)
Line 1087: 
Line 1088:         if image['status']['code']:


https://gerrit.ovirt.org/#/c/46417/5/vdsm/rpc/vdsmapi-schema.json
File vdsm/rpc/vdsmapi-schema.json:

Line 8268
Line 8269
Line 8270
Line 8271
Line 8272
Document that value that unit is bytes if sent as string and sectors if sent as 
integer.


Line 8283: # @srcImgUUID:       If specified, create a snapshot from this Image
Line 8284: #
Line 8285: # @srcVolUUID:       If specified, create a snapshot from this Volume
Line 8286: #
Line 8287: # @initialSize:      #optional if specified, initial size  in sectors
Document that value that unit is bytes if sent as string and sectors if sent as 
integer.
Line 8288: #                    of volume for thin provisioning on block 
storage.
Line 8289: #                    When using preallocated volumes or for file 
storage
Line 8290: #                    the initial size is ignored.
Line 8291: #                    (new in version 4.17.8)


-- 
To view, visit https://gerrit.ovirt.org/46417
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iaf5f142541bf0b311a77a0544a1c4ffb689a9fde
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Freddy Rolland <froll...@redhat.com>
Gerrit-Reviewer: Adam Litke <ali...@redhat.com>
Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com>
Gerrit-Reviewer: Arik Hadas <aha...@redhat.com>
Gerrit-Reviewer: Fred Rolland <froll...@redhat.com>
Gerrit-Reviewer: Freddy Rolland <froll...@redhat.com>
Gerrit-Reviewer: Greg Padgett <gpadg...@redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsof...@redhat.com>
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to