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 <kw...@redhat.com> 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
