[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [Libguestfs] [PATCH libnbd 2/2] api: New API for reading NBD protocol.



On 9/17/19 5:02 AM, Richard W.M. Jones wrote:
> This commit adds a new API which can be used from the connected to
> state to read back which NBD protocol (eg. oldstyle, newstyle-fixed)
> we are using.

Somewhat of an overlap with my get_handshake_flags (as newstyle-fixed
corresponds to whether handshake_flags includes the FIXED_NEWSTYLE
flag), but I don't see the redundancy as an issue.

> 
> It was helpful to add a new state in newstyle negotiation
> (%NEWSTYLE.FINISHED) so we can route all successful option
> negotiations through a single path before moving to the %READY state,
> allowing us to set h->protocol in one place.
> ---

> +  "get_protocol", {
> +    default_call with
> +    args = []; ret = RStaticString;
> +    permitted_states = [ Connected; Closed ];
> +    shortdesc = "return the NBD protocol variant";
> +    longdesc = "\
> +Return the NBD protocol variant in use on the connection.  At
> +the moment this returns one of the strings C<\"oldstyle\">,
> +C<\"newstyle\"> or C<\"newstyle-fixed\">.  Other strings might
> +be returned in future.  Most modern NBD servers use C<\"newstyle-fixed\">.

s/in/in the/


> +++ b/generator/states-newstyle.c
> @@ -155,4 +155,13 @@ handle_reply_error (struct nbd_handle *h)
>    }
>    return 0;
>  
> + NEWSTYLE.FINISHED:
> +  if ((h->gflags & NBD_FLAG_FIXED_NEWSTYLE) == 0)
> +    h->protocol = "newstyle";
> +  else
> +    h->protocol = "newstyle-fixed";

Should work whether this lands before or after my set_handshake_flags patch.


> +++ b/lib/handle.c
> @@ -315,3 +315,15 @@ nbd_unlocked_supports_uri (struct nbd_handle *h)
>    return 0;
>  #endif
>  }
> +
> +const char *
> +nbd_unlocked_get_protocol (struct nbd_handle *h)
> +{
> +  /* I believe that if we reach the Connected or Closed permitted
> +   * states, then the state machine must have set h->protocol.  So if
> +   * this assertion is hit then it indicates a bug in libnbd.
> +   */
> +  assert (h->protocol);

Sounds correct to me.


> @@ -51,6 +54,14 @@ main (int argc, char *argv[])
>      exit (EXIT_FAILURE);
>    }
>  
> +  /* Even ancient versions of nbdkit only supported newstyle-fixed. */
> +  s = nbd_get_protocol (nbd);
> +  if (strcmp (s, "newstyle-fixed") != 0) {
> +    fprintf (stderr,
> +             "incorrect protocol \"%s\", expected \"newstyle-fixed\"\n", s);
> +    exit (EXIT_FAILURE);
> +  }

With nbdkit 1.16 --mask-handshake and/or my patch for
nbd_set_handshake_flags, we can also provoke a test of "newstyle" as
output; looks like it's probably better for your patch to go in and me
to rebase mine on top of yours (since I was still working on writing
interop tests for mine).

ACK.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]