[Qemu-devel] [PULL 4/4] qmp: add nbd-server-remove

Eric Blake posted 4 patches 8 years, 1 month ago
[Qemu-devel] [PULL 4/4] qmp: add nbd-server-remove
Posted by Eric Blake 8 years, 1 month ago
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


[Qemu-devel] STOP Re: [PULL 4/4] qmp: add nbd-server-remove
Posted by Vladimir Sementsov-Ogievskiy 8 years, 1 month ago
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


Re: [Qemu-devel] STOP Re: [PULL 4/4] qmp: add nbd-server-remove
Posted by Vladimir Sementsov-Ogievskiy 8 years, 1 month ago
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


Re: [Qemu-devel] STOP Re: [PULL 4/4] qmp: add nbd-server-remove
Posted by Eric Blake 8 years, 1 month ago
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

Re: [Qemu-devel] STOP Re: [PULL 4/4] qmp: add nbd-server-remove
Posted by Peter Maydell 8 years, 1 month ago
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