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

[ashish mittal]

Hi Kevin,

Changed per your suggestion in V4 patch. Please review when you get some time.

Thanks,
Ashish

On Mon, Aug 15, 2016 at 3:47 AM, Kevin Wolf <address@hidden> wrote:
> Am 15.08.2016 um 12:20 hat Daniel P. Berrange geschrieben:
>> On Sat, Aug 13, 2016 at 09:35:12PM -0700, Ashish Mittal wrote:
>> > +/*
>> > + * 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.
>
> I disagree. URI syntax is nice for human users.
>
> However, you should use the proper interfaces to implement this, which
> is .bdrv_parse_filename(). This is a function that gets a string and
> converts it into a QDict, which is then passed to .bdrv_open(). So it
> uses exactly the same code path in .bdrv_open() as if used directly with
> QAPI.
>
> Kevin