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

Catch all vim exceptions in disk_detach #460

Merged
merged 1 commit into from Jun 15, 2016
Merged

Catch all vim exceptions in disk_detach #460

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
21 changes: 12 additions & 9 deletions esx_service/vmdk_ops.py
View file
Open in desktop
Expand Up @@ -40,6 +40,7 @@
import signal
import subprocess
import sys
import traceback
import threading
import time
from ctypes import *
Expand Down Expand Up @@ -307,13 +308,14 @@ def listVMDK(path):

# Return VM managed object, reconnect if needed. Throws if fails twice.
def findVmByUuid(vm_uuid):
vm = None
try:
vm = si.content.searchIndex.FindByUuid(None, vm_uuid, True, False)
if vm:
return vm
except Exception as e:
logging.exception("Failed to find VM uuid=%s (traceback below), " \
"retrying...", vm_uuid)
except Exception as ex:
Copy link
Contributor

Choose a reason for hiding this comment

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

is there a guarantee that the second search will always complete without exception?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

there is no guarantee, but we know that on connection timeout retry fixes it, and on other issues we want to rethrow if we fail in retry

logging.warning("Failed to find VM by uuid=%s, retrying...\n%s",
vm_uuid, str(ex))
if vm:
return vm
#
# Retry. It can throw if connect/search fails. But search can't return None
# since we get UUID from VMM so VM must exist
Expand Down Expand Up @@ -648,10 +650,11 @@ def disk_detach(vmdk_path, vm):

try:
wait_for_tasks(si, [vm.ReconfigVM_Task(spec=spec)])
except vim.fault.GenericVmConfigFault as ex:
for f in ex.faultMessage:
logging.warning(f.message)
return err("Failed to detach " + vmdk_path)
except vim.Fault.VimFault as ex:
ex_type, ex_value, ex_traceback = sys.exc_info()
msg = "Failed to detach %s: %s" % (vmdk_path, ex.msg)
logging.warning("%s\n%s", msg, "".join(traceback.format_tb(ex_traceback)))
return err(msg)

Copy link
Contributor

Choose a reason for hiding this comment

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

This will leave the disk attached to the VM. Like I'd mentioned in PR #433 (original set of changes) there is nothing the plugin can do to help this situation except log the error. Does the error notify docker that the unmount has failed and docker must keep track that the volume is still in use? The volume remains in attached state if docker doesn't care to unmount again.

Copy link
Contributor Author

@msterin msterin Jun 14, 2016 •
edited

Choose a reason for hiding this comment

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

i am not sure what is your question ?

setStatusDetached(vmdk_path)
logging.info("Disk detached %s", vmdk_path)
Expand Down