Change in vdsm[master]: Fix fix fix getDomUuidFromVolumePath()

[Dan Kenigsberg]

Dan Kenigsberg has uploaded a new change for review.

Change subject: Fix fix fix getDomUuidFromVolumePath()
......................................................................

Fix fix fix getDomUuidFromVolumePath()

Change-Id: I96b71b7839666d402ec5166eefeac618685c0ab0
Bug-Url: http://bugzilla.redhat.com/1017735
Signed-off-by: Dan Kenigsberg 
---
M vdsm/storage/fileVolume.py
1 file changed, 3 insertions(+), 12 deletions(-)

git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/90/20790/1

diff --git a/vdsm/storage/fileVolume.py b/vdsm/storage/fileVolume.py
index cf26197..b28a8a5 100644
--- a/vdsm/storage/fileVolume.py
+++ b/vdsm/storage/fileVolume.py
@@ -18,13 +18,11 @@
 # Refer to the README and COPYING files for full details of the license
 #

-from os.path import normpath
 import errno
 import os
 import sanlock

import storage_exception as se
-from vdsm.config import config
 from vdsm.utils import ActionStopped, grepCmd
 from sdc import sdCache
 import outOfProcess as oop
@@ -44,16 +42,9 @@

def getDomUuidFromVolumePath(volPath):
-    # Volume path has pattern:
-    #  /rhev/data-center/spUUID/sdUUID/images/imgUUID/volUUID
-
-    # sdUUID position after data-center
-    sdUUIDOffset = 1
-
-    volList = volPath.split('/')
-    sdUUIDPos = len(normpath(config.get('irs', 'repository')).split('/')) + \
-        sdUUIDOffset
-    return volList[sdUUIDPos]
+    # fileVolume path has pattern:
+    # */sdUUID/images/imgUUID/volUUID
+    return volPath.split('/')[-4]

class FileVolume(volume.Volume):

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Fix fix fix getDomUuidFromVolumePath()
......................................................................

Patch Set 1:

Build Successful

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/4370/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/5174/ : SUCCESS

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

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: Fix fix fix getDomUuidFromVolumePath()
......................................................................

Patch Set 1: Verified+1

Saggi Mizrahi has posted comments on this change.

Change subject: Fix fix fix getDomUuidFromVolumePath()
......................................................................

Patch Set 1: Code-Review+1

Ayal Baron has posted comments on this change.

Change subject: Fix fix fix getDomUuidFromVolumePath()
......................................................................

Patch Set 1:

(1 comment)

....................................................
File vdsm/storage/fileVolume.py
Line 43: 
Line 44: def getDomUuidFromVolumePath(volPath):
Line 45:     # fileVolume path has pattern:
Line 46:     # */sdUUID/images/imgUUID/volUUID
Line 47:     return volPath.split('/')[-4]
previously the method assumed that the /rhev/dc/ part is constant and that broke.
Now it assumes that /images/imgUUID/volUUID is constant and this is also not guaranteed.
The problem here is that we're trying to infer things from the path while the calling functions probably already have this info available.
i.e. proper solution would likely be to just get rid of this method.
Line 48: 
Line 49: 
Line 50: class FileVolume(volume.Volume):
Line 51:     """ Actually represents a single volume (i.e. part of virtual disk).

Ayal Baron has posted comments on this change.

Change subject: Fix fix fix getDomUuidFromVolumePath()
......................................................................

Patch Set 1: Code-Review-1

(1 comment)

....................................................
Commit Message
Line 3: AuthorDate: 2013-10-31 21:02:44 +0200
Line 4: Commit:     Dan Kenigsberg 
Line 5: CommitDate: 2013-10-31 21:46:06 +0000
Line 6: 
Line 7: Fix fix fix getDomUuidFromVolumePath()
Please write meaningful commit message
Line 8: 
Line 9: Change-Id: I96b71b7839666d402ec5166eefeac618685c0ab0
Line 10: Bug-Url: http://bugzilla.redhat.com/1017735

Eduardo has posted comments on this change.

Change subject: Fix fix fix getDomUuidFromVolumePath()
......................................................................

Patch Set 1: Code-Review-2

(1 comment)

....................................................
File vdsm/storage/fileVolume.py
Line 43: 
Line 44: def getDomUuidFromVolumePath(volPath):
Line 45:     # fileVolume path has pattern:
Line 46:     # */sdUUID/images/imgUUID/volUUID
Line 47:     return volPath.split('/')[-4]
I'm I agree with Ayal, as we discussed by mail.

Furthermore, from the quick search over the 6 calls we already done, we need to remove this method and the hunchback callers.
Line 48: 
Line 49: 
Line 50: class FileVolume(volume.Volume):
Line 51:     """ Actually represents a single volume (i.e. part of virtual disk).

[Dan Kenigsberg]

Dan Kenigsberg has abandoned this change.

Change subject: Fix fix fix getDomUuidFromVolumePath()
......................................................................

Abandoned

I do not see any reason why this patch is lesser than the one suggested in http://gerrit.ovirt.org/20783. Both reimplement the function, and neither fixes the deeper problem of having to use it.

However, I do note care enough about this matter in order to argue.

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Fix fix fix getDomUuidFromVolumePath()
......................................................................

Patch Set 1:

Build Successful

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/6943/ : SUCCESS

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

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

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Fix fix fix getDomUuidFromVolumePath()
......................................................................

Patch Set 1:

Build Successful

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/6943/ : SUCCESS

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

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