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