<div dir="ltr"><div dir="ltr"><div class="gmail_default" style="font-size:small;color:#000000"><span style="color:rgb(34,34,34)">On Sun, Mar 17, 2019 at 2:07 PM Daniel Erez <<a href="mailto:derez@redhat.com">derez@redhat.com</a>> wrote:</span><br></div></div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">After invoking transfer_service.finalize, check operation<br>
status by examining ImageTransferPhase and DiskStatus.<br>
This is done instead of failing after a predefined timeout<br>
regardless the status.<br>
<br>
* not verified *<br>
<br>
Bug-Url: <a href="https://bugzilla.redhat.com/show_bug.cgi?id=1680361" rel="noreferrer" target="_blank">https://bugzilla.redhat.com/show_bug.cgi?id=1680361</a><br>
---<br>
v2v/rhv-upload-plugin.py | 26 +++++++++++++++++++++-----<br>
1 file changed, 21 insertions(+), 5 deletions(-)<br>
<br>
diff --git a/v2v/rhv-upload-plugin.py b/v2v/rhv-upload-plugin.py<br>
index 2a950c5ed..873c11ce1 100644<br>
--- a/v2v/rhv-upload-plugin.py<br>
+++ b/v2v/rhv-upload-plugin.py<br>
@@ -523,14 +523,30 @@ def close(h):<br>
# waiting for the transfer object to cease to exist, which<br>
# falls through to the exception case and then we can<br>
# continue.<br>
- endt = time.time() + timeout<br>
+ start = time.time()<br>
try:<br>
while True:<br>
time.sleep(1)<br>
- tmp = transfer_service.get()<br>
- if time.time() > endt:<br>
- raise RuntimeError("timed out waiting for transfer "<br>
- "to finalize")<br>
+ transfer = transfer_service.get()<br>
+<br>
+ if transfer is None:<br></blockquote><div><br></div><div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)">Are you sure this is possible? the original code assumed that we</div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)">fail with <span class="gmail_default"></span><span style="color:rgb(34,34,34)">sdk.NotFoundError.</span></div></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+ disk_service = h['disk_service']<br>
+ disk = disk_service.get()<br>
+ if disk.status == types.DiskStatus.OK:<br>
+ continue<br></blockquote><div><br></div><div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)">If the disk status is OK the upload was finished, so we should break, no?</div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)"><br></div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)">If the disk status is LOCKED, we should continue. But in this case there is</div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)">no point to check the transfer again.</div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)"><br></div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)">What if the finalize failed, and the disk was deleted? Do we get disk = None</div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)">or some exception?</div></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+<br>
+ if transfer.phase == types.ImageTransferPhase.FINISHED_SUCCESS:<br>
+ debug("finalized after %s seconds", time.time() - start)<br>
+ break<br>
+<br>
+ if transfer.phase ==<br>
types.ImageTransferPhase.FINALIZING_SUCCESS:<br>
+ if time.time() > start + timeout:<br>
+ raise RuntimeError("timed out waiting for transfer "<br>
+ "to finalize")<br>
+ continue<br>
+<br>
+ raise RuntimeError("Unexpected transfer phase while<br>
finalizing "<br>
+ "upload %r" % transfer.phase)<br>
except <span class="gmail_default" style="font-size:small;color:rgb(0,0,0)"></span>sdk.NotFoundError:<br>
pass<br></blockquote><div><br></div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)">Is this error possible? We need to add debug log here.</div><div><br></div><div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)">I think the logic should be:</div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)"> </div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)"> while True:</div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)"> wait</div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)"> get transfer<br></div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)"> if transfer does not exists (None or <span class="gmail_default"></span><span style="color:rgb(34,34,34)">sdk.NotFoundErro)</span> , break</div><div class="gmail_default" style="color:rgb(0,0,0)"> if transfer is finished, break (success)</div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)"> if transfer is finalizing:</div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)"> if timed out, fail</div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)"> continue</div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)"> fail with invalid transfer phase<br></div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)"> <br></div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)"> while True:</div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)"> wait</div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)"> get disk</div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)"> if disk does not exists (None or <span class="gmail_default"></span><span style="color:rgb(34,34,34)">sdk.NotFoundErro)</span>, fail</div><div class="gmail_default" style="color:rgb(0,0,0)"> if disk is OK, break (success)</div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)"> if disk is locked:</div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)"> if timed out, fail</div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)"> continue</div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)"> fail with invalid disk status<br></div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)"><br></div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)">This is little bit crazy that all user have to do this.</div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)"><br></div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)">We need to change engine so transfer is kept in the database after it finished for enough</div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)">time (1 hour?), so user can wait *only* for the transfer.</div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)"><br></div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)">Nir</div><br></div><div> </div></div></div>