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

[Dr. David Alan Gilbert]

* Vladimir Sementsov-Ogievskiy (vsement...@virtuozzo.com) wrote:
> 26.01.2018 18:05, Dr. David Alan Gilbert wrote:
> > * Vladimir Sementsov-Ogievskiy (vsement...@virtuozzo.com) wrote:
> > > 17.01.2018 19:03, Eric Blake wrote:
> > > > On 01/17/2018 09:51 AM, Vladimir Sementsov-Ogievskiy wrote:
> > > > 
> > > > > > > I have a script (for managing libvirt guest, but it can be 
> > > > > > > adopted for
> > > > > > > qemu or even used for qemu monitor), which allows
> > > > > > > me run qmp commands on vms as easy as:
> > > > > > > 
> > > > > > > |qmp VMNAME query-block-jobs or qmp VMNAME nbd-server-remove name 
> > > > > > > exp1
> > > > > > > mode hard or even |
> > > > > > > 
> > > > > > > |qmp VMNAME blockdev-add id disk driver qcow2 cache {writeback 
> > > > > > > true
> > > > > > > direct true} aio native discard unmap file {driver file filename
> > > > > > > /tmp/somedisk} |||
> > > > > > Yeah, there are various scripting solutions around QMP that can 
> > > > > > make it
> > > > > > easier; but HMP is often still an easy front-line interface for
> > > > > > experiments.
> > > > > > 
> > > > > isn't it because these solutions are not available directly in 
> > > > > monitor,
> > > > > when HMP is?
> > > > QMP can be directly accessed in a monitor; it just requires more typing.
> > > >If you are developing QMP commands, it may be easier to use
> > > > ./scripts/qmp/qmp-shell (couple it with a readline wrapper, and you can
> > > > even get tab-completion and history across sessions).  There's also
> > > > things like libvirt's 'virsh qmp-monitor-command' for shell-scripting
> > > > access to arbitrary QMP commands, provided your guest is run by libvirt.
> > > > 
> > > > > may be, we need third type of monitor HQMP which is QMP with 
> > > > > simplified
> > > > > syntax? Or
> > > > > allow qmp commands in simplified syntax directly in HMP?
> > > > No, I don't think we need either thing.  Wrappers around existing
> > > > monitors is better than bloating qemu proper with a third flavor of
> > > > monitor.  And HMP is for humans, with no restrictions on back-compat
> > > > changes, so if it doesn't do something you want for quick-and-dirty
> > > > testing, you can either add a new HMP command, or just use QMP (or one
> > > > of its wrappers, like qmp-shell) in the first place.  Ultimately, our
> > > > long-term concern is only about the QMP interface; HMP is supposed to be
> > > > convenient.  So if it starts costing too much time to port a QMP
> > > > interface to HMP, then don't worry about it.
> > > > 
> > > most of commands, ported to hmp are done in same style: they just call
> > > corresponding qmp command.
> > > Isn't it better to provide common interface for calling qmp commands 
> > > through
> > > HMP monitor, to never
> > > create hmp versions of new commands? they will be available automatically.
> > It would be nice to do that, but they're not that consistent in how they
> > convert parameters and options, but I occasionally wonder if we could
> > automate more of it.
> 
> 
> What about allowing some new syntax in hmp, directly mapped to qmp?
> 
> something like
> 
> >>> blockdev-add id disk driver qcow2 cache {writeback true direct true} aio
> native discard unmap file {driver file filename /tmp/somedisk}
> 
> ?

