Change in vdsm[master]: Raise specific exception when local path not exist

Tal Nisan has uploaded a new change for review.

Change subject: Raise specific exception when local path not exist
......................................................................

Raise specific exception when local path not exist

When trying to connect to local storage connection, raise a
specific exception in case the directory does not exist instead of
a general one

Change-Id: I9310e3895e30fa5b2ae9a092aa4c1fef508c8e96
Bug-Url: https://bugzilla.redhat.com/949292
Signed-off-by: Tal Nisan <tnisan(a)redhat.com&gt;
---
M vdsm/storage/storageServer.py
M vdsm/storage/storage_exception.py
2 files changed, 9 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/24/13724/1

diff --git a/vdsm/storage/storageServer.py b/vdsm/storage/storageServer.py
index 023f607..8eff315 100644
--- a/vdsm/storage/storageServer.py
+++ b/vdsm/storage/storageServer.py
@@ -463,6 +463,8 @@
                             self._path.replace("_",
"__").replace("/", "_"))
 
     def checkTarget(self):
+        if not os.path.exists(self._path):
+            raise se.StorageServerLocalDirNotExistError(self._path)
         if not os.path.isdir(self._path):
             raise se.StorageServerLocalNotDirError(self._path)
         fileSD.validateDirAccess(self._path)
diff --git a/vdsm/storage/storage_exception.py b/vdsm/storage/storage_exception.py
index c2c3dc8..cb22cd0 100644
--- a/vdsm/storage/storage_exception.py
+++ b/vdsm/storage/storage_exception.py
@@ -1200,6 +1200,13 @@
     message = "Connection Reference ID was not registered"
 
 
+class StorageServerLocalDirNotExistError(StorageException):
+    code = 480
+    message = "The specified local path does not exist"
+
+    def __init__(self, path):
+        self.value = "path = %s" % path
+
 #################################################
 #  LVM related Exceptions
 #################################################


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I9310e3895e30fa5b2ae9a092aa4c1fef508c8e96
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Tal Nisan <tnisan(a)redhat.com&gt;

Eduardo has posted comments on this change.

Change subject: Raise specific exception when local path not exist
......................................................................


Patch Set 3: Code-Review-1

(1 comment)

Please explain which issue are you trying to solve.

Raising specific exceptions is a very bad practice.
The mount will fail anyway and the OSError is descriptive enough.

Changing the responses of the engine based on specific exceptions is a disaster recipe and
should be avoided since such entanglement is fixing forever the vdsm code, making
impossible any improvement.

....................................................
File vdsm/storage/storageServer.py
Line 462:         return os.path.join(self.localPathBase,
Line 463:                             self._path.replace("_",
"__").replace("/", "_"))
Line 464: 
Line 465:     def checkTarget(self):
Line 466:         if not os.path.exists(self._path) or not os.path.isdir(self._path):
Nothing was removed. Your checks are still there.
Line 467:             raise se.StorageServerLocalNotDirError(self._path)
Line 468:         fileSD.validateDirAccess(self._path)
Line 469:         return True
Line 470: 


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I9310e3895e30fa5b2ae9a092aa4c1fef508c8e96
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Tal Nisan <tnisan(a)redhat.com&gt;
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com&gt;
Gerrit-Reviewer: Better Saggi <bettersaggi(a)gmail.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com&gt;
Gerrit-Reviewer: Tal Nisan <tnisan(a)redhat.com&gt;
Gerrit-Reviewer: Yeela Kaplan <ykaplan(a)redhat.com&gt;
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes

Saggi Mizrahi has posted comments on this change.

Change subject: Raise specific exception when local path not exist
......................................................................


Patch Set 3: Code-Review-1

(1 comment)

....................................................
File vdsm/storage/storageServer.py
Line 462:         return os.path.join(self.localPathBase,
Line 463:                             self._path.replace("_",
"__").replace("/", "_"))
Line 464: 
Line 465:     def checkTarget(self):
Line 466:         if not os.path.exists(self._path) or not os.path.isdir(self._path):
If you look at change 6be8540d you'll see that Edu (wrongly) removed
validateDirAccess()

Please change it so we use validateDirAccess() and never add se exceptions to
storageServer.
Always, throw a custom exception and convert it in HSM
Line 467:             raise se.StorageServerLocalNotDirError(self._path)
Line 468:         fileSD.validateDirAccess(self._path)
Line 469:         return True
Line 470: 


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I9310e3895e30fa5b2ae9a092aa4c1fef508c8e96
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Tal Nisan <tnisan(a)redhat.com&gt;
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com&gt;
Gerrit-Reviewer: Better Saggi <bettersaggi(a)gmail.com&gt;
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com&gt;
Gerrit-Reviewer: Tal Nisan <tnisan(a)redhat.com&gt;
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Raise specific exception when local path not exist
......................................................................


