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