On Thu, Nov 28, 2019 at 09:34:18PM +0200, Nir Soffer wrote:
We were not considering failures while initializing the transfer. In
this case the transfer phase can change to PAUSED_SYSTEM or
FINISHED_FAILURE, and transfer_url will be None, which failed the
upload with a misleading error:
RuntimeError: direct upload to host not supported, requires
ovirt-engine >= 4.2 and only works when virt-v2v is run within the
oVirt/RHV environment, eg. on an oVirt node
Change the wait loop to consider all cases:
- Transfer failed and was removed
- Transfer failed and will be removed soon
- Transfer paused by the system (cancel required)
- Unexpected transfer phase (cancel required)
- Timeout waiting for TRANSFERRING state (cancel required)
Reported-by: Xiaodai Wang
---
I could easy simulate the case when the system paused the transfer by
injecting an error in vdsm, failing transfer initialization.
The import fail with:
nbdkit: python[1]: error:
/home/nsoffer/src/virt-v2v/tmp/rhvupload.1DgXyh/rhv-upload-plugin.py: open: error:
Traceback (most recent call last):
File "/home/nsoffer/src/virt-v2v/tmp/rhvupload.1DgXyh/rhv-upload-plugin.py",
line 109, in open
transfer = create_transfer(connection, disk, host)
File "/home/nsoffer/src/virt-v2v/tmp/rhvupload.1DgXyh/rhv-upload-plugin.py",
line 539, in create_transfer
"transfer %s was paused by system" % transfer.id)
RuntimeError: transfer 32b97384-ac8b-40d5-b423-26d31faabe32 was paused by system
I could not simulate the other cases. This probaly requires injecting
errors in engine.
You might be able to inject errors more easily than that by modifying
the test harness (tests/test-v2v-o-rhv-upload-module/ovirtsdk4/).
Anyway patch looks reasonable, although I didn't test it, so:
ACK
Rich.
v2v/rhv-upload-plugin.py | 40
+++++++++++++++++++++++++++++++---------
1 file changed, 31 insertions(+), 9 deletions(-)
diff --git a/v2v/rhv-upload-plugin.py b/v2v/rhv-upload-plugin.py
index 75e4f404..3942ec72 100644
--- a/v2v/rhv-upload-plugin.py
+++ b/v2v/rhv-upload-plugin.py
@@ -513,21 +513,43 @@ def create_transfer(connection, disk, host):
# Get a reference to the created transfer service.
transfer_service = transfers_service.image_transfer_service(transfer.id)
- # After adding a new transfer for the disk, the transfer's status
- # will be INITIALIZING. Wait until the init phase is over. The
- # actual transfer can start when its status is "Transferring".
+ # Wait until transfer's phase change from INITIALIZING to TRANSFERRING. On
+ # errors transfer's phase can change to PAUSED_SYSTEM or FINISHED_FAILURE.
+ # If the transfer was paused, we need to cancel it to remove the disk,
+ # otherwise the system will remove the disk and transfer shortly after.
+
endt = time.time() + timeout
while True:
- transfer = transfer_service.get()
- if transfer.phase != types.ImageTransferPhase.INITIALIZING:
+ time.sleep(1)
+ try:
+ transfer = transfer_service.get()
+ except sdk.NotFoundError:
+ # The system has removed the disk and the transfer.
+ raise RuntimeError("transfer %s was removed" % transfer.id)
+
+ if transfer.phase == types.ImageTransferPhase.FINISHED_FAILURE:
+ # The system will remove the disk and the transfer soon.
+ raise RuntimeError(
+ "transfer %s has failed" % transfer.id)
+
+ if transfer.phase == types.ImageTransferPhase.PAUSED_SYSTEM:
+ transfer_service.cancel()
+ raise RuntimeError(
+ "transfer %s was paused by system" % transfer.id)
+
+ if transfer.phase == types.ImageTransferPhase.TRANSFERRING:
break
- if time.time() > endt:
+
+ if transfer.phase != types.ImageTransferPhase.INITIALIZING:
transfer_service.cancel()
raise RuntimeError(
- "timed out waiting for transfer %s status != INITIALIZING"
- % transfer.id)
+ "unexpected transfer %s phase %s"
+ % (transfer.id, transfer.phase))
- time.sleep(1)
+ if time.time() > endt:
+ transfer_service.cancel()
+ raise RuntimeError(
+ "timed out waiting for transfer %s" % transfer.id)
return transfer
--
2.21.0
--
Richard Jones, Virtualization Group, Red Hat
http://people.redhat.com/~rjones
Read my programming and virtualization blog:
http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html