Re: [PATCH v3 16/33] nbd/client-connection: add possibility of negotiation

2021-06-08 Thread Vladimir Sementsov-Ogievskiy 

12.05.2021 09:42, Vladimir Sementsov-Ogievskiy wrote:

11.05.2021 13:45, Roman Kagan wrote:

On Fri, Apr 16, 2021 at 11:08:54AM +0300, Vladimir Sementsov-Ogievskiy wrote:

Add arguments and logic to support nbd negotiation in the same thread
after successful connection.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
  include/block/nbd.h |   9 +++-
  block/nbd.c |   4 +-
  nbd/client-connection.c | 105 ++--
  3 files changed, 109 insertions(+), 9 deletions(-)

diff --git a/include/block/nbd.h b/include/block/nbd.h
index 57381be76f..5d86e6a393 100644
--- a/include/block/nbd.h
+++ b/include/block/nbd.h
@@ -409,11 +409,16 @@ const char *nbd_err_lookup(int err);
  /* nbd/client-connection.c */
  typedef struct NBDClientConnection NBDClientConnection;
-NBDClientConnection *nbd_client_connection_new(const SocketAddress *saddr);
+NBDClientConnection *nbd_client_connection_new(const SocketAddress *saddr,
+   bool do_negotiation,
+   const char *export_name,
+   const char *x_dirty_bitmap,
+   QCryptoTLSCreds *tlscreds);
  void nbd_client_connection_release(NBDClientConnection *conn);
  QIOChannelSocket *coroutine_fn
-nbd_co_establish_connection(NBDClientConnection *conn, Error **errp);
+nbd_co_establish_connection(NBDClientConnection *conn, NBDExportInfo *info,
+    QIOChannel **ioc, Error **errp);
  void coroutine_fn nbd_co_establish_connection_cancel(NBDClientConnection 
*conn);
diff --git a/block/nbd.c b/block/nbd.c
index 9bd68dcf10..5e63caaf4b 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -361,7 +361,7 @@ static coroutine_fn void nbd_reconnect_attempt(BDRVNBDState 
*s)
  s->ioc = NULL;
  }
-    s->sioc = nbd_co_establish_connection(s->conn, NULL);
+    s->sioc = nbd_co_establish_connection(s->conn, NULL, NULL, NULL);
  if (!s->sioc) {
  ret = -ECONNREFUSED;
  goto out;
@@ -2033,7 +2033,7 @@ static int nbd_open(BlockDriverState *bs, QDict *options, 
int flags,
  goto fail;
  }
-    s->conn = nbd_client_connection_new(s->saddr);
+    s->conn = nbd_client_connection_new(s->saddr, false, NULL, NULL, NULL);
  /*
   * establish TCP connection, return error if it fails
diff --git a/nbd/client-connection.c b/nbd/client-connection.c
index b45a0bd5f6..ae4a77f826 100644
--- a/nbd/client-connection.c
+++ b/nbd/client-connection.c
@@ -30,14 +30,19 @@
  #include "qapi/clone-visitor.h"
  struct NBDClientConnection {
-    /* Initialization constants */
+    /* Initialization constants, never change */
  SocketAddress *saddr; /* address to connect to */
+    QCryptoTLSCreds *tlscreds;
+    NBDExportInfo initial_info;
+    bool do_negotiation;
  /*
   * Result of last attempt. Valid in FAIL and SUCCESS states.
   * If you want to steal error, don't forget to set pointer to NULL.
   */
+    NBDExportInfo updated_info;
  QIOChannelSocket *sioc;
+    QIOChannel *ioc;
  Error *err;
  QemuMutex mutex;
@@ -47,12 +52,25 @@ struct NBDClientConnection {
  Coroutine *wait_co; /* nbd_co_establish_connection() wait in yield() */
  };
-NBDClientConnection *nbd_client_connection_new(const SocketAddress *saddr)
+NBDClientConnection *nbd_client_connection_new(const SocketAddress *saddr,
+   bool do_negotiation,
+   const char *export_name,
+   const char *x_dirty_bitmap,
+   QCryptoTLSCreds *tlscreds)
  {
  NBDClientConnection *conn = g_new(NBDClientConnection, 1);
+    object_ref(OBJECT(tlscreds));
  *conn = (NBDClientConnection) {
  .saddr = QAPI_CLONE(SocketAddress, saddr),
+    .tlscreds = tlscreds,
+    .do_negotiation = do_negotiation,
+
+    .initial_info.request_sizes = true,
+    .initial_info.structured_reply = true,
+    .initial_info.base_allocation = true,
+    .initial_info.x_dirty_bitmap = g_strdup(x_dirty_bitmap),
+    .initial_info.name = g_strdup(export_name ?: "")
  };
  qemu_mutex_init(>mutex);
@@ -68,9 +86,59 @@ static void 
nbd_client_connection_do_free(NBDClientConnection *conn)
  }
  error_free(conn->err);
  qapi_free_SocketAddress(conn->saddr);
+    object_unref(OBJECT(conn->tlscreds));
+    g_free(conn->initial_info.x_dirty_bitmap);
+    g_free(conn->initial_info.name);
  g_free(conn);
  }
