Re: [Qemu-devel] [PATCH v3 RFC] block/vxhs: Initial commit to add Veritas HyperScale VxHS block device support

[ashish mittal]

Need some help!

I'm trying to understand how I can parse a json command line having
multiple server list (InetSocketAddressList) without the QemuOpts ->
QDict -> QAPI conversion that I am currently doing.

block/nbd.c has been suggested, but it only parses a single host
entry, which is pretty straightforward, but not very helpful.

Could somebody please suggest a sample implementation (other than
gluster.c) that parses a list of server like options in the json
format?

Thanks,
Ashish


On Wed, Aug 17, 2016 at 2:58 PM, ashish mittal <address@hidden> wrote:
> Thanks Paolo, and everyone else for the corrections :)
>
> I will try to fix it in a patch this week or the next. I referred to
> gluster.c implementation as it was closer to what we wanted to achieve
> i.e. passing multiple servers for the block device. I picked up the
> idea of referring to gluster.c from the following email that I
> received on libvirt mailing list. I did test my implementation to make
> sure that it worked fine, but did not realize that it was not clean. I
> will fix it.
>
> /====================/
> Hmm, unless I'm mis-reading this, there is no separator between
> the two URIs you're providing - is there a missing ',' or something
> similar.
>
> Slightly off topic for the libvirt list, but I would be pretty surprised
> if the QEMU developers accepted patches using this URI syntax for providing
> multiple servers.
>
> A similar approach was originally proposed for the glusterfs driver and
> the submitter was told to write it a different way, not using the legacy
> URI syntax at all, but instead a QAPI based syntax:
>
>   https://lists.gnu.org/archive/html/qemu-devel/2016-06/msg04126.html
>
> So the sooner you can submit your QEMU patches to the qemu-devel mailing
> list the better, as we'll need to get clarity on the accepted QEMU syntax
> in order to proceed further with libvirt patch review.
>
> Broadly speaking I think the patch you've proposed looks pretty much
> fine now. I'd have a slight preference for it to be done as two patches.
> One patch adds the XML schema, parser changes, etc. The second patch
> just does the QEMU driver integration & unit tests.
> ...
> /====================/
>
> Regards,
> Ashish
>
>
> On Wed, Aug 17, 2016 at 4:22 AM, Paolo Bonzini <address@hidden> wrote:
>>
>>
>> On 15/08/2016 18:29, ashish mittal wrote:
>>>>> +/*
>>>>> + * 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.
>>
>> I think I pointed you to block/nbd.c instead, which works as Kevin
>> suggested.
>>
>> Paolo