Change in vdsm[master]: core: fixing attribute error on fenceSpmStorage

Liron Ar has uploaded a new change for review.

Change subject: core: fixing attribute error on fenceSpmStorage
......................................................................

core: fixing attribute error on fenceSpmStorage

In change I75493d1db60e51cccd5231b516f963c970d24c99 the pool metadata
operations were moved to the pool backend, while the call to
invalidateMetadata in fenceSpmStorage was left as it was, leading to an
attribute error.

This patch fixes it by executing the operation on the pool backend object
rather than on the pool, as the metadata operation are implemented only in
StoragePoolDiskBackend - there's also a check for the backend type before
executing it.
Though it's not the nicest way for doing so, it's the pattern already used in
hsm.py,therefore in the meanwhile it can be fixed the same way.

Change-Id: I5ae4c371e2dc97686621ea16b8cef971183a6ab2
Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1082365
Signed-off-by: Liron Aravot <laravot(a)redhat.com&gt;
---
M vdsm/storage/hsm.py
1 file changed, 2 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/58/26358/1

diff --git a/vdsm/storage/hsm.py b/vdsm/storage/hsm.py
index 0ebe129..d9697ef 100644
--- a/vdsm/storage/hsm.py
+++ b/vdsm/storage/hsm.py
@@ -3529,7 +3529,8 @@
             se.SpmFenceError("spUUID=%s, lastOwner=%s, lastLver=%s" %
                              (spUUID, lastOwner, lastLver)))
         pool = self.getPool(spUUID)
-        pool.invalidateMetadata()
+        if isinstance(pool.getBackend(), StoragePoolDiskBackend):
+            pool.getBackend().invalidateMetadata()
         vars.task.getExclusiveLock(STORAGE, spUUID)
         pool.forceFreeSpm()
         return dict(spm_st=self._getSpmStatusInfo(pool))


-- 
To view, visit http://gerrit.ovirt.org/26358
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I5ae4c371e2dc97686621ea16b8cef971183a6ab2
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Ar <laravot(a)redhat.com&gt;

Liron Ar has posted comments on this change.

Change subject: core: fixing attribute error on fenceSpmStorage
......................................................................


Patch Set 5:

@Allon, i'm handling the backport of those 2 patches.
the third one is removal of code which seems unneeded, but isn't needed for solving
the bugs in the fence verb.

