qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v3 RFC] block/vxhs: Initial commit to add Verita


From: ashish mittal
Subject: Re: [Qemu-devel] [PATCH v3 RFC] block/vxhs: Initial commit to add Veritas HyperScale VxHS block device support
Date: Mon, 15 Aug 2016 09:29:58 -0700

>> +/*
>> + * Convert the json formatted command line into qapi.
>> +*/
>> +
>> +static int vxhs_parse_json(BlockdevOptionsVxHS *conf,
>> +                                  QDict *options, Error **errp)
>> +{
...
>> +    errno = EINVAL;
>> +    return -errno;
>> +}
>
> Ewww this is really horrible code. It is open-coding a special purpose
> conversion of QemuOpts -> QDict -> QAPI scheme. We should really put
> my qdict_crumple() API impl as a pre-requisite of this, so you can then
> use qdict_crumple + qmp_input_visitor to do this conversion in a generic
> manner removing all this code.
>
>   https://lists.gnu.org/archive/html/qemu-devel/2016-07/msg03118.html
>

Thanks for the suggestions. I will try to incorporate them in the next
version. Actually, I used block/gluster.c for reference (as advised).
This is exactly how it parses json CLI options. It also implements the
*parse_uri() in a similar way.


>> +/*
>> + * vxhs_parse_uri: Parse the incoming URI and populate *conf with the
>> + * vdisk_id, and all the host(s) information. Host at index 0 is local 
>> storage
>> + * agent and the rest, reflection target storage agents. The local storage
>> + * agent ip is the efficient internal address in the uri, e.g. 192.168.0.2.
>> + * The local storage agent address is stored at index 0. The reflection 
>> target
>> + * ips, are the E-W data network addresses of the reflection node agents, 
>> also
>> + * extracted from the uri.
>> + */
>> +static int vxhs_parse_uri(BlockdevOptionsVxHS *conf,
>> +                               const char *filename)
>
> Delete this method entirely. We should not be adding URI syntax for any new
> block driver. The QAPI schema syntax is all we need.
>

Regards,
Ashish



reply via email to

[Prev in Thread] Current Thread [Next in Thread]