Hmm, I don't particularly find that easy to read either; however the
actual block device specification for HMP should be the same as what we
pass on the command line, so we only have to worry about any extra
things that are part of blockdev_add.
(I'm sure we can find a way of making the one we pass on the commandline
more readable as well, there's so much duplication).

Dave

> Or it may be realized as a separate hmp command "qmp" (looks more safe as a
> first step, however, I think previous variant (direct call) is better):
> 
> >>> qmp blockdev-add id disk driver qcow2 cache {writeback true direct true}
> aio native discard unmap file {driver file filename /tmp/somedisk}
> 
> what do think? This looks simple to implement and should be useful.
> 
> > 
> > Dave
> > 
> > > -- 
> > > Best regards,
> > > Vladimir
> > > 
> > --
> > Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
> 
> 
> -- 
> Best regards,
> Vladimir
> 
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

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

[Vladimir Sementsov-Ogievskiy]

06.02.2018 19:06, Eric Blake wrote:

On 02/06/2018 09:29 AM, Vladimir Sementsov-Ogievskiy wrote:


most of commands, ported to hmp are done in same style: they just call
corresponding qmp command.
Isn't it better to provide common interface for calling qmp 
commands through

HMP monitor, to never
create hmp versions of new commands? they will be available 
automatically.
It would be nice to do that, but they're not that consistent in how 
they

convert parameters and options, but I occasionally wonder if we could
automate more of it.



What about allowing some new syntax in hmp, directly mapped to qmp?

something like

 >>> blockdev-add id disk driver qcow2 cache {writeback true direct 
true} aio native discard unmap file {driver file filename /tmp/somedisk}




Personally, if I'm testing blockdev-add, I'll use QMP directly (or 
even scripts/qmp/qmp-shell or virsh qemu-monitor-command), not an HMP 
wrapper where I have to learn a new syntax of how to write something 
that will convert to QMP.  We already have enough different ways to 
write things that I don't need to learn yet another syntax wrapper.  
Or maybe what I'm saying is that instead of inventing a new syntax, 
that if you DO add an HMP command that forwards to QMP, please reuse 
an existing syntax (whether direct JSON as used by QMP, or the syntax 
used by qmp-shell).


I'm afraid, that JSON is too hard to use in human monitor. And this will 
make the whole feature useless.




If you think writing a new HMP command is worth it, I won't stop you 
from writing it.  But at this point, our current approach of writing a 
manual wrapper per command as we have interest, rather than a generic 
wrap-anything, has worked for the cases that HMP users have cared 
about.  Remember, QMP is the interface that MUST work, while HMP is 
only for convenience, and if it is not trivial to make HMP do 
everything that QMP can do, it is no real loss.




But we create hmp wrappers on demand, and for each case, we actually 
invent new syntax. I just search for the way to avoid creating new and 
new hmp wrappers, by introducing new syntax only once.

And, here is almost nothing to learn:

command := command-name parameters
parameters = [key value ]...
value = simple-value | array | map
map = '{' parameters '}'
array = '[' [value ]... ']'

another variant is to use yaml - like json, but we do not need put all 
keys into quotes.


On the other hand, implementing new parser in qemu is not trivial task 
(hmm, I don't want do it=), it should be simpler to create direct JSON 
wrapper in HMP monitor, and use some python wrapper around the monitor. 
And this looks useless, as with same result I can use wrapper around QMP 
monitor. So, may be the most interesting solution would be to make some 
easy-to-use python-based wrapper, which will give a simple way to use 
both qmp and hmp commands.. I'll think about it. However it doesn't 
solve initial problem of creating new and new hmp wrappers by hand.



?

Or it may be realized as a separate hmp command "qmp" (looks more 
safe as a first step, however, I think previous variant (direct call) 
is better):


 >>> qmp blockdev-add id disk driver qcow2 cache {writeback true 
direct true} aio native discard unmap file {driver file filename 
/tmp/somedisk}


what do think? This looks simple to implement and should be useful.


Up to you if you want to tackle anything like that, but it would be a 
new thread (a generic way to invoke QMP from HMP is independent of 
nbd-server-remove).





--
Best regards,
Vladimir

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

[Eric Blake]

On 02/06/2018 09:29 AM, Vladimir Sementsov-Ogievskiy wrote:


most of commands, ported to hmp are done in same style: they just call
corresponding qmp command.
Isn't it better to provide common interface for calling qmp commands 
through

HMP monitor, to never
create hmp versions of new commands? they will be available 
automatically.

It would be nice to do that, but they're not that consistent in how they
convert parameters and options, but I occasionally wonder if we could
automate more of it.



What about allowing some new syntax in hmp, directly mapped to qmp?

something like

 >>> blockdev-add id disk driver qcow2 cache {writeback true direct 
true} aio native discard unmap file {driver file filename /tmp/somedisk}




Personally, if I'm testing blockdev-add, I'll use QMP directly (or even 
scripts/qmp/qmp-shell or virsh qemu-monitor-command), not an HMP wrapper 
where I have to learn a new syntax of how to write something that will 
convert to QMP.  We already have enough different ways to write things 
that I don't need to learn yet another syntax wrapper.  Or maybe what 
I'm saying is that instead of inventing a new syntax, that if you DO add 
an HMP command that forwards to QMP, please reuse an existing syntax 
(whether direct JSON as used by QMP, or the syntax used by qmp-shell).


If you think writing a new HMP command is worth it, I won't stop you 
from writing it.  But at this point, our current approach of writing a 
manual wrapper per command as we have interest, rather than a generic 
wrap-anything, has worked for the cases that HMP users have cared about. 
 Remember, QMP is the interface that MUST work, while HMP is only for 
convenience, and if it is not trivial to make HMP do everything that QMP 
can do, it is no real loss.



?

Or it may be realized as a separate hmp command "qmp" (looks more safe 
as a first step, however, I think previous variant (direct call) is 
better):


 >>> qmp blockdev-add id disk driver qcow2 cache {writeback true direct 
true} aio native discard unmap file {driver file filename /tmp/somedisk}


what do think? This looks simple to implement and should be useful.


Up to you if you want to tackle anything like that, but it would be a 
new thread (a generic way to invoke QMP from HMP is independent of 
nbd-server-remove).


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

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

[Vladimir Sementsov-Ogievskiy]

26.01.2018 18:05, Dr. David Alan Gilbert wrote:

* Vladimir Sementsov-Ogievskiy (vsement...@virtuozzo.com) wrote:

17.01.2018 19:03, Eric Blake wrote:

On 01/17/2018 09:51 AM, Vladimir Sementsov-Ogievskiy wrote:


I have a script (for managing libvirt guest, but it can be adopted for
qemu or even used for qemu monitor), which allows
me run qmp commands on vms as easy as:

|qmp VMNAME query-block-jobs or qmp VMNAME nbd-server-remove name exp1
mode hard or even |

|qmp VMNAME blockdev-add id disk driver qcow2 cache {writeback true
direct true} aio native discard unmap file {driver file filename
/tmp/somedisk} |||

Yeah, there are various scripting solutions around QMP that can make it
easier; but HMP is often still an easy front-line interface for
experiments.


isn't it because these solutions are not available directly in monitor,
when HMP is?

QMP can be directly accessed in a monitor; it just requires more typing.
   If you are developing QMP commands, it may be easier to use
./scripts/qmp/qmp-shell (couple it with a readline wrapper, and you can
even get tab-completion and history across sessions).  There's also
things like libvirt's 'virsh qmp-monitor-command' for shell-scripting
access to arbitrary QMP commands, provided your guest is run by libvirt.


may be, we need third type of monitor HQMP which is QMP with simplified
syntax? Or
allow qmp commands in simplified syntax directly in HMP?

No, I don't think we need either thing.  Wrappers around existing
monitors is better than bloating qemu proper with a third flavor of
monitor.  And HMP is for humans, with no restrictions on back-compat
changes, so if it doesn't do something you want for quick-and-dirty
testing, you can either add a new HMP command, or just use QMP (or one
of its wrappers, like qmp-shell) in the first place.  Ultimately, our
long-term concern is only about the QMP interface; HMP is supposed to be
convenient.  So if it starts costing too much time to port a QMP
interface to HMP, then don't worry about it.


most of commands, ported to hmp are done in same style: they just call
corresponding qmp command.
Isn't it better to provide common interface for calling qmp commands through
HMP monitor, to never
create hmp versions of new commands? they will be available automatically.

It would be nice to do that, but they're not that consistent in how they
convert parameters and options, but I occasionally wonder if we could
automate more of it.



What about allowing some new syntax in hmp, directly mapped to qmp?

something like

>>> blockdev-add id disk driver qcow2 cache {writeback true direct 
true} aio native discard unmap file {driver file filename /tmp/somedisk}


?

Or it may be realized as a separate hmp command "qmp" (looks more safe 
as a first step, however, I think previous variant (direct call) is better):


>>> qmp blockdev-add id disk driver qcow2 cache {writeback true direct 
true} aio native discard unmap file {driver file filename /tmp/somedisk}


what do think? This looks simple to implement and should be useful.



Dave


--
Best regards,
Vladimir


--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK



--
Best regards,
Vladimir

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

[Dr. David Alan Gilbert]

* Vladimir Sementsov-Ogievskiy (vsement...@virtuozzo.com) wrote:
> 17.01.2018 19:03, Eric Blake wrote:
> > On 01/17/2018 09:51 AM, Vladimir Sementsov-Ogievskiy wrote:
> > 
> > > > > I have a script (for managing libvirt guest, but it can be adopted for
> > > > > qemu or even used for qemu monitor), which allows
> > > > > me run qmp commands on vms as easy as:
> > > > > 
> > > > > |qmp VMNAME query-block-jobs or qmp VMNAME nbd-server-remove name exp1
> > > > > mode hard or even |
> > > > > 
> > > > > |qmp VMNAME blockdev-add id disk driver qcow2 cache {writeback true
> > > > > direct true} aio native discard unmap file {driver file filename
> > > > > /tmp/somedisk} |||
> > > > Yeah, there are various scripting solutions around QMP that can make it
> > > > easier; but HMP is often still an easy front-line interface for
> > > > experiments.
> > > > 
> > > isn't it because these solutions are not available directly in monitor,
> > > when HMP is?
> > QMP can be directly accessed in a monitor; it just requires more typing.
> >   If you are developing QMP commands, it may be easier to use
> > ./scripts/qmp/qmp-shell (couple it with a readline wrapper, and you can
> > even get tab-completion and history across sessions).  There's also
> > things like libvirt's 'virsh qmp-monitor-command' for shell-scripting
> > access to arbitrary QMP commands, provided your guest is run by libvirt.
> > 
> > > may be, we need third type of monitor HQMP which is QMP with simplified
> > > syntax? Or
> > > allow qmp commands in simplified syntax directly in HMP?
> > No, I don't think we need either thing.  Wrappers around existing
> > monitors is better than bloating qemu proper with a third flavor of
> > monitor.  And HMP is for humans, with no restrictions on back-compat
> > changes, so if it doesn't do something you want for quick-and-dirty
> > testing, you can either add a new HMP command, or just use QMP (or one
> > of its wrappers, like qmp-shell) in the first place.  Ultimately, our
> > long-term concern is only about the QMP interface; HMP is supposed to be
> > convenient.  So if it starts costing too much time to port a QMP
> > interface to HMP, then don't worry about it.
> > 
> 
> most of commands, ported to hmp are done in same style: they just call
> corresponding qmp command.
> Isn't it better to provide common interface for calling qmp commands through
> HMP monitor, to never
> create hmp versions of new commands? they will be available automatically.

It would be nice to do that, but they're not that consistent in how they
convert parameters and options, but I occasionally wonder if we could
automate more of it.

Dave

> -- 
> Best regards,
> Vladimir
> 
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

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

[Eric Blake]

On 01/18/2018 12:11 PM, Vladimir Sementsov-Ogievskiy wrote:
> Add command for export removing. It is needed for cases when we

s/export removing/removing an export/

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

No HMP counterpart? I can give a quick shot at that, as a followup patch.

> Signed-off-by: Vladimir Sementsov-Ogievskiy 
> ---
>  qapi/block.json | 45 +
>  include/block/nbd.h |  1 +
>  blockdev-nbd.c  | 24 
>  nbd/server.c| 21 +
>  4 files changed, 91 insertions(+)
> 
> diff --git a/qapi/block.json b/qapi/block.json
> index 353e3a45bd..ddf73932ce 100644
> --- a/qapi/block.json
> +++ b/qapi/block.json
> @@ -228,6 +228,51 @@
>'data': {'device': 'str', '*name': 'str', '*writable': 'bool'} }
>  
>  ##
> +# @NbdServerRemoveMode:
> +#
> +# Mode of NBD export removing.

Mode for removing an NBD export.

> +#
> +# @safe: Remove export if there are no existing connections, fail otherwise.
> +#
> +# @hard: Drop all connections immediately and remove export.
> +#
> +# Postponed, not realized yet modes:

Maybe:

Potential additional modes to be added in the future:

> +#
> +# hide: Just hide export from new clients, leave existing connections as is.
> +#   Remove export after all clients are disconnected. nbd-server-remove
> +#   with mode=soft or mode=hard may be called after nbd-server-remove
> +#   with mode=hide.

mode=safe could also be called; I don't see that this last sentence adds
much.

> +#
> +# soft: Hide export from new clients, answer with ESHUTDOWN for all further
> +#   requests from existing clients. Remove export after all clients are
> +#   disconnected. nbd-server-requests with mode=hard may be called after
> +#   nbd-server-remove with mode=soft

Again, the last sentence doesn't add mouch (I guess you're arguing that
requesting a hide doesn't make much sense after a soft disconnect has
already started; but I don't see any harm in permitting it as a no-op).

> +#
> +# Since: 2.12
> +##
> +{'enum': 'NbdServerRemoveMode', 'data': ['safe', 'hard']}
> +
> +##
> +# @nbd-server-remove:
> +#
> +# Remove NBD export by name.
> +#
> +# @name: Export name.
> +#
> +# @mode: Mode of command operation. See @NbdServerRemoveMode description.
> +#Default is 'safe'.
> +#
> +# Returns: error if
> +#- the server is not running
> +#- export is not found
> +#- mode is 'safe' and there are existing connections
> +#
> +# Since: 2.12
> +##
> +{ 'command': 'nbd-server-remove',
> +  'data': {'name': 'str', '*mode': 'NbdServerRemoveMode'} }

Looks reasonable.


> +++ b/nbd/server.c
> @@ -1177,6 +1177,27 @@ void nbd_export_close(NBDExport *exp)
>  nbd_export_put(exp);
>  }
>  
> +void nbd_export_remove(NBDExport *exp, NbdServerRemoveMode mode, Error 
> **errp)
> +{
> +NBDClient *client;
> +int nb_clients = 0;
> +
> +if (mode == NBD_SERVER_REMOVE_MODE_HARD || QTAILQ_EMPTY(>clients)) {
> +nbd_export_close(exp);
> +return;
> +}
> +
> +assert(mode == NBD_SERVER_REMOVE_MODE_SAFE);
> +
> +QTAILQ_FOREACH(client, >clients, next) {
> +nb_clients++;
> +}
> +
> +error_setg(errp, "NBD export '%s' has %d active connection%s. To force "
> +   "remove it (and hard disconnect clients) use mode='hard'",
> +   exp->name, nb_clients, nb_clients == 1 ? "" : "s");

Playing games with English pluralization doesn't work too well in error
messages, if we ever want to allow translations; does the number of
active clients actually matter?  Also, error_setg() recommends against
using '.' or more than one sentence.  Better might be:

error_setg(errp, "export '%s' still in use");
error_append_hint(errp, "Use mode='hard' to force client disconnect\n");

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



signature.asc
Description: OpenPGP digital signature

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

[Vladimir Sementsov-Ogievskiy]

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 
---
 qapi/block.json | 45 +
 include/block/nbd.h |  1 +
 blockdev-nbd.c  | 24 
 nbd/server.c| 21 +
 4 files changed, 91 insertions(+)

diff --git a/qapi/block.json b/qapi/block.json
index 353e3a45bd..ddf73932ce 100644
--- a/qapi/block.json
+++ b/qapi/block.json
@@ -228,6 +228,51 @@
   'data': {'device': 'str', '*name': 'str', '*writable': 'bool'} }
 
 ##
