[Libguestfs] [PATCH v2 1/2] v2v: -o rhv-upload: Always fetch server options when opening the connection.

Nir Soffer nsoffer at redhat.com
Mon Jun 25 22:50:38 UTC 2018


On Fri, Jun 22, 2018 at 2:59 PM Richard W.M. Jones <rjones at redhat.com>
wrote:

> Previously we lazily requested the server options in the can_*
> callbacks.  The can_* callbacks are always called by nbdkit straight
> after open, so this just adds complexity for no benefit.  This change
> simply makes the code always fetch the server options during the open
> callback.
>

I agree, the simple way is much better.


>
> This is — functionally at least — mostly just refactoring.  However I
> also added a useful debug message so we can see what features the
> imageio server is offering.
> ---
>  v2v/rhv-upload-plugin.py | 77
> +++++++++++++++++++++++-------------------------
>  1 file changed, 37 insertions(+), 40 deletions(-)
>
> diff --git a/v2v/rhv-upload-plugin.py b/v2v/rhv-upload-plugin.py
> index 7c5084efd..5be426897 100644
> --- a/v2v/rhv-upload-plugin.py
> +++ b/v2v/rhv-upload-plugin.py
> @@ -165,63 +165,60 @@ def open(readonly):
>          context = context
>      )
>
> +    # The first request is to fetch the features of the server.
> +    needs_auth = not params['rhv_direct']
> +    can_flush = False
> +    can_trim = False
> +    can_zero = False
> +
> +    http.putrequest("OPTIONS", destination_url.path)
> +    http.putheader("Authorization", transfer.signed_ticket)

+    http.endheaders()
> +
> +    r = http.getresponse()
>

We should read the response data here to make sure we consume
the entire response before sending hte nex


> +    if r.status == 200:
> +        # New imageio never needs authentication.
> +        needs_auth = False
> +
> +        j = json.loads(r.read())
> +        can_flush = "flush" in j['features']
> +        can_trim = "trim" in j['features']
> +        can_zero = "zero" in j['features']
> +
> +    # Old imageio servers returned either 405 Method Not Allowed or
> +    # 204 No Content (with an empty body).  If we see that we leave
> +    # all the features as False and they will be emulated.
> +    elif r.status == 405 or r.status == 204:
> +        pass
> +
> +    else:
> +        raise RuntimeError("could not use OPTIONS request: %d: %s" %
> +                           (r.status, r.reason))
>


> +
> +    debug("imageio features: flush=%r trim=%r zero=%r" %
> +          (can_flush, can_trim, can_zero))
> +
>      # Save everything we need to make requests in the handle.
>      return {
> -        'can_flush': False,
> -        'can_trim': False,
> -        'can_zero': False,
> +        'can_flush': can_flush,
> +        'can_trim': can_trim,
> +        'can_zero': can_zero,
>          'connection': connection,
>          'disk': disk,
>          'disk_service': disk_service,
>          'failed': False,
> -        'got_options': False,
>          'highestwrite': 0,
>          'http': http,
> -        'needs_auth': not params['rhv_direct'],
> +        'needs_auth': needs_auth,
>          'path': destination_url.path,
>          'transfer': transfer,
>          'transfer_service': transfer_service,
>      }
>
> -# Can we issue zero, trim or flush requests?
> -def get_options(h):
> -    if h['got_options']:
> -        return
> -    h['got_options'] = True
> -
> -    http = h['http']
> -    transfer = h['transfer']
> -
> -    http.putrequest("OPTIONS", h['path'])
> -    http.putheader("Authorization", transfer.signed_ticket)
> -    http.endheaders()
> -
> -    r = http.getresponse()
> -    if r.status == 200:
> -        # New imageio never needs authentication.
> -        h['needs_auth'] = False
> -
> -        j = json.loads(r.read())
> -        h['can_zero'] = "zero" in j['features']
> -        h['can_trim'] = "trim" in j['features']
> -        h['can_flush'] = "flush" in j['features']
> -
> -    # Old imageio servers returned either 405 Method Not Allowed or
> -    # 204 No Content (with an empty body).  If we see that we leave
> -    # all the features as False and they will be emulated.
> -    elif r.status == 405 or r.status == 204:
> -        pass
> -
> -    else:
> -        raise RuntimeError("could not use OPTIONS request: %d: %s" %
> -                           (r.status, r.reason))
> -
>  def can_trim(h):
> -    get_options(h)
>      return h['can_trim']
>
>  def can_flush(h):
> -    get_options(h)
>      return h['can_flush']
>
>  def get_size(h):
>

Otherwise looks good.

Nir
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/libguestfs/attachments/20180626/8d94f2de/attachment.htm>


More information about the Libguestfs mailing list