[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v4 11/11] nbd-server: Allow node name for nbd-se
From: |
Max Reitz |
Subject: |
Re: [Qemu-devel] [PATCH v4 11/11] nbd-server: Allow node name for nbd-server-add |
Date: |
Fri, 15 Jul 2016 15:36:16 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.2.0 |
On 14.07.2016 23:36, Eric Blake wrote:
> On 07/14/2016 07:28 AM, Kevin Wolf wrote:
>> There is no reason why an NBD server couldn't be started for any node,
>> even if it's not on the top level. This converts nbd-server-add to
>> accept a node-name.
>>
>> Note that there is a semantic difference between using a BlockBackend
>> name and the node name of its root: In the former case, the NBD server
>> is closed on eject; in the latter case, the NBD server doesn't drop its
>> reference and keeps the image file open this way.
>>
>> Signed-off-by: Kevin Wolf <address@hidden>
>> ---
>> blockdev-nbd.c | 21 +++++++++------------
>> qapi/block.json | 4 ++--
>> 2 files changed, 11 insertions(+), 14 deletions(-)
>>
>> diff --git a/blockdev-nbd.c b/blockdev-nbd.c
>> index c437d32..ca41cc6 100644
>> --- a/blockdev-nbd.c
>> +++ b/blockdev-nbd.c
>> @@ -145,7 +145,8 @@ void qmp_nbd_server_start(SocketAddress *addr,
>> void qmp_nbd_server_add(const char *device, bool has_writable, bool
>> writable,
>> Error **errp)
>> {
>> - BlockBackend *blk;
>> + BlockDriverState *bs = NULL;
>> + BlockBackend *on_eject_blk;
>> NBDExport *exp;
>>
>> if (!nbd_server) {
>> @@ -158,26 +159,22 @@ void qmp_nbd_server_add(const char *device, bool
>> has_writable, bool writable,
>> return;
>
> Do we want to do any sanity checking that writing should only be
> permitted on a root, and that when using a node name that is not a root
> that writable must be false so as not to negatively change the BDS out
> of under the feet of the other root? Do op-blockers already cover that?
Well, one could argue that it's possible to create an NBD server on a
non-root node today anyway, since creating BBs is not restricted to root
nodes:
blockdev-add(id=foo, other arguments...)
blockdev-add(id=bar, backing=foo, other arguments...)
And then you can create an NBD server on bar. I agree that this is not
how it should be, though. However, I think that the fact that you need
to specify a BB name for now deters people from doing stuff like that.
If you can specify a node name, people will think it's completely fine
to do so.
Also note that only allowing NBD servers to be created on a root node
doesn't really help you:
blockdev-add(node-name=foo, ...)
nbd-server-add(device=foo)
blockdev-add(id=bar, backing=foo, ...)
So, yeah, I think we just need the new op-blockers for this, I don't
think the current op blockers cover this.
Max
signature.asc
Description: OpenPGP digital signature
- [Qemu-devel] [PATCH v4 06/11] block: Accept node-name for blockdev-snapshot-internal-sync, (continued)
- [Qemu-devel] [PATCH v4 06/11] block: Accept node-name for blockdev-snapshot-internal-sync, Kevin Wolf, 2016/07/14
- [Qemu-devel] [PATCH v4 01/11] block: Accept node-name for block-stream, Kevin Wolf, 2016/07/14
- [Qemu-devel] [PATCH v4 09/11] block: Accept node-name for drive-mirror, Kevin Wolf, 2016/07/14
- [Qemu-devel] [PATCH v4 10/11] nbd-server: Use a separate BlockBackend, Kevin Wolf, 2016/07/14
- [Qemu-devel] [PATCH v4 11/11] nbd-server: Allow node name for nbd-server-add, Kevin Wolf, 2016/07/14