search for: transfer_service

v1 was here: https://www.redhat.com/archives/libguestfs/2018-June/msg00015.html v2 fixes all the issues raised in Nir's review. I tested it against ovirt-engine-4.2.4.1-1.el7.noarch and it works, although I wasn't easily able to trigger the error path so that code is not really tested (except for syntax checking). Rich.

...meError 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...

...function which logs the full error, pauses the +# transfer, sets the failed state, and raises a RuntimeError +# exception. +def unexpected_response(h, r, msg): + # Setting the failed flag in the handle causes the disk to be + # cleaned up on close. + h['failed'] = True + h['transfer_service'].pause() + + status = r.status + reason = r.reason + body = r.read() + + # Log the full error if we're verbose. + debug("unexpected response from imageio server:") + debug(msg) + debug("%d: %s" % (status, reason)) + debug(body) + + # Only a sh...

...this function which logs the full error, pauses the +# transfer, 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. + h['failed'] = True + h['transfer_service'].pause() + + status = r.status + reason = r.reason + try: + body = r.read() + except EnvironmentError as e: + body = "(Unable to read response body: %s)" % e + + # Log the full error if we're verbose. + debug("unexpected response from imageio...

...t; +# exception. > +def unexpected_response(h, r, msg): > This is not really unexpected response, maybe "request_failed"? > + # Setting the failed flag in the handle causes the disk to be > + # cleaned up on close. > + h['failed'] = True > + h['transfer_service'].pause() > + > + status = r.status > + reason = r.reason > + body = r.read() > This should not fail, but if we there is a possible failure, it would be more robust to do: try: body = r.read() except EnvironmentError as e: body = "(Unable t...

Only compile tested. I'm keeping this as a separate patch for the moment until the relevant change on the oVirt side goes upstream. Rich.

On Wed, Nov 20, 2019 at 1:19 AM Nir Soffer <nirsof@gmail.com> wrote: > > Extract a cancel_transfer() function, so we can cancel a transfer in > plugin handlers without keeping the transfer_service around. > > Fixes a NameError when starting a transfer fails: > > During handling of the above exception, another exception occurred: > > Traceback (most recent call last): > File "/var/tmp/rhvupload.Vw0CIU/rhv-upload-plugin.py", line 97, in open >...

...d 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.re...

...equest_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'] > > -...

...ed in close() rhv-upload: Fix cleanup after errors rhv-upload: Group oVirt SDK functions rhv-upload: Don't keep disk_service in handle rhv-upload: Extract create_disk() function rhv-upload: Try to remove disk on timeout rhv-upload: Keep disk_id in handle rhv-upload: Don't keep transfer_service in handle rhv-upload: Get host before creating disk rhv-upload: Extract create_transfer() function rhv-upload: Extract imageio helpers rhv-upload: Extract get_options() helper rhv-upload: Extract optimize_http() helper rhv-upload: Clean up username and password v2v/rhv-upload-plugin.p...

...eletions(-) 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 t...

.../v2v/rhv-upload-plugin.py index 57e90484f..51a7f381a 100644 --- a/v2v/rhv-upload-plugin.py +++ b/v2v/rhv-upload-plugin.py @@ -193,12 +193,14 @@ def open(readonly): if transfer.phase != types.ImageTransferPhase.INITIALIZING: break if time.time() > endt: + transfer_service.cancel() raise RuntimeError("timed out waiting for transfer status " "!= INITIALIZING") # Now we have permission to start the transfer. if params['rhv_direct']: if transfer.transfer_url is None: +...

...#39;: 0, 'http': http, 'path': destination_url.path, 'transfer': transfer, @@ -189,14 +188,10 @@ def pread(h, count, offset): return r.read() def pwrite(h, buf, offset): + http = h['http'] + transfer=h['transfer'] + transfer_service=h['transfer_service'] count = len(buf) - h['highestwrite'] = max(h['highestwrite'], offset+count) - do_pwrite(h, buf, offset, count) - -def do_pwrite(h, buf, offset, count): - http = h['http'] - transfer=h['transfer'] - transfer_service=h[...

Fix NameError introduced by inocomplete change to extract create_transfer() by extracting cancel_transfer() function. Finally extract finallize_transfer() function, dealing with the poorly documented and over complicated oVirt SDK. Tested with: libguestfs-1.40.2-4.fc30.x86_64 nbdkit-1.12.8-1.fc30.x86_64 Tested flows: - Successful import - Error in close() - by injecting long sleep in vdsm

...disk_id instead of disk.id in close() v1 was here: https://www.redhat.com/archives/libguestfs/2019-November/msg00060.html Nir Soffer (11): rhv-upload: Cancel transfer if finalize failed rhv-upload: Keep disk_id in handle rhv-upload: Show disk id in error message rhv-upload: Don't keep transfer_service in handle rhv-upload: Get host before creating disk rhv-upload: Extract create_transfer() function rhv-upload: Show transfer id in error message rhv-upload: Extract imageio helpers rhv-upload: Extract get_options() helper rhv-upload: Extract optimize_http() helper rhv-upload: Clean up...

Extract a cancel_transfer() function, so we can cancel a transfer in plugin handlers without keeping the transfer_service around. Fixes a NameError when starting a transfer fails: During handling of the above exception, another exception occurred: Traceback (most recent call last): File "/var/tmp/rhvupload.Vw0CIU/rhv-upload-plugin.py", line 97, in open transfer_service.cancel() N...

...and crash later with > KeyError, and make it easier to understand what is the state managed > by the plugin. Yup, strong typing FTW ... > [snipped] > > +def pwrite(h, buf, offset): > > + http = h['http'] > > + transfer=h['transfer'] > > + transfer_service=h['transfer_service'] > > + > > + count = len(buf) > > + h['highestwrite'] = max(h['highestwrite'], offset+count) > > + > > + http.putrequest("PUT", h['path'] + "?flush=n") > > > > "?flush=n...

Remove some instances of "transfer" and "transfer_service" that are not used in current code. --- v2v/rhv-upload-plugin.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/v2v/rhv-upload-plugin.py b/v2v/rhv-upload-plugin.py index 10887c031..5c036c46a 100644 --- a/v2v/rhv-upload-plugin.py +++ b/v2v/rhv-upload-plugin.py @@ -263,7 +263,6 @@ def r...

After invoking transfer_service.finalize, check operation status by examining ImageTransferPhase and DiskStatus. This is done instead of failing after a predefined timeout regardless the status. * not verified * Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1680361 --- v2v/rhv-upload-plugin.py | 26 +++++++++++++++++++++...

...ict. This ensures that we don't forget needs_auth or other keys and crash later with KeyError, and make it easier to understand what is the state managed by the plugin. [snipped] +def pwrite(h, buf, offset): > + http = h['http'] > + transfer=h['transfer'] > + transfer_service=h['transfer_service'] > + > + count = len(buf) > + h['highestwrite'] = max(h['highestwrite'], offset+count) > + > + http.putrequest("PUT", h['path'] + "?flush=n") > "?flush=n" has effect only if h["can_fl...