Nir Soffer has posted comments on this change. Change subject: sdm: Add create_volume job ......................................................................
Patch Set 15: Code-Review-1 (5 comments) https://gerrit.ovirt.org/#/c/50221/15/vdsm/storage/sdm/api/base.py File vdsm/storage/sdm/api/base.py: Line 22: Line 23: import logging Line 24: Line 25: from vdsm import jobs Line 26: from vdsm import exception This is now in vdsm.storage Line 27: Line 28: from storage.threadLocal import vars Line 29: Line 30: Line 34: Line 35: def __init__(self, job_id, desc, host_id): Line 36: super(Job, self).__init__(job_id, desc) Line 37: self._status = jobs.STATUS.PENDING Line 38: self.host_id = host_id This works now, but we would like to support different id per storage domain. Better get the host id when you take the domain(s) lock. If you don't find an easy way we can work on this later. Line 39: Line 40: def run(self): Line 41: self._status = jobs.STATUS.RUNNING Line 42: vars.job_id = self.id https://gerrit.ovirt.org/#/c/50221/15/vdsm/storage/sdm/api/create_volume.py File vdsm/storage/sdm/api/create_volume.py: Line 39: vol_format = volume.name2type(self.vol_info.vol_format) Line 40: self.sd_manifest.validateCreateVolumeParams( Line 41: vol_format, self.vol_info.parent_vol_id) Line 42: Line 43: with self.sd_manifest.domain_lock(self.host_id): We should make the domain host id accessible to the manifest object, so we don't have to pass the host id. We want to support different host id per domain. If you don't find an easy way, we can work on this later. Line 44: image_res_ns = sd.getNamespace(self.sd_manifest.sdUUID, Line 45: IMAGE_NAMESPACE) Line 46: with rmanager.acquireResource(image_res_ns, self.vol_info.img_id, Line 47: rm.LockType.exclusive): Line 74: Line 75: Line 76: def _required(params, name): Line 77: if name not in params: Line 78: raise ValueError("Missing required property %r" % name) Why not use se.InvalidParameterException? Line 79: return params[name] Line 80: Line 81: Line 82: def _enum(params, name, values): Line 82: def _enum(params, name, values): Line 83: value = _required(params, name) Line 84: if value not in values: Line 85: raise ValueError("Invalid value %r for %r, expecting one of %s" % Line 86: (value, name, values)) Same -- 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 <ali...@redhat.com> Gerrit-Reviewer: Adam Litke <ali...@redhat.com> Gerrit-Reviewer: Ala Hino <ah...@redhat.com> Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com> Gerrit-Reviewer: Amit Aviram <aavi...@redhat.com> Gerrit-Reviewer: Daniel Erez <de...@redhat.com> Gerrit-Reviewer: Freddy Rolland <froll...@redhat.com> Gerrit-Reviewer: Greg Padgett <gpadg...@redhat.com> Gerrit-Reviewer: Idan Shaby <ish...@redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Liron Aravot <lara...@redhat.com> Gerrit-Reviewer: Maor Lipchuk <mlipc...@redhat.com> Gerrit-Reviewer: Nir Soffer <nsof...@redhat.com> Gerrit-Reviewer: Tal Nisan <tni...@redhat.com> Gerrit-Reviewer: Vered Volansky <vvola...@redhat.com> Gerrit-Reviewer: gerrit-hooks <automat...@ovirt.org> Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
