On Thu, Nov 28, 2019 at 10:58 PM Richard W.M. Jones <rjones(a)redhat.com> wrote:
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
Seems that Daniel is too busy now to review this, so I think we should
push this.