All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PULL 0/4] NBD patches through 20 Dec
@ 2017-12-21  1:35 Eric Blake
  2017-12-21  1:35 ` [Qemu-devel] [PULL 1/4] nbd/server: Implement sparse reads atop structured reply Eric Blake
                   ` (4 more replies)
  0 siblings, 5 replies; 10+ messages in thread
From: Eric Blake @ 2017-12-21  1:35 UTC (permalink / raw)
  To: qemu-devel

The following changes since commit 4da5c51cac8363f86ec92dc99c38f9382d617647:

  Merge remote-tracking branch 'remotes/armbru/tags/pull-qapi-2017-12-20' into staging (2017-12-20 20:38:36 +0000)

are available in the Git repository at:

  git://repo.or.cz/qemu/ericb.git tags/pull-nbd-2017-12-20

for you to fetch changes up to fab45bb24a35b4791deecf3ec732d64b0b987662:

  qmp: add nbd-server-remove (2017-12-20 16:55:32 -0600)

----------------------------------------------------------------
nbd patches for 2017-12-20

- Eric Blake: 0/2 Optimize sparse reads over NBD
- Vladimir Sementsov-Ogievskiy: 0/2 add qmp nbd-server-remove

----------------------------------------------------------------
Eric Blake (2):
      nbd/server: Implement sparse reads atop structured reply
      nbd/server: Optimize final chunk of sparse read

Vladimir Sementsov-Ogievskiy (2):
      nbd/server: add additional assert to nbd_export_put
      qmp: add nbd-server-remove

 qapi/block.json  | 20 ++++++++++++++
 blockdev-nbd.c   | 27 +++++++++++++++++++
 nbd/server.c     | 80 +++++++++++++++++++++++++++++++++++++++++++++++++++++---
 nbd/trace-events |  1 +
 4 files changed, 125 insertions(+), 3 deletions(-)

-- 
2.14.3

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [Qemu-devel] [PULL 1/4] nbd/server: Implement sparse reads atop structured reply
  2017-12-21  1:35 [Qemu-devel] [PULL 0/4] NBD patches through 20 Dec Eric Blake
@ 2017-12-21  1:35 ` Eric Blake
  2017-12-21  1:35 ` [Qemu-devel] [PULL 2/4] nbd/server: Optimize final chunk of sparse read Eric Blake
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 10+ messages in thread
From: Eric Blake @ 2017-12-21  1:35 UTC (permalink / raw)
  To: qemu-devel; +Cc: Paolo Bonzini, open list:Network Block Dev...

The reason that NBD added structured reply in the first place was
to allow for efficient reads of sparse files, by allowing the
reply to include chunks to quickly communicate holes to the client
without sending lots of zeroes over the wire.  Time to implement
this in the server; our client can already read such data.

We can only skip holes insofar as the block layer can query them;
and only if the client is okay with a fragmented request (if a
client requests NBD_CMD_FLAG_DF and the entire read is a hole, we
could technically return a single NBD_REPLY_TYPE_OFFSET_HOLE, but
that's a fringe case not worth catering to here).  Sadly, the
control flow is a bit wonkier than I would have preferred, but
it was minimally invasive to have a split in the action between
a fragmented read (handled directly where we recognize
NBD_CMD_READ with the right conditions, and sending multiple
chunks) vs. a single read (handled at the end of nbd_trip, for
both simple and structured replies, when we know there is only
one thing being read).  Likewise, I didn't make any effort to
optimize the final chunk of a fragmented read to set the
NBD_REPLY_FLAG_DONE, but unconditionally send that as a separate
NBD_REPLY_TYPE_NONE.

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <20171107030912.23930-2-eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 nbd/server.c     | 78 +++++++++++++++++++++++++++++++++++++++++++++++++++++---
 nbd/trace-events |  1 +
 2 files changed, 76 insertions(+), 3 deletions(-)

diff --git a/nbd/server.c b/nbd/server.c
index 92c0fdd03b..be7310cb41 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -1303,6 +1303,7 @@ static int coroutine_fn nbd_co_send_structured_read(NBDClient *client,
                                                     uint64_t offset,
                                                     void *data,
                                                     size_t size,
+                                                    bool final,
                                                     Error **errp)
 {
     NBDStructuredReadData chunk;
@@ -1313,13 +1314,73 @@ static int coroutine_fn nbd_co_send_structured_read(NBDClient *client,

     assert(size);
     trace_nbd_co_send_structured_read(handle, offset, data, size);
-    set_be_chunk(&chunk.h, NBD_REPLY_FLAG_DONE, NBD_REPLY_TYPE_OFFSET_DATA,
-                 handle, sizeof(chunk) - sizeof(chunk.h) + size);
+    set_be_chunk(&chunk.h, final ? NBD_REPLY_FLAG_DONE : 0,
+                 NBD_REPLY_TYPE_OFFSET_DATA, handle,
+                 sizeof(chunk) - sizeof(chunk.h) + size);
     stq_be_p(&chunk.offset, offset);

     return nbd_co_send_iov(client, iov, 2, errp);
 }

+static int coroutine_fn nbd_co_send_sparse_read(NBDClient *client,
+                                                uint64_t handle,
+                                                uint64_t offset,
+                                                uint8_t *data,
+                                                size_t size,
+                                                Error **errp)
+{
+    int ret = 0;
+    NBDExport *exp = client->exp;
+    size_t progress = 0;
+
+    while (progress < size) {
+        int64_t pnum;
+        int status = bdrv_block_status_above(blk_bs(exp->blk), NULL,
+                                             offset + progress,
+                                             size - progress, &pnum, NULL,
+                                             NULL);
+
+        if (status < 0) {
+            error_setg_errno(errp, -status, "unable to check for holes");
+            return status;
+        }
+        assert(pnum && pnum <= size - progress);
+        if (status & BDRV_BLOCK_ZERO) {
+            NBDStructuredReadHole chunk;
+            struct iovec iov[] = {
+                {.iov_base = &chunk, .iov_len = sizeof(chunk)},
+            };
+
+            trace_nbd_co_send_structured_read_hole(handle, offset + progress,
+                                                   pnum);
+            set_be_chunk(&chunk.h, 0, NBD_REPLY_TYPE_OFFSET_HOLE,
+                         handle, sizeof(chunk) - sizeof(chunk.h));
+            stq_be_p(&chunk.offset, offset + progress);
+            stl_be_p(&chunk.length, pnum);
+            ret = nbd_co_send_iov(client, iov, 1, errp);
+        } else {
+            ret = blk_pread(exp->blk, offset + progress + exp->dev_offset,
+                            data + progress, pnum);
+            if (ret < 0) {
+                error_setg_errno(errp, -ret, "reading from file failed");
+                break;
+            }
+            ret = nbd_co_send_structured_read(client, handle, offset + progress,
+                                              data + progress, pnum, false,
+                                              errp);
+        }
+
+        if (ret < 0) {
+            break;
+        }
+        progress += pnum;
+    }
+    if (!ret) {
+        ret = nbd_co_send_structured_done(client, handle, errp);
+    }
+    return ret;
+}
+
 static int coroutine_fn nbd_co_send_structured_error(NBDClient *client,
                                                      uint64_t handle,
                                                      uint32_t error,
@@ -1481,6 +1542,16 @@ static coroutine_fn void nbd_trip(void *opaque)
             }
         }

+        if (client->structured_reply && !(request.flags & NBD_CMD_FLAG_DF)) {
+            ret = nbd_co_send_sparse_read(req->client, request.handle,
+                                          request.from, req->data, request.len,
+                                          &local_err);
+            if (ret < 0) {
+                goto reply;
+            }
+            goto done;
+        }
+
         ret = blk_pread(exp->blk, request.from + exp->dev_offset,
                         req->data, request.len);
         if (ret < 0) {
@@ -1561,7 +1632,8 @@ reply:
         } else if (reply_data_len) {
             ret = nbd_co_send_structured_read(req->client, request.handle,
                                               request.from, req->data,
-                                              reply_data_len, &local_err);
+                                              reply_data_len, true,
+                                              &local_err);
         } else {
             ret = nbd_co_send_structured_done(req->client, request.handle,
                                               &local_err);
diff --git a/nbd/trace-events b/nbd/trace-events
index 92568edce5..2b8268ce8c 100644
--- a/nbd/trace-events
+++ b/nbd/trace-events
@@ -57,6 +57,7 @@ nbd_blk_aio_detach(const char *name, void *ctx) "Export %s: Detaching clients fr
 nbd_co_send_simple_reply(uint64_t handle, uint32_t error, const char *errname, int len) "Send simple reply: handle = %" PRIu64 ", error = %" PRIu32 " (%s), len = %d"
 nbd_co_send_structured_done(uint64_t handle) "Send structured reply done: handle = %" PRIu64
 nbd_co_send_structured_read(uint64_t handle, uint64_t offset, void *data, size_t size) "Send structured read data reply: handle = %" PRIu64 ", offset = %" PRIu64 ", data = %p, len = %zu"
+nbd_co_send_structured_read_hole(uint64_t handle, uint64_t offset, size_t size) "Send structured read hole reply: handle = %" PRIu64 ", offset = %" PRIu64 ", len = %zu"
 nbd_co_send_structured_error(uint64_t handle, int err, const char *errname, const char *msg) "Send structured error reply: handle = %" PRIu64 ", error = %d (%s), msg = '%s'"
 nbd_co_receive_request_decode_type(uint64_t handle, uint16_t type, const char *name) "Decoding type: handle = %" PRIu64 ", type = %" PRIu16 " (%s)"
 nbd_co_receive_request_payload_received(uint64_t handle, uint32_t len) "Payload received: handle = %" PRIu64 ", len = %" PRIu32
-- 
2.14.3

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [Qemu-devel] [PULL 2/4] nbd/server: Optimize final chunk of sparse read
  2017-12-21  1:35 [Qemu-devel] [PULL 0/4] NBD patches through 20 Dec Eric Blake
  2017-12-21  1:35 ` [Qemu-devel] [PULL 1/4] nbd/server: Implement sparse reads atop structured reply Eric Blake
@ 2017-12-21  1:35 ` Eric Blake
  2017-12-21  1:35 ` [Qemu-devel] [PULL 3/4] nbd/server: add additional assert to nbd_export_put Eric Blake
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 10+ messages in thread
From: Eric Blake @ 2017-12-21  1:35 UTC (permalink / raw)
  To: qemu-devel; +Cc: Paolo Bonzini, open list:Network Block Dev...

If we are careful to handle 0-length read requests correctly,
we can optimize our sparse read to send the NBD_REPLY_FLAG_DONE
bit on our last OFFSET_DATA or OFFSET_HOLE chunk rather than
needing a separate chunk.

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <20171107030912.23930-3-eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 nbd/server.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/nbd/server.c b/nbd/server.c
index be7310cb41..e443b3cf5c 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -1339,12 +1339,14 @@ static int coroutine_fn nbd_co_send_sparse_read(NBDClient *client,
                                              offset + progress,
                                              size - progress, &pnum, NULL,
                                              NULL);
+        bool final;

         if (status < 0) {
             error_setg_errno(errp, -status, "unable to check for holes");
             return status;
         }
         assert(pnum && pnum <= size - progress);
+        final = progress + pnum == size;
         if (status & BDRV_BLOCK_ZERO) {
             NBDStructuredReadHole chunk;
             struct iovec iov[] = {
@@ -1353,7 +1355,8 @@ static int coroutine_fn nbd_co_send_sparse_read(NBDClient *client,

             trace_nbd_co_send_structured_read_hole(handle, offset + progress,
                                                    pnum);
-            set_be_chunk(&chunk.h, 0, NBD_REPLY_TYPE_OFFSET_HOLE,
+            set_be_chunk(&chunk.h, final ? NBD_REPLY_FLAG_DONE : 0,
+                         NBD_REPLY_TYPE_OFFSET_HOLE,
                          handle, sizeof(chunk) - sizeof(chunk.h));
             stq_be_p(&chunk.offset, offset + progress);
             stl_be_p(&chunk.length, pnum);
@@ -1366,7 +1369,7 @@ static int coroutine_fn nbd_co_send_sparse_read(NBDClient *client,
                 break;
             }
             ret = nbd_co_send_structured_read(client, handle, offset + progress,
-                                              data + progress, pnum, false,
+                                              data + progress, pnum, final,
                                               errp);
         }

@@ -1375,9 +1378,6 @@ static int coroutine_fn nbd_co_send_sparse_read(NBDClient *client,
         }
         progress += pnum;
     }
-    if (!ret) {
-        ret = nbd_co_send_structured_done(client, handle, errp);
-    }
     return ret;
 }

@@ -1542,7 +1542,8 @@ static coroutine_fn void nbd_trip(void *opaque)
             }
         }

-        if (client->structured_reply && !(request.flags & NBD_CMD_FLAG_DF)) {
+        if (client->structured_reply && !(request.flags & NBD_CMD_FLAG_DF) &&
+            request.len) {
             ret = nbd_co_send_sparse_read(req->client, request.handle,
                                           request.from, req->data, request.len,
                                           &local_err);
-- 
2.14.3

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [Qemu-devel] [PULL 3/4] nbd/server: add additional assert to nbd_export_put
  2017-12-21  1:35 [Qemu-devel] [PULL 0/4] NBD patches through 20 Dec Eric Blake
  2017-12-21  1:35 ` [Qemu-devel] [PULL 1/4] nbd/server: Implement sparse reads atop structured reply Eric Blake
  2017-12-21  1:35 ` [Qemu-devel] [PULL 2/4] nbd/server: Optimize final chunk of sparse read Eric Blake
@ 2017-12-21  1:35 ` Eric Blake
  2017-12-21  1:36 ` [Qemu-devel] [PULL 4/4] qmp: add nbd-server-remove Eric Blake
  2017-12-21 13:13 ` [Qemu-devel] [PULL 0/4] NBD patches through 20 Dec Peter Maydell
  4 siblings, 0 replies; 10+ messages in thread
From: Eric Blake @ 2017-12-21  1:35 UTC (permalink / raw)
  To: qemu-devel
  Cc: Vladimir Sementsov-Ogievskiy, Paolo Bonzini,
	open list:Network Block Dev...

From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

This place is not obvious, nbd_export_close may theoretically reduce
refcount to 0. It may happen if someone calls nbd_export_put on named
export not through nbd_export_set_name when refcount is 1.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20171109154049.42386-2-vsementsov@virtuozzo.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
---
 nbd/server.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/nbd/server.c b/nbd/server.c
index e443b3cf5c..a6d7c24663 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -1190,6 +1190,7 @@ void nbd_export_put(NBDExport *exp)
         nbd_export_close(exp);
     }

+    assert(exp->refcount > 0);
     if (--exp->refcount == 0) {
         assert(exp->name == NULL);
         assert(exp->description == NULL);
-- 
2.14.3

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [Qemu-devel] [PULL 4/4] qmp: add nbd-server-remove
  2017-12-21  1:35 [Qemu-devel] [PULL 0/4] NBD patches through 20 Dec Eric Blake
                   ` (2 preceding siblings ...)
  2017-12-21  1:35 ` [Qemu-devel] [PULL 3/4] nbd/server: add additional assert to nbd_export_put Eric Blake
@ 2017-12-21  1:36 ` Eric Blake
  2017-12-21  9:07   ` [Qemu-devel] STOP " Vladimir Sementsov-Ogievskiy
  2017-12-21 13:13 ` [Qemu-devel] [PULL 0/4] NBD patches through 20 Dec Peter Maydell
  4 siblings, 1 reply; 10+ messages in thread
From: Eric Blake @ 2017-12-21  1:36 UTC (permalink / raw)
  To: qemu-devel
  Cc: Vladimir Sementsov-Ogievskiy, Kevin Wolf, Max Reitz,
	Paolo Bonzini, Markus Armbruster, open list:Block layer core

From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

Add command for export removing. It is needed for cases when we
don't want to keep export after the operation on it was completed.
The other example is temporary node, created with blockdev-add.
If we want to delete it we should firstly remove corresponding
NBD export.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20171109154049.42386-3-vsementsov@virtuozzo.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
---
 qapi/block.json | 20 ++++++++++++++++++++
 blockdev-nbd.c  | 27 +++++++++++++++++++++++++++
 2 files changed, 47 insertions(+)

diff --git a/qapi/block.json b/qapi/block.json
index f093fa3f27..1827940717 100644
--- a/qapi/block.json
+++ b/qapi/block.json
@@ -222,6 +222,26 @@
 ##
 { 'command': 'nbd-server-add', 'data': {'device': 'str', '*writable': 'bool'} }

+##
+# @nbd-server-remove:
+#
+# Stop exporting block node through QEMU's embedded NBD server.
+#
+# @device: The device name or node name of the exported node. Should be equal
+#          to @device parameter for corresponding nbd-server-add command call.
+#
+# @force: Whether active connections to the export should be closed. If this
+#         parameter is false the export is only removed from named exports list,
+#         so new connetions are impossible and it would be freed after all
+#         clients are disconnected (default false).
+#
+# Returns: error if the server is not running or the device is not marked for
+#          export.
+#
+# Since: 2.12
+##
+{ 'command': 'nbd-server-remove', 'data': {'device': 'str', '*force': 'bool'} }
+
 ##
 # @nbd-server-stop:
 #
diff --git a/blockdev-nbd.c b/blockdev-nbd.c
index 28f551a7b0..5f66951c33 100644
--- a/blockdev-nbd.c
+++ b/blockdev-nbd.c
@@ -203,6 +203,33 @@ void qmp_nbd_server_add(const char *device, bool has_writable, bool writable,
     nbd_export_put(exp);
 }

+void qmp_nbd_server_remove(const char *device, bool has_force, bool force,
+                           Error **errp)
+{
+    NBDExport *exp;
+
+    if (!nbd_server) {
+        error_setg(errp, "NBD server not running");
+        return;
+    }
+
+    exp = nbd_export_find(device);
+    if (exp == NULL) {
+        error_setg(errp, "'%s' is not exported", device);
+        return;
+    }
+
+    if (!has_force) {
+        force = false;
+    }
+
+    if (force) {
+        nbd_export_close(exp);
+    } else {
+        nbd_export_set_name(exp, NULL);
+    }
+}
+
 void qmp_nbd_server_stop(Error **errp)
 {
     nbd_export_close_all();
-- 
2.14.3

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [Qemu-devel] STOP Re: [PULL 4/4] qmp: add nbd-server-remove
  2017-12-21  1:36 ` [Qemu-devel] [PULL 4/4] qmp: add nbd-server-remove Eric Blake
@ 2017-12-21  9:07   ` Vladimir Sementsov-Ogievskiy
  2017-12-21  9:10     ` Vladimir Sementsov-Ogievskiy
  2017-12-21 13:13     ` Peter Maydell
  0 siblings, 2 replies; 10+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2017-12-21  9:07 UTC (permalink / raw)
  To: Eric Blake, qemu-devel
  Cc: Kevin Wolf, Max Reitz, Paolo Bonzini, Markus Armbruster,
	open list:Block layer core

no, I've updated it, we discussed with Kevin that this approach is bad

21.12.2017 04:36, Eric Blake wrote:
> From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>
> Add command for export removing. It is needed for cases when we
> don't want to keep export after the operation on it was completed.
> The other example is temporary node, created with blockdev-add.
> If we want to delete it we should firstly remove corresponding
> NBD export.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> Message-Id: <20171109154049.42386-3-vsementsov@virtuozzo.com>
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
>   qapi/block.json | 20 ++++++++++++++++++++
>   blockdev-nbd.c  | 27 +++++++++++++++++++++++++++
>   2 files changed, 47 insertions(+)
>
> diff --git a/qapi/block.json b/qapi/block.json
> index f093fa3f27..1827940717 100644
> --- a/qapi/block.json
> +++ b/qapi/block.json
> @@ -222,6 +222,26 @@
>   ##
>   { 'command': 'nbd-server-add', 'data': {'device': 'str', '*writable': 'bool'} }
>
> +##
> +# @nbd-server-remove:
> +#
> +# Stop exporting block node through QEMU's embedded NBD server.
> +#
> +# @device: The device name or node name of the exported node. Should be equal
> +#          to @device parameter for corresponding nbd-server-add command call.
> +#
> +# @force: Whether active connections to the export should be closed. If this
> +#         parameter is false the export is only removed from named exports list,
> +#         so new connetions are impossible and it would be freed after all
> +#         clients are disconnected (default false).
> +#
> +# Returns: error if the server is not running or the device is not marked for
> +#          export.
> +#
> +# Since: 2.12
> +##
> +{ 'command': 'nbd-server-remove', 'data': {'device': 'str', '*force': 'bool'} }
> +
>   ##
>   # @nbd-server-stop:
>   #
> diff --git a/blockdev-nbd.c b/blockdev-nbd.c
> index 28f551a7b0..5f66951c33 100644
> --- a/blockdev-nbd.c
> +++ b/blockdev-nbd.c
> @@ -203,6 +203,33 @@ void qmp_nbd_server_add(const char *device, bool has_writable, bool writable,
>       nbd_export_put(exp);
>   }
>
> +void qmp_nbd_server_remove(const char *device, bool has_force, bool force,
> +                           Error **errp)
> +{
> +    NBDExport *exp;
> +
> +    if (!nbd_server) {
> +        error_setg(errp, "NBD server not running");
> +        return;
> +    }
> +
> +    exp = nbd_export_find(device);
> +    if (exp == NULL) {
> +        error_setg(errp, "'%s' is not exported", device);
> +        return;
> +    }
> +
> +    if (!has_force) {
> +        force = false;
> +    }
> +
> +    if (force) {
> +        nbd_export_close(exp);
> +    } else {
> +        nbd_export_set_name(exp, NULL);
> +    }
> +}
> +
>   void qmp_nbd_server_stop(Error **errp)
>   {
>       nbd_export_close_all();


-- 
Best regards,
Vladimir

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Qemu-devel] STOP Re: [PULL 4/4] qmp: add nbd-server-remove
  2017-12-21  9:07   ` [Qemu-devel] STOP " Vladimir Sementsov-Ogievskiy
@ 2017-12-21  9:10     ` Vladimir Sementsov-Ogievskiy
  2017-12-21 14:01       ` Eric Blake
  2017-12-21 13:13     ` Peter Maydell
  1 sibling, 1 reply; 10+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2017-12-21  9:10 UTC (permalink / raw)
  To: Eric Blake, qemu-devel
  Cc: Kevin Wolf, Max Reitz, Paolo Bonzini, Markus Armbruster,
	open list:Block layer core

look at "[PATCH v2 0/6] nbd export qmp interface"

21.12.2017 12:07, Vladimir Sementsov-Ogievskiy wrote:
> no, I've updated it, we discussed with Kevin that this approach is bad
>
> 21.12.2017 04:36, Eric Blake wrote:
>> From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>>
>> Add command for export removing. It is needed for cases when we
>> don't want to keep export after the operation on it was completed.
>> The other example is temporary node, created with blockdev-add.
>> If we want to delete it we should firstly remove corresponding
>> NBD export.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>> Message-Id: <20171109154049.42386-3-vsementsov@virtuozzo.com>
>> Signed-off-by: Eric Blake <eblake@redhat.com>
>> ---
>>   qapi/block.json | 20 ++++++++++++++++++++
>>   blockdev-nbd.c  | 27 +++++++++++++++++++++++++++
>>   2 files changed, 47 insertions(+)
>>
>> diff --git a/qapi/block.json b/qapi/block.json
>> index f093fa3f27..1827940717 100644
>> --- a/qapi/block.json
>> +++ b/qapi/block.json
>> @@ -222,6 +222,26 @@
>>   ##
>>   { 'command': 'nbd-server-add', 'data': {'device': 'str', 
>> '*writable': 'bool'} }
>>
>> +##
>> +# @nbd-server-remove:
>> +#
>> +# Stop exporting block node through QEMU's embedded NBD server.
>> +#
>> +# @device: The device name or node name of the exported node. Should 
>> be equal
>> +#          to @device parameter for corresponding nbd-server-add 
>> command call.
>> +#
>> +# @force: Whether active connections to the export should be closed. 
>> If this
>> +#         parameter is false the export is only removed from named 
>> exports list,
>> +#         so new connetions are impossible and it would be freed 
>> after all
>> +#         clients are disconnected (default false).
>> +#
>> +# Returns: error if the server is not running or the device is not 
>> marked for
>> +#          export.
>> +#
>> +# Since: 2.12
>> +##
>> +{ 'command': 'nbd-server-remove', 'data': {'device': 'str', 
>> '*force': 'bool'} }
>> +
>>   ##
>>   # @nbd-server-stop:
>>   #
>> diff --git a/blockdev-nbd.c b/blockdev-nbd.c
>> index 28f551a7b0..5f66951c33 100644
>> --- a/blockdev-nbd.c
>> +++ b/blockdev-nbd.c
>> @@ -203,6 +203,33 @@ void qmp_nbd_server_add(const char *device, bool 
>> has_writable, bool writable,
>>       nbd_export_put(exp);
>>   }
>>
>> +void qmp_nbd_server_remove(const char *device, bool has_force, bool 
>> force,
>> +                           Error **errp)
>> +{
>> +    NBDExport *exp;
>> +
>> +    if (!nbd_server) {
>> +        error_setg(errp, "NBD server not running");
>> +        return;
>> +    }
>> +
>> +    exp = nbd_export_find(device);
>> +    if (exp == NULL) {
>> +        error_setg(errp, "'%s' is not exported", device);
>> +        return;
>> +    }
>> +
>> +    if (!has_force) {
>> +        force = false;
>> +    }
>> +
>> +    if (force) {
>> +        nbd_export_close(exp);
>> +    } else {
>> +        nbd_export_set_name(exp, NULL);
>> +    }
>> +}
>> +
>>   void qmp_nbd_server_stop(Error **errp)
>>   {
>>       nbd_export_close_all();
>
>


-- 
Best regards,
Vladimir

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Qemu-devel] STOP Re: [PULL 4/4] qmp: add nbd-server-remove
  2017-12-21  9:07   ` [Qemu-devel] STOP " Vladimir Sementsov-Ogievskiy
  2017-12-21  9:10     ` Vladimir Sementsov-Ogievskiy
@ 2017-12-21 13:13     ` Peter Maydell
  1 sibling, 0 replies; 10+ messages in thread
From: Peter Maydell @ 2017-12-21 13:13 UTC (permalink / raw)
  To: Vladimir Sementsov-Ogievskiy
  Cc: Eric Blake, QEMU Developers, Kevin Wolf, Paolo Bonzini,
	Markus Armbruster, open list:Block layer core, Max Reitz

On 21 December 2017 at 09:07, Vladimir Sementsov-Ogievskiy
<vsementsov@virtuozzo.com> wrote:
> no, I've updated it, we discussed with Kevin that this approach is bad

To say that a pullreq is bad you need to reply to the cover letter,
there's no guarantee I'll notice a followup to one of the
individual patches in it. (As it happens I saw this email so
I'll take the pullreq out of the queue.)

thanks
-- PMM

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Qemu-devel] [PULL 0/4] NBD patches through 20 Dec
  2017-12-21  1:35 [Qemu-devel] [PULL 0/4] NBD patches through 20 Dec Eric Blake
                   ` (3 preceding siblings ...)
  2017-12-21  1:36 ` [Qemu-devel] [PULL 4/4] qmp: add nbd-server-remove Eric Blake
@ 2017-12-21 13:13 ` Peter Maydell
  4 siblings, 0 replies; 10+ messages in thread
From: Peter Maydell @ 2017-12-21 13:13 UTC (permalink / raw)
  To: Eric Blake; +Cc: QEMU Developers

On 21 December 2017 at 01:35, Eric Blake <eblake@redhat.com> wrote:
> The following changes since commit 4da5c51cac8363f86ec92dc99c38f9382d617647:
>
>   Merge remote-tracking branch 'remotes/armbru/tags/pull-qapi-2017-12-20' into staging (2017-12-20 20:38:36 +0000)
>
> are available in the Git repository at:
>
>   git://repo.or.cz/qemu/ericb.git tags/pull-nbd-2017-12-20
>
> for you to fetch changes up to fab45bb24a35b4791deecf3ec732d64b0b987662:
>
>   qmp: add nbd-server-remove (2017-12-20 16:55:32 -0600)
>
> ----------------------------------------------------------------
> nbd patches for 2017-12-20
>
> - Eric Blake: 0/2 Optimize sparse reads over NBD
> - Vladimir Sementsov-Ogievskiy: 0/2 add qmp nbd-server-remove
>

Not applying, as Vladimir says his patches are not the right ones.

thanks
-- PMM

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Qemu-devel] STOP Re: [PULL 4/4] qmp: add nbd-server-remove
  2017-12-21  9:10     ` Vladimir Sementsov-Ogievskiy
@ 2017-12-21 14:01       ` Eric Blake
  0 siblings, 0 replies; 10+ messages in thread
From: Eric Blake @ 2017-12-21 14:01 UTC (permalink / raw)
  To: Vladimir Sementsov-Ogievskiy, qemu-devel
  Cc: Kevin Wolf, Max Reitz, Paolo Bonzini, Markus Armbruster,
	open list:Block layer core

On 12/21/2017 03:10 AM, Vladimir Sementsov-Ogievskiy wrote:
> look at "[PATCH v2 0/6] nbd export qmp interface"
> 
> 21.12.2017 12:07, Vladimir Sementsov-Ogievskiy wrote:
>> no, I've updated it, we discussed with Kevin that this approach is bad
>>

I'll respin the pull request with the updated series; thanks for the catch.

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2017-12-21 14:01 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-12-21  1:35 [Qemu-devel] [PULL 0/4] NBD patches through 20 Dec Eric Blake
2017-12-21  1:35 ` [Qemu-devel] [PULL 1/4] nbd/server: Implement sparse reads atop structured reply Eric Blake
2017-12-21  1:35 ` [Qemu-devel] [PULL 2/4] nbd/server: Optimize final chunk of sparse read Eric Blake
2017-12-21  1:35 ` [Qemu-devel] [PULL 3/4] nbd/server: add additional assert to nbd_export_put Eric Blake
2017-12-21  1:36 ` [Qemu-devel] [PULL 4/4] qmp: add nbd-server-remove Eric Blake
2017-12-21  9:07   ` [Qemu-devel] STOP " Vladimir Sementsov-Ogievskiy
2017-12-21  9:10     ` Vladimir Sementsov-Ogievskiy
2017-12-21 14:01       ` Eric Blake
2017-12-21 13:13     ` Peter Maydell
2017-12-21 13:13 ` [Qemu-devel] [PULL 0/4] NBD patches through 20 Dec Peter Maydell

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.