+# @NbdServerRemoveMode:
+#
+# Mode of NBD export removing.
+#
+# @safe: Remove export if there are no existing connections, fail otherwise.
+#
+# @hard: Drop all connections immediately and remove export.
+#
+# Postponed, not realized yet modes:
+#
+# hide: Just hide export from new clients, leave existing connections as is.
+#   Remove export after all clients are disconnected. nbd-server-remove
+#   with mode=soft or mode=hard may be called after nbd-server-remove
+#   with mode=hide.
+#
+# soft: Hide export from new clients, answer with ESHUTDOWN for all further
+#   requests from existing clients. Remove export after all clients are
+#   disconnected. nbd-server-requests with mode=hard may be called after
+#   nbd-server-remove with mode=soft
+#
+# Since: 2.12
+##
+{'enum': 'NbdServerRemoveMode', 'data': ['safe', 'hard']}
+
+##
+# @nbd-server-remove:
+#
+# Remove NBD export by name.
+#
+# @name: Export name.
+#
+# @mode: Mode of command operation. See @NbdServerRemoveMode description.
+#Default is 'safe'.
+#
+# Returns: error if
+#- the server is not running
+#- export is not found
+#- mode is 'safe' and there are existing connections
+#
+# Since: 2.12
+##
+{ 'command': 'nbd-server-remove',
+  'data': {'name': 'str', '*mode': 'NbdServerRemoveMode'} }
+
+##
 # @nbd-server-stop:
 #
 # Stop QEMU's embedded NBD server, and unregister all devices previously
