Change in vdsm[master]: Remove local mount point if storage server connection fails

Mark Wu has uploaded a new change for review.

Change subject: Remove local mount point if storage server connection fails
......................................................................

Remove local mount point if storage server connection fails

Change-Id: Ic7aa1a47b7fa3577e83daa82fbad17fdc2ebd51f
Signed-off-by: Mark Wu 
---
M vdsm/storage/storageServer.py
1 file changed, 5 insertions(+), 2 deletions(-)

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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ic7aa1a47b7fa3577e83daa82fbad17fdc2ebd51f
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Mark Wu

Mark Wu has posted comments on this change.

Change subject: Remove local mount point if storage server connection fails
......................................................................

Patch Set 1: Verified

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic7aa1a47b7fa3577e83daa82fbad17fdc2ebd51f
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Mark Wu 
Gerrit-Reviewer: Mark Wu

Shu Ming has posted comments on this change.

Change subject: Remove local mount point if storage server connection fails
......................................................................

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

The system should keep clean after the connection failed.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic7aa1a47b7fa3577e83daa82fbad17fdc2ebd51f
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Mark Wu 
Gerrit-Reviewer: Mark Wu 
Gerrit-Reviewer: Shu Ming

[Ryan Harper]

Ryan Harper has posted comments on this change.

Change subject: Remove local mount point if storage server connection fails
......................................................................

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

I don't like this idea of removing directories that we may or maynot have created...

I see that on mount we attempt to create the directory structure, but we don't track if we did create it or not; what if the dirs already existed and there were files in there?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic7aa1a47b7fa3577e83daa82fbad17fdc2ebd51f
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Mark Wu 
Gerrit-Reviewer: Mark Wu 
Gerrit-Reviewer: Ryan Harper 
Gerrit-Reviewer: Shu Ming

Shu Ming has posted comments on this change.

Change subject: Remove local mount point if storage server connection fails
......................................................................

Patch Set 1: No score

If that directory was existing before, the code should fall in this path and exit, it will never reach os.rmdir(self._getLocalPath()).
except OSError as e:
 if e.errno != errno.EEXIST: 
       raise

The assumption here is the local path is a newly created directory. If the mount operation failed, the newly crated directory should be cleaned.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic7aa1a47b7fa3577e83daa82fbad17fdc2ebd51f
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Mark Wu 
Gerrit-Reviewer: Mark Wu 
Gerrit-Reviewer: Ryan Harper 
Gerrit-Reviewer: Shu Ming

Shu Ming has posted comments on this change.

Change subject: Remove local mount point if storage server connection fails
......................................................................

Patch Set 1:

It is a policy issue here: 
1) Should we allow to mount the local existing local directory with files under it? if we don't allow this mount operation, we can just change the exception handling as:
except OSError as e:
 if （e.errno != errno.EEXIST) || (not_empty(local_directory)):
     raise

2) if we do allow it.  we have a flag to test if rmdir should be done in the other exception handling.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic7aa1a47b7fa3577e83daa82fbad17fdc2ebd51f
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Mark Wu 
Gerrit-Reviewer: Mark Wu 
Gerrit-Reviewer: Ryan Harper 
Gerrit-Reviewer: Shu Ming

Mark Wu has posted comments on this change.

Change subject: Remove local mount point if storage server connection fails
......................................................................

Patch Set 1:

Ryan,
I can't figure out a case in which the directory to be removed is not empty, because it's just a local mount point. Except, you manually create directory under vdsm repository and put files into it. Is it a misusage? 
Thanks.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic7aa1a47b7fa3577e83daa82fbad17fdc2ebd51f
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Mark Wu 
Gerrit-Reviewer: Mark Wu 
Gerrit-Reviewer: Ryan Harper 
Gerrit-Reviewer: Shu Ming

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Remove local mount point if storage server connection fails
......................................................................

Patch Set 1: No score

Build Started http://jenkins.ovirt.info/job/vdsm_unit_tests_by_patch/86/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic7aa1a47b7fa3577e83daa82fbad17fdc2ebd51f
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Mark Wu 
Gerrit-Reviewer: Mark Wu 
Gerrit-Reviewer: Ryan Harper 
Gerrit-Reviewer: Shu Ming 
Gerrit-Reviewer: oVirt Jenkins CI Server

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Remove local mount point if storage server connection fails
......................................................................

Patch Set 1:

Build Successful

http://jenkins.ovirt.info/job/vdsm_unit_tests_by_patch/86/ : SUCCESS

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic7aa1a47b7fa3577e83daa82fbad17fdc2ebd51f
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Mark Wu 
Gerrit-Reviewer: Mark Wu 
Gerrit-Reviewer: Ryan Harper 
Gerrit-Reviewer: Shu Ming 
Gerrit-Reviewer: oVirt Jenkins CI Server

Mark Wu has abandoned this change.

Change subject: Remove local mount point if storage server connection fails
......................................................................

Patch Set 1: Abandoned

Already fixed in http://gerrit.ovirt.org/#/c/8695/4

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

Gerrit-MessageType: abandon
Gerrit-Change-Id: Ic7aa1a47b7fa3577e83daa82fbad17fdc2ebd51f
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Mark Wu 
Gerrit-Reviewer: Mark Wu 
Gerrit-Reviewer: Royce Lv 
Gerrit-Reviewer: Ryan Harper 
Gerrit-Reviewer: Shu Ming 
Gerrit-Reviewer: oVirt Jenkins CI Server