Change in vdsm[master]: sdm: add create_volume_container API stub

nsoffer at redhat.com nsoffer at redhat.com
Wed Dec 9 21:26:24 UTC 2015


Nir Soffer has posted comments on this change.

Change subject: sdm: add create_volume_container API stub
......................................................................


Patch Set 1: Code-Review-1

(10 comments)

https://gerrit.ovirt.org/#/c/50220/1/client/vdsClient.py
File client/vdsClient.py:

Line 1094:         return 0, image['uuid']
Line 1095: 
Line 1096:     def createVolumeContainer(self, args):
Line 1097:         (job_id, sd_id, img_id, vol_id,
Line 1098:          size, vol_format, disk_type, desc) = args[:8]
To many parameters X 4

It is impossible to sue such verb from the command line. This should get a json/dict input.
Line 1099: 
Line 1100:         parent_img_id = args[8] if len(args) > 8 else BLANK_UUID
Line 1101:         parent_vol_id = args[9] if len(args) > 9 else BLANK_UUID
Line 1102: 


https://gerrit.ovirt.org/#/c/50220/1/lib/api/vdsmapi-schema.json
File lib/api/vdsmapi-schema.json:

Line 8694: ##
Line 8695: {'class': 'SDM'}
Line 8696: 
Line 8697: ##
Line 8698: # @SDM.create_volume_container:
Nice!
Line 8699: #
Line 8700: # Create a new container to hold a volume.
Line 8701: #
Line 8702: # @job_id:         A UUID to be used for tracking the job progress


Line 8704: # @sd_id:          The Storage Domain associated with the Volume
Line 8705: #
Line 8706: # @image_id:       The Image associated with the Volume
Line 8707: #
Line 8708: # @volume_id:      The UUID of the Volume
Lets decide vol_id or volume_id, but not both. Since we use sd_id, lets keep img_id and vol_id.

This is new api, we must not copy errors from old interfaces.
Line 8709: #
Line 8710: # @size:           The Volume size in sectors
Line 8711: #
Line 8712: # @vol_format:     The data format to use for the destination Volume


Line 8706: # @image_id:       The Image associated with the Volume
Line 8707: #
Line 8708: # @volume_id:      The UUID of the Volume
Line 8709: #
Line 8710: # @size:           The Volume size in sectors
vol_size, for symmetry with vol_id, or virtual_size, for symmetry with initial_size.
Line 8711: #
Line 8712: # @vol_format:     The data format to use for the destination Volume
Line 8713: #
Line 8714: # @disk_type:      An advisory disk usage type


Line 8712: # @vol_format:     The data format to use for the destination Volume
Line 8713: #
Line 8714: # @disk_type:      An advisory disk usage type
Line 8715: #
Line 8716: # @desc:           The Volume description
description, or vol_description
Line 8717: #
Line 8718: # @parent_img_id:  #optional If specified, create a snapshot from this Image
Line 8719: #
Line 8720: # @parent_vol_id:  #optional If specified, create a snapshot from this Volume


Line 8718: # @parent_img_id:  #optional If specified, create a snapshot from this Image
Line 8719: #
Line 8720: # @parent_vol_id:  #optional If specified, create a snapshot from this Volume
Line 8721: #
Line 8722: # @initial_size:   #optional If specified, initial size of volume in bytes
Should be near the other volume parameters, this is a new api so we do not have to make it similar to the old api.
Line 8723: #                  (as string). Allowed only when creating thin provisioned
Line 8724: #                  volume on block storage.
Line 8725: #
Line 8726: # Since: 4.18.0


Line 8720: # @parent_vol_id:  #optional If specified, create a snapshot from this Volume
Line 8721: #
Line 8722: # @initial_size:   #optional If specified, initial size of volume in bytes
Line 8723: #                  (as string). Allowed only when creating thin provisioned
Line 8724: #                  volume on block storage.
How about packing the arguments in vol_info and parent_vol types?

- job_id
- vol_info - information needed to create a volume
- parent_vol - optional info about parent volume (can be also part of vol_info).

An api with so many parameters is horrible, we should stop doing it.

Any parameter you specify here must be implemented 5 times (xmlrpc.py, API.py, hsm.py, sdm/jobs/CreateVolumeContainer.py, vdsClient.py).
Line 8725: #
Line 8726: # Since: 4.18.0
Line 8727: ##
Line 8728: {'command': {'class': 'SDM', 'name': 'create_volume_container'},


https://gerrit.ovirt.org/#/c/50220/1/vdsm/API.py
File vdsm/API.py:

Line 1755:     def create_volume_container(self, job_id, sd_id, img_id, vol_id, size,
Line 1756:                                 vol_format, disk_type, desc,
Line 1757:                                 parent_img_id=Image.BLANK_UUID,
Line 1758:                                 parent_vol_id=Volume.BLANK_UUID,
Line 1759:                                 initial_size=None):
Too many parameters.
Line 1760:         return self._irs.create_volume_container(
Line 1761:             job_id, sd_id, img_id, vol_id, size, vol_format, disk_type, desc,


https://gerrit.ovirt.org/#/c/50220/1/vdsm/rpc/bindingxmlrpc.py
File vdsm/rpc/bindingxmlrpc.py:

Line 1029:         sdm = API.SDM()
Line 1030:         return sdm.create_volume_container(job_id, sd_id, img_id, vol_id, size,
Line 1031:                                            vol_format, disk_type, desc,
Line 1032:                                            parent_img_id, parent_vol_id,
Line 1033:                                            initial_size)
Too many parameters X 2
Line 1034: 
Line 1035:     def getGlobalMethods(self):
Line 1036:         return ((self.vmDestroy, 'destroy'),
Line 1037:                 (self.vmCreate, 'create'),


https://gerrit.ovirt.org/#/c/50220/1/vdsm/storage/hsm.py
File vdsm/storage/hsm.py:

Line 3687: 
Line 3688:     @public
Line 3689:     def create_volume_container(self, job_id, sd_id, img_id, vol_id, size,
Line 3690:                                 vol_format, disk_type, desc,
Line 3691:                                 parent_img_id, parent_vol_id, initial_size):
To many parameters X 3


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1ff2656f2dd427812e557e6587429759a9c0a845
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke <alitke at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation at ovirt.org>
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list