Change in vdsm[master]: virt: Move all guest agents related cleanup together
Michal Skrivanek has posted comments on this change. Change subject: virt: Move all guest agents related cleanup together .. Patch Set 1: Code-Review-1 (keeping -1 till we decide if we want to swallow all errors on cleanup or not) -- To view, visit http://gerrit.ovirt.org/26280 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I99da4b5b9dcb160974205ebbb3c6d6727415e617 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Vinzenz Feenstra Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: Move all guest agents related cleanup together
Michal Skrivanek has posted comments on this change. Change subject: virt: Move all guest agents related cleanup together .. Patch Set 1: -Code-Review (1 comment) http://gerrit.ovirt.org/#/c/26280/1/vdsm/virt/vm.py File vdsm/virt/vm.py: Line 2786: except Exception: Line 2787: pass Line 2788: Line 2789: self._guestSockCleanup(self._guestSocketFile) Line 2790: self._guestSockCleanup(self._qemuguestSocketFile) > I would like to see "except Exception" gone, or at least limited to probabl @Dan: well, then we need to decide if we want to make the code as you noted above to succeed in all possible and impossible cases. We basically have "try…except: pass" around everything, even the rmFile at line 2776. If we want to keep such careful behavior it would be better to have a try/catch arounf the whole cleanup routine instead of x-times inside each subfunction. That was my motivation. (I'd say not in all cases the failure to remove the socket means it can't be used again) I don't think we have much of a choice when things our out of our control. I think in case of a failure in cleanup we should note an error but continue with the flow hoping it will work next time. Line 2791: Line 2792: def setDownStatus(self, code, exitReasonCode, exitMessage=''): Line 2793: if not exitMessage: Line 2794: exitMessage = vmexitreason.exitReasons.get(exitReasonCode, -- To view, visit http://gerrit.ovirt.org/26280 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I99da4b5b9dcb160974205ebbb3c6d6727415e617 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Vinzenz Feenstra Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: pep8: clean remaining E713
Francesco Romani has posted comments on this change. Change subject: pep8: clean remaining E713 .. Patch Set 1: Code-Review+1 -- To view, visit http://gerrit.ovirt.org/26458 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia6790ccf38c4807ce06de8edd5e92673cf5fddca Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Yeela Kaplan Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: pep8: silence remaining errors
Francesco Romani has posted comments on this change. Change subject: pep8: silence remaining errors .. Patch Set 1: Code-Review+1 -- To view, visit http://gerrit.ovirt.org/26457 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic24b1a871d6f4cff448d2bb39b88014a9810c9c3 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Dan Kenigsberg Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: fileSD: enable mailbox on file domains
Dan Kenigsberg has posted comments on this change. Change subject: fileSD: enable mailbox on file domains .. Patch Set 2: Code-Review-1 (3 comments) http://gerrit.ovirt.org/#/c/26414/2//COMMIT_MSG Commit Message: Line 9: Since we now support mixed-type storage pools (block and file) we Line 10: must enable the mailbox on file domains in order to satisfy the Line 11: extension requests needed by block domains. Line 12: On file domains the inbox and outbox files were prepared empty so Line 13: we also need to extend them to the proper size as soon as possible: Why do we diverge from the block-domain behavior (of pre-creating the mailbox in its maximum size)? Line 14: Line 15: * on SPM start Line 16: * on master migration Line 17: http://gerrit.ovirt.org/#/c/26414/2/vdsm/storage/fileSD.py File vdsm/storage/fileSD.py: Line 188: metaFilePath = os.path.join(domPath, sd.DOMAIN_META_DATA, metaFile) Line 189: prevMetaFileSize = None Line 190: Line 191: try: Line 192: metaStat = procPool.os.stat(metaFilePath) Is it dangerous to make the mailbox shorter? I think that truncateFile() is pretty cheap, so why we want to limit? Line 193: except OSError as e: Line 194: if e.errno != os.errno.ENOENT: Line 195: raise Line 196: else: http://gerrit.ovirt.org/#/c/26414/2/vdsm/storage/sp.py File vdsm/storage/sp.py: Line 301 Line 302 Line 303 Line 304 Line 305 The mailbox is an IO and CPU hog. Can we limit its preparation to pools that actually need it (since they have at least one block domain)? This is particularly painful for local storage pool. -- To view, visit http://gerrit.ovirt.org/26414 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba8036e33ed8718d05e9d83cad1d63949794f067 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Tal Nisan Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: vm: avoid to reply with half-baked statistics
Dan Kenigsberg has posted comments on this change.
Change subject: vm: avoid to reply with half-baked statistics
..
Patch Set 5: Code-Review-1
(4 comments)
Please add a unit test, documenting the expected result for getAllVmStats if
one of two VMs has partial statistics?
http://gerrit.ovirt.org/#/c/25803/5//COMMIT_MSG
Commit Message:
Line 9: commit 8fedf8bde3c28edb07add23c3e9b72681cb48e49 introduced a tiny
Line 10: window for races between libvirt notifications (vm._onQemuDeath)
Line 11: and polling for stats from engine.
Line 12:
Line 13: This patch address tackles this problem by moving toward a more
s/address//
Line 14: 'transactional' behaviour for statistics gathering.
Line 15: Either we reply to the engine's enquiry with a well formed,
Line 16: complete statistic sample, or we don't reply at all, and instead
Line 17: we raise an error code.
Line 16: complete statistic sample, or we don't reply at all, and instead
Line 17: we raise an error code.
Line 18:
Line 19: Change-Id: I65197459cd183af5e7a1634a5ffb01719364a070
Line 20: Bug-Url: https://bugzilla.redhat.com/1073478
Please explain how this patch resolves this bug - I do not understand it at all.
http://gerrit.ovirt.org/#/c/25803/5/vdsm/API.py
File vdsm/API.py:
Line 361: """
Line 362: v = self._cif.vmContainer.get(self._UUID)
Line 363: if not v:
Line 364: return errCode['noVM']
Line 365: stats = v.getStats().copy()
why do we need the intermediate conversion of Exception to an empty dict? it
would be a little less ugly to catch Exception up here instead of in vm.py.
Line 366: if not stats:
Line 367: return errCode['statsErr']
Line 368: stats['vmId'] = self._UUID
Line 369: return {'status': doneCode, 'statsList': [stats]}
http://gerrit.ovirt.org/#/c/25803/5/vdsm/virt/vm.py
File vdsm/virt/vm.py:
Line 2901: stats['timeOffset'] = self.conf.get('timeOffset', '0')
Line 2902: stats['clientIp'] = self.conf.get('clientIp', '')
Line 2903: if 'pauseCode' in self.conf:
Line 2904: stats['pauseCode'] = self.conf['pauseCode']
Line 2905: stats.update(self.guestAgent.getGuestInfo())
much better, but please pre-verify that guestAgent is not None.
Line 2906: memUsage = 0
Line 2907: realMemUsage = int(stats['memUsage'])
Line 2908: if realMemUsage != 0:
Line 2909: memUsage = (100 - float(realMemUsage) /
--
To view, visit http://gerrit.ovirt.org/25803
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I65197459cd183af5e7a1634a5ffb01719364a070
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Francesco Romani
Gerrit-Reviewer: Jarod.w
Gerrit-Reviewer: Michal Skrivanek
Gerrit-Reviewer: Omer Frenkel
Gerrit-Reviewer: Roy Golan
Gerrit-Reviewer: Vinzenz Feenstra
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: oop: remove unused attributes
Dan Kenigsberg has submitted this change and it was merged. Change subject: oop: remove unused attributes .. oop: remove unused attributes Change-Id: I35f3fae66a4ec71dea99f22d7e085f811f1e1b55 Signed-off-by: Yeela Kaplan Reviewed-on: http://gerrit.ovirt.org/26174 Reviewed-by: Dan Kenigsberg --- M lib/vdsm/config.py.in M vdsm/storage/outOfProcess.py 2 files changed, 0 insertions(+), 6 deletions(-) Approvals: Yeela Kaplan: Verified Dan Kenigsberg: Looks good to me, approved -- To view, visit http://gerrit.ovirt.org/26174 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: merged Gerrit-Change-Id: I35f3fae66a4ec71dea99f22d7e085f811f1e1b55 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yeela Kaplan Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Yeela Kaplan Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Explicitly close libvirt connections at exist
Yeela Kaplan has posted comments on this change. Change subject: Explicitly close libvirt connections at exist .. Patch Set 3: Code-Review+1 -- To view, visit http://gerrit.ovirt.org/17192 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2b15f3ab017828a63960ba9311fa2bf6bfab4729 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Dan Kenigsberg Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Itamar Heim Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: Yeela Kaplan Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: oop: remove unused attributes
Yeela Kaplan has posted comments on this change. Change subject: oop: remove unused attributes .. Patch Set 2: Verified+1 verified it is unused + pep8 + make -- To view, visit http://gerrit.ovirt.org/26174 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I35f3fae66a4ec71dea99f22d7e085f811f1e1b55 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yeela Kaplan Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Yeela Kaplan Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[ovirt-3.4]: Allow moving of sparse images to a block domains
Allon Mureinik has posted comments on this change. Change subject: Allow moving of sparse images to a block domains .. Patch Set 2: Code-Review+1 -- To view, visit http://gerrit.ovirt.org/26469 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id95d700cf6acb46464d6c5d063966f9331a15028 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.4 Gerrit-Owner: Tal Nisan Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: pep8: clean remaining E713
Yeela Kaplan has posted comments on this change. Change subject: pep8: clean remaining E713 .. Patch Set 1: Code-Review+1 -- To view, visit http://gerrit.ovirt.org/26458 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia6790ccf38c4807ce06de8edd5e92673cf5fddca Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Yeela Kaplan Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[ovirt-3.4]: Allow moving of sparse images to a block domains
Hello Federico Simoncelli, Dan Kenigsberg, Allon Mureinik, I'd like you to do a code review. Please visit http://gerrit.ovirt.org/26469 to review the following change. Change subject: Allow moving of sparse images to a block domains .. Allow moving of sparse images to a block domains When attempting to move a sparse image from a file domain to a block domain the destination image is created as sparse which is not supported on block domains. This patch allows to create the images preallocated on domains that do not support sparseness Change-Id: Id95d700cf6acb46464d6c5d063966f9331a15028 Signed-off-by: Tal Nisan Bug-Url: https://bugzilla.redhat.com/1063996 Reviewed-on: http://gerrit.ovirt.org/25778 Reviewed-by: Federico Simoncelli Reviewed-by: Allon Mureinik Reviewed-by: Dan Kenigsberg --- M vdsm/storage/image.py 1 file changed, 8 insertions(+), 7 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/69/26469/1 diff --git a/vdsm/storage/image.py b/vdsm/storage/image.py index a24bcc3..637d4c8 100644 --- a/vdsm/storage/image.py +++ b/vdsm/storage/image.py @@ -358,11 +358,10 @@ # we create the target as a sparse volume (since it will be # soon filled with the data coming from the copy) and then # we change its metadata back to the original value. -if (volParams['prealloc'] == volume.PREALLOCATED_VOL -and destDom.supportsSparseness): +if destDom.supportsSparseness: tmpVolPreallocation = volume.SPARSE_VOL else: -tmpVolPreallocation = volParams['prealloc'] +tmpVolPreallocation = volume.PREALLOCATED_VOL destDom.createVolume(imgUUID=imgUUID, size=volParams['size'], @@ -380,10 +379,12 @@ # Extend volume (for LV only) size to the actual size dstVol.extend((volParams['apparentsize'] + 511) / 512) -# Change destination volume metadata back to the original -# type. -if tmpVolPreallocation != volParams['prealloc']: -dstVol.setType(volParams['prealloc']) +# Change destination volume metadata to preallocated in +# case we've used a sparse volume to accelerate the +# volume creation +if volParams['prealloc'] == volume.PREALLOCATED_VOL \ +and tmpVolPreallocation != volume.PREALLOCATED_VOL: +dstVol.setType(volume.PREALLOCATED_VOL) dstChain.append(dstVol) except se.StorageException: -- To view, visit http://gerrit.ovirt.org/26469 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Id95d700cf6acb46464d6c5d063966f9331a15028 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.4 Gerrit-Owner: Tal Nisan Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Federico Simoncelli ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: oop: remove unused attributes
Dan Kenigsberg has posted comments on this change. Change subject: oop: remove unused attributes .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.ovirt.org/26174 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I35f3fae66a4ec71dea99f22d7e085f811f1e1b55 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yeela Kaplan Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: oop: remove unused attributes
oVirt Jenkins CI Server has posted comments on this change. Change subject: oop: remove unused attributes .. Patch Set 2: Build Successful http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/7869/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/7981/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/7079/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/26174 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I35f3fae66a4ec71dea99f22d7e085f811f1e1b55 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yeela Kaplan Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: vm: avoid to reply with half-baked statistics
Roy Golan has posted comments on this change.
Change subject: vm: avoid to reply with half-baked statistics
..
Patch Set 2:
(1 comment)
http://gerrit.ovirt.org/#/c/25803/2/vdsm/API.py
File vdsm/API.py:
Line 362: if not v:
Line 363: return errCode['noVM']
Line 364: stats = v.getStats().copy()
Line 365: if not stats:
Line 366: return errCode['down']
> Agreed. Then, if we want to save the concept (which I think is still sound)
why simply the regular errCode isn't enough? no stats, that's ok. engine will
try you later.
special error code is suggesting a potential specific care while this isn't the
case.
Line 367: stats['vmId'] = self._UUID
Line 368: return {'status': doneCode, 'statsList': [stats]}
Line 369:
Line 370: def hibernate(self, hibernationVolHandle):
--
To view, visit http://gerrit.ovirt.org/25803
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I65197459cd183af5e7a1634a5ffb01719364a070
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Francesco Romani
Gerrit-Reviewer: Michal Skrivanek
Gerrit-Reviewer: Omer Frenkel
Gerrit-Reviewer: Roy Golan
Gerrit-Reviewer: Vinzenz Feenstra
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: pep8: clean remaining E713
oVirt Jenkins CI Server has posted comments on this change. Change subject: pep8: clean remaining E713 .. Patch Set 1: Build Successful http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/7078/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/7868/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/7980/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/26458 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia6790ccf38c4807ce06de8edd5e92673cf5fddca Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: ifcfg: Log unhandled exception for new Thread
oVirt Jenkins CI Server has posted comments on this change. Change subject: ifcfg: Log unhandled exception for new Thread .. Patch Set 2: Build Failed http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/7075/ : FAILURE http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/7977/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/7866/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_network_functional_tests/1223/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/23085 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2ce44b4586e85438898fcdcd2d62d80813caa5ba Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Maor Lipchuk Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Itamar Heim Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: pep8: silence remaining errors
oVirt Jenkins CI Server has posted comments on this change. Change subject: pep8: silence remaining errors .. Patch Set 1: Code-Review-1 Verified-1 Build Failed http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/7076/ : FAILURE http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/7978/ : FAILURE http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/7867/ : UNSTABLE -- To view, visit http://gerrit.ovirt.org/26457 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic24b1a871d6f4cff448d2bb39b88014a9810c9c3 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Dan Kenigsberg Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: pep8: clean remaining E713
oVirt Jenkins CI Server has posted comments on this change. Change subject: pep8: clean remaining E713 .. Patch Set 1: Build Failed http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/7077/ : FAILURE http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/7979/ : FAILURE http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/7868/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/26458 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia6790ccf38c4807ce06de8edd5e92673cf5fddca Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Dan Kenigsberg Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: pep8: silence remaining errors
Dan Kenigsberg has posted comments on this change. Change subject: pep8: silence remaining errors .. Patch Set 1: Verified+1 gitpydiff comes up clean on this patch -- To view, visit http://gerrit.ovirt.org/26457 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic24b1a871d6f4cff448d2bb39b88014a9810c9c3 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Dan Kenigsberg Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: pep8: silence remaining errors
Dan Kenigsberg has uploaded a new change for review.
Change subject: pep8: silence remaining errors
..
pep8: silence remaining errors
Change-Id: Ic24b1a871d6f4cff448d2bb39b88014a9810c9c3
Signed-off-by: Dan Kenigsberg
---
M client/vdsClient.py
M tests/functional/dhcp.py
M tests/miscTests.py
M tests/outOfProcessTests.py
M tests/persistentDictTests.py
M tests/remoteFileHandlerTests.py
M tests/utilsTests.py
M vdsm/dmidecodeUtil.py
8 files changed, 79 insertions(+), 75 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/57/26457/1
diff --git a/client/vdsClient.py b/client/vdsClient.py
index b324eb1..9764ea5 100644
--- a/client/vdsClient.py
+++ b/client/vdsClient.py
@@ -1888,16 +1888,16 @@
'all bootable devices'
)),
'hotunplugNic': (serv.hotunplugNic,
-(' ',
- 'Hotunplug NIC from existing VM',
- 'nicspec parameters list: r=required, o=optional',
- 'r device: bridge|sriov|vnlink|bridgeless.',
- 'r network: network name',
- 'r macAddr: mac address',
- 'r nicModel: pv|rtl8139|e1000',
- 'o bootOrder: - global boot order across '
- 'all bootable devices'
- )),
+ (' ',
+ 'Hotunplug NIC from existing VM',
+ 'nicspec parameters list: r=required, o=optional',
+ 'r device: bridge|sriov|vnlink|bridgeless.',
+ 'r network: network name',
+ 'r macAddr: mac address',
+ 'r nicModel: pv|rtl8139|e1000',
+ 'o bootOrder: - global boot order across '
+ 'all bootable devices'
+ )),
'hotplugDisk': (serv.hotplugDisk,
(' ',
'Hotplug disk to existing VM',
@@ -1917,23 +1917,24 @@
'o optional: True|False'
)),
'hotunplugDisk': (serv.hotunplugDisk,
- (' ',
- 'Hotunplug disk from existing VM',
- 'drivespec parameters list: r=required, o=optional',
- 'r iface: - Unique identification of '
- 'the existing VM.',
- 'r index: - disk index unique per interface '
- 'virtio|ide',
- 'r [pool:UUID,domain:UUID,image:UUID,volume:UUID]|'
- '[GUID:guid]|[UUID:uuid]',
- 'r format: cow|raw',
- 'r readonly: True|False - default is False',
- 'r propagateErrors: off|on - default is off',
- 'o bootOrder: - global boot order across '
- 'all bootable devices',
- 'o shared: exclusive|shared|none',
- 'o optional: True|False'
- )),
+ (' ',
+ 'Hotunplug disk from existing VM',
+ 'drivespec parameters list: r=required, o=optional',
+ 'r iface: - Unique identification of '
+ 'the existing VM.',
+ 'r index: - disk index unique per interface '
+ 'virtio|ide',
+ 'r '
+ '[pool:UUID,domain:UUID,image:UUID,volume:UUID]|'
+ '[GUID:guid]|[UUID:uuid]',
+ 'r format: cow|raw',
+ 'r readonly: True|False - default is False',
+ 'r propagateErrors: off|on - default is off',
+ 'o bootOrder: - global boot order across '
+ 'all bootable devices',
+ 'o shared: exclusive|shared|none',
+ 'o optional: True|False'
+ )),
'changeCD': (serv.do_changeCD,
(' ',
'Changes the iso image of the cdrom'
@@ -2020,13 +2021,13 @@
'Get hardware info of the VDS'
)),
'getVdsStats': (serv.do_getVdsStats,
- ('',
-'Get Statistics info on the VDS'
-)),
+('',
+ 'Get Statistics info on the VDS'
+ )),
'getVmStats': (serv.do_getVmStats,
-
Change in vdsm[master]: pep8: clean remaining E713
Dan Kenigsberg has uploaded a new change for review.
Change subject: pep8: clean remaining E713
..
pep8: clean remaining E713
Change-Id: Ia6790ccf38c4807ce06de8edd5e92673cf5fddca
Signed-off-by: Dan Kenigsberg
---
M client/vdsClient.py
M tests/functional/storageTests.py
M tests/miscTests.py
M vds_bootstrap/miniyum.py
M vdsm/dumpStorageTable.py.in
M vdsm/gluster/hooks.py
6 files changed, 12 insertions(+), 12 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/58/26458/1
diff --git a/client/vdsClient.py b/client/vdsClient.py
index 9764ea5..85a372f 100644
--- a/client/vdsClient.py
+++ b/client/vdsClient.py
@@ -182,7 +182,7 @@
elif param == 'guestNumaNodes':
guestNumaNodes.append(self._parseDriveSpec(value))
elif param.startswith('custom_'):
-if not 'custom' in params:
+if 'custom' not in params:
params['custom'] = {}
params['custom'][param[7:]] = value
else:
@@ -1093,7 +1093,7 @@
for entry in volumes['uuidlist']:
message = entry + ' : '
res = self.s.getVolumeInfo(sdUUID, spUUID, imgUUID, entry)
-if not 'info' in res:
+if 'info' not in res:
print 'ERROR:', entry, ':', res
continue
info = res['info']
@@ -1543,14 +1543,14 @@
spec = spec[1:]
while True:
-if not spec or not '}' in spec:
+if not spec or '}' not in spec:
raise Exception("nested spec not terminated "
"with '}' in '%s'" % spec)
if spec[0] == '}':
return d, spec[1:]
# Split into first name + the rest
-if not ':' in spec:
+if ':' not in spec:
raise Exception("missing name value separator "
"':' in '%s'" % spec)
name, spec = spec.split(":", 1)
diff --git a/tests/functional/storageTests.py b/tests/functional/storageTests.py
index 87de040..fe56939 100644
--- a/tests/functional/storageTests.py
+++ b/tests/functional/storageTests.py
@@ -370,7 +370,7 @@
class IscsiServer(BackendServer):
def __init__(self, vdsmServer, asserts):
# check if the system supports configuring iSCSI target
-if not "rtslib" in globals().keys():
+if "rtslib" not in globals().keys():
raise SkipTest("python-rtslib is not installed.")
cmd = [_modprobe.cmd, "iscsi_target_mod"]
@@ -499,7 +499,7 @@
# Check if gluster volume is created & started
glusterVolName = spec.split(':')[1]
-if not glusterVolName in self.glusterVolInfo['volumes']:
+if glusterVolName not in self.glusterVolInfo['volumes']:
raise SkipTest("Test volume %s not found. Pls create it "
"and start it" % glusterVolName)
diff --git a/tests/miscTests.py b/tests/miscTests.py
index ca00ec7..69cf690 100644
--- a/tests/miscTests.py
+++ b/tests/miscTests.py
@@ -1169,7 +1169,7 @@
try:
os.write(pipe, longStr)
except OSError as e:
-if not e.errno in (errno.EINTR, errno.EAGAIN):
+if e.errno not in (errno.EINTR, errno.EAGAIN):
raise
myPipe, hisPipe = os.pipe()
diff --git a/vds_bootstrap/miniyum.py b/vds_bootstrap/miniyum.py
index eb2e4f5..531c0a1 100755
--- a/vds_bootstrap/miniyum.py
+++ b/vds_bootstrap/miniyum.py
@@ -477,7 +477,7 @@
if self.buildTransaction():
self.processTransaction()
-if not 'selinux' in globals():
+if 'selinux' not in globals():
selinux = __import__('selinux', globals(), locals(), [], -1)
if selinux.is_selinux_enabled() and "MINIYUM_2ND" not in os.environ:
env = os.environ.copy()
diff --git a/vdsm/dumpStorageTable.py.in b/vdsm/dumpStorageTable.py.in
index ab013ba..997aecb 100644
--- a/vdsm/dumpStorageTable.py.in
+++ b/vdsm/dumpStorageTable.py.in
@@ -55,7 +55,7 @@
self.vmList[vmUUID] = {'Name': vmName, 'Refs': []}
for x in range(1, len(cont)):
image = cont[x].split(' ')[0].split('/')[0]
-if not image in images:
+if image not in images:
s = self.serverConnection
sd = s.getImageDomainsList(pool, image)['domainslist']
if len(sd) > 0:
@@ -69,7 +69,7 @@
def _setSdInfo(self, sd):
-if not sd in self.domainsList.keys():
+if sd not in self.domainsList.keys():
sd_info = self.serverConnection.getStorageDomainInfo(sd)['info']
self.domainsLi
Change in vdsm[master]: vm: Log unhandled exception for new Thread
Maor Lipchuk has abandoned this change. Change subject: vm: Log unhandled exception for new Thread .. Abandoned -- To view, visit http://gerrit.ovirt.org/23084 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: abandon Gerrit-Change-Id: I1a1d0c63234bc5a9d8287f3c92dc52b3eb125be1 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Maor Lipchuk Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Itamar Heim Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: oVirt Jenkins CI Server ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: misc: Log unhandled exception for new thread
Maor Lipchuk has abandoned this change. Change subject: misc: Log unhandled exception for new thread .. Abandoned -- To view, visit http://gerrit.ovirt.org/23083 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: abandon Gerrit-Change-Id: Id61236a8a2e546f80c23c86d878347f5bd98784d Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Maor Lipchuk Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Itamar Heim Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: oVirt Jenkins CI Server ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: pad memory volume only when the storage domain is file based
Allon Mureinik has posted comments on this change.
Change subject: pad memory volume only when the storage domain is file based
..
Patch Set 3:
(1 comment)
http://gerrit.ovirt.org/#/c/26407/3/vdsm/virt/vm.py
File vdsm/virt/vm.py:
Line 3550: if sdType == sd.NFS_DOMAIN:
Line 3551: oop.getProcessPool(sdUUID).fileUtils. \
Line 3552: padToBlockSize(memoryVolPath)
Line 3553: else:
Line 3554: fileUtils.padToBlockSize(memoryVolPath)
> if I recall correctly, I was asked to do it that way because I've been told
Note this line was here in the original code as well - let's fix the wrong
check of storage pool type in /this/ patch, and discuss the OOP usage in a
different one?
Line 3555:
Line 3556: snap = xml.dom.minidom.Element('domainsnapshot')
Line 3557: disks = xml.dom.minidom.Element('disks')
Line 3558: newDrives = {}
--
To view, visit http://gerrit.ovirt.org/26407
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0cebdbba2cc763963b64dcbc142f979802f18c16
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Arik Hadas
Gerrit-Reviewer: Allon Mureinik
Gerrit-Reviewer: Arik Hadas
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Eduardo
Gerrit-Reviewer: Federico Simoncelli
Gerrit-Reviewer: Francesco Romani
Gerrit-Reviewer: Michal Skrivanek
Gerrit-Reviewer: Tal Nisan
Gerrit-Reviewer: Vinzenz Feenstra
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: pad memory volume only when the storage domain is file based
Arik Hadas has posted comments on this change.
Change subject: pad memory volume only when the storage domain is file based
..
Patch Set 3:
(3 comments)
http://gerrit.ovirt.org/#/c/26407/3/vdsm/virt/vm.py
File vdsm/virt/vm.py:
Line 3544: 'elapsedTimeOffset': time.time() -
self._startTime}
Line 3545:
Line 3546: def _padMemoryVolume(memoryVolPath, sdUUID):
Line 3547: sdType = sd.name2type(
Line 3548:
self.cif.irs.getStorageDomainInfo(sdUUID)['info']['type'])
> Let's not use this costly (and possibly failing) call. We already have:
ok, I can switch to use it if we don't need to know the particular type of the
domain in case it is file-based storage (see the comment below), but is it less
costly to access the file than query "getStorageDomainInfo" ?
Line 3549: if sdType in sd.FILE_DOMAIN_TYPES:
Line 3550: if sdType == sd.NFS_DOMAIN:
Line 3551: oop.getProcessPool(sdUUID).fileUtils. \
Line 3552: padToBlockSize(memoryVolPath)
Line 3550: if sdType == sd.NFS_DOMAIN:
Line 3551: oop.getProcessPool(sdUUID).fileUtils. \
Line 3552: padToBlockSize(memoryVolPath)
Line 3553: else:
Line 3554: fileUtils.padToBlockSize(memoryVolPath)
> Why don't we need oop here? What about cifs, posix and gluster?
if I recall correctly, I was asked to do it that way because I've been told
that in NFS the operation might get stuck and in other types of storage the
operation will just fail on error. Edu?
Line 3555:
Line 3556: snap = xml.dom.minidom.Element('domainsnapshot')
Line 3557: disks = xml.dom.minidom.Element('disks')
Line 3558: newDrives = {}
Line 3685: # This code should be removed once qemu-img will handle
files
Line 3686: # with size that is not multiple of block size correctly.
Line 3687: if memoryParams:
Line 3688: _padMemoryVolume(memoryVolPath,
memoryVol['domainID'])
Line 3689: self.cif.teardownVolumePath(memoryVol)
> +1, but please do that in a separate patch!
ok, I will fix it in a separate patch
Line 3690:
Line 3691: for drive in newDrives.values(): # Update the drive
information
Line 3692: try:
Line 3693: self.updateDriveParameters(drive)
--
To view, visit http://gerrit.ovirt.org/26407
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0cebdbba2cc763963b64dcbc142f979802f18c16
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Arik Hadas
Gerrit-Reviewer: Allon Mureinik
Gerrit-Reviewer: Arik Hadas
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Eduardo
Gerrit-Reviewer: Federico Simoncelli
Gerrit-Reviewer: Francesco Romani
Gerrit-Reviewer: Michal Skrivanek
Gerrit-Reviewer: Tal Nisan
Gerrit-Reviewer: Vinzenz Feenstra
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
