thr3ads.net - Libguestfs - [Libguestfs] [nbdkit PATCH] connections: Extract common export flag computation code [Nov 2017]

If this information is useful, please help other people find it:
Share via:

Eric Blake

2017-Nov-15 22:52 UTC

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

No need to duplicate maintenance of export flag computation between
old and new style handshakes.

Signed-off-by: Eric Blake <eblake@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);

-- 
2.13.6

Richard W.M. Jones

2017-Nov-16 11:47 UTC

head link

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

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@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

Possibly Parallel Threads

Search for more reasonably related threads

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

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

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

Possibly Parallel Threads