Skip to content
This repository has been archived by the owner on Nov 9, 2020. It is now read-only.

Confusing error message from volume.create when datastore is not writ… #718

Merged
merged 6 commits into from
Nov 12, 2016
Merged

Confusing error message from volume.create when datastore is not writ… #718

Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
646bd04
Confusing error message from volume.create when datastore is not writ…
Nov 8, 2016
56fe3be
Addressed review comments from Mark and Prashanth - Use OS errcode fu…
Nov 9, 2016
832fbdf
Removed OSFS error code global variables
Nov 9, 2016
02d74b2
Merge branch 'master' into Bug_CreateVolumeOnReadOnlyDatastore3
Nov 9, 2016
a528ed6
Fix to make sure dockvols is created before tenant directory is created
Nov 9, 2016
11631ab
Added specific error message during tenane creation failure
Nov 11, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 12 additions & 9 deletions esx_service/vmdk_ops.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -490,24 +490,27 @@ def get_vol_path(datastore, tenant_name=None):
if os.path.isdir(path):
# If the path exists then return it as is.
logging.debug("Found %s, returning", path)
return path
return path, None

if not os.path.isdir(dock_vol_path):
# The osfs tools are usable for DOCK_VOLS_DIR on all datastores
cmd = "{0} {1}".format(OSFS_MKDIR_CMD, dock_vol_path)
rc, out = RunCommand(cmd)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did you delete this code? This need to exist. See #687
OSFS_MKDIR_CMD is used to create dock_vol_path and MKDIR_CMD is used to create Tenant folder.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't delete any code. It achieves the same functionality as before. I just remodified the code so that it looks simpler now.

if rc != 0:
logging.warning("Failed to create %s", dock_vol_path)
return None
errMsg = "{0} creation failed - {1} on {2}".format(DOCK_VOLS_DIR, os.strerror(rc), datastore)
logging.warning(errMsg)
return None, err(errMsg)
if tenant_name and not os.path.isdir(path):
# The mkdir command is used to create "tenant_name" folder inside DOCK_VOLS_DIR on "datastore"
cmd = "{0} {1}".format(MKDIR_CMD, path)
rc, out = RunCommand(cmd)
if rc != 0:
logging.warning("Failed to create %s", path)
return None
errMsg = "Failed to initialize volume path {0}".format(path)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not include error string here?
errMsg = "Failed to initialize volume path {0} : {1}".format(path, os.strerror(rc))

Copy link
Contributor Author

@BaluDontu BaluDontu Nov 10, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem is mkdir is that it returns either 0 or 1. 0 on success and 1 on failure for any reason.

Inorder to simulate this condition, where it fails on creating a tenant and is successful on dockvol creation. I have changed the access permissions on dockvol.

For example, when I printed the rc, out values to log
[photon-new-1-type@nfsstore-thanks] [DEBUG ] ****rc: 1, out: mkdir: can't create directory '/vmfs/volumes/nfsstore-thanks/dockvols/T12': Permission denied

Here, it clearly shows the error statement but return code is 1 which stands for "Operation not permitted".

os.strerror(1)
'Operation not permitted'

For that reason, I just settled with a normal message without the reason.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not then log "out" which already has error string?

logging.warning(errMsg)
return None, err(errMsg)

logging.info("Created %s", path)
return path
return path, None

def parse_vol_name(full_vol_name):
"""
Expand Down Expand Up @@ -599,10 +602,10 @@ def executeRequest(vm_uuid, vm_name, config_path, cmd, full_vol_name, opts):
return err(error_info)

# get /vmfs/volumes/<volid>/dockvols path on ESX:
path = get_vol_path(datastore, tenant_name)
path, errMsg = get_vol_path(datastore, tenant_name)
logging.debug("executeRequest %s %s", tenant_name, path)
if path is None:
return err("Failed to initialize volume path {0}".format(path))
return errMsg

vmdk_path = vmdk_utils.get_vmdk_path(path, vol_name)

Expand Down Expand Up @@ -1097,7 +1100,7 @@ def set_vol_opts(name, options):
return False

# get /vmfs/volumes/<datastore>/dockvols path on ESX:
path = get_vol_path(datastore)
path, errMsg = get_vol_path(datastore)

if path is None:
msg = "Failed to get datastore path {0}".format(path)
Expand Down