[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: |
Eric Blake |
Subject: |
Re: [Qemu-block] [PATCH v2 3/6] qapi: add nbd-server-remove |
Date: |
Thu, 18 Jan 2018 16:13:56 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.2 |
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 <address@hidden>
> ---
> 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(&exp->clients)) {
> + nbd_export_close(exp);
> + return;
> + }
> +
> + assert(mode == NBD_SERVER_REMOVE_MODE_SAFE);
> +
> + QTAILQ_FOREACH(client, &exp->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 0/6] nbd export qmp interface, Vladimir Sementsov-Ogievskiy, 2018/01/18
- [Qemu-block] [PATCH v2 2/6] hmp: add name parameter to nbd_server_add, Vladimir Sementsov-Ogievskiy, 2018/01/18
- [Qemu-block] [PATCH v2 5/6] iotests: implement QemuIoInteractive class, Vladimir Sementsov-Ogievskiy, 2018/01/18
- [Qemu-block] [PATCH v2 1/6] qapi: add name parameter to nbd-server-add, Vladimir Sementsov-Ogievskiy, 2018/01/18
- [Qemu-block] [PATCH v2 3/6] qapi: add nbd-server-remove, Vladimir Sementsov-Ogievskiy, 2018/01/18
- Re: [Qemu-block] [PATCH v2 3/6] qapi: add nbd-server-remove,
Eric Blake <=
- [Qemu-block] [PATCH v2 4/6] iotest 147: add cases to test new @name parameter of nbd-server-add, Vladimir Sementsov-Ogievskiy, 2018/01/18
- [Qemu-block] [PATCH v2 6/6] iotest 201: new test for qmp nbd-server-remove, Vladimir Sementsov-Ogievskiy, 2018/01/18
- Re: [Qemu-block] [PATCH v2 0/6] nbd export qmp interface, Eric Blake, 2018/01/18