+/*
+ * Connect to @addr and do NBD negotiation if @info is not null. If @tlscreds
+ * given @outioc is provided. @outioc is provided only on success.  The call 
may


s/given/are given/
s/provided/returned/g


+ * be cancelled in parallel by simply qio_channel_shutdown(sioc).


I assume by "in parallel" you mean "from another thread", I'd suggest to

Re: [PATCH v3 16/33] nbd/client-connection: add possibility of negotiation

2021-05-12 Thread Vladimir Sementsov-Ogievskiy 

11.05.2021 13:45, Roman Kagan wrote:

On Fri, Apr 16, 2021 at 11:08:54AM +0300, Vladimir Sementsov-Ogievskiy wrote:

Add arguments and logic to support nbd negotiation in the same thread
after successful connection.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
  include/block/nbd.h |   9 +++-
  block/nbd.c |   4 +-
  nbd/client-connection.c | 105 ++--
  3 files changed, 109 insertions(+), 9 deletions(-)

diff --git a/include/block/nbd.h b/include/block/nbd.h
index 57381be76f..5d86e6a393 100644
--- a/include/block/nbd.h
+++ b/include/block/nbd.h
@@ -409,11 +409,16 @@ const char *nbd_err_lookup(int err);
  /* nbd/client-connection.c */
  typedef struct NBDClientConnection NBDClientConnection;
  
-NBDClientConnection *nbd_client_connection_new(const SocketAddress *saddr);

+NBDClientConnection *nbd_client_connection_new(const SocketAddress *saddr,
+   bool do_negotiation,
+   const char *export_name,
+   const char *x_dirty_bitmap,
+   QCryptoTLSCreds *tlscreds);
  void nbd_client_connection_release(NBDClientConnection *conn);
  
  QIOChannelSocket *coroutine_fn

-nbd_co_establish_connection(NBDClientConnection *conn, Error **errp);
+nbd_co_establish_connection(NBDClientConnection *conn, NBDExportInfo *info,
+QIOChannel **ioc, Error **errp);
  
  void coroutine_fn nbd_co_establish_connection_cancel(NBDClientConnection *conn);
  
diff --git a/block/nbd.c b/block/nbd.c

index 9bd68dcf10..5e63caaf4b 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -361,7 +361,7 @@ static coroutine_fn void nbd_reconnect_attempt(BDRVNBDState 
*s)
  s->ioc = NULL;
  }
  
-s->sioc = nbd_co_establish_connection(s->conn, NULL);