Patch Set 3:

Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/1927/ (2/2)

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I9310e3895e30fa5b2ae9a092aa4c1fef508c8e96
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Tal Nisan <tnisan(a)redhat.com&gt;
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Tal Nisan <tnisan(a)redhat.com&gt;
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server

[Federico Simoncelli]

Federico Simoncelli has posted comments on this change.

Change subject: Raise specific exception when local path not exist
......................................................................


Patch Set 3: (1 inline comment)

....................................................
File vdsm/storage/storageServer.py
Line 462:         return os.path.join(self.localPathBase,
Line 463:                             self._path.replace("_",
"__").replace("/", "_"))
Line 464: 
Line 465:     def checkTarget(self):
Line 466:         if not os.path.exists(self._path) or not os.path.isdir(self._path):
If there's any code that relies on validateDirAccess being successful when the path is
missing (I doubt that there is) it should be changed asap.
Line 467:             raise se.StorageServerLocalNotDirError(self._path)
Line 468:         fileSD.validateDirAccess(self._path)
Line 469:         return True
Line 470: 


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I9310e3895e30fa5b2ae9a092aa4c1fef508c8e96
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Tal Nisan <tnisan(a)redhat.com&gt;
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com&gt;
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Tal Nisan <tnisan(a)redhat.com&gt;
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server

Tal Nisan has abandoned this change.

Change subject: Raise specific exception when local path not exist
......................................................................


Abandoned

Stale

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

Gerrit-MessageType: abandon
Gerrit-Change-Id: I9310e3895e30fa5b2ae9a092aa4c1fef508c8e96
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Tal Nisan <tnisan(a)redhat.com&gt;
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com&gt;
Gerrit-Reviewer: Tal Nisan <tnisan(a)redhat.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Raise specific exception when local path not exist
......................................................................


Patch Set 2:

Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/1913/ (1/2)

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I9310e3895e30fa5b2ae9a092aa4c1fef508c8e96
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Tal Nisan <tnisan(a)redhat.com&gt;
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Tal Nisan <tnisan(a)redhat.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Raise specific exception when local path not exist
......................................................................


Patch Set 2:

Build Successful 

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

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I9310e3895e30fa5b2ae9a092aa4c1fef508c8e96
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Tal Nisan <tnisan(a)redhat.com&gt;
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Tal Nisan <tnisan(a)redhat.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Raise specific exception when local path not exist
......................................................................


Patch Set 2:

Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/1862/ (2/2)

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I9310e3895e30fa5b2ae9a092aa4c1fef508c8e96
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Tal Nisan <tnisan(a)redhat.com&gt;
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Tal Nisan <tnisan(a)redhat.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Raise specific exception when local path not exist
......................................................................


Patch Set 1: Fails

Build Failed 

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

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/1912/ : FAILURE

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I9310e3895e30fa5b2ae9a092aa4c1fef508c8e96
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Tal Nisan <tnisan(a)redhat.com&gt;
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Raise specific exception when local path not exist
......................................................................


Patch Set 1:

Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/1912/ (2/2)

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I9310e3895e30fa5b2ae9a092aa4c1fef508c8e96
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Tal Nisan <tnisan(a)redhat.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Raise specific exception when local path not exist
......................................................................


Patch Set 1:

Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/1861/ (1/2)

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I9310e3895e30fa5b2ae9a092aa4c1fef508c8e96
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Tal Nisan <tnisan(a)redhat.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server

Tal Nisan has posted comments on this change.

Change subject: Raise specific exception when local path not exist
......................................................................


Patch Set 3: (1 inline comment)

....................................................
File vdsm/storage/storageServer.py
Line 462:         return os.path.join(self.localPathBase,
Line 463:                             self._path.replace("_",
"__").replace("/", "_"))
Line 464: 
Line 465:     def checkTarget(self):
Line 466:         if not os.path.exists(self._path) or not os.path.isdir(self._path):
I thought about it but validateDirAccess is called from 4 other places, at least one of
which is catching a specific exception which is not the one we throw here, my fear is that
I might cause regressions to other flows
Line 467:             raise se.StorageServerLocalNotDirError(self._path)
Line 468:         fileSD.validateDirAccess(self._path)
Line 469:         return True
Line 470: 


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I9310e3895e30fa5b2ae9a092aa4c1fef508c8e96
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Tal Nisan <tnisan(a)redhat.com&gt;
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com&gt;
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Tal Nisan <tnisan(a)redhat.com&gt;
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server

