[Libguestfs] [PATCH 06/18] rhv-upload: Fix cleanup after errors

[Nir Soffer]

When request failed, we paused the transfer. This is not needed since
our intent it to cancel the transfer.

When closing after failure, we canceled the transfer and removed the
disk. This is not needed since the transfer owns the disk and will
remove it when canceled.

When finalizing times out, we canceled the transfer and removed the
disk. This is not needed since the transfer will clean it self, and
likely to fail because cancelling is not possible after finalizing.
---
 v2v/rhv-upload-plugin.py | 37 +++++++++++++++----------------------
 1 file changed, 15 insertions(+), 22 deletions(-)

diff --git a/v2v/rhv-upload-plugin.py b/v2v/rhv-upload-plugin.py
index 5bdfdf49f..1f42c4a55 100644
--- a/v2v/rhv-upload-plugin.py
+++ b/v2v/rhv-upload-plugin.py
@@ -180,6 +180,10 @@ def open(readonly):
             inactivity_timeout = 3600,
         )
     )
+
+    # At this point the transfer owns the disk and will delete the disk if the
+    # transfer is canceled, or if finalizing the transfer fails.
+
     debug("transfer.id = %r" % transfer.id)
 
     # Get a reference to the created transfer service.
@@ -309,15 +313,12 @@ def can_flush(h):
 def get_size(h):
     return params['disk_size']
 
-# Any unexpected HTTP response status from the server will end up
-# calling this function which logs the full error, pauses the
-# transfer, sets the failed state, and raises a RuntimeError
-# exception.
+# Any unexpected HTTP response status from the server will end up calling this
+# function which logs the full error, sets the failed state, and raises a
+# RuntimeError exception.
 def request_failed(h, r, msg):
-    # Setting the failed flag in the handle causes the disk to be
-    # cleaned up on close.
+    # Setting the failed flag in the handle will cancel the transfer on close.
     h['failed'] = True
-    h['transfer_service'].pause()
 
     status = r.status
     reason = r.reason
@@ -490,15 +491,10 @@ def flush(h):
 
     r.read()
 
-def delete_disk_on_failure(h):
-    transfer_service = h['transfer_service']
-    transfer_service.cancel()
-    disk_service = h['disk_service']
-    disk_service.remove()
-
 def close(h):
     http = h['http']
     connection = h['connection']
+    transfer_service = h['transfer_service']
 
     # This is sometimes necessary because python doesn't set up
     # sys.stderr to be line buffered and so debug, errors or
@@ -508,15 +504,17 @@ def close(h):
 
     http.close()
 
-    # If the connection failed earlier ensure we clean up the disk.
+    # If the connection failed earlier ensure we cancel the trasfer. Canceling
+    # the transfer will delete the disk.
     if h['failed']:
-        delete_disk_on_failure(h)
-        connection.close()
+        try:
+            transfer_service.cancel()
+        finally:
+            connection.close()
         return
 
     try:
         disk = h['disk']
-        transfer_service = h['transfer_service']
 
         transfer_service.finalize()
 
@@ -548,11 +546,6 @@ def close(h):
         with builtins.open(params['diskid_file'], 'w') as fp:
             fp.write(disk.id)
 
-    except:
-        # Otherwise on any failure we must clean up the disk.
-        delete_disk_on_failure(h)
-        raise
-
     finally:
         connection.close()
 
-- 
2.21.0