[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [Patch for-2.5 v2 2/6] support nbd driver in blockdev-a
From: |
Wen Congyang |
Subject: |
Re: [Qemu-block] [Patch for-2.5 v2 2/6] support nbd driver in blockdev-add |
Date: |
Tue, 1 Sep 2015 17:35:25 +0800 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.1.0 |
On 09/01/2015 01:19 AM, Eric Blake wrote:
> On 08/11/2015 01:51 AM, Wen Congyang wrote:
>> Signed-off-by: Wen Congyang <address@hidden>
>> Signed-off-by: zhanghailiang <address@hidden>
>> Signed-off-by: Gonglei <address@hidden>
>> Reviewed-by: Alberto Garcia <address@hidden>
>> ---
>> qapi/block-core.json | 17 +++++++++++++++--
>> 1 file changed, 15 insertions(+), 2 deletions(-)
>>
>> diff --git a/qapi/block-core.json b/qapi/block-core.json
>> index 7b2efb8..3ed8114 100644
>> --- a/qapi/block-core.json
>> +++ b/qapi/block-core.json
>> @@ -1383,7 +1383,7 @@
>> 'dmg', 'file', 'ftp', 'ftps', 'host_cdrom', 'host_device',
>> 'host_floppy', 'http', 'https', 'null-aio', 'null-co',
>> 'parallels',
>> 'qcow', 'qcow2', 'qed', 'quorum', 'raw', 'tftp', 'vdi', 'vhdx',
>> - 'vmdk', 'vpc', 'vvfat' ] }
>> + 'vmdk', 'vpc', 'vvfat', 'nbd' ] }
>
> Please keep the list alphabetical. Also, this is missing documentation
> (see how BlockDeviceInfo has listed which releases have added which
> formats; so it should add a listing of 'nbd' in 2.5).
>
>>
>> ##
>> # @BlockdevOptionsBase
>> @@ -1789,6 +1789,19 @@
>> '*read-pattern': 'QuorumReadPattern' } }
>>
>> ##
>> +# @BlockdevOptionsNBD
>> +#
>> +# Driver specific block device options for NBD
>> +#
>> +# @export: #options the NBD export name
>> +#
>> +# Since: 2.5
>> +##
>> +{ 'struct': 'BlockdevOptionsNBD',
>> + 'base': 'InetSocketAddress',
>> + 'data': { '*export': 'str' } }
>
> So does NBD really support a range of ports? Or is it an error to
> specify 'to'? Perhaps what you should really be doing is making a
> simpler base class for representing a single-port IP address, then
> making InetSocketAddress a child class of the simpler one to add in
> ranging, and make BlockdevOptionsNBD a child class of the simpler one to
> add in an export name.
NBD doesn;t support a range of ports. I will update it in the next version.
>
> Also, InetSocketAddress appears to be limited to IPv4 and IPv6
> addresses; but what about nbd+unix transport? It feels like you need a
> flat union with a discriminator that describes what address family (IP
> vs. unix socket) and then further details based on that discriminator.
Hmm, It is SocketAddress, but it contains fd, which nbd doesn't support.
Another problem is that: NBD doesn't support type. It check the key path
and host:
if (qdict_haskey(options, "path") == qdict_haskey(options, "host")) {
if (qdict_haskey(options, "path")) {
error_setg(errp, "path and host may not be used at the same time.");
} else {
error_setg(errp, "one of path and host must be specified.");
}
return;
}
s->client.is_unix = qdict_haskey(options, "path");
Update NBD to support it?
Thanks
Wen Congyang
>
>> +
>> +##
>> # @BlockdevOptions
>> #
>> # Options for creating a block device.
>> @@ -1815,7 +1828,7 @@
>> 'http': 'BlockdevOptionsFile',
>> 'https': 'BlockdevOptionsFile',
>> # TODO iscsi: Wait for structured options
>> -# TODO nbd: Should take InetSocketAddress for 'host'?
>> + 'nbd': 'BlockdevOptionsNBD',
>
> So while you are indeed literally taking the TODO suggestion, I'm not
> sure that the literal interpretation is the best.
>
> Remember, the current NBD source code accepts:
>
> if (is_unix) {
> /* nbd+unix:///export?socket=path */
> if (uri->server || uri->port || strcmp(qp->p[0].name, "socket")) {
> ret = -EINVAL;
> goto out;
> }
> qdict_put(options, "path", qstring_from_str(qp->p[0].value));
> } else {
> QString *host;
> /* nbd[+tcp]://host[:port]/export */
>
> and we want to be able to represent all of those possibilities in QMP.
>
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- Re: [Qemu-block] [Patch for-2.5 v2 2/6] support nbd driver in blockdev-add,
Wen Congyang <=