-- 
To view, visit http://gerrit.ovirt.org/26358
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I5ae4c371e2dc97686621ea16b8cef971183a6ab2
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Ar <laravot(a)redhat.com&gt;
Gerrit-Reviewer: Allon Mureinik <amureini(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Liron Ar <laravot(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

[Allon Mureinik]

Allon Mureinik has posted comments on this change.

Change subject: core: fixing attribute error on fenceSpmStorage
......................................................................


Patch Set 5:

Federico/Liron - are we backporting the two merged patches, or are we waiting to merge
http://gerrit.ovirt.org/#/c/27318/ and backporting all three?

-- 
To view, visit http://gerrit.ovirt.org/26358
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I5ae4c371e2dc97686621ea16b8cef971183a6ab2
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Ar <laravot(a)redhat.com&gt;
Gerrit-Reviewer: Allon Mureinik <amureini(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Liron Ar <laravot(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

[Dan Kenigsberg]

Dan Kenigsberg has submitted this change and it was merged.

Change subject: core: fixing attribute error on fenceSpmStorage
......................................................................


core: fixing attribute error on fenceSpmStorage

In change I75493d1db60e51cccd5231b516f963c970d24c99 the pool metadata
operations were moved to the pool backend, while the call to
invalidateMetadata in fenceSpmStorage was left as it was, leading to an
attribute error.

This patch fixes it by executing the operation on the pool backend object
rather than on the pool, as the metadata operation are implemented only in
StoragePoolDiskBackend - there's also a check for the backend type before
executing it.
Though it's not the nicest way for doing so, it's the pattern already used in
hsm.py,therefore in the meanwhile it can be fixed the same way.

Change-Id: I5ae4c371e2dc97686621ea16b8cef971183a6ab2
Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1082365
Signed-off-by: Liron Aravot <laravot(a)redhat.com&gt;
Reviewed-on: http://gerrit.ovirt.org/26358
Reviewed-by: Allon Mureinik <amureini(a)redhat.com&gt;
Reviewed-by: Nir Soffer <nsoffer(a)redhat.com&gt;
Reviewed-by: Federico Simoncelli <fsimonce(a)redhat.com&gt;
---
M vdsm/storage/hsm.py
M vdsm/storage/spbackends.py
2 files changed, 5 insertions(+), 4 deletions(-)

Approvals:
  Nir Soffer: Looks good to me, but someone else must approve
  Federico Simoncelli: Looks good to me, approved
  Allon Mureinik: Looks good to me, but someone else must approve
  Liron Ar: Verified



-- 
To view, visit http://gerrit.ovirt.org/26358
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I5ae4c371e2dc97686621ea16b8cef971183a6ab2
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Ar <laravot(a)redhat.com&gt;
Gerrit-Reviewer: Allon Mureinik <amureini(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Liron Ar <laravot(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server

[Federico Simoncelli]

Federico Simoncelli has posted comments on this change.

Change subject: core: fixing attribute error on fenceSpmStorage
......................................................................


Patch Set 4: Code-Review+2

As agreed with Liron addressing SPM_FREE will be handled in a separate patch. Since I
don't trust us remembering about this I sent the patch at:
http://gerrit.ovirt.org/#/c/27318/

-- 
To view, visit http://gerrit.ovirt.org/26358
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I5ae4c371e2dc97686621ea16b8cef971183a6ab2
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Ar <laravot(a)redhat.com&gt;
Gerrit-Reviewer: Allon Mureinik <amureini(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Liron Ar <laravot(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: core: fixing attribute error on fenceSpmStorage
......................................................................


Patch Set 4:

Build Failed 

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/8487/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/7697/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/8612/ : FAILURE

-- 
To view, visit http://gerrit.ovirt.org/26358
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I5ae4c371e2dc97686621ea16b8cef971183a6ab2
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Ar <laravot(a)redhat.com&gt;
Gerrit-Reviewer: Allon Mureinik <amureini(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Liron Ar <laravot(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

Liron Ar has posted comments on this change.

Change subject: core: fixing attribute error on fenceSpmStorage
......................................................................


Patch Set 3:

(1 comment)

http://gerrit.ovirt.org/#/c/26358/3/vdsm/storage/spbackends.py
File vdsm/storage/spbackends.py:

Line 337:     def forceFreeSpm(self):
Line 338:         # DO NOT USE, STUPID, HERE ONLY FOR BC
Line 339:         # TODO: SCSI Fence the 'lastOwner'
Line 340:         self.setSpmStatus(LVER_INVALID, SPM_ID_FREE, __securityOverride=True)
Line 341:         self.pool.spmRole = SPM_FREE
>  2 things: 
Fede, I agree that it seems as unneeded from first
look.
But, that has been always the behavior here, we need to verify that there's no some
corner case before changing it (here and on engine), which isn't something that i want
to handle in this patch.

Let's agree on having the attribute errors issues fixed here which restores the
current behavior to function, while the removal of this line will be inspected regardless
on a further patch.
Line 342: 
Line 343:     @classmethod
Line 344:     def _getPoolMD(cls, domain):
Line 345:         # This might look disgusting but this makes it so that


-- 
To view, visit http://gerrit.ovirt.org/26358
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I5ae4c371e2dc97686621ea16b8cef971183a6ab2
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Ar <laravot(a)redhat.com&gt;
Gerrit-Reviewer: Allon Mureinik <amureini(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Liron Ar <laravot(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes

[Federico Simoncelli]

Federico Simoncelli has posted comments on this change.

Change subject: core: fixing attribute error on fenceSpmStorage
......................................................................


Patch Set 3: Code-Review-1

(1 comment)

http://gerrit.ovirt.org/#/c/26358/3/vdsm/storage/spbackends.py
File vdsm/storage/spbackends.py:

Line 337:     def forceFreeSpm(self):
Line 338:         # DO NOT USE, STUPID, HERE ONLY FOR BC
Line 339:         # TODO: SCSI Fence the 'lastOwner'
Line 340:         self.setSpmStatus(LVER_INVALID, SPM_ID_FREE, __securityOverride=True)
Line 341:         self.pool.spmRole = SPM_FREE
>  why? we have no setter for it..i can add one instead if you prefer
> it. 
2 things:

* yes if we want to maintain this we need a setter (and it should be handled in another
patch without touching this)
* setting spmRole = SPM_FREE is harmful

In fact if the current host is the spm (SPM_ACQUIRED) setting it to SPM_FREE without going
in stopSpm is going to generate all kind of problems.

forceFreeSpm is entirely pointless (when it's not harmful).
Line 342: 
Line 343:     @classmethod
Line 344:     def _getPoolMD(cls, domain):
Line 345:         # This might look disgusting but this makes it so that


-- 
To view, visit http://gerrit.ovirt.org/26358
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I5ae4c371e2dc97686621ea16b8cef971183a6ab2
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Ar <laravot(a)redhat.com&gt;
Gerrit-Reviewer: Allon Mureinik <amureini(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Liron Ar <laravot(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes

Liron Ar has posted comments on this change.

Change subject: core: fixing attribute error on fenceSpmStorage
......................................................................


Patch Set 3: Verified+1

-- 
To view, visit http://gerrit.ovirt.org/26358
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I5ae4c371e2dc97686621ea16b8cef971183a6ab2
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Ar <laravot(a)redhat.com&gt;
Gerrit-Reviewer: Allon Mureinik <amureini(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Liron Ar <laravot(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

Liron Ar has posted comments on this change.

Change subject: core: fixing attribute error on fenceSpmStorage
......................................................................


Patch Set 3:

(1 comment)

http://gerrit.ovirt.org/#/c/26358/3/vdsm/storage/spbackends.py
File vdsm/storage/spbackends.py:

Line 337:     def forceFreeSpm(self):
Line 338:         # DO NOT USE, STUPID, HERE ONLY FOR BC
Line 339:         # TODO: SCSI Fence the 'lastOwner'
Line 340:         self.setSpmStatus(LVER_INVALID, SPM_ID_FREE, __securityOverride=True)
Line 341:         self.pool.spmRole = SPM_FREE
>  We shouldn't need the change you introduced. 
why? we have
no setter for it..i can add one instead if you prefer it.
Line 342: 
Line 343:     @classmethod
Line 344:     def _getPoolMD(cls, domain):
Line 345:         # This might look disgusting but this makes it so that


-- 
To view, visit http://gerrit.ovirt.org/26358
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I5ae4c371e2dc97686621ea16b8cef971183a6ab2
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Ar <laravot(a)redhat.com&gt;
Gerrit-Reviewer: Allon Mureinik <amureini(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Liron Ar <laravot(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes

[Federico Simoncelli]

Federico Simoncelli has posted comments on this change.

Change subject: core: fixing attribute error on fenceSpmStorage
......................................................................


Patch Set 3:

(1 comment)

http://gerrit.ovirt.org/#/c/26358/3/vdsm/storage/spbackends.py
File vdsm/storage/spbackends.py:

Line 337:     def forceFreeSpm(self):
Line 338:         # DO NOT USE, STUPID, HERE ONLY FOR BC
Line 339:         # TODO: SCSI Fence the 'lastOwner'
Line 340:         self.setSpmStatus(LVER_INVALID, SPM_ID_FREE, __securityOverride=True)
Line 341:         self.pool.spmRole = SPM_FREE
>  it returns as part of the status result being checked by the engine
> and was 
We shouldn't need the change you introduced.
Line 342: 
Line 343:     @classmethod
Line 344:     def _getPoolMD(cls, domain):
Line 345:         # This might look disgusting but this makes it so that


-- 
To view, visit http://gerrit.ovirt.org/26358
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I5ae4c371e2dc97686621ea16b8cef971183a6ab2
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Ar <laravot(a)redhat.com&gt;
Gerrit-Reviewer: Allon Mureinik <amureini(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Liron Ar <laravot(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes

[Nir Soffer]

Nir Soffer has posted comments on this change.

Change subject: core: fixing attribute error on fenceSpmStorage
......................................................................


Patch Set 3: Code-Review+1

Ugly but seem correct. We will have to fix the ugliness later.

-- 
To view, visit http://gerrit.ovirt.org/26358
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I5ae4c371e2dc97686621ea16b8cef971183a6ab2
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Ar <laravot(a)redhat.com&gt;
Gerrit-Reviewer: Allon Mureinik <amureini(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Liron Ar <laravot(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

Liron Ar has posted comments on this change.

Change subject: core: fixing attribute error on fenceSpmStorage
......................................................................


Patch Set 3:

(1 comment)

this patch seems to be fine, i'm not marking verified till 
https://bugzilla.redhat.com/show_bug.cgi?id=1092631 will be solved (prevents from the
"fencing" to actually help in recovering the system)

http://gerrit.ovirt.org/#/c/26358/3/vdsm/storage/spbackends.py
File vdsm/storage/spbackends.py:

Line 337:     def forceFreeSpm(self):
Line 338:         # DO NOT USE, STUPID, HERE ONLY FOR BC
Line 339:         # TODO: SCSI Fence the 'lastOwner'
Line 340:         self.setSpmStatus(LVER_INVALID, SPM_ID_FREE, __securityOverride=True)
Line 341:         self.pool.spmRole = SPM_FREE
>  You shouldn't need this. 
it returns as part of the status
result being checked by the engine and was always updated, why do you think that it's
unneeded?
Line 342: 
Line 343:     @classmethod
Line 344:     def _getPoolMD(cls, domain):
Line 345:         # This might look disgusting but this makes it so that


-- 
To view, visit http://gerrit.ovirt.org/26358
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I5ae4c371e2dc97686621ea16b8cef971183a6ab2
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Ar <laravot(a)redhat.com&gt;
Gerrit-Reviewer: Allon Mureinik <amureini(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Liron Ar <laravot(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes

[Federico Simoncelli]

Federico Simoncelli has posted comments on this change.

Change subject: core: fixing attribute error on fenceSpmStorage
......................................................................


Patch Set 3:

(1 comment)

http://gerrit.ovirt.org/#/c/26358/3/vdsm/storage/spbackends.py
File vdsm/storage/spbackends.py:

Line 337:     def forceFreeSpm(self):
Line 338:         # DO NOT USE, STUPID, HERE ONLY FOR BC
Line 339:         # TODO: SCSI Fence the 'lastOwner'
Line 340:         self.setSpmStatus(LVER_INVALID, SPM_ID_FREE, __securityOverride=True)
Line 341:         self.pool.spmRole = SPM_FREE
You shouldn't need this.
Line 342: 
Line 343:     @classmethod
Line 344:     def _getPoolMD(cls, domain):
Line 345:         # This might look disgusting but this makes it so that


-- 
To view, visit http://gerrit.ovirt.org/26358
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I5ae4c371e2dc97686621ea16b8cef971183a6ab2
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Ar <laravot(a)redhat.com&gt;
Gerrit-Reviewer: Allon Mureinik <amureini(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Liron Ar <laravot(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes

[Allon Mureinik]

Allon Mureinik has posted comments on this change.

Change subject: core: fixing attribute error on fenceSpmStorage
......................................................................


Patch Set 3: Code-Review+1

-- 
To view, visit http://gerrit.ovirt.org/26358
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I5ae4c371e2dc97686621ea16b8cef971183a6ab2
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Ar <laravot(a)redhat.com&gt;
Gerrit-Reviewer: Allon Mureinik <amureini(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Liron Ar <laravot(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: core: fixing attribute error on fenceSpmStorage
......................................................................


Patch Set 3:

Build Successful 

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/8382/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/7592/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/8502/ : SUCCESS

-- 
To view, visit http://gerrit.ovirt.org/26358
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I5ae4c371e2dc97686621ea16b8cef971183a6ab2
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Ar <laravot(a)redhat.com&gt;
Gerrit-Reviewer: Allon Mureinik <amureini(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Liron Ar <laravot(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

[Nir Soffer]

Nir Soffer has posted comments on this change.

Change subject: core: fixing attribute error on fenceSpmStorage
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.ovirt.org/#/c/26358/2/vdsm/storage/hsm.py
File vdsm/storage/hsm.py:

Line 3528:         vars.task.setDefaultException(
Line 3529:             se.SpmFenceError("spUUID=%s, lastOwner=%s, lastLver=%s"
%
Line 3530:                              (spUUID, lastOwner, lastLver)))
Line 3531:         pool = self.getPool(spUUID)
Line 3532:         if isinstance(pool.getBackend(), StoragePoolDiskBackend):
>  wouldn't it be much safer to explode if Engine asks for
> fenceSpmStorage() w 
I think that Liron patch does the right thing.

fenceSpmStorage is an interface - it may be useless now, but may be useful in the future,
so practically it would be a good idea to make it a nop. Failing the request forcing the
engine to check the pool version before sending verbs is the last thing I want to do in
the engine.

Further, this is a critical bug - with the current code, if you reach this point and
fenceSpmStorage fails, whole system goes down and you don't have any way to select a
new spm.
Line 3533:             pool.getBackend().invalidateMetadata()
Line 3534:             vars.task.getExclusiveLock(STORAGE, spUUID)
Line 3535:             pool.getBackend().forceFreeSpm()
Line 3536:         return dict(spm_st=self._getSpmStatusInfo(pool))


-- 
To view, visit http://gerrit.ovirt.org/26358
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I5ae4c371e2dc97686621ea16b8cef971183a6ab2
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Ar <laravot(a)redhat.com&gt;
Gerrit-Reviewer: Allon Mureinik <amureini(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Liron Ar <laravot(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes

Liron Ar has posted comments on this change.

Change subject: core: fixing attribute error on fenceSpmStorage
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.ovirt.org/#/c/26358/2/vdsm/storage/hsm.py
File vdsm/storage/hsm.py:

Line 3528:         vars.task.setDefaultException(
Line 3529:             se.SpmFenceError("spUUID=%s, lastOwner=%s, lastLver=%s"
%
Line 3530:                              (spUUID, lastOwner, lastLver)))
Line 3531:         pool = self.getPool(spUUID)
Line 3532:         if isinstance(pool.getBackend(), StoragePoolDiskBackend):
>  what i meant was is that the engine shouldn't be aware of
> what's being done 
regardless, if we'll ever want to raise an error for
different backend implementation we could always add it when it'll be relevant, as for
the question wether we want to have the instance type check now or not - IMO it's
preferred, but let's see other opinions on that.
Line 3533:             pool.getBackend().invalidateMetadata()
Line 3534:             vars.task.getExclusiveLock(STORAGE, spUUID)
Line 3535:             pool.getBackend().forceFreeSpm()
Line 3536:         return dict(spm_st=self._getSpmStatusInfo(pool))


-- 
To view, visit http://gerrit.ovirt.org/26358
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I5ae4c371e2dc97686621ea16b8cef971183a6ab2
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Ar <laravot(a)redhat.com&gt;
Gerrit-Reviewer: Allon Mureinik <amureini(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Liron Ar <laravot(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes

Liron Ar has posted comments on this change.

Change subject: core: fixing attribute error on fenceSpmStorage
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.ovirt.org/#/c/26358/2/vdsm/storage/hsm.py
File vdsm/storage/hsm.py:

Line 3528:         vars.task.setDefaultException(
Line 3529:             se.SpmFenceError("spUUID=%s, lastOwner=%s, lastLver=%s"
%
Line 3530:                              (spUUID, lastOwner, lastLver)))
Line 3531:         pool = self.getPool(spUUID)
Line 3532:         if isinstance(pool.getBackend(), StoragePoolDiskBackend):
>  Dan, in that scenario there's nothing to do - you can't
> "explode" because w 
what i meant was is that the engine shouldn't be
aware of what's being done here, the fact that nothing will be done currently for the
other backend is an implementation detail, IMO it makes sense to keep it as is.
Line 3533:             pool.getBackend().invalidateMetadata()
Line 3534:             vars.task.getExclusiveLock(STORAGE, spUUID)
Line 3535:             pool.getBackend().forceFreeSpm()
Line 3536:         return dict(spm_st=self._getSpmStatusInfo(pool))


-- 
To view, visit http://gerrit.ovirt.org/26358
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I5ae4c371e2dc97686621ea16b8cef971183a6ab2
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Ar <laravot(a)redhat.com&gt;
Gerrit-Reviewer: Allon Mureinik <amureini(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Liron Ar <laravot(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes

Liron Ar has posted comments on this change.

Change subject: core: fixing attribute error on fenceSpmStorage
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.ovirt.org/#/c/26358/2/vdsm/storage/hsm.py
File vdsm/storage/hsm.py:

Line 3528:         vars.task.setDefaultException(
Line 3529:             se.SpmFenceError("spUUID=%s, lastOwner=%s, lastLver=%s"
%
Line 3530:                              (spUUID, lastOwner, lastLver)))
Line 3531:         pool = self.getPool(spUUID)
Line 3532:         if isinstance(pool.getBackend(), StoragePoolDiskBackend):
>  wouldn't it be much safer to explode if Engine asks for
> fenceSpmStorage() w 
Dan, in that scenario there's nothing to do - you can't
"explode" because we need BC with older engines which are not aware about the
implementation here.
Line 3533:             pool.getBackend().invalidateMetadata()
Line 3534:             vars.task.getExclusiveLock(STORAGE, spUUID)
Line 3535:             pool.getBackend().forceFreeSpm()
Line 3536:         return dict(spm_st=self._getSpmStatusInfo(pool))


-- 
To view, visit http://gerrit.ovirt.org/26358
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I5ae4c371e2dc97686621ea16b8cef971183a6ab2
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Ar <laravot(a)redhat.com&gt;
Gerrit-Reviewer: Allon Mureinik <amureini(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Liron Ar <laravot(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: core: fixing attribute error on fenceSpmStorage
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.ovirt.org/#/c/26358/2/vdsm/storage/hsm.py
File vdsm/storage/hsm.py:

Line 3528:         vars.task.setDefaultException(
Line 3529:             se.SpmFenceError("spUUID=%s, lastOwner=%s, lastLver=%s"
%
Line 3530:                              (spUUID, lastOwner, lastLver)))
Line 3531:         pool = self.getPool(spUUID)
Line 3532:         if isinstance(pool.getBackend(), StoragePoolDiskBackend):
wouldn't it be much safer to explode if Engine asks for fenceSpmStorage() while our
backend does not support it? Why would we want to silenetly ignore such a request?
Line 3533:             pool.getBackend().invalidateMetadata()
Line 3534:             vars.task.getExclusiveLock(STORAGE, spUUID)
Line 3535:             pool.getBackend().forceFreeSpm()
Line 3536:         return dict(spm_st=self._getSpmStatusInfo(pool))


-- 
To view, visit http://gerrit.ovirt.org/26358
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I5ae4c371e2dc97686621ea16b8cef971183a6ab2
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Ar <laravot(a)redhat.com&gt;
Gerrit-Reviewer: Allon Mureinik <amureini(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Liron Ar <laravot(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes

[Allon Mureinik]

Allon Mureinik has posted comments on this change.

Change subject: core: fixing attribute error on fenceSpmStorage
......................................................................


Patch Set 2:

@Liron, can you please verify so we can get this merged?

-- 
To view, visit http://gerrit.ovirt.org/26358
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I5ae4c371e2dc97686621ea16b8cef971183a6ab2
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Ar <laravot(a)redhat.com&gt;
Gerrit-Reviewer: Allon Mureinik <amureini(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Liron Ar <laravot(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

[Federico Simoncelli]

Federico Simoncelli has posted comments on this change.

Change subject: core: fixing attribute error on fenceSpmStorage
......................................................................


Patch Set 2: Code-Review+2

-- 
To view, visit http://gerrit.ovirt.org/26358
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I5ae4c371e2dc97686621ea16b8cef971183a6ab2
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Ar <laravot(a)redhat.com&gt;
Gerrit-Reviewer: Allon Mureinik <amureini(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Liron Ar <laravot(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

[Allon Mureinik]

Allon Mureinik has posted comments on this change.

Change subject: core: fixing attribute error on fenceSpmStorage
......................................................................


Patch Set 2:

Fede, ping?

-- 
To view, visit http://gerrit.ovirt.org/26358
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I5ae4c371e2dc97686621ea16b8cef971183a6ab2
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Ar <laravot(a)redhat.com&gt;
Gerrit-Reviewer: Allon Mureinik <amureini(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Liron Ar <laravot(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

[Allon Mureinik]

Allon Mureinik has posted comments on this change.

Change subject: core: fixing attribute error on fenceSpmStorage
......................................................................


Patch Set 2: Code-Review+1

Fede - what are we missing to get a +2 here?

-- 
To view, visit http://gerrit.ovirt.org/26358
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I5ae4c371e2dc97686621ea16b8cef971183a6ab2
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Ar <laravot(a)redhat.com&gt;
Gerrit-Reviewer: Allon Mureinik <amureini(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Liron Ar <laravot(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

[Federico Simoncelli]

Federico Simoncelli has posted comments on this change.

Change subject: core: fixing attribute error on fenceSpmStorage
......................................................................


Patch Set 2: Code-Review+1

(1 comment)

Looks ok. I am not confident enough for a +2 today yet.

http://gerrit.ovirt.org/#/c/26358/2/vdsm/storage/hsm.py
File vdsm/storage/hsm.py:

Line 3532:         if isinstance(pool.getBackend(), StoragePoolDiskBackend):
Line 3533:             pool.getBackend().invalidateMetadata()
Line 3534:             vars.task.getExclusiveLock(STORAGE, spUUID)
Line 3535:             pool.getBackend().forceFreeSpm()
Line 3536:         return dict(spm_st=self._getSpmStatusInfo(pool))
Can you double check what engine is going to do with this data? Anyway it's not
relevant for the vote on this patch.
Line 3537: 
Line 3538:     @public
Line 3539:     def upgradeStoragePool(self, spUUID, targetDomVersion):
Line 3540:         targetDomVersion = int(targetDomVersion)


-- 
To view, visit http://gerrit.ovirt.org/26358
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I5ae4c371e2dc97686621ea16b8cef971183a6ab2
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Ar <laravot(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Liron Ar <laravot(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes

[Nir Soffer]

Nir Soffer has posted comments on this change.

Change subject: core: fixing attribute error on fenceSpmStorage
......................................................................


Patch Set 2: Code-Review+1

Even worse, but good enough if this fix is urgent.

-- 
To view, visit http://gerrit.ovirt.org/26358
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I5ae4c371e2dc97686621ea16b8cef971183a6ab2
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Ar <laravot(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Liron Ar <laravot(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: core: fixing attribute error on fenceSpmStorage
......................................................................


Patch Set 2:

Build Failed 

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/7019/ : FAILURE

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/7921/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/7810/ : SUCCESS

-- 
To view, visit http://gerrit.ovirt.org/26358
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I5ae4c371e2dc97686621ea16b8cef971183a6ab2
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Ar <laravot(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Liron Ar <laravot(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

[Federico Simoncelli]

Federico Simoncelli has posted comments on this change.

Change subject: core: fixing attribute error on fenceSpmStorage
......................................................................


Patch Set 1: Code-Review-1

(1 comment)

http://gerrit.ovirt.org/#/c/26358/1/vdsm/storage/hsm.py
File vdsm/storage/hsm.py:

Line 3531:         pool = self.getPool(spUUID)
Line 3532:         if isinstance(pool.getBackend(), StoragePoolDiskBackend):
Line 3533:             pool.getBackend().invalidateMetadata()
Line 3534:         vars.task.getExclusiveLock(STORAGE, spUUID)
Line 3535:         pool.forceFreeSpm()
Also forceFreeSpm is also relevant only for StoragePoolDiskBackend.
Line 3536:         return dict(spm_st=self._getSpmStatusInfo(pool))
Line 3537: 
Line 3538:     @public
Line 3539:     def upgradeStoragePool(self, spUUID, targetDomVersion):


-- 
To view, visit http://gerrit.ovirt.org/26358
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I5ae4c371e2dc97686621ea16b8cef971183a6ab2
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Ar <laravot(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Liron Ar <laravot(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes

[Nir Soffer]

Nir Soffer has posted comments on this change.

Change subject: core: fixing attribute error on fenceSpmStorage
......................................................................


Patch Set 1: Code-Review+1

Ugly, but good enough for temporary bug fix.

-- 
To view, visit http://gerrit.ovirt.org/26358
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I5ae4c371e2dc97686621ea16b8cef971183a6ab2
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Ar <laravot(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Liron Ar <laravot(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

Liron Ar has posted comments on this change.

Change subject: core: fixing attribute error on fenceSpmStorage
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.ovirt.org/#/c/26358/1/vdsm/storage/hsm.py
File vdsm/storage/hsm.py:

Line 3529:             se.SpmFenceError("spUUID=%s, lastOwner=%s, lastLver=%s"
%
Line 3530:                              (spUUID, lastOwner, lastLver)))
Line 3531:         pool = self.getPool(spUUID)
Line 3532:         if isinstance(pool.getBackend(), StoragePoolDiskBackend):
Line 3533:             pool.getBackend().invalidateMetadata()
already wrote about that in the commit message - but just so reviewers will notice - nicer
way might be to add it to the "Storage pool backend" interface and implement it
for all the "interface" implementations and add invalidateMetadata() method in
sp.py that'll refer there so we could leave the code here as is - i don't mind to
implement either of the soltions, just let me know what you prefer.
Line 3534:         vars.task.getExclusiveLock(STORAGE, spUUID)
Line 3535:         pool.forceFreeSpm()
Line 3536:         return dict(spm_st=self._getSpmStatusInfo(pool))
Line 3537: 


-- 
To view, visit http://gerrit.ovirt.org/26358
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I5ae4c371e2dc97686621ea16b8cef971183a6ab2
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Ar <laravot(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Liron Ar <laravot(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: core: fixing attribute error on fenceSpmStorage
......................................................................


Patch Set 1:

Build Failed 

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/7018/ : FAILURE

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/7920/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/7809/ : SUCCESS

-- 
To view, visit http://gerrit.ovirt.org/26358
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I5ae4c371e2dc97686621ea16b8cef971183a6ab2
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Ar <laravot(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No