+s->sioc = nbd_co_establish_connection(s->conn, NULL, NULL, NULL);
  if (!s->sioc) {
  ret = -ECONNREFUSED;
  goto out;
@@ -2033,7 +2033,7 @@ static int nbd_open(BlockDriverState *bs, QDict *options, 
int flags,
  goto fail;
  }
  
-s->conn = nbd_client_connection_new(s->saddr);

+s->conn = nbd_client_connection_new(s->saddr, false, NULL, NULL, NULL);
  
  /*

   * establish TCP connection, return error if it fails
diff --git a/nbd/client-connection.c b/nbd/client-connection.c
index b45a0bd5f6..ae4a77f826 100644
--- a/nbd/client-connection.c
+++ b/nbd/client-connection.c
@@ -30,14 +30,19 @@
  #include "qapi/clone-visitor.h"
  
  struct NBDClientConnection {

-/* Initialization constants */
+/* Initialization constants, never change */
  SocketAddress *saddr; /* address to connect to */
+QCryptoTLSCreds *tlscreds;
+NBDExportInfo initial_info;
+bool do_negotiation;
  
  /*

   * Result of last attempt. Valid in FAIL and SUCCESS states.
   * If you want to steal error, don't forget to set pointer to NULL.
   */
+NBDExportInfo updated_info;
  QIOChannelSocket *sioc;
+QIOChannel *ioc;
  Error *err;
  
  QemuMutex mutex;

@@ -47,12 +52,25 @@ struct NBDClientConnection {
  Coroutine *wait_co; /* nbd_co_establish_connection() wait in yield() */
  };
  
-NBDClientConnection *nbd_client_connection_new(const SocketAddress *saddr)

+NBDClientConnection *nbd_client_connection_new(const SocketAddress *saddr,
+   bool do_negotiation,
+   const char *export_name,
+   const char *x_dirty_bitmap,
+   QCryptoTLSCreds *tlscreds)
  {
  NBDClientConnection *conn = g_new(NBDClientConnection, 1);
  
+object_ref(OBJECT(tlscreds));

  *conn = (NBDClientConnection) {
  .saddr = QAPI_CLONE(SocketAddress, saddr),
+.tlscreds = tlscreds,
+.do_negotiation = do_negotiation,
+
+.initial_info.request_sizes = true,
+.initial_info.structured_reply = true,
+.initial_info.base_allocation = true,
+.initial_info.x_dirty_bitmap = g_strdup(x_dirty_bitmap),
+.initial_info.name = g_strdup(export_name ?: "")
  };
  
  qemu_mutex_init(>mutex);

@@ -68,9 +86,59 @@ static void 
nbd_client_connection_do_free(NBDClientConnection *conn)
  }
  error_free(conn->err);
  qapi_free_SocketAddress(conn->saddr);
+object_unref(OBJECT(conn->tlscreds));
+g_free(conn->initial_info.x_dirty_bitmap);
+g_free(conn->initial_info.name);
  g_free(conn);
  }
  