[Federico Simoncelli]

Federico Simoncelli has posted comments on this change.

Change subject: Raise specific exception when local path not exist
......................................................................


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

(1 inline comment)

....................................................
File vdsm/storage/storageServer.py
Line 462:         return os.path.join(self.localPathBase,
Line 463:                             self._path.replace("_",
"__").replace("/", "_"))
Line 464: 
Line 465:     def checkTarget(self):
Line 466:         if not os.path.exists(self._path) or not os.path.isdir(self._path):
I strongly suggest to move these validations inside validateDirAccess and get rid of
checkTarget altogether. I can't think of a reason for validateDirAccess to pass if the
directory doesn't exists (or if it's a file instead).
Line 467:             raise se.StorageServerLocalNotDirError(self._path)
Line 468:         fileSD.validateDirAccess(self._path)
Line 469:         return True
Line 470: 


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I9310e3895e30fa5b2ae9a092aa4c1fef508c8e96
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Tal Nisan <tnisan(a)redhat.com&gt;
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com&gt;
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Tal Nisan <tnisan(a)redhat.com&gt;
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Raise specific exception when local path not exist
......................................................................


Patch Set 3:

Build Successful 

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

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I9310e3895e30fa5b2ae9a092aa4c1fef508c8e96
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Tal Nisan <tnisan(a)redhat.com&gt;
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Tal Nisan <tnisan(a)redhat.com&gt;
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Raise specific exception when local path not exist
......................................................................


Patch Set 3:

Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/1876/ (1/2)

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I9310e3895e30fa5b2ae9a092aa4c1fef508c8e96
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Tal Nisan <tnisan(a)redhat.com&gt;
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Tal Nisan <tnisan(a)redhat.com&gt;
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server

Tal Nisan has posted comments on this change.

Change subject: Raise specific exception when local path not exist
......................................................................


Patch Set 2: (1 inline comment)

....................................................
File vdsm/storage/storageServer.py
Line 463:                             self._path.replace("_",
"__").replace("/", "_"))
Line 464: 
Line 465:     def checkTarget(self):
Line 466:         if not os.path.exists(self._path):
Line 467:             raise se.StorageServerLocalDirNotExistError(self._path)
Discussed with Ayal, we'll go with the other idea we had, to include it all in the
StorageServerLocalNotDirError exception and change the message to directory not found or
is not a directory, will send an updated patch soon
Line 468:         if not os.path.isdir(self._path):
Line 469:             raise se.StorageServerLocalNotDirError(self._path)
Line 470:         fileSD.validateDirAccess(self._path)
Line 471:         return True


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I9310e3895e30fa5b2ae9a092aa4c1fef508c8e96
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Tal Nisan <tnisan(a)redhat.com&gt;
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Tal Nisan <tnisan(a)redhat.com&gt;
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server

[Federico Simoncelli]

Federico Simoncelli has posted comments on this change.

Change subject: Raise specific exception when local path not exist
......................................................................


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

(1 inline comment)

Marking for visibility.

....................................................
File vdsm/storage/storageServer.py
Line 463:                             self._path.replace("_",
"__").replace("/", "_"))
Line 464: 
Line 465:     def checkTarget(self):
Line 466:         if not os.path.exists(self._path):
Line 467:             raise se.StorageServerLocalDirNotExistError(self._path)
I don't think that we need an additional (and so specific) exception for this case. I
also feel like this check should be included in fileSD.validateDirAccess (how a missing
path could pass the validation?).
Line 468:         if not os.path.isdir(self._path):
Line 469:             raise se.StorageServerLocalNotDirError(self._path)
Line 470:         fileSD.validateDirAccess(self._path)
Line 471:         return True


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I9310e3895e30fa5b2ae9a092aa4c1fef508c8e96
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Tal Nisan <tnisan(a)redhat.com&gt;
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Tal Nisan <tnisan(a)redhat.com&gt;
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server

Zhou Zheng Sheng has posted comments on this change.

Change subject: Raise specific exception when local path not exist
......................................................................


Patch Set 2:

The bugzilla content is private. It says "You are not authorized to access bug
#949292." Could you copy some general information from that bugzilla to the commit
message?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I9310e3895e30fa5b2ae9a092aa4c1fef508c8e96
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Tal Nisan <tnisan(a)redhat.com&gt;
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Tal Nisan <tnisan(a)redhat.com&gt;
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server