diff --git a/include/block/nbd.h b/include/block/nbd.h
index 978e443366..ee74ec391a 100644
--- a/include/block/nbd.h
+++ b/include/block/nbd.h
@@ -261,6 +261,7 @@ NBDExport *nbd_export_new(BlockDriverState *bs, off_t 
dev_offset, off_t size,
   bool writethrough, BlockBackend *on_eject_blk,
   Error **errp);
 void nbd_export_close(NBDExport *exp);
+void nbd_export_remove(NBDExport *exp, NbdServerRemoveMode mode, Error **errp);
 void nbd_export_get(NBDExport *exp);
 void nbd_export_put(NBDExport *exp);
 
diff --git a/blockdev-nbd.c b/blockdev-nbd.c
index 104789e521..a9f79c6778 100644
--- a/blockdev-nbd.c
+++ b/blockdev-nbd.c
@@ -189,6 +189,30 @@ void qmp_nbd_server_add(const char *device, bool has_name, 
const char *name,
 nbd_export_put(exp);
 }
 
+void qmp_nbd_server_remove(const char *name,
+   bool has_mode, NbdServerRemoveMode mode,
+   Error **errp)
+{
+NBDExport *exp;
+
+if (!nbd_server) {
+error_setg(errp, "NBD server not running");
+return;
+}
+
+exp = nbd_export_find(name);
+if (exp == NULL) {
+error_setg(errp, "Export '%s' is not found", name);
+return;
+}
+
+if (!has_mode) {
+mode = NBD_SERVER_REMOVE_MODE_SAFE;
+}
+
+nbd_export_remove(exp, mode, errp);
+}
+
 void qmp_nbd_server_stop(Error **errp)
 {
 nbd_export_close_all();
diff --git a/nbd/server.c b/nbd/server.c
index 6cf2eeb2c1..fdb6be7016 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -1177,6 +1177,27 @@ void nbd_export_close(NBDExport *exp)
 nbd_export_put(exp);
 }
 
+void nbd_export_remove(NBDExport *exp, NbdServerRemoveMode mode, Error **errp)
+{
+NBDClient *client;
+int nb_clients = 0;
+
+if (mode == NBD_SERVER_REMOVE_MODE_HARD || QTAILQ_EMPTY(>clients)) {
+nbd_export_close(exp);
+return;
+}
+
+assert(mode == NBD_SERVER_REMOVE_MODE_SAFE);
+
+QTAILQ_FOREACH(client, >clients, next) {
+nb_clients++;
+}
+
+error_setg(errp, "NBD export '%s' has %d active connection%s. To force "
+   "remove it (and hard disconnect clients) use mode='hard'",
+   exp->name, nb_clients, nb_clients == 1 ? "" : "s");
+}
+
 void nbd_export_get(NBDExport *exp)
 {
 assert(exp->refcount > 0);
-- 
2.11.1

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

[Vladimir Sementsov-Ogievskiy]

17.01.2018 19:03, Eric Blake wrote:

On 01/17/2018 09:51 AM, Vladimir Sementsov-Ogievskiy wrote:


I have a script (for managing libvirt guest, but it can be adopted for
qemu or even used for qemu monitor), which allows
me run qmp commands on vms as easy as:

|qmp VMNAME query-block-jobs or qmp VMNAME nbd-server-remove name exp1
mode hard or even |

|qmp VMNAME blockdev-add id disk driver qcow2 cache {writeback true
direct true} aio native discard unmap file {driver file filename
/tmp/somedisk} |||

Yeah, there are various scripting solutions around QMP that can make it
easier; but HMP is often still an easy front-line interface for
experiments.


isn't it because these solutions are not available directly in monitor,
when HMP is?

QMP can be directly accessed in a monitor; it just requires more typing.
  If you are developing QMP commands, it may be easier to use
./scripts/qmp/qmp-shell (couple it with a readline wrapper, and you can
even get tab-completion and history across sessions).  There's also
things like libvirt's 'virsh qmp-monitor-command' for shell-scripting
access to arbitrary QMP commands, provided your guest is run by libvirt.


may be, we need third type of monitor HQMP which is QMP with simplified
syntax? Or
allow qmp commands in simplified syntax directly in HMP?

No, I don't think we need either thing.  Wrappers around existing
monitors is better than bloating qemu proper with a third flavor of
monitor.  And HMP is for humans, with no restrictions on back-compat
changes, so if it doesn't do something you want for quick-and-dirty
testing, you can either add a new HMP command, or just use QMP (or one
of its wrappers, like qmp-shell) in the first place.  Ultimately, our
long-term concern is only about the QMP interface; HMP is supposed to be
convenient.  So if it starts costing too much time to port a QMP
interface to HMP, then don't worry about it.



most of commands, ported to hmp are done in same style: they just call 
corresponding qmp command.
Isn't it better to provide common interface for calling qmp commands 
through HMP monitor, to never

create hmp versions of new commands? they will be available automatically.

--
Best regards,
Vladimir

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

[Eric Blake]

On 01/17/2018 09:51 AM, Vladimir Sementsov-Ogievskiy wrote:

>>> I have a script (for managing libvirt guest, but it can be adopted for
>>> qemu or even used for qemu monitor), which allows
>>> me run qmp commands on vms as easy as:
>>>
>>> |qmp VMNAME query-block-jobs or qmp VMNAME nbd-server-remove name exp1
>>> mode hard or even |
>>>
>>> |qmp VMNAME blockdev-add id disk driver qcow2 cache {writeback true
>>> direct true} aio native discard unmap file {driver file filename
>>> /tmp/somedisk} |||
>> Yeah, there are various scripting solutions around QMP that can make it
>> easier; but HMP is often still an easy front-line interface for
>> experiments.
>>
> 
> isn't it because these solutions are not available directly in monitor,
> when HMP is?

QMP can be directly accessed in a monitor; it just requires more typing.
 If you are developing QMP commands, it may be easier to use
./scripts/qmp/qmp-shell (couple it with a readline wrapper, and you can
even get tab-completion and history across sessions).  There's also
things like libvirt's 'virsh qmp-monitor-command' for shell-scripting
access to arbitrary QMP commands, provided your guest is run by libvirt.

> may be, we need third type of monitor HQMP which is QMP with simplified
> syntax? Or
> allow qmp commands in simplified syntax directly in HMP?

No, I don't think we need either thing.  Wrappers around existing
monitors is better than bloating qemu proper with a third flavor of
monitor.  And HMP is for humans, with no restrictions on back-compat
changes, so if it doesn't do something you want for quick-and-dirty
testing, you can either add a new HMP command, or just use QMP (or one
of its wrappers, like qmp-shell) in the first place.  Ultimately, our
long-term concern is only about the QMP interface; HMP is supposed to be
convenient.  So if it starts costing too much time to port a QMP
interface to HMP, then don't worry about it.

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



signature.asc
Description: OpenPGP digital signature

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

[Vladimir Sementsov-Ogievskiy]

17.01.2018 18:23, Eric Blake wrote:

On 01/17/2018 07:36 AM, Vladimir Sementsov-Ogievskiy wrote:


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)

Or even a fourth mode that causes an immediate error return without
state change if there are any connected clients, but otherwise removes
the server.


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

Seems reasonable.  Are you planning on tackling a respin of this series
incorporating that idea?


yes, will do.



Discussed with Nikolay.
For now we actually need only one mode: hard.
In near future we _may be_ will need your proposed fourth mode (what
about "safe" name for it ?)

'safe' sounds reasonable.

Of course, if we only have two modes at front ('safe' which returns an
error if a client is connected, and 'hard' which disconnects all clients
immediately; leaving 'hide' and 'soft' for the future), then we don't
have to worry about a state transition or any hidden exports.

A QAPI enum with only two values now is at least extensible in the
future if someone has a need for another mode, and introspectible to
learn which modes are currently supported.


I was going to implement all 4 modes, but now I doubt, isn't it too
hastily, to introduce 3 new modes to the
interface, which we (personally) do not need. May be it is better to
start from one or two modes.

Starting with just two modes is fine as well.


Finally what do you think, Eric? Which modes do you need?

'hide' may be interesting for the purpose of connecting a single client,
then hiding the export so no other clients can connect, while waiting
for the first client to take its time.  But right now, I don't have
actual use cases in mind so much as making sure we aren't limiting
ourself from future expansion as needs are identified, so a conservative
choice of just 'safe' and 'hard' for now is reasonable.


ok, I agree. 'safe' looks like better option for default behavior. So 
I'll post these two options in v3.





ps: I've created hmp version for 2/6, it will be in v2.
  also, I'm going to add query-nbd-server, which should list all exports

Sounds good.


also, about HMP: If I understand correctly, people use it because
writing qmp command by hand is not very comfortable.
I have a script (for managing libvirt guest, but it can be adopted for
qemu or even used for qemu monitor), which allows
me run qmp commands on vms as easy as:

|qmp VMNAME query-block-jobs or qmp VMNAME nbd-server-remove name exp1
mode hard or even |

|qmp VMNAME blockdev-add id disk driver qcow2 cache {writeback true
direct true} aio native discard unmap file {driver file filename
/tmp/somedisk} |||

Yeah, there are various scripting solutions around QMP that can make it
easier; but HMP is often still an easy front-line interface for experiments.



isn't it because these solutions are not available directly in monitor, 
when HMP is?
may be, we need third type of monitor HQMP which is QMP with simplified 
syntax? Or

allow qmp commands in simplified syntax directly in HMP?

--
Best regards,
Vladimir

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

[Eric Blake]

On 01/17/2018 07:36 AM, Vladimir Sementsov-Ogievskiy wrote:

 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)
>>> Or even a fourth mode that causes an immediate error return without
>>> state change if there are any connected clients, but otherwise removes
>>> the server.
>>>
 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
>>> Seems reasonable.  Are you planning on tackling a respin of this series
>>> incorporating that idea?
>>>
>>
>> yes, will do.
>>
>>
> 
> Discussed with Nikolay.
> For now we actually need only one mode: hard.
> In near future we _may be_ will need your proposed fourth mode (what
> about "safe" name for it ?)

'safe' sounds reasonable.

Of course, if we only have two modes at front ('safe' which returns an
error if a client is connected, and 'hard' which disconnects all clients
immediately; leaving 'hide' and 'soft' for the future), then we don't
have to worry about a state transition or any hidden exports.

A QAPI enum with only two values now is at least extensible in the
future if someone has a need for another mode, and introspectible to
learn which modes are currently supported.

> 
> I was going to implement all 4 modes, but now I doubt, isn't it too
> hastily, to introduce 3 new modes to the
> interface, which we (personally) do not need. May be it is better to
> start from one or two modes.

Starting with just two modes is fine as well.

> 
> Finally what do you think, Eric? Which modes do you need?

'hide' may be interesting for the purpose of connecting a single client,
then hiding the export so no other clients can connect, while waiting
for the first client to take its time.  But right now, I don't have
actual use cases in mind so much as making sure we aren't limiting
ourself from future expansion as needs are identified, so a conservative
choice of just 'safe' and 'hard' for now is reasonable.

> 
> ps: I've created hmp version for 2/6, it will be in v2.
>  also, I'm going to add query-nbd-server, which should list all exports

Sounds good.

> 
> also, about HMP: If I understand correctly, people use it because
> writing qmp command by hand is not very comfortable.
> I have a script (for managing libvirt guest, but it can be adopted for
> qemu or even used for qemu monitor), which allows
> me run qmp commands on vms as easy as:
> 
> |qmp VMNAME query-block-jobs or qmp VMNAME nbd-server-remove name exp1
> mode hard or even |
> 
> |qmp VMNAME blockdev-add id disk driver qcow2 cache {writeback true
> direct true} aio native discard unmap file {driver file filename
> /tmp/somedisk} |||

Yeah, there are various scripting solutions around QMP that can make it
easier; but HMP is often still an easy front-line interface for experiments.

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



signature.asc
Description: OpenPGP digital signature

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

[Vladimir Sementsov-Ogievskiy]

15.01.2018 20:47, Vladimir Sementsov-Ogievskiy wrote:

15.01.2018 18:09, Eric Blake wrote:

On 01/12/2018 03:47 AM, Vladimir Sementsov-Ogievskiy wrote:

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 


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

Or even a fourth mode that causes an immediate error return without
state change if there are any connected clients, but otherwise removes
the server.


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

Seems reasonable.  Are you planning on tackling a respin of this series
incorporating that idea?



yes, will do.




Discussed with Nikolay.
For now we actually need only one mode: hard.
In near future we _may be_ will need your proposed fourth mode (what 
about "safe" name for it ?)


I was going to implement all 4 modes, but now I doubt, isn't it too 
hastily, to introduce 3 new modes to the
interface, which we (personally) do not need. May be it is better to 
start from one or two modes.


Finally what do you think, Eric? Which modes do you need?

ps: I've created hmp version for 2/6, it will be in v2.
 also, I'm going to add query-nbd-server, which should list all exports

also, about HMP: If I understand correctly, people use it because 
writing qmp command by hand is not very comfortable.
I have a script (for managing libvirt guest, but it can be adopted for 
qemu or even used for qemu monitor), which allows

me run qmp commands on vms as easy as:

|qmp VMNAME query-block-jobs or qmp VMNAME nbd-server-remove name exp1 
mode hard or even |


|qmp VMNAME blockdev-add id disk driver qcow2 cache {writeback true 
direct true} aio native discard unmap file {driver file filename 
/tmp/somedisk} |||


--
Best regards,
Vladimir

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

[Vladimir Sementsov-Ogievskiy]

15.01.2018 18:09, Eric Blake wrote:

On 01/12/2018 03:47 AM, Vladimir Sementsov-Ogievskiy wrote:

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

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)

