Laszlo Hornyak has uploaded a new change for review.
Change subject: pep8 fixes ......................................................................
pep8 fixes
fixes in the code to make vdsm build on rhel 6.3
Change-Id: Ifbbba925fa0c9be78b0eb5eb3d07066dc3b3c5ab Signed-off-by: Laszlo Hornyak lhornyak@redhat.com --- M vdsm/storage/misc.py M vdsm/vm.py 2 files changed, 4 insertions(+), 5 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/66/9366/1
diff --git a/vdsm/storage/misc.py b/vdsm/storage/misc.py index 6360dcf..9f3d1bc 100644 --- a/vdsm/storage/misc.py +++ b/vdsm/storage/misc.py @@ -733,12 +733,10 @@ # keep the earliest exception info if not firstException: firstException = e - # keep the original traceback info - traceback = sys.exc_info()[2]
# re-raise the earliest exception if firstException is not None: - raise firstException, None, traceback + raise firstException
def defer(self, func, *args, **kwargs): self._finally.append((func, args, kwargs)) diff --git a/vdsm/vm.py b/vdsm/vm.py index e6d5947..73e3f3f 100644 --- a/vdsm/vm.py +++ b/vdsm/vm.py @@ -363,7 +363,7 @@ SOUND_DEVICES: [], VIDEO_DEVICES: [], CONTROLLER_DEVICES: [], GENERAL_DEVICES: [], BALLOON_DEVICES: [], REDIR_DEVICES: [], - WATCHDOG_DEVICES: [],} + WATCHDOG_DEVICES: []}
def _get_lastStatus(self): PAUSED_STATES = ('Powering down', 'RebootInProgress', 'Up') @@ -495,7 +495,8 @@ if not 'specParams' in devices[WATCHDOG_DEVICES][0]: devices[WATCHDOG_DEVICES][0]['specParams'] = {} if not 'model' in devices[WATCHDOG_DEVICES][0]['specParams']: - devices[WATCHDOG_DEVICES][0]['specParams']['model'] = 'i6300esb' + devices[WATCHDOG_DEVICES][0]['specParams']['model'] = \ + 'i6300esb' if not 'action' in devices[WATCHDOG_DEVICES][0]['specParams']: devices[WATCHDOG_DEVICES][0]['specParams']['action'] = 'none'
-- To view, visit http://gerrit.ovirt.org/9366 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange Gerrit-Change-Id: Ifbbba925fa0c9be78b0eb5eb3d07066dc3b3c5ab Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Laszlo Hornyak lhornyak@redhat.com
oVirt Jenkins CI Server has posted comments on this change.
Change subject: pep8 fixes ......................................................................
Patch Set 1:
Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/75/ (1/2)
-- To view, visit http://gerrit.ovirt.org/9366 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Ifbbba925fa0c9be78b0eb5eb3d07066dc3b3c5ab Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Laszlo Hornyak lhornyak@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: pep8 fixes ......................................................................
Patch Set 1:
Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/109/ (2/2)
-- To view, visit http://gerrit.ovirt.org/9366 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Ifbbba925fa0c9be78b0eb5eb3d07066dc3b3c5ab Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Laszlo Hornyak lhornyak@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: pep8 fixes ......................................................................
Patch Set 1:
Build Successful
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/75/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/109/ : SUCCESS
-- To view, visit http://gerrit.ovirt.org/9366 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Ifbbba925fa0c9be78b0eb5eb3d07066dc3b3c5ab Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Laszlo Hornyak lhornyak@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
Dan Kenigsberg has posted comments on this change.
Change subject: pep8 fixes ......................................................................
Patch Set 1: I would prefer that you didn't submit this
(1 inline comment)
ShaoHe Feng, Zhou, please pitch in to help with these too issues.
Laszlo, on the mean time, you can fetch pep8 from upstream (or rebuild it from fedora 18) to solve the issue with "raise".
.................................................... File vdsm/storage/misc.py Line 735: firstException = e Line 736: Line 737: # re-raise the earliest exception Line 738: if firstException is not None: Line 739: raise firstException this is more than just a pep8 change. please work with Zhou to solve this properly. Line 740: Line 741: def defer(self, func, *args, **kwargs): Line 742: self._finally.append((func, args, kwargs)) Line 743:
-- To view, visit http://gerrit.ovirt.org/9366 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Ifbbba925fa0c9be78b0eb5eb3d07066dc3b3c5ab Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Laszlo Hornyak lhornyak@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Laszlo Hornyak has posted comments on this change.
Change subject: pep8 fixes ......................................................................
Patch Set 1: (1 inline comment)
So I have to build and install pep8 from source?
.................................................... File vdsm/storage/misc.py Line 735: firstException = e Line 736: Line 737: # re-raise the earliest exception Line 738: if firstException is not None: Line 739: raise firstException Agreed. Wouldn't it be better to log all the exceptions rather than just trying to rethrow the first with its original stack trace? Line 740: Line 741: def defer(self, func, *args, **kwargs): Line 742: self._finally.append((func, args, kwargs)) Line 743:
-- To view, visit http://gerrit.ovirt.org/9366 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Ifbbba925fa0c9be78b0eb5eb3d07066dc3b3c5ab Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Laszlo Hornyak lhornyak@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Laszlo Hornyak lhornyak@redhat.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Dan Kenigsberg has posted comments on this change.
Change subject: pep8 fixes ......................................................................
Patch Set 1: (1 inline comment)
Well, it would be best if you help Zhou fix the Fedora bug in pep8. But maybe rebuilding of http://kojipkgs.fedoraproject.org//packages/python-pep8/1.3.3/3.fc18/src/pyt... would be sufficient to you.
.................................................... File vdsm/storage/misc.py Line 735: firstException = e Line 736: Line 737: # re-raise the earliest exception Line 738: if firstException is not None: Line 739: raise firstException oh no. reraising is the proper way, so that the traceback is continued to the calling functions. Catching and logging should be done only on the topmost level. Line 740: Line 741: def defer(self, func, *args, **kwargs): Line 742: self._finally.append((func, args, kwargs)) Line 743:
-- To view, visit http://gerrit.ovirt.org/9366 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Ifbbba925fa0c9be78b0eb5eb3d07066dc3b3c5ab Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Laszlo Hornyak lhornyak@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Laszlo Hornyak lhornyak@redhat.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
ShaoHe Feng has posted comments on this change.
Change subject: pep8 fixes ......................................................................
Patch Set 1: I would prefer that you didn't submit this
(4 inline comments)
.................................................... File vdsm/storage/misc.py Line 731: undo(*args, **kwargs) Line 732: except Exception as e: Line 733: # keep the earliest exception info Line 734: if not firstException: Line 735: firstException = e traceback should not be removed. It record the stack frame information of firstException
also, I have check by the latest pep8 version. there is no error. Line 736: Line 737: # re-raise the earliest exception Line 738: if firstException is not None: Line 739: raise firstException
Line 735: firstException = e Line 736: Line 737: # re-raise the earliest exception Line 738: if firstException is not None: Line 739: raise firstException agree with Dan. also, I have check by the latest pep8 version. there is no error. Line 740: Line 741: def defer(self, func, *args, **kwargs): Line 742: self._finally.append((func, args, kwargs)) Line 743:
.................................................... File vdsm/vm.py Line 362: self._devices = {DISK_DEVICES: [], NIC_DEVICES: [], Line 363: SOUND_DEVICES: [], VIDEO_DEVICES: [], Line 364: CONTROLLER_DEVICES: [], GENERAL_DEVICES: [], Line 365: BALLOON_DEVICES: [], REDIR_DEVICES: [], Line 366: WATCHDOG_DEVICES: []} agree Line 367: Line 368: def _get_lastStatus(self): Line 369: PAUSED_STATES = ('Powering down', 'RebootInProgress', 'Up') Line 370: if not self._guestCpuRunning and self._lastStatus in PAUSED_STATES:
Line 495: if not 'specParams' in devices[WATCHDOG_DEVICES][0]: Line 496: devices[WATCHDOG_DEVICES][0]['specParams'] = {} Line 497: if not 'model' in devices[WATCHDOG_DEVICES][0]['specParams']: Line 498: devices[WATCHDOG_DEVICES][0]['specParams']['model'] = \ Line 499: 'i6300esb' agree Line 500: if not 'action' in devices[WATCHDOG_DEVICES][0]['specParams']: Line 501: devices[WATCHDOG_DEVICES][0]['specParams']['action'] = 'none' Line 502: Line 503: # Normalize vdsm images
-- To view, visit http://gerrit.ovirt.org/9366 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Ifbbba925fa0c9be78b0eb5eb3d07066dc3b3c5ab Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Laszlo Hornyak lhornyak@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Laszlo Hornyak lhornyak@redhat.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Zhou Zheng Sheng has posted comments on this change.
Change subject: pep8 fixes ......................................................................
Patch Set 1: (1 inline comment)
fix of vdsm/vm.py is good, but I do not like removing the stack trace info in misc.py.
.................................................... File vdsm/storage/misc.py Line 735: firstException = e Line 736: Line 737: # re-raise the earliest exception Line 738: if firstException is not None: Line 739: raise firstException We can log the exceptions as well as re-raise the first one with original stack trace. In this case the following exception sometimes is caused by an earlier exception, so the first exception is the most valuable one for investigating the problem. If the other exceptions are related to other problems, their log can be valuable as well.
Reporting error W602 on three-args from of raise statement is a pep8 bug. The python-pep8 in EPEL6 is old, this bug has already been fixed in upstream. I submit a bug report at https://bugzilla.redhat.com/show_bug.cgi?id=878753 . Hope the EPEL maintainers can backport the fix. Line 740: Line 741: def defer(self, func, *args, **kwargs): Line 742: self._finally.append((func, args, kwargs)) Line 743:
-- To view, visit http://gerrit.ovirt.org/9366 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Ifbbba925fa0c9be78b0eb5eb3d07066dc3b3c5ab Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Laszlo Hornyak lhornyak@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Laszlo Hornyak lhornyak@redhat.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: pep8 fixes ......................................................................
Patch Set 2:
Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/118/ (1/2)
-- To view, visit http://gerrit.ovirt.org/9366 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Ifbbba925fa0c9be78b0eb5eb3d07066dc3b3c5ab Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Laszlo Hornyak lhornyak@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Laszlo Hornyak lhornyak@redhat.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: pep8 fixes ......................................................................
Patch Set 2:
Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/84/ (2/2)
-- To view, visit http://gerrit.ovirt.org/9366 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Ifbbba925fa0c9be78b0eb5eb3d07066dc3b3c5ab Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Laszlo Hornyak lhornyak@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Laszlo Hornyak lhornyak@redhat.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: pep8 fixes ......................................................................
Patch Set 2:
Build Successful
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/84/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/118/ : SUCCESS
-- To view, visit http://gerrit.ovirt.org/9366 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Ifbbba925fa0c9be78b0eb5eb3d07066dc3b3c5ab Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Laszlo Hornyak lhornyak@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Laszlo Hornyak lhornyak@redhat.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Dan Kenigsberg has posted comments on this change.
Change subject: pep8 fixes ......................................................................
Patch Set 2: Verified; Looks good to me, approved
People are nagging to often, I cannot wait any longer, so I'm fixing this myself.
-- To view, visit http://gerrit.ovirt.org/9366 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Ifbbba925fa0c9be78b0eb5eb3d07066dc3b3c5ab Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Laszlo Hornyak lhornyak@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Laszlo Hornyak lhornyak@redhat.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Dan Kenigsberg has submitted this change and it was merged.
Change subject: pep8 fixes ......................................................................
pep8 fixes
fixes in the code to make vdsm build on rhel 6.3
Change-Id: Ifbbba925fa0c9be78b0eb5eb3d07066dc3b3c5ab Signed-off-by: Laszlo Hornyak lhornyak@redhat.com --- M vdsm/vm.py 1 file changed, 3 insertions(+), 2 deletions(-)
Approvals: Dan Kenigsberg: Verified; Looks good to me, approved
-- To view, visit http://gerrit.ovirt.org/9366 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: merged Gerrit-Change-Id: Ifbbba925fa0c9be78b0eb5eb3d07066dc3b3c5ab Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Laszlo Hornyak lhornyak@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Laszlo Hornyak lhornyak@redhat.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
vdsm-patches@lists.fedorahosted.org