[Libguestfs] [nbdkit PATCH] connections: Extract common export flag computation code

[Richard W.M. Jones]

On Wed, Nov 15, 2017 at 04:52:37PM -0600, Eric Blake wrote:
> No need to duplicate maintenance of export flag computation between
> old and new style handshakes.
> 
> Signed-off-by: Eric Blake <eblake at redhat.com>
> ---
>  src/connections.c | 120 +++++++++++++++++++++---------------------------------
>  1 file changed, 47 insertions(+), 73 deletions(-)
> 
> diff --git a/src/connections.c b/src/connections.c
> index 8dc1925..f9edea7 100644
> --- a/src/connections.c
> +++ b/src/connections.c
> @@ -237,13 +237,57 @@ free_connection (struct connection *conn)
>  }
> 
>  static int
> +compute_eflags (struct connection *conn, uint16_t *flags)
> +{
> +  uint16_t eflags = NBD_FLAG_HAS_FLAGS;
> +  int fl;
> +
> +  fl = plugin_can_write (conn);
> +  if (fl == -1)
> +    return -1;
> +  if (readonly || !fl) {
> +    eflags |= NBD_FLAG_READ_ONLY;
> +    conn->readonly = 1;
> +  }
> +  if (!conn->readonly) {
> +    eflags |= NBD_FLAG_SEND_WRITE_ZEROES;
> +  }
> +
> +  fl = plugin_can_flush (conn);
> +  if (fl == -1)
> +    return -1;
> +  if (fl) {
> +    eflags |= NBD_FLAG_SEND_FLUSH | NBD_FLAG_SEND_FUA;
> +    conn->can_flush = 1;
> +  }
> +
> +  fl = plugin_is_rotational (conn);
> +  if (fl == -1)
> +    return -1;
> +  if (fl) {
> +    eflags |= NBD_FLAG_ROTATIONAL;
> +    conn->is_rotational = 1;
> +  }
> +
> +  fl = plugin_can_trim (conn);
> +  if (fl == -1)
> +    return -1;
> +  if (fl) {
> +    eflags |= NBD_FLAG_SEND_TRIM;
> +    conn->can_trim = 1;
> +  }
> +
> +  *flags = eflags;
> +  return 0;
> +}
> +
> +static int
>  _negotiate_handshake_oldstyle (struct connection *conn)
>  {
>    struct old_handshake handshake;
>    int64_t r;
>    uint64_t exportsize;
>    uint16_t gflags, eflags;
> -  int fl;
> 
>    /* In --tls=require / FORCEDTLS mode, old style handshakes are
>     * rejected because they cannot support TLS.
> @@ -265,43 +309,8 @@ _negotiate_handshake_oldstyle (struct connection *conn)
>    conn->exportsize = exportsize;
> 
>    gflags = 0;
> -  eflags = NBD_FLAG_HAS_FLAGS;
> -
> -  fl = plugin_can_write (conn);
> -  if (fl == -1)
> +  if (compute_eflags (conn, &eflags) < 0)
>      return -1;
> -  if (readonly || !fl) {
> -    eflags |= NBD_FLAG_READ_ONLY;
> -    conn->readonly = 1;
> -  }
> -  if (!conn->readonly) {
> -    eflags |= NBD_FLAG_SEND_WRITE_ZEROES;
> -  }
> -
> -
> -  fl = plugin_can_flush (conn);
> -  if (fl == -1)
> -    return -1;
> -  if (fl) {
> -    eflags |= NBD_FLAG_SEND_FLUSH | NBD_FLAG_SEND_FUA;
> -    conn->can_flush = 1;
> -  }
> -
> -  fl = plugin_is_rotational (conn);
> -  if (fl == -1)
> -    return -1;
> -  if (fl) {
> -    eflags |= NBD_FLAG_ROTATIONAL;
> -    conn->is_rotational = 1;
> -  }
> -
> -  fl = plugin_can_trim (conn);
> -  if (fl == -1)
> -    return -1;
> -  if (fl) {
> -    eflags |= NBD_FLAG_SEND_TRIM;
> -    conn->can_trim = 1;
> -  }
> 
>    debug ("oldstyle negotiation: flags: global 0x%x export 0x%x",
>           gflags, eflags);
> @@ -552,7 +561,6 @@ _negotiate_handshake_newstyle (struct connection *conn)
>    int64_t r;
>    uint64_t exportsize;
>    uint16_t eflags;
> -  int fl;
> 
>    gflags = NBD_FLAG_FIXED_NEWSTYLE | NBD_FLAG_NO_ZEROES;
> 
> @@ -596,42 +604,8 @@ _negotiate_handshake_newstyle (struct connection *conn)
>    exportsize = (uint64_t) r;
>    conn->exportsize = exportsize;
> 
> -  eflags = NBD_FLAG_HAS_FLAGS;
> -
> -  fl = plugin_can_write (conn);
> -  if (fl == -1)
> +  if (compute_eflags (conn, &eflags) < 0)
>      return -1;
> -  if (readonly || !fl) {
> -    eflags |= NBD_FLAG_READ_ONLY;
> -    conn->readonly = 1;
> -  }
> -  if (!conn->readonly) {
> -    eflags |= NBD_FLAG_SEND_WRITE_ZEROES;
> -  }
> -
> -  fl = plugin_can_flush (conn);
> -  if (fl == -1)
> -    return -1;
> -  if (fl) {
> -    eflags |= NBD_FLAG_SEND_FLUSH | NBD_FLAG_SEND_FUA;
> -    conn->can_flush = 1;
> -  }
> -
> -  fl = plugin_is_rotational (conn);
> -  if (fl == -1)
> -    return -1;
> -  if (fl) {
> -    eflags |= NBD_FLAG_ROTATIONAL;
> -    conn->is_rotational = 1;
> -  }
> -
> -  fl = plugin_can_trim (conn);
> -  if (fl == -1)
> -    return -1;
> -  if (fl) {
> -    eflags |= NBD_FLAG_SEND_TRIM;
> -    conn->can_trim = 1;
> -  }
> 
>    debug ("newstyle negotiation: flags: export 0x%x", eflags);
> 

Pretty simple refactoring, so:

ACK

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW