[Top][All Lists]

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

Re: [Qemu-block] [PATCH v2 3/6] qapi: add nbd-server-remove

From: Vladimir Sementsov-Ogievskiy
Subject: Re: [Qemu-block] [PATCH v2 3/6] qapi: add nbd-server-remove
Date: Fri, 12 Jan 2018 12:47:55 +0300
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.2

09.01.2018 22:52, Eric Blake wrote:
On 12/07/2017 09:50 AM, Vladimir Sementsov-Ogievskiy wrote:
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 <address@hidden>
  qapi/block.json     | 18 ++++++++++++++++++
  include/block/nbd.h |  3 ++-
  blockdev-nbd.c      | 29 ++++++++++++++++++++++++++++-
  nbd/server.c        | 25 ++++++++++++++++++++++---
  4 files changed, 70 insertions(+), 5 deletions(-)

diff --git a/qapi/block.json b/qapi/block.json
index 503d4b287b..f83485c325 100644
--- a/qapi/block.json
+++ b/qapi/block.json
@@ -228,6 +228,24 @@
    'data': {'device': 'str', '*name': 'str', '*writable': 'bool'} }
+# @nbd-server-remove:
+# Remove NBD export by name.
+# @name: Export name.
+# @force: Whether active connections to the export should be closed. If this
+#         parameter is false the export only becomes hidden from clients (new
+#         connections are impossible), and would be automatically freed after
+#         all clients are disconnected (default false).
Unstated, but if the parameter is true, existing clients are forcefully
disconnected, possibly losing pending transactions.

Do we want a third mode in the middle, where the server starts replying
to all existing clients with ESHUTDOWN errors for all new requests
rather than abruptly disconnecting (no new I/O, but no forced disconnect
and pending in-flight transactions can still complete gracefully)?

looks interesting. what about the following naming?

@mode: possible values:
              hide - just hide server from new clients, maintain existing connections,
                          remove after all clients disconnected
              soft - like hide, but answer with ESHUTDOWN for all further requests from
                          existing connections
              hard - hard disconnect all clients and remove server
              (default: soft)

new corresponding states of nbd export:
hidden, shutting_down

and we allow transitions:

normal_execution -> hidden
normal_execution -> shutting_down
normal_execution -> exit
hidden -> shutting_down
hidden -> exit
shutting_down -> exit

+# Returns: error if the server is not running or export is not found.
+# Since: 2.12
+{ 'command': 'nbd-server-remove', 'data': {'name': 'str', '*force': 'bool'} }
If we're okay with just the bool parameter, then this patch looks good;
but if we want a third mode, then we want '*force' to be an enum type.
So tentative:

Reviewed-by: Eric Blake <address@hidden>

Best regards,

reply via email to

[Prev in Thread] Current Thread [Next in Thread]