Or even a fourth mode that causes an immediate error return without
state change if there are any connected clients, but otherwise removes
the server.


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

Seems reasonable.  Are you planning on tackling a respin of this series
incorporating that idea?



yes, will do.


--
Best regards,
Vladimir

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

[Eric Blake]

On 01/12/2018 03:47 AM, Vladimir Sementsov-Ogievskiy wrote:
> 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 
>>> ---

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

Or even a fourth mode that causes an immediate error return without
state change if there are any connected clients, but otherwise removes
the server.

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

Seems reasonable.  Are you planning on tackling a respin of this series
incorporating that idea?

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



signature.asc
Description: OpenPGP digital signature

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

[Vladimir Sementsov-Ogievskiy]

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




--
Best regards,
Vladimir

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

[Eric Blake]

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 
> ---
>  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)?

> +#
> +# 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 

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



signature.asc
Description: OpenPGP digital signature

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

[Vladimir Sementsov-Ogievskiy]

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 
---
 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).
+#
+# 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'} }
+
+##
 # @nbd-server-stop:
 #
 # Stop QEMU's embedded NBD server, and unregister all devices previously
diff --git a/include/block/nbd.h b/include/block/nbd.h
index 113c707a5e..b89d116597 100644
--- a/include/block/nbd.h
+++ b/include/block/nbd.h
@@ -261,12 +261,13 @@ NBDExport *nbd_export_new(BlockDriverState *bs, off_t 
dev_offset, off_t size,
   bool writethrough, BlockBackend *on_eject_blk,
   Error **errp);
 void nbd_export_close(NBDExport *exp);
+void nbd_export_hide(NBDExport *exp);
 void nbd_export_get(NBDExport *exp);
 void nbd_export_put(NBDExport *exp);
 
 BlockBackend *nbd_export_get_blockdev(NBDExport *exp);
 
-NBDExport *nbd_export_find(const char *name);
+NBDExport *nbd_export_find(const char *name, bool include_hidden);
 void nbd_export_set_name(NBDExport *exp, const char *name);
 void nbd_export_set_description(NBDExport *exp, const char *description);
 void nbd_export_close_all(void);
diff --git a/blockdev-nbd.c b/blockdev-nbd.c
index 46c885aa35..2495d38f2c 100644
--- a/blockdev-nbd.c
+++ b/blockdev-nbd.c
@@ -174,7 +174,7 @@ void qmp_nbd_server_add(const char *device, bool has_name, 
const char *name,
 name = device;
 }
 
-if (nbd_export_find(name)) {
+if (nbd_export_find(name, true)) {
 error_setg(errp, "NBD server already has export named '%s'", name);
 return;
 }
@@ -207,6 +207,33 @@ void qmp_nbd_server_add(const char *device, bool has_name, 
const char *name,
 nbd_export_put(exp);
 }
 
+void qmp_nbd_server_remove(const char *name, bool has_force, bool force,
+   Error **errp)
+{
+NBDExport *exp;
+
+if (!nbd_server) {
+error_setg(errp, "NBD server not running");
+return;
+}
+
+exp = nbd_export_find(name, true);
+if (exp == NULL) {
+error_setg(errp, "Export '%s' is not found", name);
+return;
+}
+
+if (!has_force) {
+force = false;
+}
+
+if (force) {
+nbd_export_close(exp);
+} else {
+nbd_export_hide(exp);
+}
+}
+
 void qmp_nbd_server_stop(Error **errp)
 {
 nbd_export_close_all();
diff --git a/nbd/server.c b/nbd/server.c
index e817c48087..d85f2dc747 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -78,6 +78,10 @@ struct NBDExport {
 
 BlockBackend *eject_notifier_blk;
 Notifier eject_notifier;
+
+/* hidden export is not available for new connections and should be removed
+ * after last client disconnect */
+bool hidden;
 };
 
 static QTAILQ_HEAD(, NBDExport) exports = QTAILQ_HEAD_INITIALIZER(exports);
@@ -300,7 +304,7 @@ static int nbd_negotiate_handle_export_name(NBDClient 
*client, uint32_t length,
 
 trace_nbd_negotiate_handle_export_name_request(name);
 
-client->exp = nbd_export_find(name);
+client->exp = nbd_export_find(name, false);
 if (!client->exp) {
 error_setg(errp, "export not found");
 return -EINVAL;
@@ -429,7 +433,7 @@ static int nbd_negotiate_handle_info(NBDClient *client, 
uint32_t length,
 }
 assert(length == 0);
 
-exp = nbd_export_find(name);
+exp = nbd_export_find(name, false);
 if (!exp) {
 return nbd_negotiate_send_rep_err(client->ioc, NBD_REP_ERR_UNKNOWN,
   opt, errp, "export '%s' not present",
@@ -966,6 +970,9 @@ void nbd_client_put(NBDClient *client)
 if (client->exp) {
 QTAILQ_REMOVE(>exp->clients, client, next);
 nbd_export_put(client->exp);
+if (client->exp->hidden && QTAILQ_EMPTY(>exp->clients)) {
+nbd_export_close(client->exp);
+