Change in vdsm[master]: Fix remotePath comperision in getStorageDomainsList

Saggi Mizrahi has uploaded a new change for review.

Change subject: Fix remotePath comperision in getStorageDomainsList
......................................................................

Fix remotePath comperision in getStorageDomainsList

Change-Id: I50c856f0a612b27672a5c6ccda877979455b00f5
---
M vdsm/storage/hsm.py
1 file changed, 1 insertion(+), 1 deletion(-)

git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/44/1244/1
--
To view, visit http://gerrit.ovirt.org/1244
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I50c856f0a612b27672a5c6ccda877979455b00f5
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi

[Igor Lvovsky]

Igor Lvovsky has posted comments on this change.

Change subject: Fix remotePath comperision in getStorageDomainsList
......................................................................

Patch Set 3: (1 inline comment)

....................................................
File vdsm/storage/hsm.py
Line 2207:                 if remotePath and remotePath != dom.getRemotePath():
What happened with transformPath() ? do we depreciated it? 
If so what is dom.getRemotePath() returns?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I50c856f0a612b27672a5c6ccda877979455b00f5
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi 
Gerrit-Reviewer: Igor Lvovsky 
Gerrit-Reviewer: Saggi Mizrahi

[Igor Lvovsky]

Igor Lvovsky has posted comments on this change.

Change subject: Fix remotePath comperision in getStorageDomainsList
......................................................................

Patch Set 4: (1 inline comment)

....................................................
File vdsm/storage/hsm.py
Line 2118:                 if remotePath and remotePath != dom.getRemotePath():
was it a bug? how it worked before?
What happened with transformPath() ? do we decided to depreciate it? 
Hmmm, what happens with block domains ?  getRemotePath() doesn't defined there

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I50c856f0a612b27672a5c6ccda877979455b00f5
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi 
Gerrit-Reviewer: Igor Lvovsky 
Gerrit-Reviewer: Saggi Mizrahi

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: Fix remotePath comperision in getStorageDomainsList
......................................................................

Patch Set 5: I would prefer that you didn't submit this

Saggi, please relate to comments, and provide more descriptive commit messages.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I50c856f0a612b27672a5c6ccda877979455b00f5
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Igor Lvovsky 
Gerrit-Reviewer: Saggi Mizrahi

Saggi Mizrahi has posted comments on this change.

Change subject: Fix remotePath comperision in getStorageDomainsList
......................................................................

Patch Set 4: (1 inline comment)

....................................................
File vdsm/storage/hsm.py
Line 2118:                 if remotePath and remotePath != dom.getRemotePath():
yes
it didn't
it got transformed and never validated
deprecate what?
I don't know\don't care

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I50c856f0a612b27672a5c6ccda877979455b00f5
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Igor Lvovsky 
Gerrit-Reviewer: Saggi Mizrahi

Saggi Mizrahi has posted comments on this change.

Change subject: Fix remotePath comperision in getStorageDomainsList
......................................................................

Patch Set 5:

I fixed the comparison, if there are other things broken you are welcome to fix them yourself.

Obviously no one used that filter so I wouldn't overextend myself if I were you.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I50c856f0a612b27672a5c6ccda877979455b00f5
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Igor Lvovsky 
Gerrit-Reviewer: Saggi Mizrahi

[Federico Simoncelli]

Federico Simoncelli has posted comments on this change.

Change subject: Fix remotePath comperision in getStorageDomainsList
......................................................................

Patch Set 6: I would prefer that you didn't submit this

(1 inline comment)

....................................................
Commit Message
Line 7: Fix remotePath comperision in getStorageDomainsList
s/comperision/comparison/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I50c856f0a612b27672a5c6ccda877979455b00f5
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Igor Lvovsky 
Gerrit-Reviewer: Saggi Mizrahi

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: Fix remotePath comperision in getStorageDomainsList
......................................................................

Patch Set 6: (1 inline comment)

....................................................
Commit Message
Line 7: Fix remotePath comperision in getStorageDomainsList
Please also add a rant about this feature not really working and never have been worked before.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I50c856f0a612b27672a5c6ccda877979455b00f5
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Igor Lvovsky 
Gerrit-Reviewer: Saggi Mizrahi

Change in vdsm[master]: Fix remotePath comparison in getStorageDomainsList

[Federico Simoncelli]

Federico Simoncelli has posted comments on this change.

Change subject: Fix remotePath comparison in getStorageDomainsList
......................................................................

Patch Set 7: Looks good to me, but someone else must approve

There is one comment in patch set 6 that was probably overlooked. Anyway it's good for me.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I50c856f0a612b27672a5c6ccda877979455b00f5
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Igor Lvovsky 
Gerrit-Reviewer: Saggi Mizrahi

Change in vdsm[master]: Fix remotePath comparison in getStorageDomainsList

Ayal Baron has posted comments on this change.

Change subject: Fix remotePath comparison in getStorageDomainsList
......................................................................

Patch Set 7: (1 inline comment)

....................................................
File vdsm/storage/hsm.py
Line 2119:                 if remotePath and remotePath != dom.getRemotePath():
before all kinds of refactoring and stuff dom.getRemotePath used to return a path with '/' replaced with underscores.  remotePath however contained the path with '/'.  Hence, in order to make the comparison you had to transform remotePath to the same format as was returned from the domain.

Are you sure that remotePath and dom.getRemotePath now contain the same kind of path? or does this comparison always evaluate to False?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I50c856f0a612b27672a5c6ccda877979455b00f5
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Igor Lvovsky 
Gerrit-Reviewer: Saggi Mizrahi

Change in vdsm[master]: Fix remotePath comparison in getStorageDomainsList

[Igor Lvovsky]

Igor Lvovsky has posted comments on this change.

Change subject: Fix remotePath comparison in getStorageDomainsList
......................................................................

Patch Set 7: Looks good to me, but someone else must approve

(1 inline comment)

comment inside

....................................................
File vdsm/storage/hsm.py
Line 2119:                 if remotePath and remotePath != dom.getRemotePath():
Actually, this particular fix is right. It looks like this code was broken for the long time (just no one use this call with 'remotePath' parameter).
My only problem is that it's still broken for iSCSI domains (because getRemotePath doesn't exists there), but because this is not a regression we can push it.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I50c856f0a612b27672a5c6ccda877979455b00f5
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Igor Lvovsky 
Gerrit-Reviewer: Saggi Mizrahi

Change in vdsm[master]: Fix remotePath comparison in getStorageDomainsList

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: Fix remotePath comparison in getStorageDomainsList
......................................................................

Patch Set 8: Verified; Looks good to me, approved

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I50c856f0a612b27672a5c6ccda877979455b00f5
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Igor Lvovsky 
Gerrit-Reviewer: Saggi Mizrahi

Change in vdsm[master]: Fix remotePath comparison in getStorageDomainsList

[Dan Kenigsberg]

Dan Kenigsberg has submitted this change and it was merged.

Change subject: Fix remotePath comparison in getStorageDomainsList
......................................................................

Fix remotePath comparison in getStorageDomainsList

Note that filtering getStorageDomainsList according to remotePath is
still broken for block domains (luckily it is never used).

Change-Id: I50c856f0a612b27672a5c6ccda877979455b00f5
---
M vdsm/storage/hsm.py
1 file changed, 1 insertion(+), 1 deletion(-)

Approvals:
  Dan Kenigsberg: Verified; Looks good to me, approved

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

Gerrit-MessageType: merged
Gerrit-Change-Id: I50c856f0a612b27672a5c6ccda877979455b00f5
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Igor Lvovsky 
Gerrit-Reviewer: Saggi Mizrahi