+/*

+ * Connect to @addr and do NBD negotiation if @info is not null. If @tlscreds
+ * given @outioc is provided. @outioc is provided only on success.  The call 
may


s/given/are given/
s/provided/returned/g


+ * be cancelled in parallel by simply qio_channel_shutdown(sioc).


I assume by "in parallel" you mean "from another thread", I'd suggest to
spell

Re: [PATCH v3 16/33] nbd/client-connection: add possibility of negotiation

2021-05-11 Thread Roman Kagan 
On Fri, Apr 16, 2021 at 11:08:54AM +0300, Vladimir Sementsov-Ogievskiy wrote:
> Add arguments and logic to support nbd negotiation in the same thread
> after successful connection.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy 
> ---
>  include/block/nbd.h |   9 +++-
>  block/nbd.c |   4 +-
>  nbd/client-connection.c | 105 ++--
>  3 files changed, 109 insertions(+), 9 deletions(-)
> 
> diff --git a/include/block/nbd.h b/include/block/nbd.h
> index 57381be76f..5d86e6a393 100644
> --- a/include/block/nbd.h
> +++ b/include/block/nbd.h
> @@ -409,11 +409,16 @@ const char *nbd_err_lookup(int err);
>  /* nbd/client-connection.c */
>  typedef struct NBDClientConnection NBDClientConnection;
>  
> -NBDClientConnection *nbd_client_connection_new(const SocketAddress *saddr);
> +NBDClientConnection *nbd_client_connection_new(const SocketAddress *saddr,
> +   bool do_negotiation,
> +   const char *export_name,
> +   const char *x_dirty_bitmap,
> +   QCryptoTLSCreds *tlscreds);
>  void nbd_client_connection_release(NBDClientConnection *conn);
>  
>  QIOChannelSocket *coroutine_fn
> -nbd_co_establish_connection(NBDClientConnection *conn, Error **errp);
> +nbd_co_establish_connection(NBDClientConnection *conn, NBDExportInfo *info,
> +QIOChannel **ioc, Error **errp);
>  
>  void coroutine_fn nbd_co_establish_connection_cancel(NBDClientConnection 
> *conn);
>  
> diff --git a/block/nbd.c b/block/nbd.c
> index 9bd68dcf10..5e63caaf4b 100644
> --- a/block/nbd.c
> +++ b/block/nbd.c
> @@ -361,7 +361,7 @@ static coroutine_fn void 
> nbd_reconnect_attempt(BDRVNBDState *s)
>  s->ioc = NULL;
>  }
>  
> -s->sioc = nbd_co_establish_connection(s->conn, NULL);
> +s->sioc = nbd_co_establish_connection(s->conn, NULL, NULL, NULL);
>  if (!s->sioc) {
>  ret = -ECONNREFUSED;
>  goto out;
> @@ -2033,7 +2033,7 @@ static int nbd_open(BlockDriverState *bs, QDict 
> *options, int flags,
>  goto fail;
>  }
>  
> -s->conn = nbd_client_connection_new(s->saddr);
> +s->conn = nbd_client_connection_new(s->saddr, false, NULL, NULL, NULL);
>  
>  /*
>   * establish TCP connection, return error if it fails
> diff --git a/nbd/client-connection.c b/nbd/client-connection.c
> index b45a0bd5f6..ae4a77f826 100644
> --- a/nbd/client-connection.c
> +++ b/nbd/client-connection.c
> @@ -30,14 +30,19 @@
>  #include "qapi/clone-visitor.h"
>  
>  struct NBDClientConnection {
> -/* Initialization constants */
> +/* Initialization constants, never change */
>  SocketAddress *saddr; /* address to connect to */
> +QCryptoTLSCreds *tlscreds;
> +NBDExportInfo initial_info;
> +bool do_negotiation;
>  
>  /*
>   * Result of last attempt. Valid in FAIL and SUCCESS states.
>   * If you want to steal error, don't forget to set pointer to NULL.
>   */
> +NBDExportInfo updated_info;
>  QIOChannelSocket *sioc;
> +QIOChannel *ioc;
>  Error *err;
>  
>  QemuMutex mutex;
> @@ -47,12 +52,25 @@ struct NBDClientConnection {
>  Coroutine *wait_co; /* nbd_co_establish_connection() wait in yield() */
>  };
>  
> -NBDClientConnection *nbd_client_connection_new(const SocketAddress *saddr)
> +NBDClientConnection *nbd_client_connection_new(const SocketAddress *saddr,
> +   bool do_negotiation,
> +   const char *export_name,
> +   const char *x_dirty_bitmap,
> +   QCryptoTLSCreds *tlscreds)
>  {
>  NBDClientConnection *conn = g_new(NBDClientConnection, 1);
>  
> +object_ref(OBJECT(tlscreds));
>  *conn = (NBDClientConnection) {
>  .saddr = QAPI_CLONE(SocketAddress, saddr),
> +.tlscreds = tlscreds,
> +.do_negotiation = do_negotiation,
> +
> +.initial_info.request_sizes = true,
> +.initial_info.structured_reply = true,
> +.initial_info.base_allocation = true,
> +.initial_info.x_dirty_bitmap = g_strdup(x_dirty_bitmap),
> +.initial_info.name = g_strdup(export_name ?: "")
>  };
>  
>  qemu_mutex_init(>mutex);
> @@ -68,9 +86,59 @@ static void 
> nbd_client_connection_do_free(NBDClientConnection *conn)
>  }
>  error_free(conn->err);
>  qapi_free_SocketAddress(conn->saddr);
> +object_unref(OBJECT(conn->tlscreds));
> +g_free(conn->initial_info.x_dirty_bitmap);
> +g_free(conn->initial_info.name);
>  g_free(conn);
>  }
>  
> +/*
> + * Connect to @addr and do NBD negotiation if @info is not null. If @tlscreds
> + * given @outioc is provided. @outioc is provided only on success.  The call 
> may

s/given/are given/
s/provided/returned/g

> + *