Change in vdsm[master]: sdm: Add create_volume job

alitke at redhat.com alitke at redhat.com
Fri Apr 1 19:14:57 UTC 2016


Adam Litke has posted comments on this change.

Change subject: sdm: Add create_volume job
......................................................................


Patch Set 15:

(8 comments)

https://gerrit.ovirt.org/#/c/50221/8/vdsm/storage/hsm.py
File vdsm/storage/hsm.py:

Line 3688
Line 3689
Line 3690
Line 3691
Line 3692
> No need for name parameter, use the job.id or add job.name.
Done


Line 3695
Line 3696
Line 3697
Line 3698
Line 3699
> Leftover?
yep, removed,


Line 3701
Line 3702
Line 3703
Line 3704
Line 3705
> We don't need _params - we get a vol_info dict, and wrap it in CreateVolume
Done


https://gerrit.ovirt.org/#/c/50221/8/vdsm/storage/sdm/api/create_volume.py
File vdsm/storage/sdm/api/create_volume.py:

Line 31: 
Line 32: class Job(base.Job):
Line 33:     def __init__(self, job_id, host_id, sd_manifest, vol_info):
Line 34:         super(Job, self).__init__(job_id, 'create_volume', host_id)
Line 35:         self.sd_manifest = sd_manifest
> Lets use "sd" instead of "dom".
Done
Line 36:         self.vol_info = vol_info
Line 37: 
Line 38:     def _run(self):
Line 39:         vol_format = volume.name2type(self.vol_info.vol_format)


Line 54:                 artifacts.commit()
Line 55: 
Line 56: 
Line 57: # TODO: Adopt the properties framework for managing complex verb parameters
Line 58: 
> Too many parameter, can we pass vol_info as is?
Let's worry about this for a future patch.
Line 59: 
Line 60: class CreateVolumeInfo(object):
Line 61:     def __init__(self, params):
Line 62:         self.sd_id = _required(params, 'sd_id')


Line 59: 
Line 60: class CreateVolumeInfo(object):
Line 61:     def __init__(self, params):
Line 62:         self.sd_id = _required(params, 'sd_id')
Line 63:         self.img_id = _required(params, 'img_id')
> This error translation is bad, and hiding the original error. The concept o
Done
Line 64:         self.vol_id = _required(params, 'vol_id')
Line 65:         self.virtual_size = _required(params, 'virtual_size')
Line 66:         vol_types = [volume.VOLUME_TYPES[vt]
Line 67:                      for vt in (volume.RAW_FORMAT, volume.COW_FORMAT)]


Line 73:         self.initial_size = params.get('initial_size', 0)
Line 74: 
Line 75: 
Line 76: def _required(params, name):
Line 77:     if name not in params:
> This must a string in new code - no more magic integers in the api, but the
Agreed.  This evaluates to ['COW', 'RAW']
Line 78:         raise ValueError("Missing required property %r" % name)
Line 79:     return params[name]
Line 80: 
Line 81: 


Line 75: 
Line 76: def _required(params, name):
Line 77:     if name not in params:
Line 78:         raise ValueError("Missing required property %r" % name)
Line 79:     return params[name]
> This must a string in new code - no more magic integers in the api, but the
Agreed.  This evaluates to ['UNKNOWN', 'SYSTEM', 'DATA', 'SHARED', 'SWAP', 'TEMP']
Line 80: 
Line 81: 
Line 82: def _enum(params, name, values):
Line 83:     value = _required(params, name)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia614059f52c9625da7841ea9fbca2b2f2375cd75
Gerrit-PatchSet: 15
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke <alitke at redhat.com>
Gerrit-Reviewer: Adam Litke <alitke at redhat.com>
Gerrit-Reviewer: Ala Hino <ahino at redhat.com>
Gerrit-Reviewer: Allon Mureinik <amureini at redhat.com>
Gerrit-Reviewer: Amit Aviram <aaviram at redhat.com>
Gerrit-Reviewer: Daniel Erez <derez at redhat.com>
Gerrit-Reviewer: Freddy Rolland <frolland at redhat.com>
Gerrit-Reviewer: Greg Padgett <gpadgett at redhat.com>
Gerrit-Reviewer: Idan Shaby <ishaby at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Liron Aravot <laravot at redhat.com>
Gerrit-Reviewer: Maor Lipchuk <mlipchuk at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Tal Nisan <tnisan at redhat.com>
Gerrit-Reviewer: Vered Volansky <vvolansk at redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation at ovirt.org>
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list