search for: disk_service

...quot;to finalize") >> + transfer = transfer_service.get() >> + >> + if transfer is None: > > > Are you sure this is possible? the original code assumed that we > fail with sdk.NotFoundError. > >> >> + disk_service = h['disk_service'] >> + disk = disk_service.get() >> + if disk.status == types.DiskStatus.OK: >> + continue > > > If the disk status is OK the upload was finished, so we should break, no? Yes, should f...

...if time.time() > endt: - raise RuntimeError("timed out waiting for transfer " - "to finalize") + transfer = transfer_service.get() + + if transfer is None: + disk_service = h['disk_service'] + disk = disk_service.get() + if disk.status == types.DiskStatus.OK: + continue + + if transfer.phase == types.ImageTransferPhase.FINISHED_SUCCESS: + debug("finalized afte...

This is a patch which Daniel Erez wrote originally. I have modified it only to fix a small bug in the debug() statement, and Ilanit kindly tested it here: https://bugzilla.redhat.com/show_bug.cgi?id=1680361#c38 Rich.

...time.sleep(1) > - tmp = transfer_service.get() > - if time.time() > endt: > - raise RuntimeError("timed out waiting for transfer " > - "to finalize") > + disk_service = h['disk_service'] > We can clean this a little bit by doing this outside of the loop. > + disk = disk_service.get() > + if disk.status == types.DiskStatus.LOCKED: > + if time.time() > start + timeout: > +...

...class rhv-upload: Check status more frequently rhv-upload: Don't flush() in close() rhv-upload: Ensure connection is closed in close() rhv-upload: Ensure http is closed 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:...

...time.sleep(1) > - tmp = transfer_service.get() > - if time.time() > endt: > - raise RuntimeError("timed out waiting for transfer " > - "to finalize") > + disk_service = h['disk_service'] > + disk = disk_service.get() > + if disk.status == types.DiskStatus.LOCKED: > + if time.time() > start + timeout: > + raise RuntimeError("timed out waiting for > transfer &quo...

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

...time.sleep(1) > - tmp = transfer_service.get() > - if time.time() > endt: > - raise RuntimeError("timed out waiting for transfer " > - "to finalize") > + disk_service = h['disk_service'] > + disk = disk_service.get() > + if disk.status == types.DiskStatus.LOCKED: > + if time.time() > start + timeout: > + raise RuntimeError("timed out waiting for transfer " &g...

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.

...while True: time.sleep(1) - tmp = transfer_service.get() - if time.time() > endt: - raise RuntimeError("timed out waiting for transfer " - "to finalize") + disk_service = h['disk_service'] + disk = disk_service.get() + if disk.status == types.DiskStatus.LOCKED: + if time.time() > start + timeout: + raise RuntimeError("timed out waiting for transfer " +...

...while True: time.sleep(1) - tmp = transfer_service.get() - if time.time() > endt: - raise RuntimeError("timed out waiting for transfer " - "to finalize") + disk_service = h['disk_service'] + disk = disk_service.get() + if disk.status == types.DiskStatus.LOCKED: + if time.time() > start + timeout: + raise RuntimeError("timed out waiting for transfer " +...

...while True: time.sleep(1) - tmp = transfer_service.get() - if time.time() > endt: - raise RuntimeError("timed out waiting for transfer " - "to finalize") + disk_service = h['disk_service'] + disk = disk_service.get() + if disk.status == types.DiskStatus.LOCKED: + if time.time() > start + timeout: + raise RuntimeError("timed out waiting for transfer " +...

...n.py b/v2v/rhv-upload-plugin.py index 4d61a089b..57e90484f 100644 --- a/v2v/rhv-upload-plugin.py +++ b/v2v/rhv-upload-plugin.py @@ -485,6 +485,8 @@ 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() -- 2.21.0

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

...- "to finalize") > + transfer = transfer_service.get() > + > + if transfer is None: > Are you sure this is possible? the original code assumed that we fail with sdk.NotFoundError. > + disk_service = h['disk_service'] > + disk = disk_service.get() > + if disk.status == types.DiskStatus.OK: > + continue > If the disk status is OK the upload was finished, so we should break, no? If the disk status is LOCKED, we s...

...(offset, count)) + r.read() + def flush(h): http = h['http'] transfer = h['transfer'] @@ -394,6 +405,8 @@ def flush(h): if r.status != 200: request_failed(h, r, "could not flush") + r.read() + def delete_disk_on_failure(h): disk_service = h['disk_service'] disk_service.remove() -- 2.17.1

...tus = 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 someti...

...6 +575,11 @@ def create_disk(connection): > if disk.status == types.DiskStatus.OK: > break > if time.time() > endt: > - raise RuntimeError("timed out waiting for disk to become unlocked") > + try: > + disk_service.remove() > + except Exception as e: > + debug("error removing disk %s: %s" % (disk.id, e)) > + raise RuntimeError( > + "timed out waiting for disk %s to become unlocked" % disk.id) > > return disk > --...

...> - tmp = transfer_service.get() > > - if time.time() > endt: > > - raise RuntimeError("timed out waiting for transfer " > > - "to finalize") > > + disk_service = h['disk_service'] > > + disk = disk_service.get() > > + if disk.status == types.DiskStatus.LOCKED: > > + if time.time() > start + timeout: > > + raise RuntimeError("timed out waiting fo...

...plugin just follows flush requests issued by the client (ie. qemu-img). qemu-img probably won't issue its own flush request. Should it issue a flush or is that the job of oVirt? > > + # Otherwise if we did fail then we should delete the disk. > > + else: > > + disk_service = h['disk_service'] > > + disk_service.remove() > > > > This should be done only if we created the disk - is this always the case? Yes. If we failed to create a disk, open() fails, and close() is never called. I made all the other changes you suggested, see upco...