Change in vdsm[master]: sdm: Add create_volume job
gerrit-hooks has posted comments on this change. Change subject: sdm: Add create_volume job .. Patch Set 2: * Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- 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: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sdm: Add create_volume job
Nir Soffer has posted comments on this change. Change subject: sdm: Add create_volume job .. Patch Set 2: Code-Review-1 (5 comments) https://gerrit.ovirt.org/#/c/50221/2/vdsm/storage/sdm/api/sdm_job.py File vdsm/storage/sdm/api/sdm_job.py: Line 31: from .. import volume_artifacts Line 32: Line 33: Line 34: class SdmJob(jobs.Job): Line 35: log = logging.getLogger('Storage.SdmJob') Lets use only lowercase logger names in new code: "storage.sdmjob", or "storage.sdm" Line 36: Line 37: def __init__(self, job_id, desc, host_id): Line 38: super(SdmJob, self).__init__(job_id, desc) Line 39: self._error = None Line 35: log = logging.getLogger('Storage.SdmJob') Line 36: Line 37: def __init__(self, job_id, desc, host_id): Line 38: super(SdmJob, self).__init__(job_id, desc) Line 39: self._error = None Why not move error up to Job? Line 40: self._progress = None Line 41: self.host_id = host_id Line 42: Line 43: @property Line 45: return self._progress Line 46: Line 47: def info(self): Line 48: sdm_info = super(SdmJob, self).info() Line 49: sdm_info['extra_info'] = {'error': self._error} Why not add error to standard job info? Line 50: return sdm_info Line 51: Line 52: def run(self): Line 53: vars.job_id = self.id Line 58:self.id, self.description) Line 59: self._status = jobs.STATUS.FAILED Line 60: self._error = e Line 61: except Exception as e: Line 62: self.log.exception("Job (id:%s desc:%s) failed", We are using mostly (id=%s, desc=%s) in other places, and it is also the format used by Pyhton builtins such as namedtuple, so better keep this format. Line 63:self.id, self.description) Line 64: self._error = se.GENERAL_EXCEPTION(str(e)) Line 65: self._status = jobs.STATUS.FAILED Line 66: else: Line 60: self._error = e Line 61: except Exception as e: Line 62: self.log.exception("Job (id:%s desc:%s) failed", Line 63:self.id, self.description) Line 64: self._error = se.GENERAL_EXCEPTION(str(e)) This returns a tuple, but we want a GeneralExcpetion instance here. If creating a GeneralException instance works with current code (it should), lets use it. In the future, we should raise everywhere an error which has a response() method, and kill the error handling code in storage_exception.py. Line 65: self._status = jobs.STATUS.FAILED Line 66: else: Line 67: self._status = jobs.STATUS.DONE -- 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: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sdm: Add create_volume job
Nir Soffer has posted comments on this change. Change subject: sdm: Add create_volume job .. Patch Set 2: (2 comments) https://gerrit.ovirt.org/#/c/50221/2/vdsm/storage/sdm/api/create_volume.py File vdsm/storage/sdm/api/create_volume.py: Line 32: class Job(sdm_job.SdmJob): Line 33: def __init__(self, job_id, host_id, dom_manifest, vol_params): Line 34: super(Job, self).__init__(job_id, 'create_volume', host_id) Line 35: self.dom_manifest = dom_manifest Line 36: self.vol_params = _CreateVolumeParams(vol_params) Why not _VolumeParameters? Line 37: Line 38: def _run(self): Line 39: self.dom_manifest.validateCreateVolumeParams( Line 40: self.vol_params.vol_format, self.vol_params.parent_vol_id) Line 38: def _run(self): Line 39: self.dom_manifest.validateCreateVolumeParams( Line 40: self.vol_params.vol_format, self.vol_params.parent_vol_id) Line 41: Line 42: self.dom_manifest.acquireDomainLock(self.host_id) We can use now with dom_manifest.domain_lock(self.host_id) Line 43: try: Line 44: image_res_ns = sd.getNamespace(self.dom_manifest.sdUUID, Line 45:IMAGE_NAMESPACE) Line 46: with rmanager.acquireResource(image_res_ns, self.vol_params.img_id, -- 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: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sdm: Add create_volume job
Adam Litke has posted comments on this change. Change subject: sdm: Add create_volume job .. Patch Set 2: (7 comments) https://gerrit.ovirt.org/#/c/50221/2/vdsm/storage/sdm/api/create_volume.py File vdsm/storage/sdm/api/create_volume.py: Line 32: class Job(sdm_job.SdmJob): Line 33: def __init__(self, job_id, host_id, dom_manifest, vol_params): Line 34: super(Job, self).__init__(job_id, 'create_volume', host_id) Line 35: self.dom_manifest = dom_manifest Line 36: self.vol_params = _CreateVolumeParams(vol_params) > Why not _VolumeParameters? Changed to _CreateVolumeInfo for symmetry with the schema. Line 37: Line 38: def _run(self): Line 39: self.dom_manifest.validateCreateVolumeParams( Line 40: self.vol_params.vol_format, self.vol_params.parent_vol_id) Line 38: def _run(self): Line 39: self.dom_manifest.validateCreateVolumeParams( Line 40: self.vol_params.vol_format, self.vol_params.parent_vol_id) Line 41: Line 42: self.dom_manifest.acquireDomainLock(self.host_id) > We can use now with dom_manifest.domain_lock(self.host_id) Done Line 43: try: Line 44: image_res_ns = sd.getNamespace(self.dom_manifest.sdUUID, Line 45:IMAGE_NAMESPACE) Line 46: with rmanager.acquireResource(image_res_ns, self.vol_params.img_id, https://gerrit.ovirt.org/#/c/50221/2/vdsm/storage/sdm/api/sdm_job.py File vdsm/storage/sdm/api/sdm_job.py: Line 31: from .. import volume_artifacts Line 32: Line 33: Line 34: class SdmJob(jobs.Job): Line 35: log = logging.getLogger('Storage.SdmJob') > Lets use only lowercase logger names in new code: "storage.sdmjob", or "sto Done Line 36: Line 37: def __init__(self, job_id, desc, host_id): Line 38: super(SdmJob, self).__init__(job_id, desc) Line 39: self._error = None Line 35: log = logging.getLogger('Storage.SdmJob') Line 36: Line 37: def __init__(self, job_id, desc, host_id): Line 38: super(SdmJob, self).__init__(job_id, desc) Line 39: self._error = None > Why not move error up to Job? Done Line 40: self._progress = None Line 41: self.host_id = host_id Line 42: Line 43: @property Line 45: return self._progress Line 46: Line 47: def info(self): Line 48: sdm_info = super(SdmJob, self).info() Line 49: sdm_info['extra_info'] = {'error': self._error} > Why not add error to standard job info? Done Line 50: return sdm_info Line 51: Line 52: def run(self): Line 53: vars.job_id = self.id Line 58:self.id, self.description) Line 59: self._status = jobs.STATUS.FAILED Line 60: self._error = e Line 61: except Exception as e: Line 62: self.log.exception("Job (id:%s desc:%s) failed", > We are using mostly (id=%s, desc=%s) in other places, and it is also the fo Done Line 63:self.id, self.description) Line 64: self._error = se.GENERAL_EXCEPTION(str(e)) Line 65: self._status = jobs.STATUS.FAILED Line 66: else: Line 60: self._error = e Line 61: except Exception as e: Line 62: self.log.exception("Job (id:%s desc:%s) failed", Line 63:self.id, self.description) Line 64: self._error = se.GENERAL_EXCEPTION(str(e)) > This returns a tuple, but we want a GeneralExcpetion instance here. Now using utils.GeneralException Line 65: self._status = jobs.STATUS.FAILED Line 66: else: Line 67: self._status = jobs.STATUS.DONE -- 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: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sdm: Add create_volume job
Adam Litke has posted comments on this change. Change subject: sdm: Add create_volume job .. Patch Set 1: (17 comments) https://gerrit.ovirt.org/#/c/50221/1/tests/sdm_verbs_test.py File tests/sdm_verbs_test.py: Line 38: self.sdUUID = sd_id Line 39: Line 40: def validateCreateVolumeParams(self, *args): Line 41: pass Line 42: > Use @recorded and validate that the job acquired and released the lock? Done Line 43: def acquireDomainLock(self, *args): Line 44: pass Line 45: Line 46: def releaseDomainLock(self, *args): Line 53: class FakeVolumeArtifacts(object): Line 54: def __init__(self, img_id, vol_id): Line 55: self.img_id = img_id Line 56: self.vol_id = vol_id Line 57: > Use @recorded and validate the the job created and committed? Added TODO since I depend on the classmethod support in @recorded. Line 58: def create(self, *args): Line 59: pass Line 60: Line 61: def commit(self): Line 73: break Line 74: Line 75: Line 76: @expandPermutations Line 77: class CreateVolumeContainerTests(SdmVerbTests): > Lets have separate tests for each verb, and put common helpers in sdmtestli Done Line 78: Line 79: def _get_args(self): Line 80: dom_manifest = FakeDomainManifest(str(uuid.uuid4())) Line 81: return dict(dom_manifest=dom_manifest, job_id=str(uuid.uuid4()), https://gerrit.ovirt.org/#/c/50221/1/tests/storagefakelib.py File tests/storagefakelib.py: Line 218: return ''.join(random.choice(chars) for _ in range(size)) Line 219: return '-'.join(part(size) for size in [6, 4, 4, 4, 4, 6]) Line 220: Line 221: Line 222: class FakeResourceManager(object): > Nice! - lets also @record operation so we can validate the locking? I used a bit of custom recording logic to achieve this in the next version. Line 223: @contextmanager Line 224: def acquireResource(self, namespace, name, lockType, timeout=None): Line 225: yield Line 226: Line 224: def acquireResource(self, namespace, name, lockType, timeout=None): Line 225: yield Line 226: Line 227: def releaseResource(self, namespace, name): Line 228: pass > Split this into its own patch. Done https://gerrit.ovirt.org/#/c/50221/1/vdsm/storage/sdm/Makefile.am File vdsm/storage/sdm/Makefile.am: Line 17: # Line 18: # Refer to the README and COPYING files for full details of the license Line 19: # Line 20: Line 21: SUBDIRS = jobs > Good point, lets keep the package. Done Line 22: Line 23: include $(top_srcdir)/build-aux/Makefile.subs Line 24: Line 25: vdsmsdmdir = $(vdsmdir)/storage/sdm https://gerrit.ovirt.org/#/c/50221/1/vdsm/storage/sdm/jobs/Makefile.am File vdsm/storage/sdm/jobs/Makefile.am: Line 22: Line 23: vdsmsdmjobsdir = $(vdsmdir)/storage/sdm/jobs Line 24: Line 25: dist_vdsmsdmjobs_PYTHON = \ Line 26:createVolumeContainer.py \ > While you rename it, lets use "create_volume.py" Done Line 27:$(NULL) https://gerrit.ovirt.org/#/c/50221/1/vdsm/storage/sdm/jobs/createVolumeContainer.py File vdsm/storage/sdm/jobs/createVolumeContainer.py: Line 16: # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA Line 17: # Line 18: # Refer to the README and COPYING files for full details of the license Line 19: # Line 20: > Add this in every new module. Done Line 21: from vdsm import jobs Line 22: from storage import resourceManager as rm Line 23: from storage import blockVolume, fileVolume, sd Line 24: from storage.resourceFactories import IMAGE_NAMESPACE Line 21: from vdsm import jobs Line 22: from storage import resourceManager as rm Line 23: from storage import blockVolume, fileVolume, sd Line 24: from storage.resourceFactories import IMAGE_NAMESPACE Line 25: from storage.sdm import volumeartifacts > We can use relative imports to import stuff from storage and storage.sdm: Done Line 26: Line 27: import sdmJob Line 28: Line 29: rmanager = rm.ResourceManager.getInstance() Line 23: from storage import blockVolume, fileVolume, sd Line 24: from storage.resourceFactories import IMAGE_NAMESPACE Line 25: from storage.sdm import volumeartifacts Line 26: Line 27: import sdmJob > Use relative imports when importing stuff from same package: Done Line 28: Line 29: rmanager = rm.ResourceManager.getInstance() Line 30: Line 31: Line 47: self._progress = 0 Line 48: Line 49: @property Line 50: def progress(self): Line 51: return self._progress > Lets keep it, and return special progress value that engine will treat as i Let's go with None. It seems logical. Line 52: Line 53: def _run(self): Line 54: self.dom_manifest.validateCreateVolumeParams(self.vol_format, Line 55: self.parent_vol_id) Line 65: artifacts = self._get_artifacts_instance() Line 66: artifacts.create(sel
Change in vdsm[master]: sdm: Add create_volume job
gerrit-hooks has posted comments on this change. Change subject: sdm: Add create_volume job .. Patch Set 3: * Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- 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: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sdm: Add create_volume job
Nir Soffer has posted comments on this change. Change subject: sdm: Add create_volume job .. Patch Set 3: Code-Review-1 (15 comments) Partial review, I like this very much. https://gerrit.ovirt.org/#/c/50221/3/tests/sdm_create_volume_test.py File tests/sdm_create_volume_test.py: Line 38: Line 39: class FakeDomainManifest(object): Line 40: def __init__(self, sd_id): Line 41: self.sdUUID = sd_id Line 42: We can@record this an verify that a job verified the arguments. Lets also check the case when this raises. Line 43: def validateCreateVolumeParams(self, *args): Line 44: pass Line 45: Line 46: @recorded Line 73: self.vol_id = vol_id Line 74: Line 75: # TODO: record these calls and verify them in the tests. Line 76: Line 77: def create(self, *args): Lets test that we deal correctly with failures here (locks released, error reported) Line 78: pass Line 79: Line 80: def commit(self): Line 81: pass Line 76: Line 77: def create(self, *args): Line 78: pass Line 79: Line 80: def commit(self): Same Line 81: pass Line 82: Line 83: Line 84: class CreateVolumeTests(VdsmTestCase): Line 82: Line 83: Line 84: class CreateVolumeTests(VdsmTestCase): Line 85: Line 86: def _get_args(self): Better call this setUp, and set the needed test state in self. Line 87: job_id = str(uuid.uuid4()) Line 88: host_id = 1 Line 89: dom_manifest = FakeDomainManifest(str(uuid.uuid4())) Line 90: vol_info = dict(img_id=str(uuid.uuid4()), vol_id=str(uuid.uuid4()), Line 101: self.rm)]): Line 102: yield Line 103: Line 104: def test_create_volume(self): Line 105: args = self._get_args() You are re-inventing setUp() Line 106: job = storage.sdm.api.create_volume.Job(**args) Line 107: Line 108: with self._fake_env(): Line 109: job.run() Line 127: def test_create_volume_domainlock_contended(self): Line 128: def error(*args): Line 129: raise se.AcquireLockFailure('id', 'rc', 'out', 'err') Line 130: Line 131: args = self._get_args() You are re-inventing setUp() Line 132: args['dom_manifest'].acquireDomainLock = error Line 133: job = storage.sdm.api.create_volume.Job(**args) Line 134: job.run() Line 135: wait_for_job(job) https://gerrit.ovirt.org/#/c/50221/3/vdsm/storage/sdm/api/__init__.py File vdsm/storage/sdm/api/__init__.py: Line 17: # Line 18: # Refer to the README and COPYING files for full details of the license Line 19: # Line 20: Line 21: __all__ = [ Please avoid this, we don't want code that does "from sdm import *", adding 20 names to the global namespace. Line 22: 'create_volume', https://gerrit.ovirt.org/#/c/50221/3/vdsm/storage/sdm/api/create_volume.py File vdsm/storage/sdm/api/create_volume.py: Line 28: Line 29: rmanager = rm.ResourceManager.getInstance() Line 30: Line 31: Line 32: class Job(sdm_job.SdmJob): Please keep empty line after between class and __init__. Line 33: def __init__(self, job_id, host_id, dom_manifest, vol_info): Line 34: super(Job, self).__init__(job_id, 'create_volume', host_id) Line 35: self.dom_manifest = dom_manifest Line 36: self.vol_info = _CreateVolumeInfo(vol_info) Line 36: self.vol_info = _CreateVolumeInfo(vol_info) Line 37: Line 38: def _run(self): Line 39: self.dom_manifest.validateCreateVolumeParams( Line 40: self.vol_info.vol_format, self.vol_info.parent_vol_id) Since we validate vol_info in __init__, we should also validate these parameters in __init__. I would copy the logic from dom_manifest into CreateVolumeInfo, so we don't have to depend on this class method, or if you want to avoid the duplication, send the manifest to CreateVolumeInfo.__init__. Line 41: Line 42: with self.dom_manifest.domain_lock(self.host_id): Line 43: image_res_ns = sd.getNamespace(self.dom_manifest.sdUUID, Line 44:IMAGE_NAMESPACE) Line 50: self.vol_info.vol_id) Line 51: artifacts.create( Line 52: self.vol_info.size, self.vol_info.vol_format, Line 53: self.vol_info.disk_type, self.vol_info.desc, Line 54: self.vol_info.parent_vol_id) Wny not pass vol_info to artifacts.create? Line 55: artifacts.commit() Line 56: Line 57: # TODO: Adopt the properties framework for managing complex verb parameters Line 58: Line 56: Line 57: # TODO: Adopt the properties framework for managing complex verb parameters Line 58: Line 59: Line 60: class _CreateVolumeInfo(object): We don't need to keep this private. In the future, it would be nice if the bridge creates this class (based on the type name in
Change in vdsm[master]: sdm: Add create_volume job
gerrit-hooks has posted comments on this change. Change subject: sdm: Add create_volume job .. Patch Set 4: * Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- 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: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sdm: Add create_volume job
gerrit-hooks has posted comments on this change. Change subject: sdm: Add create_volume job .. Patch Set 5: * Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- 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: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sdm: Add create_volume job
gerrit-hooks has posted comments on this change. Change subject: sdm: Add create_volume job .. Patch Set 6: * Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- 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: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sdm: Add create_volume job
gerrit-hooks has posted comments on this change. Change subject: sdm: Add create_volume job .. Patch Set 7: * Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- 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: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Amit Aviram Gerrit-Reviewer: Daniel Erez Gerrit-Reviewer: Freddy Rolland Gerrit-Reviewer: Greg Padgett Gerrit-Reviewer: Idan Shaby Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Liron Aravot Gerrit-Reviewer: Maor Lipchuk Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Tal Nisan Gerrit-Reviewer: Vered Volansky Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sdm: Add create_volume job
gerrit-hooks has posted comments on this change. Change subject: sdm: Add create_volume job .. Patch Set 8: * Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- 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: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Amit Aviram Gerrit-Reviewer: Daniel Erez Gerrit-Reviewer: Freddy Rolland Gerrit-Reviewer: Greg Padgett Gerrit-Reviewer: Idan Shaby Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Liron Aravot Gerrit-Reviewer: Maor Lipchuk Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Tal Nisan Gerrit-Reviewer: Vered Volansky Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sdm: Add create_volume job
Nir Soffer has posted comments on this change. Change subject: sdm: Add create_volume job .. Patch Set 8: Code-Review-1 (13 comments) Partial review https://gerrit.ovirt.org/#/c/50221/8/vdsm/storage/hsm.py File vdsm/storage/hsm.py: Line 3688: their meaning. Line 3689: """ Line 3690: return {'domains': self.domainMonitor.getHostStatus(domains)} Line 3691: Line 3692: def sdm_schedule(self, name, job): No need for name parameter, use the job.id or add job.name. Line 3693: """ Line 3694: SDM jobs currently run using the old TaskManager thread pool but none Line 3695: of the other old Task features (ie. rollbacks, persistence) are Line 3696: supported. SDM tasks are managed using the Host Jobs API in jobs.py. Line 3695: of the other old Task features (ie. rollbacks, persistence) are Line 3696: supported. SDM tasks are managed using the Host Jobs API in jobs.py. Line 3697: """ Line 3698: def start_job(*args, **kwargs): Line 3699: pass Leftover? Line 3700: Line 3701: jobs.add(job) Line 3702: self.taskMng.scheduleJob("sdm", None, vars.task, name, job.run) Line 3703: Line 3701: jobs.add(job) Line 3702: self.taskMng.scheduleJob("sdm", None, vars.task, name, job.run) Line 3703: Line 3704: @public Line 3705: def sdm_create_volume(self, job_id, vol_info_params): We don't need _params - we get a vol_info dict, and wrap it in CreateVolumeInfo, replacing vol_info so we cannot touch it. vol_info = Wapper(vol_info) This is good pattern when you do this in the start of the function. In the future, the bridge will wrap the vol_info dict for us, based on the schema type. Line 3706: vol_info = sdm.api.create_volume.CreateVolumeInfo(vol_info_params) Line 3707: dom_manifest = sdCache.produce(vol_info.sd_id).manifest Line 3708: host_id = self.domainMonitor.getHostId(vol_info.sd_id) Line 3709: job = sdm.api.create_volume.Job(job_id, host_id, dom_manifest, https://gerrit.ovirt.org/#/c/50221/8/vdsm/storage/sdm/api/create_volume.py File vdsm/storage/sdm/api/create_volume.py: Line 31: rmanager = rm.ResourceManager.getInstance() Line 32: Line 33: Line 34: class Job(sdm_job.SdmJob): Line 35: def __init__(self, job_id, host_id, dom_manifest, vol_info): Lets use "sd" instead of "dom". Line 36: super(Job, self).__init__(job_id, 'create_volume', host_id) Line 37: self.dom_manifest = dom_manifest Line 38: self.vol_info = vol_info Line 39: Line 37: self.dom_manifest = dom_manifest Line 38: self.vol_info = vol_info Line 39: Line 40: def _run(self): Line 41: vol_format = volume.name2type(self.vol_info.vol_format) vol_info.vol_format can be the format enum - the api must use only strings, the code can use enums. If we use this system, we don't have to do this kind of translation by hand. Line 42: self.dom_manifest.validateCreateVolumeParams( Line 43: vol_format, self.vol_info.parent_vol_id) Line 44: Line 45: with self.dom_manifest.domain_lock(self.host_id): Line 54: try: Line 55: artifacts.create( Line 56: self.vol_info.virtual_size, vol_format, Line 57: self.vol_info.disk_type, self.vol_info.description, Line 58: self.vol_info.parent_vol_id) Too many parameter, can we pass vol_info as is? Line 59: artifacts.commit() Line 60: except (volume_artifacts.CannotCreateVolumeArtifacts, Line 61: volume_artifacts.VolumeArtifactsNotFound): Line 62: self.log.exception("Error creating volume artifacts") Line 59: artifacts.commit() Line 60: except (volume_artifacts.CannotCreateVolumeArtifacts, Line 61: volume_artifacts.VolumeArtifactsNotFound): Line 62: self.log.exception("Error creating volume artifacts") Line 63: raise se.VolumeCreationError() This error translation is bad, and hiding the original error. The concept of raising specific error in vdsm for each verb is usless, and we should stop it now. We should have set of common errors, and raise any of them from any verb, based the error (e.g OutOfSpace error is good for create volume or snapshot or most operations). Engine has all the context to show translated error about creating a volume, it does not need CreateVolumeError from vdsm. If can use more specific errors from vdsm like OutOfSpace or PermisionDenied, or NoSuchFile to show useful error message. For example: Cannot create volume "Foo" There is no available space on storage domain "Bar". The first part of the error is based on what engine knows. Th
Change in vdsm[master]: sdm: Add create_volume job
gerrit-hooks has posted comments on this change. Change subject: sdm: Add create_volume job .. Patch Set 9: * Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- 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: 9 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Amit Aviram Gerrit-Reviewer: Daniel Erez Gerrit-Reviewer: Freddy Rolland Gerrit-Reviewer: Greg Padgett Gerrit-Reviewer: Idan Shaby Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Liron Aravot Gerrit-Reviewer: Maor Lipchuk Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Tal Nisan Gerrit-Reviewer: Vered Volansky Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sdm: Add create_volume job
Nir Soffer has posted comments on this change. Change subject: sdm: Add create_volume job .. Patch Set 9: Code-Review-1 (3 comments) Nice https://gerrit.ovirt.org/#/c/50221/9/vdsm/storage/sdm/api/Makefile.am File vdsm/storage/sdm/api/Makefile.am: Line 24: Line 25: dist_vdsmsdmapi_PYTHON = \ Line 26:__init__.py \ Line 27:create_volume.py \ Line 28:sdm_job.py \ sdm_job.Job looks like the base class for all sdm.api..Job, so maybe call it base.py: # sdm/api/delete_image.py from . import base class Job(base.Job): ... https://gerrit.ovirt.org/#/c/50221/9/vdsm/storage/sdm/api/sdm_job.py File vdsm/storage/sdm/api/sdm_job.py: Line 37: self._status = jobs.STATUS.PENDING Line 38: self.host_id = host_id Line 39: Line 40: def run(self): Line 41: vars.job_id = self.id You missed my comments from previous versions: - Move vars.job_id = self.id just before the try: - Add finally: to clean it up Line 42: self._status = jobs.STATUS.RUNNING Line 43: try: Line 44: self._run() Line 45: except se.StorageException as e: Line 42: self._status = jobs.STATUS.RUNNING Line 43: try: Line 44: self._run() Line 45: except se.StorageException as e: Line 46: self.log.exception("Job (id:%s desc:%s) failed", Use (id=xxx desc=yyy) Line 47:self.id, self.description) Line 48: self._status = jobs.STATUS.FAILED Line 49: self._error = e Line 50: except Exception as e: -- 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: 9 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Amit Aviram Gerrit-Reviewer: Daniel Erez Gerrit-Reviewer: Freddy Rolland Gerrit-Reviewer: Greg Padgett Gerrit-Reviewer: Idan Shaby Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Liron Aravot Gerrit-Reviewer: Maor Lipchuk Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Tal Nisan Gerrit-Reviewer: Vered Volansky Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sdm: Add create_volume job
gerrit-hooks has posted comments on this change. Change subject: sdm: Add create_volume job .. Patch Set 10: * Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- 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: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Amit Aviram Gerrit-Reviewer: Daniel Erez Gerrit-Reviewer: Freddy Rolland Gerrit-Reviewer: Greg Padgett Gerrit-Reviewer: Idan Shaby Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Liron Aravot Gerrit-Reviewer: Maor Lipchuk Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Tal Nisan Gerrit-Reviewer: Vered Volansky Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sdm: Add create_volume job
Adam Litke has posted comments on this change. Change subject: sdm: Add create_volume job .. Patch Set 10: Verified+1 Verified with 'make check' and by doing LSM+Live Merge. -- 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: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Amit Aviram Gerrit-Reviewer: Daniel Erez Gerrit-Reviewer: Freddy Rolland Gerrit-Reviewer: Greg Padgett Gerrit-Reviewer: Idan Shaby Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Liron Aravot Gerrit-Reviewer: Maor Lipchuk Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Tal Nisan Gerrit-Reviewer: Vered Volansky Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sdm: Add create_volume job
Nir Soffer has posted comments on this change. Change subject: sdm: Add create_volume job .. Patch Set 10: Code-Review-1 (1 comment) https://gerrit.ovirt.org/#/c/50221/10/vdsm/storage/sdm/api/base.py File vdsm/storage/sdm/api/base.py: Line 50: except Exception as e: Line 51: self.log.exception("Job (id=%s desc=%s) failed", Line 52:self.id, self.description) Line 53: self._error = utils.GeneralException(str(e)) Line 54: self._status = jobs.STATUS.FAILED The only difference between the exception handlers is the wrapping of the exeption, so we can do this instead: except Exception as e: self.log.exception(...) if not isinstance(e, utils.GeneralException): e = utils.GeneralException(str(e)) self._error = e ... Line 55: else: Line 56: self._status = jobs.STATUS.DONE Line 57: finally: -- 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: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Amit Aviram Gerrit-Reviewer: Daniel Erez Gerrit-Reviewer: Freddy Rolland Gerrit-Reviewer: Greg Padgett Gerrit-Reviewer: Idan Shaby Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Liron Aravot Gerrit-Reviewer: Maor Lipchuk Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Tal Nisan Gerrit-Reviewer: Vered Volansky Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sdm: Add create_volume job
Adam Litke has posted comments on this change. Change subject: sdm: Add create_volume job .. Patch Set 10: (1 comment) https://gerrit.ovirt.org/#/c/50221/10/vdsm/storage/sdm/api/base.py File vdsm/storage/sdm/api/base.py: Line 50: except Exception as e: Line 51: self.log.exception("Job (id=%s desc=%s) failed", Line 52:self.id, self.description) Line 53: self._error = utils.GeneralException(str(e)) Line 54: self._status = jobs.STATUS.FAILED > The only difference between the exception handlers is the wrapping of the e Done Line 55: else: Line 56: self._status = jobs.STATUS.DONE Line 57: finally: -- 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: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Amit Aviram Gerrit-Reviewer: Daniel Erez Gerrit-Reviewer: Freddy Rolland Gerrit-Reviewer: Greg Padgett Gerrit-Reviewer: Idan Shaby Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Liron Aravot Gerrit-Reviewer: Maor Lipchuk Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Tal Nisan Gerrit-Reviewer: Vered Volansky Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sdm: Add create_volume job
gerrit-hooks has posted comments on this change. Change subject: sdm: Add create_volume job .. Patch Set 11: * Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- 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: 11 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Amit Aviram Gerrit-Reviewer: Daniel Erez Gerrit-Reviewer: Freddy Rolland Gerrit-Reviewer: Greg Padgett Gerrit-Reviewer: Idan Shaby Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Liron Aravot Gerrit-Reviewer: Maor Lipchuk Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Tal Nisan Gerrit-Reviewer: Vered Volansky Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sdm: Add create_volume job
Nir Soffer has posted comments on this change. Change subject: sdm: Add create_volume job .. Patch Set 11: Code-Review-1 (2 comments) partial review https://gerrit.ovirt.org/#/c/50221/11/vdsm/storage/sdm/api/create_volume.py File vdsm/storage/sdm/api/create_volume.py: Line 77: volume.BLANK_UUID) Line 78: self.initial_size = _get_param_default(params, 'initial_size', 0) Line 79: Line 80: Line 81: def _get_required(params, name): _get_xxx is ugly. How about _required() and _enum()? Line 82: if name not in params: Line 83: raise ValueError("Missing required property %r" % name) Line 84: return params[name] Line 85: Line 91: (value, name, values)) Line 92: return value Line 93: Line 94: Line 95: def _get_param_default(params, name, default): We can drop this, and use instead: self.foo = params.get("key", "default") Line 96: try: Line 97: return params[name] Line 98: except KeyError: -- 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: 11 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Amit Aviram Gerrit-Reviewer: Daniel Erez Gerrit-Reviewer: Freddy Rolland Gerrit-Reviewer: Greg Padgett Gerrit-Reviewer: Idan Shaby Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Liron Aravot Gerrit-Reviewer: Maor Lipchuk Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Tal Nisan Gerrit-Reviewer: Vered Volansky Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sdm: Add create_volume job
gerrit-hooks has posted comments on this change. Change subject: sdm: Add create_volume job .. Patch Set 12: * Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- 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: 12 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Amit Aviram Gerrit-Reviewer: Daniel Erez Gerrit-Reviewer: Freddy Rolland Gerrit-Reviewer: Greg Padgett Gerrit-Reviewer: Idan Shaby Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Liron Aravot Gerrit-Reviewer: Maor Lipchuk Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Tal Nisan Gerrit-Reviewer: Vered Volansky Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sdm: Add create_volume job
Adam Litke has posted comments on this change. Change subject: sdm: Add create_volume job .. Patch Set 11: (2 comments) https://gerrit.ovirt.org/#/c/50221/11/vdsm/storage/sdm/api/create_volume.py File vdsm/storage/sdm/api/create_volume.py: Line 77: volume.BLANK_UUID) Line 78: self.initial_size = _get_param_default(params, 'initial_size', 0) Line 79: Line 80: Line 81: def _get_required(params, name): > _get_xxx is ugly. How about _required() and _enum()? Done Line 82: if name not in params: Line 83: raise ValueError("Missing required property %r" % name) Line 84: return params[name] Line 85: Line 91: (value, name, values)) Line 92: return value Line 93: Line 94: Line 95: def _get_param_default(params, name, default): > We can drop this, and use instead: Done Line 96: try: Line 97: return params[name] Line 98: except KeyError: -- 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: 11 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Amit Aviram Gerrit-Reviewer: Daniel Erez Gerrit-Reviewer: Freddy Rolland Gerrit-Reviewer: Greg Padgett Gerrit-Reviewer: Idan Shaby Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Liron Aravot Gerrit-Reviewer: Maor Lipchuk Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Tal Nisan Gerrit-Reviewer: Vered Volansky Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sdm: Add create_volume job
Adam Litke has posted comments on this change. Change subject: sdm: Add create_volume job .. Patch Set 9: (2 comments) https://gerrit.ovirt.org/#/c/50221/9/vdsm/storage/sdm/api/sdm_job.py File vdsm/storage/sdm/api/sdm_job.py: Line 37: self._status = jobs.STATUS.PENDING Line 38: self.host_id = host_id Line 39: Line 40: def run(self): Line 41: vars.job_id = self.id > You missed my comments from previous versions: Done Line 42: self._status = jobs.STATUS.RUNNING Line 43: try: Line 44: self._run() Line 45: except se.StorageException as e: Line 42: self._status = jobs.STATUS.RUNNING Line 43: try: Line 44: self._run() Line 45: except se.StorageException as e: Line 46: self.log.exception("Job (id:%s desc:%s) failed", > Use (id=xxx desc=yyy) Done Line 47:self.id, self.description) Line 48: self._status = jobs.STATUS.FAILED Line 49: self._error = e Line 50: except Exception as e: -- 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: 9 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Amit Aviram Gerrit-Reviewer: Daniel Erez Gerrit-Reviewer: Freddy Rolland Gerrit-Reviewer: Greg Padgett Gerrit-Reviewer: Idan Shaby Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Liron Aravot Gerrit-Reviewer: Maor Lipchuk Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Tal Nisan Gerrit-Reviewer: Vered Volansky Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sdm: Add create_volume job
Nir Soffer has posted comments on this change. Change subject: sdm: Add create_volume job .. Patch Set 12: (1 comment) https://gerrit.ovirt.org/#/c/50221/12/vdsm/storage/sdm/api/base.py File vdsm/storage/sdm/api/base.py: Line 44: self._run() Line 45: except Exception as e: Line 46: self.log.exception("Job (id=%s desc=%s) failed", Line 47:self.id, self.description) Line 48: if not isinstance(e, utils.GeneralException): Needs a rebase, this exceptions is in lib/vdsm/exception.py Line 49: e = utils.GeneralException(str(e)) Line 50: self._error = e Line 51: self._status = jobs.STATUS.FAILED Line 52: else: -- 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: 12 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Amit Aviram Gerrit-Reviewer: Daniel Erez Gerrit-Reviewer: Freddy Rolland Gerrit-Reviewer: Greg Padgett Gerrit-Reviewer: Idan Shaby Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Liron Aravot Gerrit-Reviewer: Maor Lipchuk Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Tal Nisan Gerrit-Reviewer: Vered Volansky Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sdm: Add create_volume job
gerrit-hooks has posted comments on this change. Change subject: sdm: Add create_volume job .. Patch Set 13: * Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- 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: 13 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Amit Aviram Gerrit-Reviewer: Daniel Erez Gerrit-Reviewer: Freddy Rolland Gerrit-Reviewer: Greg Padgett Gerrit-Reviewer: Idan Shaby Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Liron Aravot Gerrit-Reviewer: Maor Lipchuk Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Tal Nisan Gerrit-Reviewer: Vered Volansky Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sdm: Add create_volume job
gerrit-hooks has posted comments on this change. Change subject: sdm: Add create_volume job .. Patch Set 14: * Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- 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: 14 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Amit Aviram Gerrit-Reviewer: Daniel Erez Gerrit-Reviewer: Freddy Rolland Gerrit-Reviewer: Greg Padgett Gerrit-Reviewer: Idan Shaby Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Liron Aravot Gerrit-Reviewer: Maor Lipchuk Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Tal Nisan Gerrit-Reviewer: Vered Volansky Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sdm: Add create_volume job
gerrit-hooks has posted comments on this change. Change subject: sdm: Add create_volume job .. Patch Set 15: * Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- 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 Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Amit Aviram Gerrit-Reviewer: Daniel Erez Gerrit-Reviewer: Freddy Rolland Gerrit-Reviewer: Greg Padgett Gerrit-Reviewer: Idan Shaby Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Liron Aravot Gerrit-Reviewer: Maor Lipchuk Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Tal Nisan Gerrit-Reviewer: Vered Volansky Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sdm: Add create_volume job
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 Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Amit Aviram Gerrit-Reviewer: Daniel Erez Gerrit-Reviewer: Freddy Rolland Gerrit-Reviewer: Greg Padgett Gerrit-Reviewer: Idan Shaby Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Liron Aravot Gerrit-Reviewer: Maor Lipchuk Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Tal Nisan Gerrit-Reviewer: Vered Volansky Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sdm: Add create_volume job
Adam Litke has posted comments on this change. Change subject: sdm: Add create_volume job .. Patch Set 15: Verified+1 -- 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 Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Amit Aviram Gerrit-Reviewer: Daniel Erez Gerrit-Reviewer: Freddy Rolland Gerrit-Reviewer: Greg Padgett Gerrit-Reviewer: Idan Shaby Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Liron Aravot Gerrit-Reviewer: Maor Lipchuk Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Tal Nisan Gerrit-Reviewer: Vered Volansky Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sdm: Add create_volume job
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 Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Amit Aviram Gerrit-Reviewer: Daniel Erez Gerrit-Reviewer: Freddy Rolland Gerrit-Reviewer: Greg Padgett Gerrit-Reviewer: Idan Shaby Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Liron Aravot Gerrit-Reviewer: Maor Lipchuk Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Tal Nisan Gerrit-Reviewer: Vered Volansky Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sdm: Add create_volume job
Adam Litke has posted comments on this change. Change subject: sdm: Add create_volume job .. Patch Set 15: (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 Nope. Using base exceptions from vdsm only 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 domai Right now the only way to get the host ID is to look up the domainMonitor from the cif singleton and get it from there. For one, that is not clean at all and we are going to be making some changes in that area as part of the sdm pool refactoring later. I prefer to keep this simple for now and we can always drop the host id parameter later if there ends up being a better way to look it up. 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 As stated in another response, let's leave this change until after the pool refactoring. 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? Because InvalidParameterException is for reporting an invalid value, not a missing value. I can switch to exception.MissingParameter instead. 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 Done -- 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 Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Amit Aviram Gerrit-Reviewer: Daniel Erez Gerrit-Reviewer: Freddy Rolland Gerrit-Reviewer: Greg Padgett Gerrit-Reviewer: Idan Shaby Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Liron Aravot Gerrit-Reviewer: Maor Lipchuk Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Tal Nisan Gerrit-Reviewer: Vered Volansky Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sdm: Add create_volume job
gerrit-hooks has posted comments on this change. Change subject: sdm: Add create_volume job .. Patch Set 16: * Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- 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: 16 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Amit Aviram Gerrit-Reviewer: Daniel Erez Gerrit-Reviewer: Freddy Rolland Gerrit-Reviewer: Greg Padgett Gerrit-Reviewer: Idan Shaby Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Liron Aravot Gerrit-Reviewer: Maor Lipchuk Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Tal Nisan Gerrit-Reviewer: Vered Volansky Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sdm: Add create_volume job
Adam Litke has posted comments on this change. Change subject: sdm: Add create_volume job .. Patch Set 16: Verified+1 -- 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: 16 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Amit Aviram Gerrit-Reviewer: Daniel Erez Gerrit-Reviewer: Freddy Rolland Gerrit-Reviewer: Greg Padgett Gerrit-Reviewer: Idan Shaby Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Liron Aravot Gerrit-Reviewer: Maor Lipchuk Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Tal Nisan Gerrit-Reviewer: Vered Volansky Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sdm: Add create_volume job
Nir Soffer has posted comments on this change. Change subject: sdm: Add create_volume job .. Patch Set 15: (4 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 > Nope. Using base exceptions from vdsm only OK 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 > Right now the only way to get the host ID is to look up the domainMonitor f OK 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): > As stated in another response, let's leave this change until after the pool OK 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) > Because InvalidParameterException is for reporting an invalid value, not a MissingParameter sounds good. Line 79: return params[name] Line 80: Line 81: Line 82: def _enum(params, name, values): -- 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 Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Amit Aviram Gerrit-Reviewer: Daniel Erez Gerrit-Reviewer: Freddy Rolland Gerrit-Reviewer: Greg Padgett Gerrit-Reviewer: Idan Shaby Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Liron Aravot Gerrit-Reviewer: Maor Lipchuk Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Tal Nisan Gerrit-Reviewer: Vered Volansky Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sdm: Add create_volume job
Nir Soffer has posted comments on this change. Change subject: sdm: Add create_volume job .. Patch Set 16: Code-Review+2 -- 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: 16 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Amit Aviram Gerrit-Reviewer: Daniel Erez Gerrit-Reviewer: Freddy Rolland Gerrit-Reviewer: Greg Padgett Gerrit-Reviewer: Idan Shaby Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Liron Aravot Gerrit-Reviewer: Maor Lipchuk Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Tal Nisan Gerrit-Reviewer: Vered Volansky Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sdm: Add create_volume job
Nir Soffer has posted comments on this change. Change subject: sdm: Add create_volume job .. Patch Set 16: Looks good, but lets have another review. -- 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: 16 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Amit Aviram Gerrit-Reviewer: Daniel Erez Gerrit-Reviewer: Freddy Rolland Gerrit-Reviewer: Greg Padgett Gerrit-Reviewer: Idan Shaby Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Liron Aravot Gerrit-Reviewer: Maor Lipchuk Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Tal Nisan Gerrit-Reviewer: Vered Volansky Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sdm: Add create_volume job
Freddy Rolland has posted comments on this change. Change subject: sdm: Add create_volume job .. Patch Set 16: Code-Review-1 (2 comments) Please remove vdsm/storage/sdm/jobs/__init__.py https://gerrit.ovirt.org/#/c/50221/16/vdsm/storage/sdm/api/create_volume.py File vdsm/storage/sdm/api/create_volume.py: PS16, Line 79: def _required(params, name): I guess we will need '_required' and '_enum' in a lot of other places. Maybe we should have it in some common util module? https://gerrit.ovirt.org/#/c/50221/16/vdsm/storage/sdm/jobs/__init__.py File vdsm/storage/sdm/jobs/__init__.py: This file is not needed -- 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: 16 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Amit Aviram Gerrit-Reviewer: Daniel Erez Gerrit-Reviewer: Freddy Rolland Gerrit-Reviewer: Greg Padgett Gerrit-Reviewer: Idan Shaby Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Liron Aravot Gerrit-Reviewer: Maor Lipchuk Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Tal Nisan Gerrit-Reviewer: Vered Volansky Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sdm: Add create_volume job
gerrit-hooks has posted comments on this change. Change subject: sdm: Add create_volume job .. Patch Set 17: * Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- 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: 17 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Amit Aviram Gerrit-Reviewer: Daniel Erez Gerrit-Reviewer: Freddy Rolland Gerrit-Reviewer: Greg Padgett Gerrit-Reviewer: Idan Shaby Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Liron Aravot Gerrit-Reviewer: Maor Lipchuk Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Tal Nisan Gerrit-Reviewer: Vered Volansky Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sdm: Add create_volume job
Adam Litke has posted comments on this change. Change subject: sdm: Add create_volume job .. Patch Set 16: (2 comments) https://gerrit.ovirt.org/#/c/50221/16/vdsm/storage/sdm/api/create_volume.py File vdsm/storage/sdm/api/create_volume.py: Line 75: self.parent_vol_id = params.get('parent_vol_id', volume.BLANK_UUID) Line 76: self.initial_size = params.get('initial_size', 0) Line 77: Line 78: Line 79: def _required(params, name): > I guess we will need '_required' and '_enum' in a lot of other places. Yes, we talked about this. Nir has a series that will address this but I don't want to hold up sdm work on it. For now, we'll use sdm-specific validation routines but switch to the shared version once it hits the repo. https://gerrit.ovirt.org/#/q/status:open+project:vdsm+branch:master+topic:properties Line 80: if name not in params: Line 81: raise exception.MissingParameter() Line 82: return params[name] Line 83: https://gerrit.ovirt.org/#/c/50221/16/vdsm/storage/sdm/jobs/__init__.py File vdsm/storage/sdm/jobs/__init__.py: > This file is not needed Done -- 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: 16 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Amit Aviram Gerrit-Reviewer: Daniel Erez Gerrit-Reviewer: Freddy Rolland Gerrit-Reviewer: Greg Padgett Gerrit-Reviewer: Idan Shaby Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Liron Aravot Gerrit-Reviewer: Maor Lipchuk Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Tal Nisan Gerrit-Reviewer: Vered Volansky Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sdm: Add create_volume job
Adam Litke has posted comments on this change. Change subject: sdm: Add create_volume job .. Patch Set 17: Verified+1 -- 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: 17 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Amit Aviram Gerrit-Reviewer: Daniel Erez Gerrit-Reviewer: Freddy Rolland Gerrit-Reviewer: Greg Padgett Gerrit-Reviewer: Idan Shaby Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Liron Aravot Gerrit-Reviewer: Maor Lipchuk Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Tal Nisan Gerrit-Reviewer: Vered Volansky Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sdm: Add create_volume job
Freddy Rolland has posted comments on this change. Change subject: sdm: Add create_volume job .. Patch Set 17: Code-Review+1 -- 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: 17 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Amit Aviram Gerrit-Reviewer: Daniel Erez Gerrit-Reviewer: Freddy Rolland Gerrit-Reviewer: Greg Padgett Gerrit-Reviewer: Idan Shaby Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Liron Aravot Gerrit-Reviewer: Maor Lipchuk Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Tal Nisan Gerrit-Reviewer: Vered Volansky Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sdm: Add create_volume job
Nir Soffer has submitted this change and it was merged. Change subject: sdm: Add create_volume job .. sdm: Add create_volume job This is the first SDM verb to be wired up using all of the infrastructure (jobs and getHostJobs API, VolumeArtifacts, etc.). HSM creates a create_volume.Job and schedules it using the existing TaskManager thread pool. Eventually we can switch to using the executor framework but that can be for a later patch. Change-Id: Ia614059f52c9625da7841ea9fbca2b2f2375cd75 Signed-off-by: Adam Litke Reviewed-on: https://gerrit.ovirt.org/50221 Reviewed-by: Freddy Rolland Continuous-Integration: Jenkins CI Reviewed-by: Nir Soffer --- M configure.ac M debian/vdsm.install M tests/Makefile.am A tests/sdmtestlib.py A tests/storage_sdm_api_test.py A tests/storage_sdm_create_volume_test.py M vdsm.spec.in M vdsm/storage/hsm.py M vdsm/storage/sdm/Makefile.am A vdsm/storage/sdm/api/Makefile.am A vdsm/storage/sdm/api/__init__.py A vdsm/storage/sdm/api/base.py A vdsm/storage/sdm/api/create_volume.py 13 files changed, 480 insertions(+), 1 deletion(-) Approvals: Nir Soffer: Looks good to me, approved Adam Litke: Verified Jenkins CI: Passed CI tests Freddy Rolland: Looks good to me, but someone else must approve -- To view, visit https://gerrit.ovirt.org/50221 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: merged Gerrit-Change-Id: Ia614059f52c9625da7841ea9fbca2b2f2375cd75 Gerrit-PatchSet: 18 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Amit Aviram Gerrit-Reviewer: Daniel Erez Gerrit-Reviewer: Freddy Rolland Gerrit-Reviewer: Greg Padgett Gerrit-Reviewer: Idan Shaby Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Liron Aravot Gerrit-Reviewer: Maor Lipchuk Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Tal Nisan Gerrit-Reviewer: Vered Volansky Gerrit-Reviewer: gerrit-hooks ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sdm: Add create_volume job
Nir Soffer has posted comments on this change. Change subject: sdm: Add create_volume job .. Patch Set 17: Code-Review+2 -- 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: 17 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Amit Aviram Gerrit-Reviewer: Daniel Erez Gerrit-Reviewer: Freddy Rolland Gerrit-Reviewer: Greg Padgett Gerrit-Reviewer: Idan Shaby Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Liron Aravot Gerrit-Reviewer: Maor Lipchuk Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Tal Nisan Gerrit-Reviewer: Vered Volansky Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sdm: Add create_volume job
gerrit-hooks has posted comments on this change. Change subject: sdm: Add create_volume job .. Patch Set 18: * Update tracker: IGNORE, no Bug-Url found * Set MODIFIED::IGNORE, no Bug-Url found. -- 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: 18 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Amit Aviram Gerrit-Reviewer: Daniel Erez Gerrit-Reviewer: Freddy Rolland Gerrit-Reviewer: Greg Padgett Gerrit-Reviewer: Idan Shaby Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Liron Aravot Gerrit-Reviewer: Maor Lipchuk Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Tal Nisan Gerrit-Reviewer: Vered Volansky Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches