Libguestfs - Sep 2019 - Re: [PATCH] v2v: rhv-upload-plugin - improve wait logic after finalize (RHBZ#1680361)

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.

From: Daniel Erez <derez@redhat.com>

After invoking transfer_service.finalize, check operation status by
examining DiskStatus.  This is done instead of failing after a
predefined timeout regardless the status.

Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1680361
Signed-off-by: Richard W.M. Jones <rjones@redhat.com>
Tested-by: Ilanit Stein <istein@redhat.com>
---
 v2v/rhv-upload-plugin.py | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/v2v/rhv-upload-plugin.py b/v2v/rhv-upload-plugin.py
index 9e71021..8f13ce1 100644
--- a/v2v/rhv-upload-plugin.py
+++ b/v2v/rhv-upload-plugin.py
@@ -549,16 +549,23 @@ def close(h):
         # waiting for the transfer object to cease to exist, which
         # falls through to the exception case and then we can
         # continue.
-        endt = time.time() + timeout
+        disk_id = disk.id
+        start = time.time()
         try:
             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
"
+                                           "to finalize")
+                    continue
+                if disk.status == types.DiskStatus.OK:
+                    debug("finalized after %s seconds" % (time.time()
- start))
+                    break
         except sdk.NotFoundError:
-            pass
+            raise RuntimeError("transfer failed: disk %s not found" %
disk_id)
 
         # Write the disk ID file.  Only do this on successful completion.
         with builtin_open(params['diskid_file'], 'w') as fp:
-- 
1.8.3.1

On Mon, Sep 9, 2019 at 7:02 PM Richard W.M. Jones <rjones@redhat.com>
wrote:
> From: Daniel Erez <derez@redhat.com>
>
> After invoking transfer_service.finalize, check operation status by
> examining DiskStatus.  This is done instead of failing after a
> predefined timeout regardless the status.
>
> Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1680361
> Signed-off-by: Richard W.M. Jones <rjones@redhat.com>
> Tested-by: Ilanit Stein <istein@redhat.com>
> ---
>  v2v/rhv-upload-plugin.py | 19 +++++++++++++------
>  1 file changed, 13 insertions(+), 6 deletions(-)
>
> diff --git a/v2v/rhv-upload-plugin.py b/v2v/rhv-upload-plugin.py
> index 9e71021..8f13ce1 100644
> --- a/v2v/rhv-upload-plugin.py
> +++ b/v2v/rhv-upload-plugin.py
> @@ -549,16 +549,23 @@ def close(h):
>          # waiting for the transfer object to cease to exist, which
>          # falls through to the exception case and then we can
>          # continue.
> -        endt = time.time() + timeout
> +        disk_id = disk.id
> +        start = time.time()
>          try:
>              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 "
> +                                           "to finalize")
> +                    continue
> +                if disk.status == types.DiskStatus.OK:
> +                    debug("finalized after %s seconds" %
(time.time() -
> start))
> +                    break
>          except sdk.NotFoundError:
> -            pass
> +            raise RuntimeError("transfer failed: disk %s not
found" %
> disk_id)
>
>          # Write the disk ID file.  Only do this on successful completion.
>          with builtin_open(params['diskid_file'], 'w') as
fp:
> --
> 1.8.3.1
>
Looks good.

Nir

On Monday, 9 September 2019 18:00:35 CEST Richard W.M. Jones
wrote:
> From: Daniel Erez <derez@redhat.com>
> 
> After invoking transfer_service.finalize, check operation status by
> examining DiskStatus.  This is done instead of failing after a
> predefined timeout regardless the status.
> 
> Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1680361
> Signed-off-by: Richard W.M. Jones <rjones@redhat.com>
> Tested-by: Ilanit Stein <istein@redhat.com>
> ---
>  v2v/rhv-upload-plugin.py | 19 +++++++++++++------
>  1 file changed, 13 insertions(+), 6 deletions(-)
> 
> diff --git a/v2v/rhv-upload-plugin.py b/v2v/rhv-upload-plugin.py
> index 9e71021..8f13ce1 100644
> --- a/v2v/rhv-upload-plugin.py
> +++ b/v2v/rhv-upload-plugin.py
> @@ -549,16 +549,23 @@ def close(h):
>          # waiting for the transfer object to cease to exist, which
>          # falls through to the exception case and then we can
>          # continue.
> -        endt = time.time() + timeout
> +        disk_id = disk.id
> +        start = time.time()
>          try:
>              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 "
> +                                           "to finalize")
> +                    continue
> +                if disk.status == types.DiskStatus.OK:
> +                    debug("finalized after %s seconds" %
(time.time() - start))
> +                    break
>          except sdk.NotFoundError:
> -            pass
> +            raise RuntimeError("transfer failed: disk %s not
found" % disk_id)
Seems to be OK (although I did not test it).

-- 
Pino Toscano